build(linux-musl): statically link libstdc++/libgcc - #29683
Conversation
|
Updated 11:38 AM PT - May 4th, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 29683That installs a local version of the PR into your bun-29683 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
I didn't find any bugs, but this reverses an explicit linkage decision from #15186 and changes the runtime dependency surface of the linux-musl release binary — worth a human confirming the history and an ldd check on the CI-built musl artifact before merging.
Extended reasoning...
Overview
This PR collapses two linker-flag table entries in scripts/build/flags.ts so that -static-libstdc++ -static-libgcc is emitted for all Linux targets (previously: glibc only; musl got -lstdc++ -lgcc dynamically). It adds a regression test that evaluates the linkerFlags table for musl/gnu × x64/aarch64 and asserts the static flags are present and the dynamic ones are absent.
Security risks
None identified. This is a build-time linker-flag change with no auth, crypto, input-handling, or permission implications. Statically linking libstdc++/libgcc is standard practice for portable Linux binaries and is what the glibc build already does.
Level of scrutiny
Moderate-to-high. The diff is small and mechanically correct, but it deliberately reverses a choice made in PR #15186, whose comment claimed "static unavailable" on musl. The PR description argues convincingly that the comment was wrong (Alpine build-base ships the .a archives), but a maintainer should sanity-check why #15186 went dynamic in the first place — there may have been a toolchain-specific reason at the time that no longer applies, or one that still does. This change alters the runtime dependency surface of an official release artifact, so the real validation is the CI musl build succeeding plus an ldd showing no libstdc++.so.6/libgcc_s.so.1, which I can't verify here.
Other factors
- The added regression test guards the flag-table edit but, as the author notes, the harness's stash-based gate (
src/ packages/only) won't toggle it — so it passes either way in the gate. It's still useful as a forward guard. - The Buildkite build (#47729) is still in progress per the timeline; the musl link step succeeding is the key signal.
- The accompanying code comment is detailed and accurate; the test's
FakeConfigshape covers all fields thelinkerFlagspredicates touch. - No CODEOWNERS cover
scripts/build/.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughLinux C++ runtime linker flags were changed to always use static C++ runtime flags on Linux; comments and descriptions about Alpine toolchains were added. A regression test was added to validate resolved Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/regression/issue/29681.test.ts`:
- Around line 39-65: The fake config returned by makeLinuxConfig is missing the
canonical keys used by real Config (add the canonical os and arch fields), so
update the object in makeLinuxConfig to include os: "linux" and arch: arch
(mapping "x64" | "aarch64" to the real arch string) so tests exercise lookups
like linkerFlags.when(c => c.os === ... || c.arch === ...); ensure the added
properties match the types expected by FakeConfig.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba548e29-e3b8-488f-8da4-a0fd0d6fd66d
📒 Files selected for processing (1)
test/regression/issue/29681.test.ts
There was a problem hiding this comment.
The reasoning looks sound and the diff is tiny, but this reverts a deliberate (if likely mistaken) choice from #15186 and changes how the shipped musl release binary links its C++ runtime — worth a maintainer's eyes and a green musl CI build before merging.
Extended reasoning...
Overview
This PR collapses two entries in the linkerFlags table in scripts/build/flags.ts so that -static-libstdc++ -static-libgcc is emitted for all Linux targets, dropping the musl-only -lstdc++ -lgcc dynamic-link branch. It adds test/regression/issue/29681.test.ts, which source-parses the linkerFlags array literal and asserts the static flags are present (and dynamic ones absent) for {gnu, musl} × {x64, aarch64}.
Security risks
None identified. No auth, crypto, network, or user-input handling. The change only affects which form of the C++/gcc runtime is linked into the musl binary.
Level of scrutiny
Higher than the line count suggests. The functional edit is one predicate (c.linux && c.abi !== "musl" → c.linux) plus deletion of the dynamic-link entry, but it alters how the production musl release artifact is linked. The branch being removed was introduced intentionally in #15186 with a "static unavailable" rationale; the PR argues convincingly that the rationale was wrong (Alpine build-base ships libstdc++.a/libgcc.a), but that's a toolchain claim best confirmed by the actual musl CI build + ldd check the description references — and Buildkite #47731 was still running at review time. A maintainer familiar with the musl build environment should confirm there wasn't some other constraint (binary size, EH/unwind interaction, cross-toolchain availability) behind the original split.
Other factors
- The regression test guards the flag table but, as the author notes, doesn't exercise an actual link; it's a guard against future regressions in
flags.ts, not proof the musl link succeeds. Its source-parse +new Functioneval approach is a bit unusual but self-contained. - No CODEOWNERS entry covers
scripts/build/. - The one inline finding is a follow-up nit (drop
apk add libgcc libstdc++fromdockerhub/alpine/Dockerfileafter a release ships this fix) and is explicitly not actionable in this PR. - glibc builds are unchanged (they already used
-static-libstdc++ -static-libgcc), so blast radius is limited to the musl artifacts.
There was a problem hiding this comment.
No issues found in the diff, but this changes how the official musl release binary is linked and reverses a deliberate choice from #15186 — worth a human confirming the original "static unavailable" rationale is indeed obsolete and that the Alpine CI link/ldd actually comes back clean.
Extended reasoning...
Overview
The functional change is a 2-line edit in scripts/build/flags.ts: the musl-specific -lstdc++ -lgcc linker rule is dropped and the existing -static-libstdc++ -static-libgcc rule's predicate is widened from c.linux && c.abi !== "musl" to c.linux. The rest is a well-documented in-code comment, a new regression test that source-parses the linkerFlags table to assert the flags, and an unrelated ASAN skip in test/expectations.txt.
Security risks
None. This is build/link-flag configuration; no auth, crypto, input handling, or runtime code paths are touched. The regression test uses new Function(...) on a slice of the repo's own flags.ts — that's controlled input, not a security boundary.
Level of scrutiny
High, despite the small diff. This governs how the shipped bun-linux-*-musl artifacts are linked, and it reverses a branch that PR #15186 added intentionally (with a "static unavailable" comment). The PR's argument that the comment was wrong is well-reasoned and matches Alpine's toolchain reality, but the original author of #15186 may have had a build-environment constraint that isn't captured here. The PR description itself defers final verification to "once CI ships the next musl artifacts" — i.e., the regression test only proves the flag table emits the right strings, not that the musl link actually succeeds or that the resulting binary runs cleanly on bare Alpine.
Other factors
The glibc path is unchanged (it already used these flags), so blast radius is limited to musl builds. The added test is a reasonable guard against silent reintroduction. My earlier inline comment about the dockerhub/alpine/Dockerfile cleanup is a post-release follow-up, not a blocker. Given this affects release artifacts and undoes a prior explicit decision, a human with context on #15186 and visibility into the musl CI build should sign off rather than auto-approval.
There was a problem hiding this comment.
No issues found in the diff itself, but this flips release linker flags for all linux-musl artifacts (reversing #15186's deliberate choice) and touches test/expectations.txt which is CODEOWNERS-gated — worth a human sign-off, ideally after eyeballing ldd on the CI musl artifact.
Extended reasoning...
Overview
This PR collapses the musl-specific -lstdc++ -lgcc linker rule into the existing -static-libstdc++ -static-libgcc rule so it applies to all Linux ABIs (scripts/build/flags.ts). Supporting changes: an allowlist entry for __arm_tpidr2_save in scripts/verify-baseline-static/allowlist-aarch64.txt (newly pulled in from Alpine's statically-linked libgcc), an ASAN skip in test/expectations.txt, and a regression test that source-parses linkerFlags and asserts the static flags are present for {gnu,musl}×{x64,aarch64}.
Security risks
None. This is a build-time linker-flag change; no runtime auth, crypto, input handling, or user-controlled data paths are touched. Statically linking libstdc++/libgcc is the same posture the glibc build already uses.
Level of scrutiny
High. The semantic diff is two lines, but it changes how every shipped linux-musl release binary is linked. The branch being removed was intentionally added in PR #15186 with a (likely wrong) "static unavailable" rationale — a human should confirm there wasn't a secondary reason behind that choice. The PR description's end-to-end check (ldd bun on the next CI musl artifact) hasn't been confirmed in-thread yet. The new __arm_tpidr2_save allowlist entry is well-justified but is exactly the kind of baseline-verification exception that benefits from a second pair of eyes.
Other factors
test/expectations.txtis owned by a CODEOWNER per.github/CODEOWNERS, so the ASAN-skip addition needs owner review regardless.- The added ASAN skip (
build-prefetch/prefetch.test.ts) papers over a JSC assertion in the build-script import graph rather than fixing it; that's a reasonable trade-off but a human should agree. - The regression test is thorough but uses a hand-rolled bracket-matching parser +
new Functioneval of a slice offlags.ts; it works for the current file shape but is somewhat fragile to future edits (e.g. block comments or template-literal interpolation inside the array). Not a blocker. - I previously left an inline note about the follow-up
dockerhub/alpine/Dockerfilecleanup; nothing further to add there.
7cd2eb2 to
ad16527
Compare
There was a problem hiding this comment.
No bugs found, but this changes how the official musl release binaries are linked (reverting a deliberate choice from #15186) and adds a baseline-static allowlist entry — worth a human sanity-check that the original reason for dynamic linking no longer applies and that __arm_tpidr2_save is genuinely unreachable on armv8-a baseline.
Extended reasoning...
Overview
Four files: scripts/build/flags.ts collapses the musl-specific -lstdc++ -lgcc linker rule into the existing -static-libstdc++ -static-libgcc rule so it fires for all Linux ABIs; scripts/verify-baseline-static/allowlist-aarch64.txt adds __arm_tpidr2_save (a libgcc SME helper now pulled in by Alpine's newer static libgcc); test/expectations.txt skips a build-prefetch test under ASAN; and a new regression test source-parses the linkerFlags table to assert the static flags are present for {gnu,musl}×{x64,aarch64}.
Security risks
None. No auth, crypto, network, or user-input handling. The only runtime-adjacent concern is the baseline-static allowlist entry — allowlisting a symbol containing SVE instructions is safe only if it's truly unreachable on armv8-a CPUs. The in-file comment argues it is (SME streaming-mode prologue helper, never emitted by armv8-a code), which is plausible, but that's a toolchain-ABI claim I can't verify mechanically.
Level of scrutiny
Medium-high. The flags.ts edit is tiny and the rationale is well-argued, but it directly changes how every musl release artifact is linked. PR #15186 (Nov 2024) deliberately switched musl to dynamic libstdc++/libgcc with a "static unavailable" note — this PR asserts that note was simply wrong, which is likely true (Alpine build-base does ship the .a files), but a maintainer who remembers the #15186 context should confirm there wasn't a different motivation (e.g. a since-fixed toolchain bug). The end-to-end proof is the CI musl artifact's ldd output, which isn't visible from the diff alone.
Other factors
The regression test is clever (transpiles the array literal in isolation to dodge a debug-loader bug) but is a config-shape test, not an end-to-end link test — it would not catch a link failure or runtime issue from static libstdc++ on musl. The ASAN expectations addition and the allowlist entry are both CI fix-ups stemming from this change rather than independent edits. I already left an inline note about the follow-up Dockerfile cleanup; nothing further to add there.
ad16527 to
0aedc1d
Compare
8a3257d to
755f4ad
Compare
Before, bun-linux-*-musl linked libstdc++.so.6 and libgcc_s.so.1 dynamically and refused to launch on a clean Alpine image with 'Error loading shared library libstdc++.so.6' until `apk add libstdc++ libgcc` was run (#29681). PR #15186 had added that dynamic branch; earlier Bun releases linked them statically. Alpine's build-base ships libstdc++.a / libgcc.a / libgcc_eh.a via the g++ / gcc packages, so static linking works on musl just like it does on glibc. Collapse the two rules into one -static-libstdc++ -static-libgcc for all Linux targets. Bun still links musl libc dynamically (via /lib/ld-musl-*.so.1), so dynamic TLS keeps working for mimalloc etc. Closes #29681
…der bug
bun bd test was crashing on 'import { linkerFlags } from ../../../scripts/build/flags.ts'
with a JSC assertion in JSGlobalObject::moduleEvaluation (dependency.isAsync).
The transitive config.ts import graph trips a module-loader bug in the debug
build; release bun imports the same chain fine.
Read flags.ts from disk, extract the linkerFlags array literal, transpile TS
to JS, and eval it in a stubbed Function scope. Same assertions as before;
passes on bun bd and USE_SYSTEM_BUN=1 bun test.
755f4ad to
6c7d31d
Compare
…l [skip size check] Alpine's libgcc ships `__arm_tpidr2_save` — the AAPCS helper that saves TPIDR2_EL0 when a caller is invoked in SME streaming mode or has live ZA state. The function body contains 2 SVE instructions but is unreachable on armv8-a+crc baseline CPUs: SME/ZA callers don't exist there, so the helper is never entered. Ubuntu's libgcc doesn't ship the helper yet, so the glibc verify-baseline step on the same commit passes — but musl-aarch64 started failing once this PR switched it from `-lgcc` to `-static-libgcc`. Matches the pattern of the libgcc DWARF unwinder SVE entries above: runtime-unreachable on baseline hardware, statically-present in the archive. The musl binaries also grow by ~500 KB (static libstdc++ + libgcc + SME helper). That's the whole point of this PR — the binary is now self-contained on Alpine without `apk add libstdc++ libgcc`. Tag the commit with `[skip size check]` so the binary-size step doesn't reject the intentional growth.
6c7d31d to
410aa5e
Compare
|
Dropped the |
|
I rechecked this on today's main canary ( I also tested the x64-musl CI artifact for the current PR head ( The PR artifact has only However, it also reproduces the native-addon compatibility regression that #15186 was intended to avoid. With both Preloading So this is a concrete compatibility regression, rather than only an ELF-level concern. @Jarred-Sumner, should preserving the musl native-addon behavior introduced by #15186 be a merge requirement here? If so, I’m happy to prepare the regression coverage and a compatibility-preserving update. If you no longer plan to rebase this branch, I can instead prepare an attributed successor on current main that preserves this behavior and drops the part already landed through #34782. Separately, the |
|
Thanks for the thorough testing, this is exactly the kind of end-to-end verification this change needed, and the addon finding is a real regression I had not covered. I can confirm the mechanism from the link configuration: bun's Linux link uses a version script ( The compatibility-preserving shape would be to keep Since you have the repro infrastructure already set up, I think an attributed successor on current main is the cleaner path, especially as this branch needs another rebase across the src/ restructure anyway, and the |
|
Thanks, that gives me a clear next step, and I’ll make sure #29683 is credited. One detail I want to test first: the current artifact does not contain I’ll try that on current main branch. If the canvas repro passes and the export/size impact looks reasonable, I’ll open an attributed draft successor and link it here. |
|
Sounds good. Your read on the missing Node solves exactly this with the whole-archive trick: it wraps libstdc++.a in Happy to review the successor when it's up. This branch stays as-is until then. |
|
Thanks again for handing this off. I opened #38152 as the current-main successor and credited #29683 in the description. I ended up taking a different compatibility route from exporting or whole-archiving Bun's embedded C++ runtime: Bun stays statically linked for clean Alpine startup, then makes one best-effort The PR has native x64 coverage for startup, legacy canvas, self-contained addons, new/delete, real exception/unwind, and provider-absent behavior, plus a production aarch64 build and supplemental QEMU checks. I would especially appreciate review of the loader-policy choice. If the direction looks good, feel free to close this in favor of the successor. |
|
#38152 looks like the right home for this work, thanks for carrying it over the finish line with the addon coverage this branch lacked. On the loader-policy choice, as handoff context for reviewers rather than a review: the lazy Closing this PR in favor of #38152. The regression test and flag-table change here are superseded by the equivalents there. |

Fixes #29681.
Repro
Cause
scripts/build/flags.tsexplicitly opts the musl build out of-static-libstdc++ -static-libgcc:The
static unavailablecomment is wrong. PR #15186 (Nov 2024) introduced this branch; earlier Bun releases linked these statically. Alpine'sbuild-baseshipslibstdc++.a/libgcc.a/libgcc_eh.aviag++/gcc, so static linking works on musl just like it does on glibc — and it's what theoven/bun:alpineimage has papered over withapk add libgcc libstdc++ever since.Fix
Collapse the two rules into one
-static-libstdc++ -static-libgccrule for all Linux targets. Bun still links musl libc dynamically (via/lib/ld-musl-*.so.1), so dynamic TLS keeps working for mimalloc etc. Scope matches the reported bug — this narrowslibstdc++.so.6/libgcc_s.so.1dependencies only, no attempt at a fully-static-staticbuild (#27375 is a separate, larger concern).Verification
test/regression/issue/29681.test.tsevaluateslinkerFlagsfor musl/gnu × x64/aarch64 configurations and asserts-static-libstdc++/-static-libgccare present and-lstdc++/-lgccare not. Passes with the fix; reintroducing the musl dynamic branch fails the two musl cases.End-to-end confirmation once CI ships the next musl artifacts:
ldd bunonbun-linux-x64-muslshould show only/lib/ld-musl-x86_64.so.1+libc.musl-*.so.1, withlibstdc++.so.6/libgcc_s.so.1gone.