Fix memory leak by fourcolor · Pull Request #716 · sysprog21/rv32emu

fourcolor · 2026-02-09T03:51:22Z

This PR fixes several ASan-reported memory leaks:

Dispose tk/utk builders returned from instruction handlers in t2c_trace_ebb().
Dispose messages returned by LLVMGetHostCPUName() / LLVMGetHostCPUFeatures().
Free branch_tables for all cached blocks before teardown.
Free fd_map and mem in rv_delete().
Free all live cache entries in cache_free().

Summary by cubic

Fixes ASan-reported memory leaks in the JIT and teardown for a clean shutdown and lower memory use. Disposes temporary LLVM builders/messages and frees branch tables on eviction/teardown, plus fd_map/mem and cache entries.

Bug Fixes
- t2c: dispose tk/utk builders; dispose LLVMGetHostCPUName/Features messages.
- eviction/teardown: free ir->branch_table when replacing/evicting blocks and for all remaining blocks before freeing the cache; delete fd_map and mem in rv_delete().
- cache: free all live and ghost cache entries in cache_free().

^{Written for commit 2138efe. Summary will update on new commits.}

The tk/utk builders passed back from instruction handlers are never disposed in t2c_trace_ebb. This change make sure them deposed at the end.

Fix memory leak by disposing messages from LLVMGetHostCPUName/Features

- Free branch_tables for all cached blocks - Free fd_map/mem in rv_delete() - Free all live cache entries in cache_free()

jserv

Review Summary

All five leak categories are correctly fixed. No double-free, no use-after-free, no race conditions, no regressions. Cross-validated with Gemini and Codex (GPT) — all three reviewers agree the fixes are sound.

Per-commit assessment

970f619 Dispose LLVM builders in t2c_trace_ebb — The tk != *builder / utk != *builder guard is correct; avoids double-disposing the caller's builder.
72f3e75 Free LLVMGetHostCPU* return strings — Textbook fix. LLVM C API returns malloc'd strings; LLVMDisposeMessage is the right cleanup.
3cbd3f0 Free fd_map/mem in JIT teardown + cache entries — fd_map/mem were only freed in the non-JIT #else path. The cache_free() entry cleanup is also necessary (list nodes were previously leaked).
2138efe Free branch_table on runtime block eviction — Covers all three eviction loops. free(NULL) is a no-op per C standard, so calling unconditionally on every IR node is safe and simple.

Ordering correctness

The teardown sequence in rv_delete is the only correct ordering: clear_cache_hot(free_block_branch_tables) → jit_state_exit → cache_free → mpool_destroy. Reversing free_block_branch_tables after mpool_destroy would be use-after-free on IR nodes. The current code is correct.

Pre-existing leak (not introduced by this PR)

In src/t2c_template.c, the hand-written cbeqz (line 1214-1218) and cbnez handlers do not call LLVMDisposeBuilder(builder2/builder3) in their else branches, unlike the macro version (line 343/355) which correctly disposes them. Worth a follow-up fix.

jserv · 2026-03-06T14:24:13Z

src/t2c.c

+    if (tk && tk != *builder)
+        LLVMDisposeBuilder(tk);
+    if (utk && utk != *builder)
+        LLVMDisposeBuilder(utk);


Good. The tk != *builder and utk != *builder guards are necessary and correct — when the branch is taken/untaken within the EBB, tk/utk alias *builder and must not be double-disposed. When they point to distinct builders created by instruction handlers, they are properly cleaned up here.

One subtlety: tk/utk are initialized to NULL and only set by branch instruction handlers. The tk && / utk && null checks handle the case where no branch was encountered. Clean.

jserv · 2026-03-06T14:24:13Z

src/t2c.c

    LLVMDisposeTargetMachine(tm);
    LLVMDisposeMessage(triple);
+    LLVMDisposeMessage(cpu_name);
+    LLVMDisposeMessage(cpu_features);


Correct. LLVMGetHostCPUName() and LLVMGetHostCPUFeatures() return malloc'd C strings per LLVM C API contract. LLVMDisposeMessage (which calls free) is the documented cleanup. Previously these were passed as temporaries directly to LLVMCreateTargetMachine, leaking on every T2C compilation.

jserv · 2026-03-06T14:24:13Z

src/emulate.c

         ir = next_ir) {
        next_ir = ir->next;
-
+        free(ir->branch_table);


Correct. This is the runtime eviction path in block_find_or_translate — when the cache is full and a block is replaced. Without this, blocks with indirect branches (JALR dispatch tables) leaked their branch_table allocation on every eviction cycle. free(NULL) is a no-op for the majority of IR nodes that have no branch table.

jserv · 2026-03-06T14:24:13Z

src/riscv.c

    clear_cache_hot(rv->block_cache, t2c_dispose_block_engine);
 #endif
+    /* Free branch tables for all remaining blocks before freeing cache */
+    clear_cache_hot(rv->block_cache, free_block_branch_tables);


This must remain before mpool_destroy(rv->block_ir_mp) at line 1114. free_block_branch_tables dereferences ir->branch_table through IR nodes allocated from that pool — destroying the pool first would be use-after-free.

Consider adding a brief comment noting this ordering constraint, e.g.:

/* Must precede mpool_destroy(block_ir_mp) -- IR nodes live in that pool */

jserv · 2026-03-06T14:24:13Z

src/riscv.c

    mpool_destroy(rv->block_ir_mp);
    mpool_destroy(rv->block_mp);
    mpool_destroy(rv->fuse_mp);
+    map_delete(attr->fd_map);


Good catch. These were previously only freed in the #if !RV32_HAS(JIT) path (line 1081-1082), meaning every JIT-enabled build leaked fd_map and mem on exit.

jserv · 2026-03-06T14:24:13Z

src/cache.c

+#else
+    list_for_each_entry_safe (entry, safe, &cache->list, list, cache_entry_t)
+#endif
+        free(entry);


Nit: The #ifdef __HAVE_TYPEOF / #else pattern appears twice (lines 401-406 for live, 408-414 for ghost). Consider factoring into a helper macro or static function that takes a struct list_head * to reduce duplication. Not a blocker.

jserv · 2026-03-06T14:24:13Z

src/t2c.c

            /* Free IRs that main thread skipped during deferred eviction */
            for (rv_insn_t *ir = block->ir_head, *next_ir; ir; ir = next_ir) {
                next_ir = ir->next;
+                free(ir->branch_table);


Correct. This deferred eviction path runs under cache_lock (acquired at line 600). The block was evicted by the main thread while T2C was compiling it — now we properly free branch_table before returning the IR node to the pool. Same pattern correctly applied at line 635 for the successful-compilation-but-evicted case.

jserv · 2026-03-06T14:27:14Z

Thank @fourcolor for contributing!

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

fourcolor force-pushed the fix-memory-leak branch from 4f38e79 to bd13459 Compare February 9, 2026 04:09

jserv added this to the release-2026.2 milestone Feb 9, 2026

fourcolor mentioned this pull request Feb 10, 2026

Handle jump table overflow in JIT block chaining #717

Closed

This comment was marked as resolved.

Sign in to view

fourcolor force-pushed the fix-memory-leak branch from bd13459 to b11bf0f Compare March 2, 2026 09:23

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

sysprog21 deleted a comment from cubic-dev-ai bot Mar 4, 2026

This comment was marked as resolved.

Sign in to view

fourcolor added 4 commits March 6, 2026 14:19

Dispose LLVM builders in t2c_trace_ebb

970f619

The tk/utk builders passed back from instruction handlers are never disposed in t2c_trace_ebb. This change make sure them deposed at the end.

Free LLVMGetHostCPU* return strings in T2C init

72f3e75

Fix memory leak by disposing messages from LLVMGetHostCPUName/Features

Free fd_map/mem in JIT teardown path

3cbd3f0

- Free branch_tables for all cached blocks - Free fd_map/mem in rv_delete() - Free all live cache entries in cache_free()

Free branch_table on runtime block eviction

2138efe

fourcolor force-pushed the fix-memory-leak branch from b11bf0f to 2138efe Compare March 6, 2026 13:16

jserv reviewed Mar 6, 2026

View reviewed changes

jserv merged commit 960ed8b into sysprog21:master Mar 6, 2026
17 checks passed

jserv mentioned this pull request Mar 6, 2026

T2C: LLVM builder leak in cbeqz/cbnez/fuse12 else branches #722

Closed

Conversation

fourcolor commented Feb 9, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Review Summary

Per-commit assessment

Ordering correctness

Pre-existing leak (not introduced by this PR)

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jserv Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jserv commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fourcolor commented Feb 9, 2026 •

edited by cubic-dev-ai bot

Loading