ci: trace the linux-aarch64 symbol order file on every main build - #37711
ci: trace the linux-aarch64 symbol order file on every main build#37711robobun wants to merge 1 commit into
Conversation
|
Important Review skippedThis review includes 5 billable files. This on-demand review is free during your promotion. Your included review limit has been reached. Run
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
You can disable this status message by setting the Comment |
9f89563 to
ef08395
Compare
|
Updated 12:46 AM PT - Aug 19th, 2026
✅ @robobun, your commit 89a3d6e0eb3ff5bd4390221a3d597288ff95f0aa passed in 🧪 To try this PR locally: bunx bun-pr 37711That installs a local version of the PR into your bun-37711 --bun |
|
Status: rebased onto main (89a3d6e) at the maintainer's request; the trace-order steps on build 101120 all passed, the rest of the build is still running.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this and didn't find any bugs. Since it changes how the order-file artifact is published for every target (removing the build-lane publisher and the canary self-bootstrap fallback), and the PR's own [generate symbol order] verification build is still pending, a human look at the transition plan would be worthwhile.
What was reviewed:
usesOrderFile()targets (linux-gnu x64/aarch64, darwin-arm64) are exactly the three now intraceOrderTargets, so no target loses its only publisher.traceOrderStepKey()output matchesgetTraceOrderStep's${getTargetKey(target)}-trace-orderfor all three targets.- No dangling references to the deleted
mustGenerateOrderFile/reportOrderFileBootstrap/reportOrderFileCannotTraceexports; the onecanTraceOrderFilemention in flags.ts is a comment and the function still exists (now unexported). - The build.ts control-flow change drops
verifyOrderFileAppliedonly when!inherited— where the file is the empty placeholder anyway.
Extended reasoning...
Overview
Adds a linux-aarch64-trace-order step to .buildkite/ci.mjs (mirroring the existing darwin-aarch64 and linux-x64 steps), makes the trace-order step the sole publisher of the standalone .order artifact by removing the build-lane upload from packageAndUpload(), and deletes the canary self-bootstrap path (mustGenerateOrderFile, reportOrderFileBootstrap) since the trace-order step now seeds the chain. reportOrderFileCannotTrace is generalized to reportOrderFileNotInherited. Tests and comments updated to match; generate.ts change is comment-only.
Security risks
None. This is CI pipeline generation and build-script orchestration; no runtime code, auth, crypto, or user-input parsing is touched.
Level of scrutiny
Moderate. It's build infrastructure, not shipped runtime, and the order file is a perf optimization (a broken chain ships a correct-but-unordered binary with an annotation). But it does change the artifact-publishing contract for every ordered target and removes a self-healing fallback, so the transition needs to be right — the PR description reasons through it (first post-merge main build inherits the old file while its trace step publishes fresh; second inherits fresh), and I couldn't find a hole in it.
Other factors
I verified usesOrderFile() in flags.ts returns true for exactly linux-gnu (both arches) and darwin-arm64, matching the three traceOrderTargets entries — so no target that consumes an order file is left without a publisher. The deleted exports have no remaining callers (grep confirms; flags.ts has one comment mentioning canTraceOrderFile, which still exists locally). The regenerateOrderFile "why" simplification is correct now that the "nothing to inherit" reason is gone. The status comment on the PR notes the opt-in verification build (92745) is still running and the trace-order steps are soft-fail, so their outcome won't show in PR checks — worth confirming before merge.
|
The verification build has now run the part that matters: in Buildkite build 92745, On the transition: the first main build after this merges still inherits the old file, since |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes the order-file publishing model (trace-order becomes the sole publisher, and the canary bootstrap-trace-on-no-inherit path is removed), a human look at the design tradeoff and the transition on the first post-merge main build would be worthwhile.
What was reviewed:
traceOrderTargetsnow covers all threeusesOrderFile()targets; the new debian-13 aarch64 entry reuses an existing test-fleet image/agent.packageAndUpload()no longer uploads the standalone.order, soinheritOrderFile()(no--step) sees only the trace-order step's artifact — no stale/fresh race.- Removed exports (
mustGenerateOrderFile,reportOrderFileBootstrap,canTraceOrderFilede-exported) have no remaining importers;traceOrderStepKey()matchesgetTargetKey() + '-trace-order'. - Checked the
build.tscontrol-flow rewrite:verifyOrderFileAppliednow only runs wheninheritedis true, which is correct since an un-inherited build has only the empty placeholder.
Extended reasoning...
Overview
This PR adds linux-aarch64 to traceOrderTargets in .buildkite/ci.mjs so it gets a -trace-order step like linux-x64 and darwin-aarch64 already have, and reworks the order-file publishing model so the trace-order step is the sole publisher for every target. It removes the canary bootstrap path (a native-host canary that inherited nothing used to trace + relink itself to seed the chain), deletes the now-dead mustGenerateOrderFile/reportOrderFileBootstrap, drops the unused ctx param from regenerateOrderFile, renames/generalizes the not-inherited annotation, stops packageAndUpload() uploading a standalone .order, and updates the tests and comments accordingly.
Security risks
None. This is Buildkite pipeline generation and build-script orchestration only; no runtime code, no user input parsing, no auth/crypto. The trace-order step is soft-fail and the order file is a link-time optimization — worst case is an unordered (larger-RSS but correct) binary.
Level of scrutiny
Moderate. It doesn't touch shipped runtime code, but it does change how canary/release build lanes behave on main (removing a fallback relink and changing which step publishes the inherited artifact). The diagnosis is well-documented with concrete build numbers, and the failure modes are all annotate-and-continue. Still, this is a design change to CI behavior — dropping the bootstrap relink trades ~13 min of critical-path link time for a one-build unordered window on a cold chain — which is the kind of tradeoff a maintainer should sign off on.
Other factors
- Verified no remaining importers of the removed/de-exported symbols (
mustGenerateOrderFile,reportOrderFileBootstrap,reportOrderFileCannotTrace, exportedcanTraceOrderFile); the only remainingcanTraceOrderFilereference outsideci.tsis a doc comment inflags.ts. traceOrderStepKey(cfg)derives<target>-trace-orderfromcomputeBunTriplet(cfg)and matchesgetTargetKey(target) + '-trace-order'inci.mjsfor all three eligible targets.- The new
traceOrderTargetslinux-aarch64onplatform is identical to the existing debian-13 aarch64testPlatformsentry, sogetTestAgent/image resolution is already exercised. - The test file removes assertions about deleted behavior and adds a comment documenting that a native canary lane still doesn't self-trace, which matches the new model.
- The PR's own push build carries the
[generate symbol order]opt-in tag so all three trace-order steps (including the new one) run on this build; results are still pending in the status comment.
|
Same conclusions as the first pass, so just the CI state for whoever looks at this: build 92745 is at 190 passed with two red jobs, both Windows jobs cancelled before an agent was assigned (the other 19 Windows jobs passed, and this diff does not reach the Windows lanes). |
### Problem - `darwin-aarch64` and `linux-x64` builds link with a traced symbol order file; Windows release builds link without one, so `windows-x64` and `windows-aarch64` miss the startup win. - The existing tracer is an `LD_PRELOAD` library, which has no Windows counterpart, and a Windows executable has no symbol table to map traced addresses back to names. - Related: #37711, #35328. ### Fix - Windows release links take `/order:@linker.order` (plus `/ignore:4037` for traced names that no longer exist), and both windows targets get a `trace-order` step on their existing test images. The generated pipeline differs by exactly those two steps. - A new tracer runs the binary as its debuggee: with the process stopped at creation it writes a breakpoint over every function start; each hit restores the instruction, records the address and resumes there. Every function traps once and then runs unmodified, and the trace file format is the unix tracer's. - Names come from two maps the release link now writes into the profile zip: `/map` for names, `/lldmap` for where each input chunk landed. Only names at chunk starts count as functions, because the MSVC CRT leaves labels (`Table`, `$LN123`) on data inside `.text` and a breakpoint there killed the debuggee. Chunk starts are exactly what `/order` can move, so nothing orderable is lost. - Verification: the linker-order test covers the flag and map wiring, both map parsers against fixtures, and on windows the tracer itself against a C fixture (exact entry order, exit code, argv, ConPTY stdio). The generator was also run by hand on Server 2019 x64 and Windows 11 arm64 boxes, all eight workloads under the real tracer, and this PR's own build runs all four trace-order steps on the real binaries. ### Background - Symbol order file: one function name per line; the linker lays those out first in `.text` so startup-hot code shares pages. Generated in CI, never committed. lld takes `--symbol-ordering-file`, Apple ld `-order_file`, lld-link `/order:@file`. - trace-order step: a soft-fail, non-PR CI step that downloads the build lane's unstripped `bun-profile`, runs a few workloads under a tracer that records first-entry order, and uploads the `.order` file the next build links with. - `/order` can only move a function that is in its own COMDAT section. Bun's C++ and the WebKit prebuilt use `/Gy`, rustc emits function sections, and LTO output is per-function, so all of bun's code already qualifies. - ELF and Mach-O binaries carry a symbol table that `nm` reads. A PE keeps its names in the PDB, so on Windows the linker's map files stand in as the address-to-name source. - A Windows debugger gets its debuggee's creation event before any of it has run, may write its memory, and receives every breakpoint exception, so nothing has to be loaded into the traced process. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test-only change; deferring to CI. <!-- robobun:evidence:end --> <details> <summary>Original description</summary> ## What `darwin-aarch64` and `linux-x64` have a `-trace-order` step that traces the freshly built `bun-profile` on the native test fleet and publishes the symbol order file the next build links with. Windows had none of this: `usesOrderFile()` was false there, and the tracer is an `LD_PRELOAD` library. This adds it for `windows-x64` and `windows-aarch64`. - **Link** (`scripts/build/flags.ts`): windows release links get `/order:@<buildDir>/linker.order` plus `/ignore:4037` (the `--no-warn-symbol-ordering` counterpart), and `linker.order` becomes a link dependency so an inherited file relinks. Every function is its own COMDAT already (`/Gy` for bun's C++ and the WebKit prebuilt, function sections for rustc, per-function LTO output), which is what `/order` needs; checked against lld-link 21 that it orders statics and ThinLTO-internalized functions too. - **Tracer** (`scripts/orderfile/functrace-windows.c`): a debugger rather than an injected library. It starts the binary as its debuggee, writes a breakpoint (x64 `INT3`, arm64 `BRK #0xF000`) over every function start while the process is stopped at its creation event, and on each breakpoint exception restores the instruction, records the address and resumes the thread there. Everything else (the loader's initial breakpoint, exceptions the program handles itself) is passed through. With `BUN_FUNCTRACE_TTY` it starts the debuggee on a ConPTY and types our stdin into it, standing in for `ptyrun.c`. Same trace file format as `functrace.c`. Built on the test VM by `generate.ts` with clang-cl (on the images' machine PATH) or cl. - **Symbols** (`scripts/orderfile/windows-symbols.ts`): a PE has no symbol table, so the windows release link writes two maps that ship in the profile zip next to `bun-profile.pdb`: lld-link's MSVC-style `/map` (`bun-profile.map`, every symbol under the exact name `/order` takes) and `/lldmap` (`bun-profile.linker-map`, where every input chunk was placed). Only names at chunk starts are treated as functions. This turned out to be necessary, not cosmetic: the MSVC CRT defines symbols on data it keeps in `.text`. The arm64 `memset` keeps the byte table for its computed branch there under a symbol named `Table`, and MSVC-compiled code leaves a `$LN123` symbol on every slot of the jump tables it emits after a function. A breakpoint written over those corrupts the table and the debuggee died in the first small `memset` / in `printf`. Chunk starts are also exactly the set `/order` can move, so the filter loses nothing orderable. Cost: roughly 10 to 15 MB more per compressed windows profile zip (the linux map compresses about 10x). - **CI** (`.buildkite/ci.mjs`): `windows-x64` and `windows-aarch64` join `traceOrderTargets`, tracing on the existing Server 2019 x64 and Windows 11 arm64 test images. The step runs under cmd.exe through `vs-shell.ps1`, like the test runner. Generating the pipeline as main before and after this change differs by exactly those two steps. - `linkerMapPath` moves from `rust.ts` to `flags.ts` next to the new `symbolMapPath`/`linkerMapOutputs`; `bun.ts` declares the maps to ninja and `ci.ts` packs them from that one list. `verifyOrderFileApplied()` now reads symbols through the same `readTextSymbols()` the generator uses, so it works on the windows build lanes too. ## Verification `test/js/bun/perf/linker-order.test.ts` (26 pass / 4 platform skips on each of linux x64, windows x64 and windows arm64): - flag, link-dependency and map-output wiring for both windows targets, and that the maps are written exactly where `linkerMapOutputs()` declares them; - parsers for both maps against fixtures shaped like real output (long names, folded functions, absolute symbols, demangled lld-map names, `Table`/`$LN` labels being dropped); - the tracer itself, windows only: `functrace-fixture.c` (now also compiled with clang-cl; it links the static CRT, so it crashes on arm64 without the chunk filter) must record `f0..f31` in order, `main`, and `after` as distinct first entries with a child process spawned in between; the debuggee's exit code and argv come through and a starts file for another binary is refused; and a C probe reports console stdio 80 columns wide and the typed line under `BUN_FUNCTRACE_TTY`, pipe stdio otherwise. The unix tracer test now makes the same exact-order assertions. Also run by hand on the two windows boxes (Server 2019 x64, Windows 11 24H2 arm64, both bootstrapped like the CI images): the whole of `generate.ts` against the installed `bun.exe` with maps faked down to its entry point, so all eight workloads run under the real tracer, ConPTY one included (`cli-fixture.js` reports `tty=true 80x24` and exits on `quit`); and a 20k-function program to size the cost, about 65 to 70 us per breakpoint hit on both, so a full trace of bun is a few seconds. The unix path was re-run against a linux release build (13973 functions). The commit message carries the opt-in tag `ci.mjs` honours outside main, so this PR's own build runs all four trace-order steps; the two windows ones are the end-to-end check on the real binaries and maps. The tag is deliberately not in this title or body. Related: #37711 (linux-aarch64 step), #35328. </details>
|
@robobun fix conflicts |
…nerate symbol order] Add linux-aarch64 to traceOrderTargets, so it gets the same trace-order step darwin-aarch64 and linux-x64 already have. Its build lane runs on the aarch64 host and so was left to publish its own file, but a canary lane only inherits, so it re-published the same file on every build and the file was only refreshed by a release or an opt-in trace. With the trace-order step publishing for every target, the build lane no longer uploads the standalone .order artifact (it would race the fresh trace under the same name), and a canary that inherits nothing links unordered and annotates instead of tracing and relinking itself, which removes mustGenerateOrderFile and reportOrderFileBootstrap. The cross-compile-specific annotation becomes reportOrderFileNotInherited and names the step that should have published the file.
ef08395 to
89a3d6e
Compare
|
Rebased onto main (89a3d6e). The only textual conflict was the two doc comments in Re-verified on the rebased tree: generating the main pipeline from origin/main and from this branch still differs by exactly the one |
There was a problem hiding this comment.
LGTM — CI-only change adding a soft-fail linux-aarch64-trace-order step and making trace-order steps the sole publisher of .order artifacts.
What was reviewed:
- Verified the new
traceOrderTargetsentry resolves to a valid test image (getTestAgent/getImageKeyfor linux-aarch64 debian-13 →c8g.xlarge, same image key as the existing test lane). - Checked that
traceOrderStepKey(cfg)matchesgetTraceOrderStep's key format (computeBunTripletminusbun-prefix ≡getTargetKeyfor the targets in the list). - Confirmed no dangling references to the deleted/de-exported
mustGenerateOrderFile,reportOrderFileBootstrap,reportOrderFileCannotTrace,canTraceOrderFile(only a comment in flags.ts mentions the last, and the function still exists locally). - The
!inheritedbranch in build.ts no longer callsverifyOrderFileApplied— fine, since with no inherited file the seeded placeholder has 0 functions and verify returns early anyway.
Extended reasoning...
Overview
This PR fixes a stale symbol order file for linux-aarch64 canary builds by adding it to traceOrderTargets in .buildkite/ci.mjs (one new soft-fail step on main), and simplifies the publisher model so trace-order steps are the only source of the standalone .order artifact. In scripts/build/ci.ts it removes the build lane's standalone .order upload from packageAndUpload, deletes mustGenerateOrderFile/reportOrderFileBootstrap (the self-trace-and-relink fallback), renames reportOrderFileCannotTrace→reportOrderFileNotInherited with a message that names the responsible step, and drops the now-unused ctx param from regenerateOrderFile. scripts/build.ts updates the call sites; the test file drops the assertions for the deleted function; scripts/orderfile/generate.ts gets a comment-only update.
Security risks
None. This is CI pipeline generation and build-script orchestration — no runtime code, no user-facing surface, no credentials or network changes.
Level of scrutiny
Low-medium. The order file is a startup RSS optimization; a stale, missing, or wrong one produces a correct binary that just faults in more pages. The new Buildkite step is soft_fail: true with a 15-minute timeout, so it cannot break a build. The removed self-trace fallback was itself an optimization (seeding the inheritance chain at the cost of a second LTO link); its replacement — link unordered and annotate — degrades gracefully and is already the behavior for cross-compiled lanes. Worst case for any bug here is a slower canary startup, surfaced by the existing verifyOrderFileApplied warning annotation.
Other factors
- The change was exercised end-to-end in Buildkite build 92745 via the
[generate symbol order]opt-in tag:linux-aarch64-trace-orderpassed (11234 functions traced), and the build lane uploaded only its two zips (no standalone.order). The two red jobs in that build were unrelated Azure agent-creation failures and a pre-existing darwinrustupissue. - Unit tests in
linker-order.test.tswere updated to remove the deleted-function assertions and re-document the canary-does-not-generate case; 18 pass / 1 skip. - Dead code is deleted in the same PR that makes it dead, per REVIEW.md.
- The maintainer's only comment ("fix conflicts") has been addressed — the head commit
89a3d6e0is at the repo tip. - The transition after merge is well-reasoned: the first main build inherits the old file (nothing new races it), its trace-order step publishes a fresh one, and the second build onward inherits fresh.
|
Results from the rebased build (101120) for the record: all five trace-order steps passed. The new |
2 participants
Problem
linux-aarch64canary on main since build #68409 (July 5) has linked against the same symbol order file; of its 1000 hottest names, 513 still resolve today. The x64 and darwin files are re-traced every build.-trace-orderstep.linux-aarch64does not: it builds natively, so its build lane published its own file.Fix
linux-aarch64to the trace-order targets, on the debian 13 aarch64 test image. On main the generated pipeline gains exactly one step..orderartifact (the copy in the profile zip stays), so the trace-order step is the only publisher and an inherited copy can never race a fresh one.test/js/bun/perf/linker-order.test.tsupdated (25 pass, 4 windows-only skips on linux); the branch head opts in, so this PR's push build (Buildkite 101120, previously 92745) runs all five trace-order steps, soft-fail, reported in the status comment.Background
bun-profileunder a function-entry tracer to produce that list, then relink. Releases do this to their own binary; canaries inherit to skip the second link.bun-<target>.orderfrom recent builds on the branch by name alone, so two publishers of one name would race.-trace-orderstep: a soft-fail Buildkite step on the target's native test fleet that traces the build lane'sbun-profileand uploads the.orderthe next build inherits. One build of lag.no test proof · iteration 2 · Platform-specific test-only change; deferring to CI.
Original description
Problem
darwin-aarch64andlinux-x64have a-trace-orderstep that traces the freshly builtbun-profileon the native test fleet and publishesbun-<target>.orderfor the next build to inherit.linux-aarch64had none: its build lane runs on the aarch64 host, so it was left to publish its own file (packageAndUpload()uploaded whatever the lane linked with). A canary lane only inherits, so on main it re-published the same file on every build, and the file was only ever refreshed by a release or an opt-in trace.Following the
inherited ... from #Nlines in thelinux-aarch64-build-bunlogs on main back from build 92687: every build since #68409 (July 5, an opt-in trace, 19625 functions, still from the page-fault tracer that #35085 replaced) inherited that one file.verifyOrderFileAppliedreports how many of its 1000 hottest names still resolve in the binary being linked: 1000 on July 5, 978 by July 20, 522 by July 28, 513 today. The x64 and darwin files are re-traced every build.Change
.buildkite/ci.mjs: add linux-aarch64 totraceOrderTargets, traced on the debian 13 aarch64 test image. On main the generated pipeline gains exactly one step,linux-aarch64-trace-order, in the linux-aarch64 group (verified by generating the pipeline before and after and diffing).scripts/build/ci.ts: the build lane no longer uploads the standalone.orderartifact; the trace-order step is the only publisher for every target, soinheritOrderFile()(which downloads by name, without--step) never sees a stale copy racing a fresh one. Thelinker.ordercopy inside the profile zip stays.mustGenerateOrderFileandreportOrderFileBootstrap, andregenerateOrderFileloses its now-unusedctxparameter.reportOrderFileCannotTracebecomesreportOrderFileNotInherited, fires for any eligible build that inherited nothing, and names the<target>-trace-orderstep that should have published the file.After this merges, the first main build still inherits the old file from an earlier build (nothing publishes under that name any more, so there is nothing to race) while its trace-order step publishes a fresh one; the build after that inherits the fresh one.
Tests
test/js/bun/perf/linker-order.test.ts: themustGenerateOrderFileassertions are removed with the function. The test "a canary that inherited nothing generates anyway, seeding the chain" asserted the behaviour this PR removes, so it is deleted; the "canary does not generate" test now documents that a lane able to run its own binary is no exception. 18 pass, 1 skip (the unsupported-platform case) withbun bd test.The head commit on this branch carries the opt-in tag that
ci.mjshonours outside main, so the push build for this PR (Buildkite build 92745) schedules all three trace-order steps, including the newlinux-aarch64-trace-order. They are soft-fail steps, so their result is reported in the status comment below rather than in the PR check. The tag is deliberately kept out of this title and body, since the squash commit is built from those.