build: trace the symbol order file on windows x64 and arm64 - #37745
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 5 included reviews per hour; 2 remain after this review. WalkthroughChangesWindows x64 and arm64 now support linker maps, Windows linker-order support
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
e332286 to
13025d1
Compare
|
Updated 9:49 PM PT - Aug 18th, 2026
❌ @robobun, your commit e3e0f32 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 37745That installs a local version of the PR into your bun-37745 --bun |
|
Status: ready for review. Verified locally on a Server 2019 x64 box and a Windows 11 arm64 box (both bootstrapped like the CI images): End to end in CI: this branch's build (https://buildkite.com/bun/bun/builds/92842) ran CI state: rebased onto main (ddc9b3e) on Aug 19 at Jarred's request, see the last comment below. The rebased build is https://buildkite.com/bun/bun/builds/101034. 181 of 182 jobs passed, including all four trace-order steps, both windows build lanes and both windows test lanes. The one red job is the macOS 26 arm64 test shard: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds a new ~540-line Windows debugger-based tracer (functrace-windows.c), changes how Windows release binaries are linked (/order, /map, /lldmap), and rewires CI pipeline steps and profile-zip packaging, a human look would still be worthwhile — especially with the end-to-end check depending on this branch's own Buildkite run.
What was reviewed:
usesOrderFile/linkDepends/linkerMapOutputsgating and that the new flags line up with the declared ninja outputs and packaged files.functrace-windows.c: PE header bounds checks, the per-arch breakpoint encoding/PC handling, argv quoting againstCommandLineToArgvWrules, ConPTY stdio handoff, and the debug-event loop's exception passthrough.windows-symbols.tsparsers against the fixture shapes and the chunk-start filter droppingTable/$LNlabels;readTextSymbolsfallback ordering and its use inverifyOrderFileApplied..buildkite/ci.mjscmd.exe command shape (|| exit /b 1,vs-shell.ps1) matches the existing verify-baseline/test-runner conventions.
Extended reasoning...
Overview
This PR extends the symbol-order-file infrastructure to Windows x64 and arm64. It adds a new debugger-based function-entry tracer written in C (scripts/orderfile/functrace-windows.c, ~540 lines) that plants INT3/BRK breakpoints via WriteProcessMemory and drives a ConPTY for the tty workload; a linker-map parser (scripts/orderfile/windows-symbols.ts) that stands in for nm on PE binaries; new lld-link flags (/order:@, /ignore:4037, /map:, /lldmap:) in scripts/build/flags.ts; refactored linkerMapOutputs()/linkDepends() consumed by bun.ts/compile.ts/ci.ts; two new traceOrderTargets in .buildkite/ci.mjs with a Windows-specific cmd.exe command block; and substantial test additions in test/js/bun/perf/linker-order.test.ts. The refactor also moves linkerMapPath from rust.ts to flags.ts and routes verifyOrderFileApplied() through the shared readTextSymbols().
Security risks
None user-facing. The tracer is a build-time tool that runs only in CI/local dev against the freshly built bun-profile.exe; it doesn't ship in the release binary. The PE-header parsing in map_image() bounds-checks e_lfanew and the section table before dereferencing, and the starts-file reader validates magic/version/count against the file size. The argv quoting in append_argument() follows the CommandLineToArgvW backslash-doubling rule. No new attack surface on the shipped runtime.
Level of scrutiny
High. This is ~1400 lines of net-new code across 11 files, including low-level Win32 debugging APIs (WaitForDebugEvent, Get/SetThreadContext, VirtualProtectEx, FlushInstructionCache), per-architecture breakpoint encoding, ConPTY lifecycle management with a quiescence heuristic, and changes to release-link flags and CI artifact packaging. The /order flag now affects every Windows release binary's .text layout. The trace-order step is soft-fail so a broken tracer can't fail main, but the linker-flag and packaging changes are on the critical path. The author notes the definitive end-to-end check is this PR's own Buildkite run (via the [generate symbol order] commit tag), which is still in progress.
Other factors
The test coverage is thorough for the pure-function pieces (flag wiring, map parsers, chunk-start filtering) and the tracer has three concurrent Windows-only integration tests, but the real-binary path (bun-profile.exe with its actual maps under all eight workloads) is only exercised by the CI trace-order step itself. The refactor of linkDepends() and linkerMapOutputs() is behavior-preserving for existing platforms per the test matrix, and the darwin .linker-map now goes through linkerMapPath() instead of an inline string (checked equivalent). The writesLinkerMap() gate for darwin no longer excludes asan (previously cfg.unix && !cfg.asan in packageAndUpload; now cfg.darwin unconditionally under release), which is a minor behavior change but darwin+asan isn't a shipped configuration. Given the scope, cross-arch subtlety, and pending CI verification, deferring to a human reviewer.
|
The trace-order steps in this branch's build (https://buildkite.com/bun/bun/builds/92842) passed on both Windows targets, against the real
All eight workloads ran on both (the arm64 count is higher because that build is not LTO). Most of the wall time is compiling the tracer and reading the two maps; the tracing itself is a few seconds. The two maps grew each Windows profile zip by 7 MiB (x64 200 -> 207 MiB, arm64 167 -> 174 MiB, against main build 92794). Both On the review's note about darwin: yes, |
13025d1 to
8711726
Compare
|
Rebased onto current main (8711726). The only conflict was the import line at the top of New build, again with all four trace-order steps: https://buildkite.com/bun/bun/builds/99438 |
There was a problem hiding this comment.
I reviewed this PR and the automated bug hunt found no issues. Given the scope — a new 540-line Windows debugger-based tracer, a two-map symbol resolution scheme, and build-lane wiring across half a dozen files — a human look at the design is still worthwhile.
What was reviewed:
- The
/order,/map,/lldmapflag wiring andlinkDepends/linkerMapOutputsgating against the existing linux/darwin paths. functrace-windows.c: PE header bounds checks, breakpoint arm/restore on both arches, the ConPTY std-handle dance, andappend_argument's CommandLineToArgvW quoting.windows-symbols.tsparsers against the fixture shapes (chunk-start filter droppingTable/$LNlabels; image-base handling).packageAndUpload's newlinkerMapOutputs()list vs the darwin-asan case the author already noted — the-Wl,-mapflag is release-gated so the file exists.
Extended reasoning...
Overview
This PR extends Bun's symbol-order-file infrastructure to Windows x64 and arm64. It adds /order:@linker.order to the Windows release link, a new debugger-based function-entry tracer (scripts/orderfile/functrace-windows.c, ~540 lines) that plants INT3/BRK breakpoints via WriteProcessMemory and drives the debuggee through the Win32 debug loop, a two-map symbol resolver (scripts/orderfile/windows-symbols.ts) that reads lld-link's /map and /lldmap outputs to recover function names and filter out non-function labels, and CI wiring (traceOrderTargets, getTraceOrderStep) to run the tracer on the Windows test fleet. It also refactors linkerMapPath/linkerMapOutputs from rust.ts into flags.ts and centralizes what bun.ts, compile.ts, and ci.ts each declare/pack. Test coverage in linker-order.test.ts is thorough (flag wiring, both map parsers against realistic fixtures, and on Windows the tracer end-to-end against a compiled C fixture including ConPTY stdio).
Security risks
None material. The tracer runs only in a soft-fail CI step against Bun's own freshly-built binary; it does not ship in the release. Inputs (the starts file, the two maps) are produced by the same build. The C code does bounds-check the PE headers before reading the section table and validates the starts file's magic/count against its size. append_argument implements the standard MSVCRT argv quoting rules correctly.
Level of scrutiny
Medium-high. This is build/CI tooling, not runtime code — the trace-order step is soft-fail and non-PR-only, and /order on an empty seeded file is a no-op — so a defect degrades to "Windows ships unordered" plus a Buildkite annotation, not a broken release. But it is a substantial new subsystem: a hand-written Win32 debugger that patches another process's code pages, a novel chunk-start heuristic for distinguishing functions from CRT data-in-.text labels, and cross-cutting refactors of the linker-map plumbing. The design choices (debugger vs injection, two maps vs PDB, chunk-start filter) are well-reasoned in the description and comments, but they are choices a maintainer should sign off on.
Other factors
The PR has strong evidence of working end-to-end: the author ran the full pipeline on this branch with the [generate symbol order] opt-in, both Windows trace-order steps passed and produced 11k–13k function order files, and linker-order.test.ts passed on all three test platforms. The one behavior change I checked outside Windows — packageAndUpload now lists the darwin map via linkerMapOutputs() for any release config including asan, where before it was gated on !asan — is benign because the -Wl,-map flag has always been release-only (so the file exists) and makeZip skips missing files anyway; the author already addressed this in a PR comment. No prior human review comments to track.
|
@robobun rebase |
|
Rebased onto current main (ddc9b3e) and pushed as e3e0f32. The PR is mergeable again. One conflict, in Checks on the rebased tree:
Main did not touch New build, again with all four trace-order steps: https://buildkite.com/bun/bun/builds/101034. All four passed on the rebased tree and published their order files: |
… symbol order] Link the windows release binaries with lld-link's /order:@linker.order (plus /ignore:4037, the counterpart of --no-warn-symbol-ordering), and add a trace-order step for windows-x64 and windows-aarch64 that traces the cross-built bun-profile.exe on the matching test fleet and publishes the .order file the next build inherits, as the darwin-aarch64 and linux-x64 steps already do. The tracer (scripts/orderfile/functrace-windows.c) is a small debugger rather than an injected library: it starts the binary as its debuggee, writes a breakpoint (INT3 on x64, BRK on arm64) over every function start while the process is still stopped at its creation event, and on each breakpoint exception restores the instruction, records the address and resumes the thread there. With BUN_FUNCTRACE_TTY it starts the debuggee on a pseudo console, standing in for ptyrun.c. It is built on the test VM with clang-cl (or cl). A PE has no symbol table, so the windows release link now writes two maps that ship in the profile zip (scripts/orderfile/windows-symbols.ts): the MSVC-style /map for the names /order takes, and lld's own /lldmap for which of those names start an input chunk. The second is needed because the MSVC CRT defines symbols on data it keeps in .text (arm64 memset's branch table, the $LN labels on MSVC-emitted jump tables); a breakpoint there corrupts the table. Only chunk starts are functions, which is also exactly what /order can move. linkerMapPath moves from rust.ts to flags.ts next to the new symbolMapPath and linkerMapOutputs, which bun.ts uses to declare the maps to ninja and ci.ts to pack them; verifyOrderFileApplied reads symbols through the same readTextSymbols the generator uses. The tag in the subject makes this branch's own CI build run the trace-order steps (ci.mjs only adds them on main otherwise); Buildkite's message for a PR build is the commit subject alone.
8711726 to
e3e0f32
Compare
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Given the scope — a new 541-line Windows debugger-tracer in C, linker-map parsing, and release-link flag changes across the build system — a human look is still warranted.
Checked: the /order//map//lldmap flag gating matches linkerMapOutputs() so ninja and the profile zip agree; linkDepends() refactor preserves the existing linux/darwin/freebsd outputs; packageAndUpload's switch to linkerMapOutputs() now includes the darwin-asan map, which the darwin -Wl,-map flag already writes (gated on release alone). The tracer's breakpoint-restore path handles the seen-twice race and passes non-breakpoint exceptions through with DBG_EXCEPTION_NOT_HANDLED.
Extended reasoning...
Overview
This PR extends the symbol-ordering-file infrastructure (previously linux + darwin-aarch64 only) to Windows x64 and arm64. It adds: a new native Windows debugger-based function tracer (scripts/orderfile/functrace-windows.c, 541 lines) that plants INT3/BRK breakpoints over every function start via WriteProcessMemory and records first-entry order; a linker-map parser (scripts/orderfile/windows-symbols.ts) that reads lld-link's /map and /lldmap outputs to recover function names (a PE has no symbol table); /order:@, /ignore:4037, /map:, and /lldmap: flags in scripts/build/flags.ts for the Windows release link; two new soft-fail trace-order CI steps in .buildkite/ci.mjs; and refactors of linkerMapPath/linkerMapOutputs/linkDepends to centralize the map-output declarations that bun.ts, compile.ts, and ci.ts all consume. The unix generate.ts path is refactored into a Tracer interface with per-platform buildUnixTracer/buildWindowsTracer. Tests in linker-order.test.ts grow substantially (flag wiring, both map parsers against fixtures, and on Windows the tracer against a compiled C fixture including ConPTY stdio).
Security risks
None user-facing. The tracer is a build-time CI tool that runs only in the soft-fail trace-order step against Bun's own bun-profile.exe; it never ships and never processes untrusted input. The PE header parsing in map_image() bounds-checks e_lfanew and the section table against the 4 KB read. The starts-file reader validates magic/version/count against file size before allocating. append_argument() implements the standard MSVC argv quoting rules. No changes touch runtime code paths.
Level of scrutiny
High — this warrants human review despite no bugs found. The new C tracer is non-trivial systems code (cross-arch breakpoint encoding, VirtualProtectEx/WriteProcessMemory/FlushInstructionCache, thread-context manipulation, ConPTY lifecycle with the Server 2019 output-drain quirk). The build-system changes alter release-link flags on two shipping targets and change what goes into the profile zip. The writesLinkerMap()/linkerMapOutputs() centralization touches darwin/linux gating that was previously duplicated across three files, and one behavioral edge (darwin+asan now lists the map where before it was skipped) was already noted and justified in the PR thread. These are the kind of cross-cutting infrastructure decisions a maintainer should sign off on.
Other factors
The PR is thoroughly self-documented and comes with end-to-end CI evidence (build 92842 ran both new Windows trace-order steps successfully, publishing 10985/13233-function order files). Test coverage is strong: fixture-based parser tests run everywhere, and the Windows tracer tests compile and run the real functrace-windows.c against a C fixture that would crash on arm64 without the chunk-start filter. Jarred has already engaged (rebase request), so human review is in progress. The linkDepends() refactor was traced to confirm freebsd, linux, darwin, and windows outputs are unchanged except for the intended addition of orderFilePath on windows. The verifyOrderFileApplied() change to use readTextSymbols() was checked to preserve the linux/darwin nm path while adding the windows map path.
2 participants
Problem
darwin-aarch64andlinux-x64builds link with a traced symbol order file; Windows release builds link without one, sowindows-x64andwindows-aarch64miss the startup win.LD_PRELOADlibrary, which has no Windows counterpart, and a Windows executable has no symbol table to map traced addresses back to names.Fix
/order:@linker.order(plus/ignore:4037for traced names that no longer exist), and both windows targets get atrace-orderstep on their existing test images. The generated pipeline differs by exactly those two steps./mapfor names,/lldmapfor where each input chunk landed. Only names at chunk starts count as functions, because the MSVC CRT leaves labels (Table,$LN123) on data inside.textand a breakpoint there killed the debuggee. Chunk starts are exactly what/ordercan move, so nothing orderable is lost.Background
.textso startup-hot code shares pages. Generated in CI, never committed. lld takes--symbol-ordering-file, Apple ld-order_file, lld-link/order:@file.bun-profile, runs a few workloads under a tracer that records first-entry order, and uploads the.orderfile the next build links with./ordercan 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.nmreads. A PE keeps its names in the PDB, so on Windows the linker's map files stand in as the address-to-name source.no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.
Original description
What
darwin-aarch64andlinux-x64have a-trace-orderstep that traces the freshly builtbun-profileon 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 anLD_PRELOADlibrary. This adds it forwindows-x64andwindows-aarch64.scripts/build/flags.ts): windows release links get/order:@<buildDir>/linker.orderplus/ignore:4037(the--no-warn-symbol-orderingcounterpart), andlinker.orderbecomes a link dependency so an inherited file relinks. Every function is its own COMDAT already (/Gyfor bun's C++ and the WebKit prebuilt, function sections for rustc, per-function LTO output), which is what/orderneeds; checked against lld-link 21 that it orders statics and ThinLTO-internalized functions too.scripts/orderfile/functrace-windows.c): a debugger rather than an injected library. It starts the binary as its debuggee, writes a breakpoint (x64INT3, arm64BRK #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. WithBUN_FUNCTRACE_TTYit starts the debuggee on a ConPTY and types our stdin into it, standing in forptyrun.c. Same trace file format asfunctrace.c. Built on the test VM bygenerate.tswith clang-cl (on the images' machine PATH) or cl.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 tobun-profile.pdb: lld-link's MSVC-style/map(bun-profile.map, every symbol under the exact name/ordertakes) 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 arm64memsetkeeps the byte table for its computed branch there under a symbol namedTable, and MSVC-compiled code leaves a$LN123symbol 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 smallmemset/ inprintf. Chunk starts are also exactly the set/ordercan 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)..buildkite/ci.mjs):windows-x64andwindows-aarch64jointraceOrderTargets, tracing on the existing Server 2019 x64 and Windows 11 arm64 test images. The step runs under cmd.exe throughvs-shell.ps1, like the test runner. Generating the pipeline as main before and after this change differs by exactly those two steps.linkerMapPathmoves fromrust.tstoflags.tsnext to the newsymbolMapPath/linkerMapOutputs;bun.tsdeclares the maps to ninja andci.tspacks them from that one list.verifyOrderFileApplied()now reads symbols through the samereadTextSymbols()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):linkerMapOutputs()declares them;Table/$LNlabels being dropped);functrace-fixture.c(now also compiled with clang-cl; it links the static CRT, so it crashes on arm64 without the chunk filter) must recordf0..f31in order,main, andafteras 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 underBUN_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.tsagainst the installedbun.exewith maps faked down to its entry point, so all eight workloads run under the real tracer, ConPTY one included (cli-fixture.jsreportstty=true 80x24and exits onquit); 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.mjshonours 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.