leaksan: follow the Bun::generateModule rename to Bun::generateInternalModule

[robobun]

Problem

• test/bundler/transpiler/macro-test.test.ts is red on the debian 13 x64-asan lane. bun build with a macro that imports node:fs exits 134: LeakSanitizer: detected memory leaks, Direct leak of 4104 byte(s) from Box::new in bun_runtime::node::node_fs_binding::create_binding (src/runtime/node/node_fs_binding.rs:329).
• compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597 renamed Bun::generateModule to Bun::generateInternalModule (src/jsc/bindings/InternalModuleRegistry.cpp:103). test/leaksan.supp:26 kept the old name, so the entry matched nothing. Every build with compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597 fails, including its own. Builds without it pass.

Fix

• Replace the entry with leak:^Bun::generateInternalModule(. The anchor matches the function only, not the lambda in Bun::generateInternalModuleSourceCode, which the old entry never covered.
• The leak is a false positive and not new. The NodeJSFS JSC cell owns the box. The macro VM inside bun build is never torn down, so the finalizer never runs. Before compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597 the entry suppressed exactly this allocation (print_suppressions=1: 1 4104 Bun::generateModule).
• Verified: bun bd test test/bundler/transpiler/macro-test.test.ts with the CI LSan environment. Old file: 21 pass, 1 fail (the CI output). New file: 22 pass, suppression used: 1 4104 ^Bun::generateInternalModule(. CI build 106730: the asan lane passes.

Background

• LSan reports a leak when no memory it scans (globals, stacks, TLS, its own heap chunks) points to the allocation. bmalloc uses the system allocator only when __asan_init is exported. bun does not export it, so JSC allocates through libpas, which LSan does not scan.
• test/leaksan.supp is the allowlist for that class of report. An entry matches a frame of the recorded allocation stack (30 frames in CI). ^ anchors the pattern at the start of the function name.

Notes

• Culprit check. Builds 106555, 106643, 106656 (compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597's own), 106687, 106693 and 106694 include 595f97b (compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597) and fail. Builds 106531, 106594 and 106642 include 0e395c2 (node:fs: async calls with a Buffer path no longer keep the Buffer alive forever #40511) but not compile: builtin module sources in a dedicated section so --bytecode covers non-host targets #40597 and pass macro-test.test.ts on the asan lane (106531 job log: test/bundler/transpiler/macro-test.test.ts (5.81s)). The asan lane runs on PR builds only, so main never showed it.
• A debug build truncates the recorded stack before the frame (malloc_context_size=30 reaches JSC::profiledCall), so locally this test leaks under LSan with any suppressions file unless malloc_context_size is raised. The release-asan build in CI inlines enough that the frame is recorded at position 12. The local runs above used malloc_context_size=80 with BUN_DESTRUCT_VM_ON_EXIT=1, ASAN_OPTIONS=detect_leaks=1 and LSAN_OPTIONS=suppressions=test/leaksan.supp.
• The rename affected no other function: Bun::generateModule was the only definition the old entry matched.
• Follow-ups, not done here: bun build could tear down its macro VM on exit under BUN_DESTRUCT_VM_ON_EXIT, or create_binding could mark the box LSan-ignored (node:fs: mark the per-VM Binding box as LSan-ignored (fixes worker-terminate-lifetime.test.ts on main) #35159 proposes that). Either makes this entry deletable. Other entries in the file use Zig-era names (runtime.node.node_fs_binding.Bindings(.mkdtemp).runSync, ...) and are dead since the Rust port.

---

no test proof · iteration 0 · no src or test change; test-proof not applicable

[robobun]

Status: reproduced locally with bun bd test test/bundler/transpiler/macro-test.test.ts under the CI LSan environment (BUN_DESTRUCT_VM_ON_EXIT=1, ASAN_OPTIONS=detect_leaks=1, LSAN_OPTIONS=suppressions=test/leaksan.supp, malloc_context_size=80 for the debug build). Old suppressions file: 21 pass, 1 fail with the CI output. This branch: 22 pass. The diff is the suppressions file only, so a plain bun bd test run passes with or without it.

CI: test/bundler/transpiler/macro-test.test.ts passes on the debian 13 x64-asan lane in build 106730 (revision 1) and build 106887 (revision 2, the anchored entry). All 21 asan jobs are green in both. The remaining red in both builds is test/js/web/url/url.test.ts on darwin x64, which also fails on main and is reported separately, plus tests that passed on retry. None of it touches this change. Ready for review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 87cf9cf8-a012-42e9-9a55-c4e0e0b12988

📥 Commits

Reviewing files that changed from the base of the PR and between ecca0d1 and 9d44e99.

📒 Files selected for processing (1)

• test/leaksan.supp

Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review.

---

Walkthrough

Changes

LSAN suppression update

Layer / File(s)	Summary
Update builtin-module leak suppression test/leaksan.supp	The suppression now targets Bun::generateInternalModule. Comments describe builtin-module evaluation and JSC cell-owned native state reported as leaked when the VM remains active.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly describes the main change: updating the LeakSanitizer suppression to follow the rename from Bun::generateModule to Bun::generateInternalModule.
Description check	✅ Passed	The description explains the problem, the suppression change, the technical cause, and detailed verification results. It does not use the template headings exactly, but it provides the required inform…

Full details: Description check

Explanation

The description explains the problem, the suppression change, the technical cause, and detailed verification results. It does not use the template headings exactly, but it provides the required information and is sufficiently complete.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical rename of an existing LSAN suppression to track the Bun::generateModule → Bun::generateInternalModule rename from #40597.

What was reviewed:

• Verified Bun::generateInternalModule exists at src/jsc/bindings/InternalModuleRegistry.cpp:103 inside namespace Bun, and that #40597 (595f97b) is where the old name disappeared.
• Confirmed this restores an existing suppression rather than adding new coverage — not a weakening of the LSAN safety net.
• The added comment matches the file's established convention (see the TemporalCore::withTimeZone / RunLoop::dispatchAfter entries) and explains the libpas-heap-invisible-to-LSAN mechanism plus the rename dependency.

Extended reasoning...

Overview

This PR touches a single file, test/leaksan.supp, replacing one line (leak:Bun::generateModule → leak:Bun::generateInternalModule) and adding a seven-line explanatory comment. The suppression file is the allowlist for LeakSanitizer false positives on the ASAN CI lane; entries substring-match frames in the recorded allocation stack. PR #40597 renamed the C++ function this entry targeted, so the old entry stopped matching and test/bundler/transpiler/macro-test.test.ts went red on the ASAN lane.

Security risks

None. This is a test-infrastructure config file that only affects whether LSAN reports are suppressed during CI test runs. It has no bearing on shipped runtime behavior, auth, crypto, or user-facing surface.

Level of scrutiny

Low. This is a one-line config rename plus a comment. I verified the factual claims: generateInternalModule is defined at InternalModuleRegistry.cpp:103 inside namespace Bun {, and git log -S confirms 595f97b (#40597) is where the generateModule symbol changed in that file. The old substring Bun::generateModule is not a substring of Bun::generateInternalModule (the "Internal" infix breaks it), so the PR's premise that the rename silently orphaned the suppression checks out. REVIEW.md's "never silently weaken a safety net" rule applies here in the change's favor: this restores a pre-existing suppression, it does not introduce new leak-hiding.

Other factors

The added comment satisfies CLAUDE.md's comment-quality bar (rules 13/14): it explains why the suppression exists (libpas heap invisible to LSAN, macro VM not torn down before exit) and warns future readers that the entry must track the function's name — both are things a future maintainer would otherwise spend real effort rediscovering. The style and depth match the other commented entries lower in the same file. No CODEOWNERS entry covers test/leaksan.supp. No prior review comments exist on the PR. Exit reason was dry_streak with zero findings.

[claude]

LGTM — the follow-up commit anchors the entry and states its removal condition, which addresses the earlier feedback.

What was reviewed:

• Confirmed Bun::generateInternalModule exists at src/jsc/bindings/InternalModuleRegistry.cpp:103 and that generateInternalModuleSourceCode is a distinct sibling the anchor now excludes.
• Checked that ^ is a real start-of-frame anchor in compiler-rt's TemplateMatch (sanitizer suppression matcher) and ( is matched literally, so ^Bun::generateInternalModule( narrows rather than breaks the match.
• Comment style and removal-condition phrasing match the existing entries in this file (e.g. the TemporalCore and RunLoop::dispatchAfter blocks).

Extended reasoning...

Overview

This PR updates a single line in test/leaksan.supp, the LeakSanitizer suppressions allowlist. PR #40597 renamed Bun::generateModule to Bun::generateInternalModule in InternalModuleRegistry.cpp, which made the existing leak:Bun::generateModule entry match nothing and turned test/bundler/transpiler/macro-test.test.ts red on the ASAN lane. The first commit renamed the entry; the second (pushed after the earlier review) tightened it to leak:^Bun::generateInternalModule( and added a comment naming what it covers, why LSAN reports it (JSC cells live in libpas memory that LSAN does not scan as a root region), and the concrete condition under which the entry should be deleted.

Security risks

None. This is a test-infrastructure change to a sanitizer suppressions file; no runtime code, no user-facing surface, no data handling.

Level of scrutiny

Low-to-moderate. REVIEW.md flags "never disable sanitizers or weaken CI verification to get green" and requires a stated reason for suppression edits. This change goes the right direction: it narrows an existing suppression (anchored with ^ and a trailing ( so Bun::generateInternalModuleSourceCode and any future generateInternalModule* siblings are not swept in), documents the exact allocation it covers (the node:fs Binding box owned by a NodeJSFS cell in the never-torn-down macro VM), and states a removal condition. I verified against the codebase that both Bun::generateInternalModule (InternalModuleRegistry.cpp:103) and generateInternalModuleSourceCode (ModuleLoader.cpp:71) exist, so the anchoring is meaningful. I also confirmed compiler-rt's suppression matcher (TemplateMatch) treats a leading ^ as a start-of-string anchor and other characters literally, so the "anchored" claim in the comment is accurate — this was the one point worth checking since the file has no other ^-anchored entries.

Other factors

The comment format mirrors the well-documented entries already in this file (WTF::RunLoop::dispatchAfter, TemporalCore::withTimeZone, TemporalCore::buildCalendarTemplate), which each explain the ownership chain, why LSAN can't see the root, and when to delete. The PR description includes verification (bun bd test with the CI env: 21 pass / 1 fail on the old file, 22 pass on the new) and a culprit bisection across build numbers. The earlier COMMENTED review on this PR preceded the anchoring commit, and that commit's changes (anchor + removal condition) are the plausible response to it; there are no outstanding CHANGES_REQUESTED reviews.