diff --git a/src/crash_handler/lib.rs b/src/crash_handler/lib.rs index a5cb8c8966ea..6953fd661192 100644 --- a/src/crash_handler/lib.rs +++ b/src/crash_handler/lib.rs @@ -793,7 +793,7 @@ mod draft { Output::disable_scoped_debug_writer(); } - let mut trace_str_buf = BoundedArray::::default(); + let mut trace_str_buf = TraceStringBuf::default(); match PANIC_STAGE.with(|s| s.get()) { 0 => { @@ -1777,7 +1777,7 @@ mod draft { let reason = CrashReason::Panic(unsafe { bun_collections::detach_lifetime(msg_buf.const_slice()) }); - let mut trace_str_buf = BoundedArray::::default(); + let mut trace_str_buf = TraceStringBuf::default(); { let _panic_guard = PANIC_MUTEX.lock(); let writer = &mut stderr_writer(); @@ -2599,7 +2599,11 @@ mod draft { action: TraceStringAction, } - #[derive(Clone, Copy, PartialEq, Eq)] + /// Trace strings are built in a stack buffer of this size; the panic message is cut to fit. + const TRACE_STRING_MAX_LEN: usize = 1024; + type TraceStringBuf = BoundedArray; + + #[derive(Clone, Copy)] enum TraceStringAction { /// Open a pre-filled GitHub issue with the expanded trace OpenIssue, @@ -2609,13 +2613,19 @@ mod draft { impl<'a> fmt::Display for TraceString<'a> { fn fmt(&self, writer: &mut fmt::Formatter<'_>) -> fmt::Result { - // encode_trace_string takes a byte writer; bridge fmt::Formatter via adapter - let _ = encode_trace_string(self, &mut FmtAdapter::new(writer)); - Ok(()) + let mut buf = TraceStringBuf::default(); + // Whatever was encoded before an error is still worth printing. + let _ = encode_trace_string(self, &mut buf); + FmtAdapter::new(writer) + .write_all(buf.const_slice()) + .map_err(|_| fmt::Error) } } - fn encode_trace_string(opts: &TraceString<'_>, writer: &mut impl Write) -> crate::Result<()> { + fn encode_trace_string( + opts: &TraceString<'_>, + writer: &mut TraceStringBuf, + ) -> crate::Result<()> { writer.write_all(report_base_url())?; writer.write_all(b"/")?; writer.write_all(Environment::VERSION_STRING.as_bytes())?; @@ -2638,50 +2648,16 @@ mod draft { writer.write_all(VLQ::ZERO.slice())?; + let suffix: &[u8] = match opts.action { + TraceStringAction::OpenIssue => b"", + TraceStringAction::ViewTrace => b"/view", + }; + // The following switch must be kept in sync with `bun.report`'s decoder implementation. match opts.reason { CrashReason::Panic(message) => { writer.write_byte(b'0')?; - - let mut compressed_bytes: [u8; 2048] = [0; 2048]; - let mut len: bun_zlib::uLong = compressed_bytes.len() as bun_zlib::uLong; - // SAFETY: buffers and lengths are valid - let ret = unsafe { - bun_zlib::compress2( - compressed_bytes.as_mut_ptr(), - &raw mut len, - message.as_ptr(), - u32::try_from(message.len()).expect("int cast") as bun_zlib::uLong, - 9, - ) - }; - // Match on the raw zlib return code so this stays ABI-correct - // regardless of whether the platform `compress2` binding returns - // `c_int` (posix) or the `ReturnCode` enum (win32). - let compressed = match ret as i32 { - r if r == bun_zlib::ReturnCode::Ok as i32 => { - &compressed_bytes[0..usize::try_from(len).expect("int cast")] - } - // Insufficient memory. - r if r == bun_zlib::ReturnCode::MemError as i32 => { - return Err(crate::Error::Alloc(bun_alloc::AllocError)); - } - // The buffer dest was not large enough to hold the compressed data. - r if r == bun_zlib::ReturnCode::BufError as i32 => { - return Err(crate::Error::Sys(bun_errno::SystemErrno::ENOSPC)); - } - // The level was not Z_DEFAULT_LEVEL, or was not between 0 and 9. - // This is technically possible but impossible because we pass 9. - _ => return Err(crate::Error::Unexpected), - }; - - let mut b64_bytes: [u8; 2048] = [0; 2048]; - if bun_base64::encode_len(compressed) > b64_bytes.len() { - return Err(crate::Error::Sys(bun_errno::SystemErrno::ENOSPC)); - } - let b64_len = bun_base64::encode(&mut b64_bytes, compressed); - - writer.write_all(strings::trim_right(&b64_bytes[0..b64_len], b"="))?; + write_panic_message(writer, message, suffix.len())?; } CrashReason::Unreachable => writer.write_byte(b'1')?, @@ -2720,12 +2696,92 @@ mod draft { } } - if opts.action == TraceStringAction::ViewTrace { - writer.write_all(b"/view")?; - } + writer.write_all(suffix)?; Ok(()) } + /// Largest zlib stream whose unpadded base64 form fits a trace string. + const MAX_COMPRESSED_MESSAGE_LEN: usize = TRACE_STRING_MAX_LEN * 3 / 4; + + /// Appends base64(zlib(p)), p the longest message prefix that fits with `reserved` bytes spare. + fn write_panic_message( + writer: &mut TraceStringBuf, + message: &[u8], + reserved: usize, + ) -> crate::Result<()> { + let b64_budget = (TRACE_STRING_MAX_LEN - writer.len()).saturating_sub(reserved); + // Unpadded base64 takes ceil(4n / 3) bytes for n input bytes. + let max_compressed_len = (b64_budget * 3 / 4).min(MAX_COMPRESSED_MESSAGE_LEN); + let mut compressed: [u8; MAX_COMPRESSED_MESSAGE_LEN] = [0; MAX_COMPRESSED_MESSAGE_LEN]; + let dest = &mut compressed[..max_compressed_len]; + + // Bisect on the prefix length; the whole message is tried first since it normally fits. + let mut fits = 0; + let mut too_long = message.len() + 1; + let mut len = message.len(); + let compressed_len = loop { + match compress_message(dest, utf8_prefix(message, len))? { + Some(compressed_len) => { + fits = len; + if too_long - fits == 1 { + break compressed_len; + } + } + None => { + too_long = len; + if too_long == fits { + return Err(crate::Error::Sys(bun_errno::SystemErrno::ENOSPC)); + } + } + } + len = fits + (too_long - fits) / 2; + }; + + const B64_LEN: usize = bun_base64::encode_len_from_size(MAX_COMPRESSED_MESSAGE_LEN); + let mut b64: [u8; B64_LEN] = [0; B64_LEN]; + let b64_len = bun_base64::encode(&mut b64, &dest[..compressed_len]); + writer.write_all(strings::trim_right(&b64[..b64_len], b"="))?; + Ok(()) + } + + /// Length of the zlib stream of `message` written into `dest`, or `None` if it does not fit. + fn compress_message(dest: &mut [u8], message: &[u8]) -> crate::Result> { + // `uLong` is 32-bit on Windows; a message zlib cannot even take does not fit either. + let Ok(source_len) = bun_zlib::uLong::try_from(message.len()) else { + return Ok(None); + }; + let mut dest_len = dest.len() as bun_zlib::uLong; + // SAFETY: both pointers are valid for the lengths passed with them, and zlib writes at + // most `dest_len` bytes. + let ret = unsafe { + bun_zlib::compress2( + dest.as_mut_ptr(), + &raw mut dest_len, + message.as_ptr(), + source_len, + 9, + ) + }; + // The `compress2` binding returns `c_int` on posix and `ReturnCode` on win32. + match ret as i32 { + r if r == bun_zlib::ReturnCode::Ok as i32 => Ok(Some(dest_len as usize)), + r if r == bun_zlib::ReturnCode::BufError as i32 => Ok(None), + r if r == bun_zlib::ReturnCode::MemError as i32 => { + Err(crate::Error::Alloc(bun_alloc::AllocError)) + } + // Only reachable with an invalid level, and 9 is valid. + _ => Err(crate::Error::Unexpected), + } + } + + /// `message[..len]` backed up off UTF-8 continuation bytes; the whole message comes back as-is. + fn utf8_prefix(message: &[u8], mut len: usize) -> &[u8] { + while len < message.len() && len > 0 && message[len] & 0xC0 == 0x80 { + len -= 1; + } + &message[..len] + } + pub fn write_u64_as_two_vlqs(writer: &mut impl Write, addr: usize) -> crate::Result<()> { // `as u32 as i32` reinterprets the 32-bit halves, preserving bits. let first = VLQ::encode((((addr as u64) & 0xFFFFFFFF00000000) >> 32) as u32 as i32); diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 9b1fed3e91d9..577073b5fcb8 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -119,7 +119,7 @@ export const cssInternals = { export const crash_handler = $rust("crash_handler.rs", "js_bindings.generate") as { getMachOImageZeroOffset: () => number; segfault: () => void; - panic: () => void; + panic: (message?: string) => void; rootError: () => void; outOfMemory: () => void; abort: () => void; diff --git a/src/runtime/api/crash_handler_jsc.rs b/src/runtime/api/crash_handler_jsc.rs index d573f171b4af..014ac0649cab 100644 --- a/src/runtime/api/crash_handler_jsc.rs +++ b/src/runtime/api/crash_handler_jsc.rs @@ -126,10 +126,19 @@ pub(crate) mod js_bindings { Ok(JSValue::UNDEFINED) } + /// `panic(message?: string)`; tests pass a message to see how it ends up in the trace string. #[bun_jsc::host_fn] - fn js_panic(_global: &JSGlobalObject, _frame: &CallFrame) -> JsResult { + fn js_panic(global: &JSGlobalObject, frame: &CallFrame) -> JsResult { + let message_arg = frame.argument(0); + let custom_message; + let message: &[u8] = if message_arg.is_string() { + custom_message = BunString::from_js(message_arg, global)?.to_utf8(); + custom_message.slice() + } else { + b"invoked crashByPanic() handler" + }; crash_handler::suppress_core_dumps_if_necessary(); - crash_handler::panic_impl(b"invoked crashByPanic() handler", None, None); + crash_handler::panic_impl(message, None, None); } #[bun_jsc::host_fn] diff --git a/test/cli/run/run-crash-handler.test.ts b/test/cli/run/run-crash-handler.test.ts index 4d436bbedc7b..64c851b90be0 100644 --- a/test/cli/run/run-crash-handler.test.ts +++ b/test/cli/run/run-crash-handler.test.ts @@ -1,7 +1,9 @@ import { crash_handler } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isASAN, isDebug, isLinux, isPosix, isWindows, mergeWindowEnvs, tempDir } from "harness"; +import { createHash } from "node:crypto"; import { rmSync } from "node:fs"; +import { inflateSync } from "node:zlib"; import path from "path"; const { getMachOImageZeroOffset } = crash_handler; @@ -500,6 +502,125 @@ describe.if(isPosix)("SIGABRT/SIGTRAP are caught by the crash handler", () => { }); }); +// The trace string is built in a 1024-byte buffer (TRACE_STRING_MAX_LEN in +// src/crash_handler/lib.rs). A panic message whose compressed form did not fit +// in what the header and frames left of it used to be dropped altogether, +// leaving a '0' (panic) reason with no payload, which bun.report rejects. +// Now the longest prefix of the message that fits is encoded instead. +describe.concurrent("trace string panic message", () => { + const TRACE_STRING_MAX_LEN = 1024; + + // TRACE_STRING_MAX_LEN bytes off a hash chain: deterministic, and with 8 bits + // of entropy per byte nothing made of them deflates below that many bytes, + // while the message payload of a trace string holds at most 3/4 as many. + // So these messages cannot fit whatever the buffer size or the header length. + const entropy = (() => { + const chunks: Buffer[] = []; + let digest = Buffer.from("trace string panic message"); + for (let bytes = 0; bytes < TRACE_STRING_MAX_LEN; bytes += digest.length) { + digest = createHash("sha512").update(digest).digest(); + chunks.push(digest); + } + return Buffer.concat(chunks).subarray(0, TRACE_STRING_MAX_LEN); + })(); + + async function panicTraceString(message: string): Promise { + // The message goes through a file rather than argv so that the report's + // "Args:" line stays short. + using dir = tempDir("crash-panic-message", { + "crash.js": `require("bun:internal-for-testing").crash_handler.panic(${JSON.stringify(message)});`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(String(dir), "crash.js"), "--debug-crash-handler-use-trace-string"], + env: noReportEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(exitCode).not.toBe(0); + + // The trace string is printed on a line of its own. noReportEnv sets + // BUN_CRASH_REPORT_URL to "", so it has no base and starts at the version. + const trace = stderr.match(/^\s*(\S*\/\d+\.\d+\.\d+\/\S+)\s*$/m); + expect(trace, `no trace string in:\n${stdout}${stderr}`).not.toBeNull(); + return trace![1]; + } + + const VLQ_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + function readVLQ(s: string, i: number): [value: number, next: number] { + let raw = 0; + let shift = 0; + for (;;) { + const digit = VLQ_ALPHABET.indexOf(s[i++]); + expect(digit).toBeGreaterThanOrEqual(0); + raw |= (digit & 31) << shift; + shift += 5; + if (!(digit & 32)) break; + } + raw >>>= 0; + return [raw & 1 ? -(raw >>> 1) : raw >>> 1, i]; + } + + // Layout after `{base}/{version}/`, mirroring encode_trace_string and + // bun.report's parser: platform char, command char, format char, 7-char sha, + // two feature VLQs, the frames, a VLQ 0, the reason char, then its payload. + // A frame is `_` (unknown), a VLQ address, or VLQ 1 + VLQ name length + name + // + VLQ address for a frame in another module. A panic's payload is the + // zlib stream of the message in base64 without the padding. + function panicMessageIn(trace: string): string { + const payload = trace.match(/\/\d+\.\d+\.\d+\/(.*)$/)![1]; + let i = 1 + 1 + 1 + 7; + [, i] = readVLQ(payload, i); + [, i] = readVLQ(payload, i); + for (;;) { + if (payload[i] === "_") { + i++; + continue; + } + let address: number; + [address, i] = readVLQ(payload, i); + if (address === 0) break; + if (address === 1) { + let nameLength: number; + [nameLength, i] = readVLQ(payload, i); + i += nameLength; + [, i] = readVLQ(payload, i); + } + } + expect(payload[i]).toBe("0"); + const compressed = payload.slice(i + 1); + expect(compressed, "panic reason has no payload: the message was dropped").not.toBe(""); + // fatal: a cut that split a multi-byte character fails here as a decoding + // error rather than as a U+FFFD mismatch in the caller's comparison. + return new TextDecoder("utf-8", { fatal: true }).decode(inflateSync(Buffer.from(compressed, "base64"))); + } + + test.each([ + ["a short message", "crash_handler.panic() called from the trace string test"], + ["an empty message", ""], + ])("%s is encoded whole", async (_, message) => { + const trace = await panicTraceString(message); + expect(trace.length).toBeLessThanOrEqual(TRACE_STRING_MAX_LEN); + expect(panicMessageIn(trace)).toBe(message); + }); + + test.each([ + // Two one-byte characters per entropy byte. + ["ASCII", entropy.toString("hex")], + // One three-byte character per entropy byte, so a cut at an arbitrary byte offset would split one. + ["multi-byte", Array.from(entropy, byte => String.fromCharCode(0x4e00 + byte)).join("")], + ])("a message too long to fit is cut to the longest prefix that fits (%s)", async (_, message) => { + const trace = await panicTraceString(message); + const decoded = panicMessageIn(trace); + expect(decoded.length).toBeGreaterThan(0); + expect(decoded.length).toBeLessThan(message.length); + expect(decoded).toBe(message.slice(0, decoded.length)); + // Longest prefix that fits: the buffer ends up full to within deflate's + // per-character granularity, not cut at some shorter convenient point. + expect(trace.length).toBeGreaterThan(TRACE_STRING_MAX_LEN - 64); + expect(trace.length).toBeLessThanOrEqual(TRACE_STRING_MAX_LEN); + }); +}); + describe("automatic crash reporter", () => { for (const approach of ["panic", "segfault", "outOfMemory"]) { test(`${approach} should report`, async () => {