Conversation
On aarch64 the fault-context extraction read only pc and fp, discarding x30 (lr). When the faulting instruction is inside a function with no frame prologue (machine-outlined sequence, ICF-folded thunk, intrinsic), fp still belongs to the caller's frame record, so the fp-walk's first hop yields the caller's return address and the immediate caller never appears in the trace. Sentry reports from such faults then show the grandparent as frame 1 and attribute the pc to whatever symbol the linker's identical-code-folding picked for the fold, which on macOS release builds (ld64.lld --icf=safe) is effectively arbitrary. Capture lr from the ucontext (x30 on aarch64; best-effort [rsp] on x86_64) and emit it between pc and the fp-walk when it is distinct from the walk's first hop, mapped, and not a stack address. The framed case where lr equals [fp+8] (not yet clobbered) is suppressed as a duplicate; a clobbered lr that still lands in the image is kept, trading one possibly-noisy frame for not losing a real one. Adds a segfaultInFramelessLeaf test hook that faults in a one-instruction asm stub with no prologue (or, under ASAN which owns SIGSEGV, synthesises the equivalent TraceSeed::Fault), and a test asserting the wrapper that called the stub is named in the symbolized trace.
|
Warning Review limit reached
More reviews will be available in 6 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe crash handler stack unwinder is extended to recover the caller frame when a crash occurs in a frameless leaf function. ChangesFrameless Leaf Frame Recovery
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
msync(MS_ASYNC) only checks that a page is mapped, not readable, so a PROT_NONE reservation (guard page, mimalloc uncommitted arena, JSC reserved region) passed is_valid_memory and the subsequent raw copy_nonoverlapping faulted. With SA_RESETHAND set on the SIGSEGV handler, a recursive fault during capture loses the entire crash report. The new lr/[rsp] recovery path feeds arbitrary-valued candidates to this probe (looks_like_code) and, on x86_64, reads [rsp] from the fault context before the handler body runs at all; a stack overflow that left rsp in the guard page would therefore die silently. mach_vm_read_overwrite does the copy in the kernel and returns KERN_PROTECTION_FAILURE / KERN_INVALID_ADDRESS instead of faulting. This also hardens the pre-existing fp-walk against corrupted saved frame pointers. Also bump the create_empty_object inline-capacity hint to 9 to match the new ENTRIES count.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/run/run-crash-handler.test.ts`:
- Line 149: Remove the explicit per-test timeout from the crash-path test so it
uses Bun’s default runner timeout instead. Update the relevant test case in
run-crash-handler.test.ts (the crash handler/symbolizer path test) to delete the
60_000 override and rely on the existing test runner behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6eb0da7c-4d57-4203-887a-ba4948ec8132
📒 Files selected for processing (5)
src/bun_core/debug.rssrc/crash_handler/lib.rssrc/js/internal-for-testing.tssrc/runtime/api/crash_handler_jsc.rstest/cli/run/run-crash-handler.test.ts
Pass raw sp through TraceSeed::Fault instead of dereferencing [rsp] in fault_context_from_ucontext. The read now happens inside the crash handler body, after the header is printed and PANIC_STAGE has advanced, so a stack overflow that left rsp in a PROT_NONE guard page cannot recursively fault before any output is produced. The read also reuses the fp-walk's MemoryAccessor (process_vm_readv / proc-mem / mach_vm_read_overwrite already primed by the first hop). Drops load_return_address_at, which removes the allow(dead_code) escape that tripped test/internal/dead-code-escapes.test.ts.
|
Status: the diff is complete and every CI lane that actually ran tests is green (280 passed on the latest build). The only red check is Same agent each time ( Review feedback from all six threads has been addressed and resolved. I am not going to keep pushing to re-roll that one agent; a retrigger once the agent recovers, or a manual re-run of the |
…ven-sh#35074) ## Problem `handle_segfault_windows` reads `ExceptionAddress` for frame 0 but then captures the rest of the stack from *inside the handler* with `RtlCaptureStackBackTrace`, trimming the handler's own frames by scanning for an address within 256 bytes of the fault PC. That trim only works when `RtlCaptureStackBackTrace` can unwind through `KiUserExceptionDispatcher` back to the faulting frame. When it cannot, the capture is just `[fault PC, capture_from_context, crash_handler, handle_segfault_windows, ntdll]` and none of the bun callers are recorded. Observed in Sentry (BUN-3K2N and siblings): 1180 events in the last 30 days on `bun@1.4.0+*` builds where `capture_from_context` is visible in the stack, 88% Windows ARM64 / 12% x64. Example trace for a fault inside `CRYPTSP.dll`: ``` <anonymous> CRYPTSP.dll (fault PC) bun_core::debug::capture_from_context debug.rs:344 bun_crash_handler::draft::crash_handler lib.rs:1123 bun_crash_handler::draft::handle_segfault_windows lib.rs:2065 <anonymous> ntdll.dll ``` On Windows Server 2019 x64 the old path happens to work (RtlCaptureStackBackTrace does unwind through the dispatcher there), so this is not reproducible on every Windows version. ## Fix Mirror the POSIX path: seed the walk from the saved register context instead of from inside the handler. The VEH receives `EXCEPTION_POINTERS::ContextRecord`; pass that through `TraceSeed::Fault` and walk with `RtlLookupFunctionEntry` + `RtlVirtualUnwind`. The handler's frames are never in the chain, and faults in external DLLs unwind into their bun callers regardless of Windows version or architecture. This is the same approach Crashpad, Breakpad and the Sentry native SDK use. - `src/windows_sys/externs.rs`: add `CONTEXT` (x64 and ARM64 layouts with static size/offset assertions), `RUNTIME_FUNCTION`, `RtlLookupFunctionEntry`, `RtlVirtualUnwind`, `UNW_FLAG_NHANDLER`. - `src/bun_core/debug.rs`: rewrite the Windows branch of `capture_from_context` to copy the CONTEXT and loop `RtlLookupFunctionEntry` / `RtlVirtualUnwind`, with leaf-function handling for both arches. - `src/crash_handler/lib.rs`: pass `info.ContextRecord` through instead of `fp: 0`. - `src/runtime/api/crash_handler_jsc.rs`: add a `segfaultInDll` test hook that faults inside `ntdll!RtlFillMemory` so the test has a fault PC outside bun.exe. ## Verification Built on Windows x64, ran `segfaultInDll`, and symbolized the captured addresses: ``` frame0 ntdll!RtlFillMemory (fault PC) frame1 crash_handler_jsc::js_bindings::js_segfault_in_dll (the bun caller) frame2 __jsc_host_js_segfault_in_dll::{closure#0} frame3 host_fn_result::{closure#0} frame4 to_js_host_call frame5 host_fn_result frame6 __jsc_host_js_segfault_in_dll frame7-8 JIT frame9 llint_entry ``` No `capture_from_context` / `crash_handler` / `handle_segfault_windows` frames. `cargo check` clean on `x86_64-pc-windows-msvc`, `aarch64-pc-windows-msvc`, and the host. The new test is Windows-only; on Server 2019 both the old and new code produce a correct trace for this case, so the test acts as a regression guard for the new path rather than a fail-before proof. The fail-before evidence is the Sentry data above. Touches the same files as oven-sh#32871 (frameless leaf handling) but is orthogonal; whichever lands second will need a small rebase. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/run/run-crash-handler.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-06-27, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
1 participant
Problem
Sentry BUN-3QBN reports a startup segfault in standalone executables on macOS aarch64 with this stack:
CallbackList::pushis provably unreachable fromVirtualMachine::init(the only thinginit_with_module_graphcalls). The release darwin binaries are cross-linked withld64.lld --icf=safe, so identical function tails are folded and the pc's debuginfo attribution is whichever source location the fold kept.The real problem is what's missing from that stack.
fault_context_from_ucontexton aarch64 reads only__ss.__pcand__ss.__fp, discarding__ss.__lr. When the faulting instruction is inside a function with no frame prologue (machine-outlined sequence, ICF-folded thunk, compiler intrinsic, hand-written asm),fpat the fault still points at the caller's frame record, so:out[0]= pc (inside the frameless leaf, symbolicated to whatever ICF picked)fp: first hop is[fp+8]= the caller's saved return address = return into the grandparentThe immediate caller never appears. In the BUN-3QBN trace that missing frame is whatever
VirtualMachine::initcalled that faulted, which is exactly the information needed to root-cause it.Fix
Capture the return-into-caller address from the fault context and emit it between
pcand the fp-walk when it's not already covered:mc.regs[30]/__ss.__lr.[rsp](the wordcallpushed), via the existing fault-tolerantMemoryAccessor.capture_from_contextemitslronly when it is non-zero, distinct from bothpcand the fp-walk's first result (so a framed function whose saved LR at[fp+8]matcheslrdoesn't get a duplicate), not within 64 MB offp(rejects the x86_64 framed case where[rsp]is a local), and lands on a mapped page (rejects small-integer garbage from a clobbered register). A stalelrthat still resolves to code is kept: one possibly-noisy frame is better than a lost one.RtlCaptureStackBackTracealready handles this via.pdata).Test
segfaultInFramelessLeafinbun:internal-for-testingcalls a one-instructionglobal_asm!stub with no prologue (x86_64:movq 0, %rax; aarch64:ldr x0, [x8]withx8 = 0) from a#[inline(never)]wrapperframeless_segv_caller. Under ASAN, where Bun's SIGSEGV handler is not installed, the helper synthesises an equivalentTraceSeed::Fault { pc, fp, lr }socapture_from_context's lr insertion is still exercised.test/cli/run/run-crash-handler.test.tsassertsframeless_segv_callerappears in the llvm-symbolizer output:bun run rust:check-allpasses on all 10 targets.Follow-up
This makes the next BUN-3QBN event diagnosable; the underlying fault inside
VirtualMachine::initis still unidentified and needs the actual pc mapped against the crashing build's linker map (already a CI artifact for darwin release).