diff --git a/src/bun_core/Global.rs b/src/bun_core/Global.rs index 56a912aec7f0..f52c18a65bac 100644 --- a/src/bun_core/Global.rs +++ b/src/bun_core/Global.rs @@ -66,6 +66,28 @@ pub static WINDOWS_SEGFAULT_HANDLE: core::sync::atomic::AtomicPtr { pub index: usize, pub instruction_addresses: &'a [usize], + /// `instruction_addresses[0]` is the faulting instruction itself (the pc a + /// fault handler was given) rather than a return address. See + /// [`Self::symbol_address`]. + pub first_frame_is_exact_pc: bool, +} + +impl StackTrace<'_> { + /// The address to symbolize for frame `i`. + /// + /// A return address points one past its call instruction, so it is stepped + /// back into the call; looked up as is, a call that ends a line (or a + /// function) is attributed to whatever follows it. A fault pc already is the + /// instruction to blame and must not be stepped back, or a fault on the first + /// instruction of a function is attributed to the function before it. + pub fn symbol_address(&self, i: usize) -> usize { + let addr = self.instruction_addresses[i]; + if i == 0 && self.first_frame_is_exact_pc { + addr + } else { + addr.saturating_sub(1) + } + } } /// Fixed 31-frame stack-trace buffer. @@ -85,6 +107,7 @@ impl StoredTrace { StackTrace { index: self.index, instruction_addresses: &self.data, + first_frame_is_exact_pc: false, } } diff --git a/src/crash_handler/lib.rs b/src/crash_handler/lib.rs index a5cb8c8966ea..b621528ee7f4 100644 --- a/src/crash_handler/lib.rs +++ b/src/crash_handler/lib.rs @@ -776,7 +776,15 @@ mod draft { /// becomes frame 0. POSIX: `fp` is the saved frame-pointer register and /// the walk follows the fp chain. Windows: `fp` is the `*const CONTEXT` /// from `EXCEPTION_POINTERS` and the walk uses `RtlVirtualUnwind`. - Fault { pc: usize, fp: usize }, + /// `exact_pc`: `pc` is the faulting instruction itself, as opposed to + /// already pointing past it the way a trap leaves it (see + /// `signal_pc_is_exact`); decides whether frame 0 is symbolized as is + /// or stepped back like a return address. + Fault { + pc: usize, + fp: usize, + exact_pc: bool, + }, /// A trace was already captured upstream. ErrorReturn(&'a StackTrace<'a>), /// Walk the current stack and trim the capture machinery above this PC. @@ -1027,7 +1035,7 @@ mod draft { let trace_buf: StackTrace; let trace: &StackTrace = 'blk: { - let idx: usize = match seed { + let (idx, first_frame_is_exact_pc): (usize, bool) = match seed { TraceSeed::ErrorReturn(ert) => break 'blk ert, // For an actual fault the signal/exception handler hands // us the saved register context. Seeding the walk from @@ -1036,19 +1044,22 @@ mod draft { // `SA_ONSTACK` altstack, so its own frame chain is // disjoint from the faulting thread's, and release builds // strip the unwind tables a CFI-based capture would need. - TraceSeed::Fault { pc, fp } => { - bun_core::debug::capture_from_context(pc, fp, &mut addr_buf) - } + TraceSeed::Fault { pc, fp, exact_pc } => ( + bun_core::debug::capture_from_context(pc, fp, &mut addr_buf), + exact_pc, + ), TraceSeed::BeginAddr(addr) => { - debug::capture_stack_trace(addr, &mut addr_buf) - } - TraceSeed::None => { - debug::capture_stack_trace(debug::return_address(), &mut addr_buf) + (debug::capture_stack_trace(addr, &mut addr_buf), false) } + TraceSeed::None => ( + debug::capture_stack_trace(debug::return_address(), &mut addr_buf), + false, + ), }; trace_buf = StackTrace { index: idx, instruction_addresses: &addr_buf, + first_frame_is_exact_pc, }; break 'blk &trace_buf; }; @@ -1568,6 +1579,25 @@ mod draft { } } + /// Whether the pc saved in the signal context is the instruction that raised + /// `sig`. Faults (SIGSEGV, SIGBUS, SIGILL, SIGFPE) are reported before the + /// instruction completes, so it is. Traps are reported after it: x86_64 + /// `int3` (WTF's CRASH()) leaves pc on the byte following it, and SIGABRT is + /// delivered on return from the kill(2) that raised it. For those, stepping + /// back one byte like a return address lands inside the instruction that + /// trapped. + #[cfg(unix)] + fn signal_pc_is_exact(sig: c_int) -> bool { + // The exception among traps: the kernel leaves pc on an aarch64 `brk` + // itself (which is also why a handler that returns re-traps forever). + const TRAP_LEAVES_PC_ON_INSTRUCTION: bool = cfg!(target_arch = "aarch64"); + match sig { + libc::SIGABRT => false, + libc::SIGTRAP => TRAP_LEAVES_PC_ON_INSTRUCTION, + _ => true, + } + } + #[cfg(unix)] extern "C" fn handle_segfault_posix(sig: c_int, info: *mut libc::siginfo_t, ctx: *mut c_void) { // SAFETY: kernel provides a valid siginfo_t; `si_addr` reads the per-platform @@ -1586,7 +1616,11 @@ mod draft { _ => unreachable!(), }, match fault_context_from_ucontext(ctx) { - Some((pc, fp)) => TraceSeed::Fault { pc, fp }, + Some((pc, fp)) => TraceSeed::Fault { + pc, + fp, + exact_pc: signal_pc_is_exact(sig), + }, None => TraceSeed::None, }, ); @@ -1818,6 +1852,7 @@ mod draft { let trace = StackTrace { index: idx, instruction_addresses: &addr_buf, + first_frame_is_exact_pc: false, }; if debug_trace { @@ -1902,6 +1937,7 @@ mod draft { let stack = StackTrace { index: n, instruction_addresses: &addrs, + first_frame_is_exact_pc: false, }; let _ = writeln!( stderr, @@ -1968,6 +2004,8 @@ mod draft { #[cfg(windows)] static WINDOWS_EXE_IMAGE_END: AtomicUsize = AtomicUsize::new(0); + /// For every code classified here `ExceptionAddress` is the faulting + /// instruction itself, hence `exact_pc: true` at the three callers. #[cfg(windows)] fn classify_exception_windows( record: &bun_sys::windows::EXCEPTION_RECORD, @@ -2038,6 +2076,7 @@ mod draft { TraceSeed::Fault { pc, fp: info.ContextRecord as usize, + exact_pc: true, }, ); } @@ -2079,6 +2118,7 @@ mod draft { TraceSeed::Fault { pc, fp: context as usize, + exact_pc: true, }, ); } @@ -2099,6 +2139,7 @@ mod draft { TraceSeed::Fault { pc, fp: info.ContextRecord as usize, + exact_pc: true, }, ); } @@ -2426,10 +2467,23 @@ mod draft { } impl StackLine { - /// `None` implies the trace is not known. - fn from_address(addr: usize, name_bytes: &mut [u8]) -> Option { + /// One frame of `trace`, as bun.report expects it. `None` implies the + /// frame is not known. + /// + /// POSIX strings carry the address to symbolize + /// (`StackTrace::symbol_address`), which bun.report hands to + /// llvm-symbolizer unchanged. Windows strings carry the addresses as + /// captured: bun.report treats the first frame as the fault pc and steps + /// the frames after it back itself, so adjusting here as well would step + /// return addresses back twice. + fn from_frame( + trace: &StackTrace, + frame: usize, + name_bytes: &mut [u8], + ) -> Option { #[cfg(windows)] { + let addr = trace.instruction_addresses[frame]; let module = bun_sys::windows::get_module_handle_from_address(addr)?; let base_address = module as usize; @@ -2462,7 +2516,7 @@ mod draft { } #[cfg(target_os = "macos")] { - let address = if addr == 0 { 0 } else { addr - 1 }; + let address = trace.symbol_address(frame); let image_count = bun_sys::c::_dyld_image_count(); @@ -2545,7 +2599,7 @@ mod draft { #[cfg(not(any(windows, target_os = "macos")))] { let _ = name_bytes; - let address = addr.saturating_sub(1); + let address = trace.symbol_address(frame); let m = bun_sys::elf::find_loaded_module(address)?; return Some(StackLine { address: i32::try_from(address - m.base_address).expect("int cast"), @@ -2631,8 +2685,8 @@ mod draft { let mut name_bytes: [u8; 1024] = [0; 1024]; - for &addr in &opts.trace.instruction_addresses[0..opts.trace.index] { - let line = StackLine::from_address(addr, &mut name_bytes); + for frame in 0..opts.trace.index { + let line = StackLine::from_frame(opts.trace, frame, &mut name_bytes); StackLine::write_encoded(line.as_ref(), writer)?; } @@ -3268,8 +3322,8 @@ mod draft { }); let mut name_bytes: [u8; 1024] = [0; 1024]; - for &addr in &trace.instruction_addresses[0..trace.index] { - let Some(line) = StackLine::from_address(addr, &mut name_bytes) else { + for frame in 0..trace.index { + let Some(line) = StackLine::from_frame(trace, frame, &mut name_bytes) else { continue; }; argv.push(format!("0x{:X}", line.address).into_bytes()); @@ -3373,15 +3427,15 @@ mod draft { if frame_index >= limits.frame_count { break; } - let return_address = stack_trace.instruction_addresses[frame_index]; - let source = match get_source_at_address(debug_info, return_address - 1)? { + let address = stack_trace.symbol_address(frame_index); + let source = match get_source_at_address(debug_info, address)? { Some(s) => s, None => { - let module_name = debug_info.get_module_name_for_address(return_address - 1); + let module_name = debug_info.get_module_name_for_address(address); print_line_info( out_stream, None, - return_address - 1, + address, b"???", module_name.as_deref().unwrap_or(b"???"), tty_config, @@ -3424,7 +3478,7 @@ mod draft { print_line_info( out_stream, source.source_location.as_ref(), - return_address - 1, + address, &source.symbol_name, &source.compile_unit_name, tty_config, diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 9b1fed3e91d9..162d4b096a55 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -119,6 +119,12 @@ export const cssInternals = { export const crash_handler = $rust("crash_handler.rs", "js_bindings.generate") as { getMachOImageZeroOffset: () => number; segfault: () => void; + /** Faults (SIGILL / illegal instruction) on the first instruction of a function. */ + faultAtFunctionEntry: () => void; + /** POSIX only. Traps (int3 / brk) on the first instruction of a function. */ + trapAtFunctionEntry?: () => void; + /** Crashes with a one-frame trace holding `faultAtFunctionEntry`'s entry as a return address. */ + functionEntryAsReturnAddress: () => void; panic: () => void; rootError: () => void; outOfMemory: () => void; diff --git a/src/runtime/api/crash_handler_jsc.rs b/src/runtime/api/crash_handler_jsc.rs index d573f171b4af..9dd7210c6695 100644 --- a/src/runtime/api/crash_handler_jsc.rs +++ b/src/runtime/api/crash_handler_jsc.rs @@ -23,6 +23,16 @@ pub(crate) mod js_bindings { ("getFeatureData", __jsc_host_js_get_feature_data), ("segfault", __jsc_host_js_segfault), ("segfaultInDll", __jsc_host_js_segfault_in_dll), + ( + "faultAtFunctionEntry", + __jsc_host_js_fault_at_function_entry, + ), + #[cfg(unix)] + ("trapAtFunctionEntry", __jsc_host_js_trap_at_function_entry), + ( + "functionEntryAsReturnAddress", + __jsc_host_js_function_entry_as_return_address, + ), ("panic", __jsc_host_js_panic), ("rootError", __jsc_host_js_root_error), ("outOfMemory", __jsc_host_js_out_of_memory), @@ -126,6 +136,91 @@ pub(crate) mod js_bindings { Ok(JSValue::UNDEFINED) } + /// Faults on its very first instruction, so a crash report's frame 0 is + /// exactly this function's entry address. Symbolized as the fault pc it is, + /// frame 0 names this function; stepped back one byte the way a return + /// address is, it names whatever the linker placed before it. + #[cfg(target_arch = "x86_64")] + #[unsafe(naked)] + extern "C" fn fault_at_function_entry() -> ! { + core::arch::naked_asm!("ud2") + } + #[cfg(target_arch = "aarch64")] + #[unsafe(naked)] + extern "C" fn fault_at_function_entry() -> ! { + core::arch::naked_asm!("udf #0") + } + + /// Trap-class counterpart of `fault_at_function_entry`: the kernel reports + /// x86_64 `int3` with pc already past it and aarch64 `brk` with pc on it, + /// and the report has to resolve frame 0 to this function either way. + #[cfg(all(unix, target_arch = "x86_64"))] + #[unsafe(naked)] + extern "C" fn trap_at_function_entry() -> ! { + core::arch::naked_asm!("int3", "ud2") + } + #[cfg(all(unix, target_arch = "aarch64"))] + #[unsafe(naked)] + extern "C" fn trap_at_function_entry() -> ! { + core::arch::naked_asm!("brk #0") + } + + #[bun_jsc::host_fn] + fn js_fault_at_function_entry( + _global: &JSGlobalObject, + _frame: &CallFrame, + ) -> JsResult { + crash_handler::suppress_core_dumps_if_necessary(); + #[cfg(unix)] + if Environment::ENABLE_ASAN { + // No fault handlers are installed under ASAN (see `js_segfault`), so + // hand the handler what SIGILL would have delivered: pc on the + // instruction itself. + let pc = fault_at_function_entry as *const () as usize; + crash_handler::crash_handler( + crash_handler::CrashReason::IllegalInstruction(pc), + crash_handler::TraceSeed::Fault { + pc, + fp: 0, + exact_pc: true, + }, + ); + } + fault_at_function_entry() + } + + /// Only meaningful with the real signal handler installed: under ASAN the + /// trap simply kills the process. + #[cfg(unix)] + #[bun_jsc::host_fn] + fn js_trap_at_function_entry( + _global: &JSGlobalObject, + _frame: &CallFrame, + ) -> JsResult { + crash_handler::suppress_core_dumps_if_necessary(); + trap_at_function_entry() + } + + /// Control for the two hooks above: the same entry address reported as an + /// ordinary return-address frame, which the report steps back one byte. + #[bun_jsc::host_fn] + fn js_function_entry_as_return_address( + _global: &JSGlobalObject, + _frame: &CallFrame, + ) -> JsResult { + crash_handler::suppress_core_dumps_if_necessary(); + let frames = [fault_at_function_entry as *const () as usize]; + let trace = bun_core::StackTrace { + index: frames.len(), + instruction_addresses: &frames, + first_frame_is_exact_pc: false, + }; + crash_handler::crash_handler( + crash_handler::CrashReason::IllegalInstruction(frames[0]), + crash_handler::TraceSeed::ErrorReturn(&trace), + ) + } + #[bun_jsc::host_fn] fn js_panic(_global: &JSGlobalObject, _frame: &CallFrame) -> JsResult { crash_handler::suppress_core_dumps_if_necessary(); diff --git a/src/sys/lib.rs b/src/sys/lib.rs index cf908fe8fec6..6c2c217af891 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -8678,7 +8678,7 @@ pub mod elf { /// Walk loaded ELF objects /// via `dl_iterate_phdr`, returning the one whose `PT_LOAD` segment contains - /// `address`. Shared by `bun_crash_handler::StackLine::from_address` and + /// `address`. Shared by `bun_crash_handler::StackLine::from_frame` and /// `bun_jsc::btjs::SelfInfo::lookup_module_dl` / `lookup_module_name_dl`. #[cfg(not(any(windows, target_os = "macos")))] pub fn find_loaded_module(address: usize) -> Option { diff --git a/test/cli/run/fixture-crash.js b/test/cli/run/fixture-crash.js index d44ba5bbc61d..fd1077d2f0e6 100644 --- a/test/cli/run/fixture-crash.js +++ b/test/cli/run/fixture-crash.js @@ -12,6 +12,7 @@ if (approach in crash_handler) { crash_handler[approach](); } else { console.error( - "usage: bun fixture-crash.js ", + "usage: bun fixture-crash.js ", ); + process.exit(2); } diff --git a/test/cli/run/run-crash-handler.test.ts b/test/cli/run/run-crash-handler.test.ts index af71635faddf..ae5c83141a9f 100644 --- a/test/cli/run/run-crash-handler.test.ts +++ b/test/cli/run/run-crash-handler.test.ts @@ -43,6 +43,123 @@ test.if(isDebug && isLinux && hasSymbolizer)( 60_000, // symbolizing the debug binary takes several seconds ); +// Frame 0 of a fault trace is the faulting instruction itself; every other +// frame is a return address, which points one past its call and is therefore +// symbolized one byte back so it lands inside the call. Applying that step to +// the fault pc as well blames the instruction before the fault. The hook +// faults on the first instruction of a function, where the skew is most +// visible: one byte back is the padding (or the function) before it. +test.if(isDebug && isLinux && hasSymbolizer)( + "a fault on the first instruction of a function is symbolized to that function", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "fixture-crash.js"), "faultAtFunctionEntry"], + env: noReportEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("Illegal instruction at address"); + expect(exitCode).not.toBe(0); + + const firstFrame = stdout.split("\n").find(line => line.trim().length > 0); + expect(firstFrame ?? "").toContain("::fault_at_function_entry"); + }, + 60_000, // symbolizing the debug binary takes several seconds +); + +// The same property at the trace-string level, which is what bun.report +// decodes, on every platform and build flavor. All three approaches crash with +// frame 0 holding the entry address of one and the same function: +// `faultAtFunctionEntry` / `trapAtFunctionEntry` really fault / trap on its +// first instruction, `functionEntryAsReturnAddress` reports the address as an +// ordinary return-address frame. Frames are encoded image-relative, so the +// offsets are comparable across the two processes regardless of ASLR, and the +// return-address encoding is by definition one below the function's entry. +describe.concurrent("fault trace strings encode frame 0 as the faulting instruction", () => { + const VLQ_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + + // Source-map style VLQ, as bun.report's decoder reads it: 5 data bits per + // character, bit 32 continues, bit 0 of the assembled value is the sign. + function decodeVLQ(encoded: string, start: number): { value: number; end: number } { + let assembled = 0; + let scale = 1; + let i = start; + for (;;) { + const digit = VLQ_ALPHABET.indexOf(encoded[i] ?? ""); + if (digit < 0) throw new Error(`no VLQ at index ${i} of ${JSON.stringify(encoded)}`); + i++; + assembled += (digit & 31) * scale; + scale *= 32; + if ((digit & 32) === 0) break; + } + const magnitude = Math.floor(assembled / 2); + return { value: assembled % 2 === 1 ? -magnitude : magnitude, end: i }; + } + + // Trace string layout (see `encode_trace_string`): platform char, command + // char, trace-string version char, 7-char commit, packed features as two + // VLQs, then one VLQ per frame. + function decodeFrameZero(trace: string): number { + let i = 1 + 1 + 1 + 7; + i = decodeVLQ(trace, i).end; + i = decodeVLQ(trace, i).end; + return decodeVLQ(trace, i).value; + } + + async function frameZeroOf(approach: string): Promise { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + path.join(import.meta.dir, "fixture-crash.js"), + approach, + // Debug builds otherwise symbolize locally instead of printing the + // trace string. + "--debug-crash-handler-use-trace-string", + ], + env: noReportEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Printed as `{report url}/{x.y.z}/{trace string}`; the report url is + // empty in `noReportEnv`. + const version = Bun.version.match(/^\d+\.\d+\.\d+/)![0]; + const trace = stderr.match(new RegExp(`/${version.replaceAll(".", "\\.")}/(\\S+)`)); + expect(trace, `no trace string in:\n${stderr}`).not.toBeNull(); + expect(exitCode).not.toBe(0); + return decodeFrameZero(trace![1]); + } + + test("fault pc is not stepped back like a return address", async () => { + const [fault, asReturnAddress] = await Promise.all([ + frameZeroOf("faultAtFunctionEntry"), + frameZeroOf("functionEntryAsReturnAddress"), + ]); + expect(asReturnAddress).toBeGreaterThan(0); + if (isWindows) { + // Windows trace strings carry every address as captured; bun.report + // steps the frames after the first back itself. + expect(fault).toBe(asReturnAddress); + } else { + expect(fault).toBe(asReturnAddress + 1); + } + }); + + // The kernel reports an x86_64 `int3` with pc already past it (stepping back + // is what lands on the trap, exactly like a return address) but an aarch64 + // `brk` with pc on the instruction; either way the report must resolve to + // the function that trapped. Needs the real signal handler, which ASAN + // builds do not install, and Windows claims no trap-class exceptions. + test.skipIf(isASAN || isWindows)("trap pc resolves to the trapping instruction", async () => { + const [trap, asReturnAddress] = await Promise.all([ + frameZeroOf("trapAtFunctionEntry"), + frameZeroOf("functionEntryAsReturnAddress"), + ]); + expect(trap).toBe(asReturnAddress + 1); + }); +}); + // `crash()` resets fatal-signal dispositions to SIG_DFL before re-raising so // that JS-registered listeners (`process.on("SIGABRT")` etc., installed by // npm's widely-used signal-exit package) cannot swallow the termination. A