crash_handler(windows): unwind through LLInt and vmEntryToJavaScript frames and stop the walk at non-code PCs

[robobun]

Problem

• Every Windows crash that starts in JS passes through frames that have no unwind info: the LLInt opcode handlers and vmEntryToJavaScript. Both of bun's Windows stack captures mishandle them (src/bun_core/debug.rs):
  • faults: capture_from_context treats any PC without a RUNTIME_FUNCTION as a leaf and pops [Rsp] as the return address. Those frames are not leaves, so the pop reads a CallFrame slot. Native function called from interpreted JS (host function or bun:ffi call in a script): the slot is the null CodeBlock slot and the trace ends one frame below the JIT thunk (... __jsc_host_js_segfault, <JIT thunk>, llint_entry, end). JIT-compiled callers get as far as vmEntryToJavaScript, after which the slots of its frame are reported as frames until the 20-entry buffer is full: the second handoff trace decodes to intlSegmenterPrototypeFuncSegment, 7 JIT frames, vmEntryToJavaScript, 5 frames in no module (pkg:"?" on bun.report); the shipped 8bb8d04c4 binary gives 11 of them for the same shape.
  • panics, RELEASE_ASSERTs, OOM (every TraceSeed other than Fault, via capture_current): RtlCaptureStackBackTrace stops at the first frame without static unwind info, which is the JIT thunk itself. crash_handler.panic() called through three JS functions reports the panic machinery, the thunk, and nothing else.
• In both cases the JS frames and the C++/Rust that entered JS, the part of a report that says what the process was doing, are missing.
• The "stale frames" the handoff was opened for turned out to be a different problem, see the details block at the end: those traces are correct, they were symbolized against the wrong one of the two x64 links of that commit.

Fix

• One walk, walk_context, used by both captures. capture_from_context seeds it from the fault CONTEXT as before; crash reports that are not faults now go through capture_current_for_crash (bun_core::capture_crash_stack_trace, which is what the crash handler's debug::capture_stack_trace wrapper now calls), seeding it with RtlCaptureContext and trimming with the same first_address logic. capture_current itself keeps RtlCaptureStackBackTrace: StoredTrace captures it on every debug-build refcount init (ThreadLock::lock), which cannot afford a VirtualQuery per frame, and those dumps are diagnostics, not reports. That is the one Windows capture deliberately left as is.
• A PC without unwind info is handled by position in the walk: frame 0 may be a genuine leaf (the handoff's ICU crash and RtlFillMemory both are), so [Rsp] (Lr on ARM64) is still taken there, but only when it holds a code address (unwind_leaf); otherwise, and for every later frame, step through the frame-pointer register, caller frame at [fp], return address at [fp+8], caller sp = fp + 16 (unwind_frame_pointer).
• Every PC the walk produces, from either fallback or from RtlVirtualUnwind, has to be in executable memory (VirtualQuery), or the walk stops. is_valid_memory and the new is_executable_memory share the query; PAGE_EXECUTE and RtlCaptureContext are added to bun_windows_sys, and the CONTEXT docs and layout asserts now cover Rbp/Fp, which the step reads.
• Why the frame-pointer step is correct: a return address never points into a leaf, so a frame past the first with no unwind info is offlineasm, and offlineasm keeps rbp/x29 on a CallFrame, whose first two slots are exactly the caller frame and return address. It is the same layout the unwind info JSC registers for the JIT pool describes (registerJITUnwindInfo, crash_handler(windows): let foreign first-chance AVs reach SEH via JSC unwind info #35083), so the two compose, and it is not a frame-pointer walk: the prebuilt C++ keeps no frame pointer, and it is RtlVirtualUnwind stepping through those frames that restores rbp/x29 by the time an offlineasm return address comes up. Offlineasm is the only such code: tinycc registers a function table for everything it compiles (tccrun.c, win64_add_function_table), and the bun:ffi call stubs are JIT-pool code; an earlier revision of this PR wrongly listed tinycc here.
• Why the leaf step is restricted to frame 0 and validated: at a later frame it is always wrong (that is this bug); at frame 0 a fault directly inside LLInt would have scratch at [Rsp], and the code-address check routes that case to the frame-pointer step too.
• Why stopping on a non-code PC: a return address is always in mapped code (image .text, JIT pool, FFI stubs; frame 1 of the FFI test is a pool address produced by the leaf step, so that case is exercised). Anything else means the chain is lost, and continuing is what produced the padding.
• This is the interim, bun-side fix. The root fix is for the WebKit build to emit one .pdata/.xdata entry over the offlineasm blob (jsc_llint_begin..jsc_llint_end, the SEH twin of the .cfi_* block LowLevelInterpreter.cpp already emits for DWARF), which is the follow-up ExecutableAllocator.cpp and crash_handler(windows): let foreign first-chance AVs reach SEH via JSC unwind info #35083 already name; it would also give LLInt faults an SEH catch point and fix ETW/WinDbg. It is a WebKit change plus a pin bump, so it is tracked separately; when it lands, unwind_frame_pointer becomes dead and its doc comment says to delete it. The frame-0 rule, the executable check and the report-path capture stay useful after it.
• Verified with test/cli/run/run-crash-handler.test.ts, Windows: crash trace continues below the JS frames (3 tests, Windows only): segfault and panic called through three interpreted JS functions, and an RtlFillMemory fault reached through JIT-compiled JS. Unfixed, they report 1, 0 and 0 bun frames below the JIT frames (the panic case measured on a build that already had the fault-path fix, so it isolates the capture_current_for_crash change); fixed, all three pass. Fixed traces symbolized with the debug PDBs give the same chain on x64 and ARM64 for both the fault and the panic: llint_entry x3, llint_entry (module body), vmEntryToJavaScript, JSC::Interpreter::executeProgram, JSC::evaluate, Bun::evaluateCommonJSModuleOnce, ... to the 20-frame cap, no unknown frames; the panic trace's frames above the thunk are byte-for-byte the ones the old capture produced, so trimming is unchanged.
• Whole file: 13 pass / 13 skip on Windows Server 2019 x64 and Windows 11 ARM64 debug builds. cargo check -p bun_crash_handler clean on x86_64-pc-windows-msvc, aarch64-pc-windows-msvc and Linux; debug.rs is clippy-clean on both Windows targets (the four borrow_as_ptr hits main has in the moved loop are fixed in passing, which is also what bun_core: fix clippy on the Windows and FreeBSD targets and lint them in CI #37605 does to those lines).
• Not covered by a test: the frame-0 decline path (a fault directly inside LLInt needs a JSC bug to trigger) and the stop conditions; both are argued above. The gate cannot run these tests on Linux, same as crash_handler(windows): walk the fault CONTEXT via RtlVirtualUnwind #35074 and crash_handler(windows): let foreign first-chance AVs reach SEH via JSC unwind info #35083.
• Open PRs touching the same lines, none of them this fix: crash_handler: recover the immediate caller when a frameless leaf faults #32871 (POSIX leaf recovery, changes capture_from_context's signature), crash_handler: tag JSC JIT-pool frames instead of discarding them #35440 (tags JIT-pool frames "JIT"; the new tests accept either encoding), bun_core: fix clippy on the Windows and FreeBSD targets and lint them in CI #37605 (the clippy lines above).

Background

• Windows stack walking: the linker emits a RUNTIME_FUNCTION (.pdata) for every compiled function that uses the stack; RtlLookupFunctionEntry finds it for a PC and RtlVirtualUnwind applies it to a register CONTEXT, restoring callee-saved registers including rbp/x29. Leaf functions (no stack use) get no entry; for them the return address is still at [rsp] (in lr on ARM64). For a PC inside a loaded image RtlLookupFunctionEntry consults only that image's static .pdata, so dynamic function tables (what crash_handler(windows): let foreign first-chance AVs reach SEH via JSC unwind info #35083 registers for the JIT pool and tinycc registers for its output, both outside the image) cannot cover in-image code. RtlCaptureContext fills a CONTEXT with the calling function's registers; RtlCaptureStackBackTrace is ntdll's own walker over the same tables, with no way to step a frame the tables do not describe.
• offlineasm: JSC's assembler that produces the LLInt interpreter and the vmEntryTo* trampolines. Its output is linked into bun's .text without .seh_* directives, so it has no .pdata, but it keeps the frame-pointer register pointing at the current CallFrame.
• CallFrame: JSC's JS stack frame. On both architectures the frame-pointer register points at it; slot 0 is the caller's frame pointer and slot 1 the return address, which is what every JIT tier's prologue and LLInt's functionPrologue push.
• TraceSeed (src/crash_handler/lib.rs): how a report's trace is obtained. Fault carries the signal/exception context; the other seeds (panic, OOM, C++ Bun__crashHandler) capture the current stack from inside the handler and trim everything above a caller-supplied return address.
• Trace string: the crash report URL encodes one entry per captured frame from a 20-entry buffer: an offset for bun's own image, a name plus offset for another DLL, or _ for an address in no module. The tests decode that and count bun-image frames below the last JIT-pool frame.

What the handed-off traces actually were

The handoff's two bun.report traces (build 8bb8d04c4) have mid-instruction addresses in every frame including frame 0, which a real fault PC cannot be. That commit has two separate x64 links, bun-windows-x64-profile and bun-windows-x64-baseline-profile (different PDB GUIDs, .text differs by 40 KB), and a trace string identifies a build only by platform character plus 7-character sha. Symbolized against the baseline link, every frame of both traces is an instruction boundary directly after a call into the previous frame's function:

• trace 1, frame 0 first: JSC::SlotVisitor::visitChildren (the fault is mov rax, [rax+0x50] with rax = 0x20200000000, matching the reported fault address 0x20200000050), SlotVisitor::drain, drainFromShared, the Heap::runBeginPhase lambda, ParallelHelperClient::runTask, AutomaticThread, WTF::Thread::entryPoint, wtfThreadEntryPoint, ucrt thread_start, BaseThreadInitThunk, RtlUserThreadStart. A GC marking thread hitting a corrupt cell, not date code.
• trace 2, frame 0 first: icu_73::RuleBasedBreakIterator::BreakCache::reset entered with a null this (fault address 0x10, PC at the function's first instruction, no .pdata, so this is the leaf step working), RuleBasedBreakIterator::operator=, RuleBasedBreakIterator::clone, ubrk_clone, JSC::cloneUBreakIterator (IntlWorkaround.cpp) inlined into IntlSegmenter::segment, intlSegmenterPrototypeFuncSegment, 7 JIT frames, vmEntryToJavaScript, then 5 frames of padding. The padding is what this PR fixes; the rest of the trace was right.

So the walker was sound for those crashes; the fix here is for what happens below vmEntryToJavaScript, plus the interpreted-caller truncation. The build-identity problem (which link a w + sha7 trace string came from) is a release/bun.report matter and is not changed here.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c86c7e4-cc56-4ff6-9ca3-211e41dc583d

📥 Commits

Reviewing files that changed from the base of the PR and between 2c2ef7c and 751e8e4.

📒 Files selected for processing (5)

• src/bun_core/debug.rs
• src/bun_core/lib.rs
• src/crash_handler/lib.rs
• src/windows_sys/externs.rs
• test/cli/run/run-crash-handler.test.ts

---

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

[robobun]

Status: ready for review (scope widened once after self-review, see below).

Reproduced on Windows Server 2019 x64 with unfixed debug builds: a fault in crash_handler.segfault() called through three JS functions produced a trace ending at the first llint_entry return address (1 bun frame below the JIT thunk); an RtlFillMemory fault reached through JIT-compiled JS produced vmEntryToJavaScript followed by 11 frames in no module; and crash_handler.panic() through the same three JS functions produced the panic machinery, the JIT thunk, and nothing else (0 frames below). The shipped 8bb8d04c4 release binary shows the same shapes, as does the second trace in the report this PR came from.

With the fix, all three walk through the LLInt frames and vmEntryToJavaScript into JSC::Interpreter::executeProgram, JSC::evaluate, Bun::evaluateCommonJSModuleOnce and on until the 20-frame buffer is full, on both x64 and Windows 11 ARM64 (symbolized against the debug PDBs). The three tests in test/cli/run/run-crash-handler.test.ts (Windows: crash trace continues below the JS frames) fail on the unfixed builds with 1 / 0 / 0 frames and pass with the fix; the whole file is 13 pass / 13 skip on both machines.

Self-review found that the panic/OOM capture (capture_current, RtlCaptureStackBackTrace) had the same gap and that the description wrongly named tinycc trampolines as needing the frame-pointer step; 751e8e4 routes crash reports through the same walk and corrects the comment, and the description now records that the bun-side step is an interim until the WebKit build emits .pdata for the offlineasm blob (tracked separately). StoredTrace deliberately keeps the cheap capture.

CI on the current head (751e8e4, build 97700): 177 jobs passed, including all 16 Windows x64 / ARM64 test shards; the shard on each Windows architecture that ran run-crash-handler.test.ts passed. The remaining two jobs are darwin 14 aarch64 - test-bun shards waiting for an agent (on the previous head one of those expired in the queue without running); the change is Windows-only code plus Windows-only tests, so if that happens again the shard just needs a retry from someone with Buildkite access. Nothing further is pending on this PR from my side.

[claude]

I re-reviewed after 06cc3c1 and 5e7752c — the stale CONTEXT doc comments and layout asserts are fixed, and the bug-hunting pass found nothing further. Given this is unsafe crash-time unwinding logic that encodes assumptions about JSC's CallFrame layout, offlineasm frame-pointer discipline, and Win32 unwind semantics across two architectures, a human look is still worthwhile.

What was reviewed:

• accessible_page_protection refactor preserves the exact is_valid_memory predicate (MEM_COMMIT, no PAGE_NOACCESS/PAGE_GUARD, then the READABLE mask).
• unwind_frame_pointer guards: alignment, frame >= sp, checked_add for caller_sp, and both slot loads go through MemoryAccessor so an unmapped frame page fails the walk rather than faulting the handler.
• Loop termination: the new is_executable_memory(next_pc) check applies to all three step sources, and the n < out.len() cap remains the ultimate bound.
• Test decoder cross-checked against the layout comment; the isJitFrame predicate accepts both current _ and #35440's "JIT" tag.

Extended reasoning...

Overview

The PR changes the Windows crash-handler stack walker in src/bun_core/debug.rs to correctly step through JSC offlineasm frames (LLInt opcode handlers and vmEntryToJavaScript) that have no .pdata unwind info. Previously these were treated as leaf functions, which either truncated the trace (interpreted callers) or padded it with garbage stack slots (JIT callers reaching vmEntryToJavaScript). The fix: (1) restrict the leaf-pop to frame 0 and only when [Rsp]/Lr holds a code address, (2) otherwise step via the frame pointer (Rbp/Fp), which offlineasm and the JIT-pool unwind info both maintain as a CallFrame chain, (3) stop the walk on any non-executable PC. Supporting changes: a shared accessible_page_protection helper factored out of is_valid_memory, a new is_executable_memory predicate, PAGE_EXECUTE added to bun_windows_sys, and the CONTEXT struct docs/layout-asserts extended to cover Rbp/Fp. Two new Windows-only tests decode the trace string and count bun-image frames below the JIT frames.

Security risks

None. This code runs only inside the crash handler after a fault has already occurred; its inputs are the kernel's fault CONTEXT and the process's own address space. All memory reads go through MemoryAccessor::load_usize (which VirtualQuerys first on Windows) or the new is_executable_memory predicate, so the walker cannot itself fault on unmapped pages. No user-controlled data reaches this path.

Level of scrutiny

High. This is unsafe Rust operating on raw register state at crash time, on two Windows architectures, with correctness depending on undocumented-in-code assumptions about JSC's CallFrame layout, offlineasm's frame-pointer discipline, and how RtlVirtualUnwind restores callee-saved registers across compiled frames. A mistake here does not corrupt user state (the process is already dying) but would degrade every Windows crash report. The PR description is unusually thorough in justifying each invariant, and the tests are well-constructed (they decode the actual trace-string format and assert structural properties, verified to fail on the unfixed build). Still, the depth of platform-specific reasoning warrants a human reviewer familiar with the Windows unwind path and prior PRs #35074/#35083.

Other factors

My earlier nit (stale CONTEXT doc comments and missing offset_of! pins for Rbp/Fp) was addressed in 06cc3c1. The comment-cop bot's length complaints were addressed in 5e7752c with a reasonable pushback — the remaining comments state invariants the code cannot express, not workaround justifications. The is_valid_memory refactor is behavior-preserving: the same MEM_COMMIT && !(PAGE_NOACCESS|PAGE_GUARD) && (Protect & READABLE) predicate, just factored through the shared helper. The new tests use test.concurrent, drain both pipes, use await using, and are gated describe.if(isWindows). The PR notes three open PRs touching adjacent lines (#32871, #35440, #37605) and the test already accepts #35440's "JIT" tag encoding.

[robobun]

^{Updated 8:05 PM PT - Aug 14th, 2026}

@robobun, your commit 5e7752c is building: #97073

[robobun]

^{Updated 11:05 PM PT - Aug 14th, 2026}

❌ @robobun, your commit 5e7752c has some failures in Build #97073 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38789

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

bun-38789 --bun

[claude]

I reviewed this PR and didn't find any bugs (my earlier nit about the CONTEXT doc comments was addressed in 06cc3c1, and the new capture_current_for_crash path in 751e8e4 looks consistent with the fault-path walk). Because this is low-level Windows unwinding logic whose correctness rests on JSC/offlineasm invariants and runs inside a crash handler on a possibly-corrupt stack, a human look from someone familiar with the Windows unwind + JSC CallFrame layout would still be worthwhile.

What was reviewed:

• unwind_leaf / unwind_frame_pointer / walk_context: the frame-0-only leaf gate, the is_executable_memory stop condition, and that every raw read goes through MemoryAccessor / VirtualQuery before dereference.
• is_valid_memory refactor: confirmed accessible_page_protection preserves the previous MEM_COMMIT / PAGE_GUARD / PAGE_NOACCESS semantics.
• capture_current_for_crash: RtlCaptureContext seeding, out.is_empty() guard, and that non-Windows delegates unchanged to capture_current.
• New tests: the VLQ decoder against the documented encode_trace_string layout, and that assertions distinguish fixed from unfixed behavior per the PR description.

Extended reasoning...

Overview

This PR reworks the Windows crash-handler stack walker in src/bun_core/debug.rs so it can step through JSC offlineasm frames (LLInt opcode handlers and vmEntryToJavaScript) that have no .pdata unwind info. The old code treated every no-.pdata PC as a leaf and popped [Rsp], which read a CallFrame slot instead of a return address — truncating traces at the first LLInt frame or padding them with garbage below vmEntryToJavaScript. The fix: (1) restrict the leaf step to frame 0 and validate its result is a code address, (2) otherwise step through rbp/x29 (unwind_frame_pointer), (3) stop the walk on any non-executable PC. It also extracts the walk loop into walk_context and adds capture_current_for_crash so panic/OOM reports (not just faults) use the same walk via RtlCaptureContext. Supporting changes: PAGE_EXECUTE constant + RtlCaptureContext extern + CONTEXT field-offset asserts in src/windows_sys/externs.rs; capture_crash_stack_trace re-export in src/bun_core/lib.rs; the crash_handler crate's internal capture_stack_trace now calls the thorough variant. Two new Windows-only tests decode the trace string and assert bun-image frames appear below the JIT frames.

Security risks

None user-facing. The code runs only inside the crash handler after a fatal fault/panic, reads process-local memory through VirtualQuery-guarded loads, and has no external inputs. The worst-case failure mode is a truncated or wrong crash trace, which is what the PR is fixing.

Level of scrutiny

High. This is (a) crash-handler code executing in a possibly-corrupt process, (b) unsafe FFI to RtlVirtualUnwind/RtlLookupFunctionEntry/RtlCaptureContext with hand-maintained CONTEXT layouts, (c) dual-arch #[cfg]-gated code where only one branch is checked per target, and (d) a correctness argument that depends on how JSC's offlineasm maintains the frame-pointer register and how RtlVirtualUnwind restores callee-saved registers across compiled frames. The PR description argues each invariant carefully and the tests demonstrably fail on the unfixed build, but validating that the frame-pointer step is sound for every offlineasm entry path (and that no other in-image code without .pdata violates it) needs domain knowledge I can't verify from the diff alone.

Other factors

• My earlier nit (stale CONTEXT doc comments listing which fields bun reads) was addressed in 06cc3c1 with the Rbp/Fp additions and matching offset_of! asserts.
• The refactor of is_valid_memory into accessible_page_protection is behavior-preserving: same MEM_COMMIT + not-PAGE_NOACCESS/PAGE_GUARD gate, same READABLE mask.
• All raw memory reads in the new fallback steps go through MemoryAccessor::load_usize, which itself gates on is_valid_memory; frame < sp, alignment, and checked_add guards are present.
• The comment-cop bot has re-flagged several multi-line doc comments after 751e8e4; the author already argued (and I agree) that these state the invariant the code depends on rather than justify a workaround, so I don't consider them blocking.
• CI on 5e7752c passed all Windows shards; the head commit 751e8e4 adds the panic-path capture and is what the fresh bot comments are on. The new tests are Windows-only and cannot run on the Linux gate, same as the prior PRs in this area.
• Open PRs #32871, #35440, #37605 touch adjacent lines; the test already accepts #35440's "JIT" tag encoding.

[robobun]

Heads-up: #38838 changes the trace string to format 4, which inserts a header right after the 7-character sha (a VLQ field count, then per field a VLQ tag, a VLQ char count and that many chars) before the two feature VLQs. The test decoder added here reads the features directly after the sha, so whichever of the two lands second needs to skip that header first; the header loop in the trace string identifies the build test in #38838 can be reused as is.

[robobun]

The root fix this PR's description points at is now up: oven-sh/WebKit#442 emits one .pdata/.xdata record over jsc_llint_begin..jsc_llint_end (same unwind codes and handler as the JIT pool's), and #39101 is the bun side (pin bump, comment updates, tests that look up both ends of the range, walk through interpreted frames with RtlVirtualUnwind, and check a jitless segfaultInDll is reported through the record's handler). Verified against the preview build on Windows x64 and ARM64: the table-driven walk goes through all the interpreted frames and vmEntryToJavaScript into the C++ below.

Interaction with this PR: once the pin lands, the PCs unwind_frame_pointer exists for all have unwind info, so that step becomes dead, as its doc comment anticipates; the frame-0 leaf validation and the non-code-PC stop are unaffected. Whichever of the two lands second should drop or delete it accordingly; #39101 does not touch debug.rs, so there is no conflict either way.

[robobun]

oven-sh/WebKit#442 is on main since #35343, so the LLInt and vmEntry* code now has a .pdata record. #39101 is closed, and #44212 adds tests for the record.

Two points for the rebase of this PR:

• The doc comment of unwind_frame_pointer says that the step goes once WebKit emits .pdata. That is the state of main now. While the step exists, the trace tests of this PR pass with or without the record.
• test(crash_handler): cover the Windows unwind record over the LLInt code #44212 has the same traceStringFrameImages decoder and a trace test for a segfault under interpreted frames. The PR that lands second must drop the duplicate.