-
Notifications
You must be signed in to change notification settings - Fork 142
T2C: LLVM builder leak in cbeqz/cbnez/fuse12 else branches #722
Description
Description
The hand-written cbeqz, cbnez, and fuse12 T2C handlers in src/t2c_template.c leak LLVMBuilderRef objects in their else branches. The BRANCH_FUNC macro (line 343/355) correctly calls LLVMDisposeBuilder in the same code path, but these three handlers omit it.
Found during review of #716.
Root Cause
When ir->branch_taken / ir->branch_untaken is false (or fails t2c_check_valid_blk in fuse12), the builder is used to emit a RetVoid but never disposed. The if branch correctly passes ownership to *taken_builder/*untaken_builder (disposed later by t2c_trace_ebb), but the else branch retains no reference after use.
6 leak sites total:
| Handler | Builder | Line |
|---|---|---|
cbeqz |
builder2 |
1218 |
cbeqz |
builder3 |
1229 |
cbnez |
builder2 |
1246 |
cbnez |
builder3 |
1257 |
fuse12 |
builder2 |
1686 |
fuse12 |
builder3 |
1698 |
Impact
Each leaked LLVMBuilderRef is a heap-allocated LLVM object. The leak accumulates per T2C compilation of cbeqz/cbnez/fuse12 instructions where the branch target is not chainable. For long-running workloads (system mode, Linux boot), this is unbounded host memory growth.
Proposed Fix
Add LLVMDisposeBuilder in each else branch, matching the pattern in BRANCH_FUNC:
cbeqz (line 1205)
T2C_OP(cbeqz, {
LLVMValueRef addr_PC = t2c_gen_PC_addr(start, builder, ir);
T2C_LLVM_GEN_LOAD_VMREG(rs1, 32, t2c_gen_rs1_addr(start, builder, ir));
T2C_LLVM_GEN_CMP_IMM32(EQ, val_rs1, 0);
LLVMBasicBlockRef taken = LLVMAppendBasicBlock(start, "taken");
LLVMBuilderRef builder2 = LLVMCreateBuilder();
LLVMPositionBuilderAtEnd(builder2, taken);
if (ir->branch_taken)
*taken_builder = builder2;
else {
T2C_LLVM_GEN_STORE_IMM32(builder2, ir->pc + ir->imm, addr_PC);
T2C_STORE_TIMER(builder2, start, insn_counter);
LLVMBuildRetVoid(builder2);
+ LLVMDisposeBuilder(builder2);
}
LLVMBasicBlockRef untaken = LLVMAppendBasicBlock(start, "untaken");
LLVMBuilderRef builder3 = LLVMCreateBuilder();
LLVMPositionBuilderAtEnd(builder3, untaken);
if (ir->branch_untaken)
*untaken_builder = builder3;
else {
T2C_LLVM_GEN_STORE_IMM32(builder3, ir->pc + 2, addr_PC);
T2C_STORE_TIMER(builder3, start, insn_counter);
LLVMBuildRetVoid(builder3);
+ LLVMDisposeBuilder(builder3);
}
LLVMBuildCondBr(*builder, cmp, taken, untaken);
})cbnez (line 1233)
Same pattern — add LLVMDisposeBuilder(builder2) after line 1245 and LLVMDisposeBuilder(builder3) after line 1256.
fuse12 (line 1666)
} else {
T2C_LLVM_GEN_STORE_IMM32(builder2, ir->pc + 4 + ir->imm2, addr_PC);
T2C_STORE_TIMER(builder2, start, insn_counter);
LLVMBuildRetVoid(builder2);
+ LLVMDisposeBuilder(builder2);
}
...
} else {
T2C_LLVM_GEN_STORE_IMM32(builder3, ir->pc + 8, addr_PC);
T2C_STORE_TIMER(builder3, start, insn_counter);
LLVMBuildRetVoid(builder3);
+ LLVMDisposeBuilder(builder3);
}Additional Note
cbeqz/cbnez check if (ir->branch_taken) without the t2c_check_valid_blk() guard that BRANCH_FUNC and fuse12 use. This is a separate behavioral discrepancy — the compressed branch handlers may chain to blocks that have been evicted or are otherwise invalid. Consider aligning with BRANCH_FUNC:
// Current (cbeqz/cbnez):
if (ir->branch_taken)
// Should match BRANCH_FUNC:
if (ir->branch_taken && t2c_check_valid_blk(rv, block, ir->branch_taken->pc))This guard discrepancy is a separate concern from the builder leak but worth addressing in the same patch.