crash_handler: give the musl and android builds their own trace string platform characters

[robobun]

Merge after oven-sh/bun.report#32 is deployed. bun.report rejects the new characters until then.

Problem

• Platform::CURRENT (src/crash_handler/lib.rs) emits 'l'/'L' for every Linux build. The glibc, musl and android builds of a commit are three binaries with three profile zips, and bun.report picks one by this character. Every musl or android crash report is symbolicated against the glibc binary and shows plausible but wrong frames.
• BUN-4MRJ is one: reported as a segfault in NestedRuleParser::parse_block (css_parser.rs:1738), it is a VariableEnvironment lookup on a bad UniquedStringImpl* against the musl profile (notes).

Fix

• musl builds emit 'u'/'U', android builds 'a'/'A', and 'l'/'L' mean glibc. Lowercase is x86_64, uppercase aarch64. bun.report#32 decodes them.
• Shipped binaries keep emitting 'l'/'L', which bun.report keeps decoding as glibc.
• The format is unchanged. crash_handler: put the executable's debug id in the trace string #38838 (a debug id in a new format) verifies the downloaded file instead. The two compose: here the first download is right, and the URL says which build crashed.
• Verified: CI build 101799 ran an assertion on the Alpine x64 and aarch64 lanes that the build emits 'u'/'U'. It passed and was then removed at review, since it only mirrored the table. Also cargo check -p bun_crash_handler for gnu, musl and android, and clippy.

Background

• Trace string: the https://bun.report/<version>/<payload> URL in a crash report. The payload starts with the platform character, then command character, format version and sha. bun.report maps the character to an artifact name.
• The baseline characters ('B', 'b', 'e') did the same for the old baseline binaries until ci: single arm64 debian-13 build host; ThinLTO everywhere; baseline-only x64; rust+link merge; sysroots; WebKit a36c188; rust 2026-07-20 #34782 made x64 one binary.

Notes

The human-readable report header already says musl or Android ... bionic; only the trace string lacked the distinction.

BUN-4MRJ, trace 1.4.0/L_134cbb9aEggggC+98pvDA2Dhggw6jC (commit 34cbb9a40), one address 0x37a79df, fault address 0xFFFFFFFFBC2C0000. llvm-symbolizer --inlines --relative-address against the 1.4.0 release profiles:

• bun-linux-aarch64-profile.zip (what bun.report used): <NestedRuleParser<BundlerAtRuleParser> as AtRuleParser>::parse_block, src/css/css_parser.rs:1738.
• bun-linux-aarch64-musl-profile.zip, fault pc 0x37a79e0: WTF::InlineMap<PackedRefPtr<UniquedStringImpl>, JSC::VariableEnvironmentEntry, 9>::findKeyOrEmpty<const UniquedStringImpl*> (InlineMap.h:725) > findKeyOrEmptyInStorage (InlineMap.h:703) > JSC::IdentifierRepHash::hash (Identifier.h:235) > SymbolImpl::existingSymbolAwareHash > StringImpl::isSymbol (StringImpl.h:334). That is the load of m_hashAndFlags (offset 0x10) through the lookup key, so the key the caller passed was 0xFFFFFFFFBC2BFFF0: a user-space pointer truncated to 32 bits and sign-extended. The scope had more than 9 variables (out-of-line storage). The trace has no caller frame, so the report says nothing more about the crash. It is not a CSS crash. It is almost certainly a musl build: the musl reading accounts for the fault address exactly (key + 0x10), the glibc reading needs a frame pointer that changed between two adjacent loads, and in the android build the address is data (.text ends at 0x32ea060).

BUN-4NQQ is a second 1.4.0 aarch64 report in the same state, and a clearer one: abort() called during bun run, 18 frames, trace 1.4.0/Lr134cbb9aAggggCmrtU2vjI2w/ktB2m42rD+/32rD+s32rD++22rD+r61oEm842oE2k2g1B2vlz3C+l6voCm5i1pCururuCusssuCunjpuCux0zsB2kgI_Aa. Against the glibc profile the frames are unrelated functions (llint_op_instanceof_wide16, bun_md::HtmlRenderer, WTF::PrintStream). Against the musl profile every frame lines up: two frames in the statically linked libc (abort), then WTF::RandomDevice::RandomDevice() (RandomDevice.cpp:95, the crashUnableToOpenURandom() call, and CRASH() is std::abort() on Linux release builds) < cryptographicallyRandomValuesFromOS < ARC4RandomNumberGenerator::stirIfNeeded < cryptographicallyRandomNumber < JSC::VM::VM (VM.cpp:258) < VM::tryCreate < Zig__GlobalObject__create (ZigGlobalObject.cpp:476) < VirtualMachine::init (VirtualMachine.rs:2567) < RunCommand::boot (run_command.rs:954) < boot_and_handle_error < exec_auto_or_run < cli::start < main. So that process could not open /dev/urandom while creating the VM. The platform character was the only thing in the way of reading that.

Which byte values: 'u'/'U' and 'a'/'A' are unused in bun.report's table (w e W m b M l B L f F). The reason and command characters are separate positions, so they do not constrain the choice.

scripts/ci-remap-server pins bun-tracestrings at bun.report 912ca63, which does not decode these characters (nor the 'a'/'b' reasons or FreeBSD from #29 and bun.report#29). A crash on the Alpine CI lanes prints the raw trace string in the annotation instead of a remap until the pin is bumped, as SIGABRT and SIGTRAP crashes do on every lane today. #38838 bumps the pin as part of its landing.

[coderabbitai]

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

• Run review for free

_{On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file.}

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 3 minutes), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f04bfc17-155d-4c80-b60b-ea7427551e5b

📥 Commits

Reviewing files that changed from the base of the PR and between c9585f7 and 7df0644.

📒 Files selected for processing (1)

• src/crash_handler/lib.rs

---

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

[robobun]

^{Updated 3:44 PM PT - Aug 20th, 2026}

✅ @robobun, your commit 7df0644fe4f506232ee6c850c9bf342646659a5d passed in Build #101855! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39801

That installs a local version of the PR into your bun-39801 executable, so you can run:

bun-39801 --bun

[robobun]

Status: ready for review. Merge after oven-sh/bun.report#32 (the decoder) is deployed.

Reproduced: decoded BUN-4MRJ's trace (1.4.0/L_134cbb9aEggggC+98pvDA2Dhggw6jC) and symbolized its one address against the 1.4.0 bun-linux-aarch64-profile.zip and bun-linux-aarch64-musl-profile.zip. The glibc profile gives the css_parser.rs:1738 frame Sentry shows. The musl profile gives a VariableEnvironment hash lookup on a bad UniquedStringImpl*, which matches the fault address. The trace string itself cannot tell the two builds apart, which is what this PR changes.

The diff is now src/crash_handler/lib.rs alone. The test was removed at review. Before its removal, CI build 101799 ran it on the Alpine x64 and aarch64 lanes and it passed, so the musl builds emit 'u' and 'U'.

Landing order:

1. Decode the musl and android platform characters u/U and a/A bun.report#32 merged and deployed.
2. This PR.

[claude]

LGTM — mechanical cfg-matrix split with a matching test. (The author's note about landing after oven-sh/bun.report#32 still applies.)

What was reviewed:

• Platform::CURRENT cfg arms: verified linux+!musl / linux+musl / android are mutually exclusive and exhaustive alongside the existing macos/windows/freebsd arms, and that the old any(linux, android) coverage is fully replaced.
• Test: confirmed isAndroid, isMusl, isFreeBSD, isArm64 all exist in test/harness.ts, and the ternary chain's precedence (android before musl before glibc fallback) matches the Rust cfg split.
• Refactor: traceStringPayloadFromCrash is a pure hoist of the existing helper; commandCharFromCrash still indexes [1] so the existing command-char tests are unchanged.

Extended reasoning...

Overview

This PR splits the crash-report trace-string platform character so musl (u/U) and Android (a/A) builds no longer alias to the glibc characters (l/L). It touches two files: the Platform::CURRENT const cfg matrix in src/crash_handler/lib.rs, and test/cli/run/crash-report-command-char.test.ts, where the crash-and-read-stderr helper is hoisted to file scope and a new describe block asserts the platform character against harness platform flags.

Security risks

None. This changes a single metadata byte in the crash-report URL and adds a test. No user input, no auth, no memory management.

Level of scrutiny

Low-to-medium. The Rust change is a compile-time const derived purely from cfg predicates — either it compiles to exactly one byte literal per target or the build fails. I confirmed the six linux-family arms (glibc/musl/android × x64/aarch64) are pairwise disjoint (Rust's target_os is single-valued, so linux and android never both match; target_env = "musl" partitions the linux arms) and that together with the unchanged macos/windows/freebsd arms they cover every target the previous code covered. The test change is a straightforward extract-helper refactor plus one new assertion; the existing command-char tests still call payload[1] via the wrapper, so their behavior is preserved.

Other factors

• All harness flags the test imports (isAndroid, isMusl, isFreeBSD, isArm64) exist in test/harness.ts. isMusl is gated on isLinux and isAndroid on process.platform === "android", so they're mutually exclusive — the ternary ordering is correct regardless.
• The PR description states the author ran cargo check -p bun_crash_handler for gnu, musl and android targets, and that the new test fails on Alpine lanes with the released bun (satisfying the fails-without-fix requirement).
• The PR carries an explicit merge-after gate on bun.report#32; that's a timing decision the author owns, not a code-review concern.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/crash_handler/lib.rs:2359-2383 — This behavioral change (musl → 'u'/'U', android → 'a'/'A') now ships without a test: commit 0562e6a reverted the test added in 237e133 exactly, so the net diff to test/cli/run/crash-report-command-char.test.ts is zero and only src/crash_handler/lib.rs remains. REVIEW.md requires every behavioral change to ship an automated test in the same PR — restore a non-tautological version (branch on harness isMusl/isAndroid/isArm64, which observe the host at runtime independently of the #[cfg] matrix under test), and update the PR description, which still claims the test exists and "fails on the Alpine lanes with the released bun".

  Extended reasoning...

  What the bug is

  The PR changes observable behavior — musl builds now emit 'u'/'U' and android builds emit 'a'/'A' in the trace-string payload instead of 'l'/'L' — but ships no automated test guarding it. Commit 237e133 added both the src/crash_handler/lib.rs change and a test in test/cli/run/crash-report-command-char.test.ts. Commit 0562e6a ("Drop the platform character test") then reverted the test file exactly, with the rationale "It derives the expected character from the same table it checks." The PR description was not updated and still cites the deleted test as its verification evidence.

  Step-by-step proof

  1. git log --oneline 01c4e2fd..HEAD shows two commits: 237e133 (adds code + test) and 0562e6a (drops test).
  2. git diff 01c4e2fd..HEAD --stat shows the only net change is src/crash_handler/lib.rs | 39 +++++++++++++++++++++++++++------------.
  3. git diff 01c4e2fd..HEAD -- test/cli/run/crash-report-command-char.test.ts produces empty output — the test file is byte-identical to the merge base.
  4. The PR description still says: "Verified: test/cli/run/crash-report-command-char.test.ts asserts the character of the running build from the host's os, libc and cpu. It fails on the Alpine lanes with the released bun and guards the other characters elsewhere." — and the robobun status comment cites Alpine shards passing that test on build 101799 (from before the drop commit). None of this reflects the current diff.

  Why the drop rationale doesn't remove the requirement

  REVIEW.md is explicit: "Every behavioral change ships an automated test in the same PR. 'Verified manually', unnamed 'existing tests', and benchmarks don't count, even for one-liners." If the added test was tautological, the fix is a non-tautological test, not deletion.

  Notably, the dropped test was already close to non-tautological: it derived the expected character from harness helpers — isMusl (test/harness.ts:39, computed at runtime from libcFamily), isAndroid, isArm64, isWindows, isMacOS, isFreeBSD — which observe the host environment at runtime, while Platform::CURRENT is a compile-time #[cfg(target_env = "musl")] constant. Those are two independent sources of the same fact; a mismatch (e.g., a musl-targeted build accidentally compiled without target_env = "musl", or a cfg arm typo) would fail the test. The "same table" concern applies only if the test literally re-imported the Rust constant, which it did not.

  Impact

  • A reviewer reading the PR description will believe the Alpine lanes are guarding 'u'/'U'. They are not — the net diff has no test coverage of the new characters.
  • A future refactor that reorders or typos a #[cfg] arm (e.g., swapping 'u' and 'U', or dropping not(target_env = "musl") from the glibc arm so musl matches both blocks) would ship silently — exactly the class of bug this PR is fixing (wrong debug file → plausible-but-wrong symbolication, per BUN-4MRJ).
  • The PR body's own verification claim is now false, which REVIEW.md's testing rules exist to prevent ("'Verified manually' … doesn't count").

  How to fix

  Restore the platform-character test, deriving the expected value from the harness host-observation helpers rather than any mirrored table:

  import { isWindows, isMacOS, isFreeBSD, isAndroid, isMusl, isArm64 } from "harness";
  
  const lower = isWindows ? "w" : isMacOS ? "m" : isFreeBSD ? "f" : isAndroid ? "a" : isMusl ? "u" : "l";
  const expected = isArm64 ? lower.toUpperCase() : lower;
  
  test(`encodes this build as '${expected}'`, async () => {
    const payload = await traceStringPayloadFromCrash([fixture, "panic"]);
    expect(payload[0]).toBe(expected);
  });

  This is exactly what 237e133 added. If the concern is that the ?: chain visually mirrors the cfg matrix, add a comment noting the two derivations are independent (runtime host detection vs. compile-time target cfg) — but the test itself is sound and satisfies the CLAUDE.md "fails with USE_SYSTEM_BUN=1 on Alpine, passes with bun bd" criterion. Also update the PR description and Notes section to drop the stale references to test-file header edits.

[robobun]

On the missing test: the assertion in 237e133 was removed in 0562e6a at dylan-conway's request. It derived the expected character from the host the same way the cfg matrix does, and he judged it not worth keeping. Before its removal it ran on the Alpine x64 and aarch64 lanes in build 101799 and passed, which is the evidence that the musl builds emit 'u' and 'U'. The description's Verified bullet says this now. If a test is wanted after all, restoring that commit is the whole change.

[claude]

I reviewed this PR and didn't find any bugs — the cfg matrix is mutually exclusive and covers glibc/musl/android × x64/aarch64 correctly. Deferring only because merge is gated on oven-sh/bun.report#32 being deployed, which needs a human to confirm before landing.

What was reviewed:

• The Platform::CURRENT cfg arms are disjoint (Rust treats target_os = "android" as distinct from "linux", and target_env = "musl" / not(musl) partition Linux), so exactly one block compiles per target.
• No other consumer of these character values exists in-tree; the only reader is bun.report.
• The removed comment ("Android folds into the Linux variants … would break older decoders") is correctly superseded — the paired decoder change makes it obsolete.

Extended reasoning...

Overview

This PR changes a single compile-time constant, Platform::CURRENT, in src/crash_handler/lib.rs. Previously, glibc, musl, and Android Linux builds all emitted 'l'/'L' in the crash-report trace string, causing bun.report to symbolicate musl/android crashes against the glibc debug binary. The fix splits the cfg matrix so musl emits 'u'/'U' and Android emits 'a'/'A', while glibc keeps 'l'/'L'. The doc comment is updated to describe the full character set.

Security risks

None. This is a compile-time byte constant embedded in crash-report URLs; no user input, no runtime branching, no memory or auth surface.

Level of scrutiny

Low for the code itself — it is a static lookup table expressed as #[cfg]-gated const blocks. I verified the arms are mutually exclusive and exhaustive over Bun's supported targets: target_os = "android" is a distinct value from "linux" in rustc, so the new android arms cannot overlap the linux arms; target_env = "musl" vs not(target_env = "musl") cleanly partitions Linux (glibc has target_env = "gnu"). macOS/Windows/FreeBSD arms are untouched. cargo check across gnu/musl/android was reported in the PR, and CI build 101799 confirmed the Alpine lanes emit 'u'/'U'.

Other factors

The reason I'm not approving outright is the explicit cross-repo landing dependency: the PR description and status comment both say "merge after oven-sh/bun.report#32 is deployed," because bun.report will reject the new characters until then. That is a human coordination step — someone needs to confirm the decoder is live before hitting merge. An automated approval could invite a premature merge that breaks crash-report ingestion for musl/android builds. The test that asserted the emitted character was intentionally removed at review as tautological, and the comment-cop threads on the doc comment were addressed in 7df0644 and resolved.