From a1f798cf0221b052f07cf39da76a53c3e3b32247 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 03:21:23 +0000 Subject: [PATCH 1/2] Decode trace format 4 and pick the debug file by the executable's debug id oven-sh/bun#38838 makes bun's crash handler emit trace format '4': after the sha, the build-flags VLQ of format 3 (bit 0 = canary) and then the id the linker stamped into the executable (PDB GUID, GNU build-id, LC_UUID) as a VLQ byte count plus lowercase hex. Until now a trace named its build by platform char plus sha, which is not a binary: a commit published as both bun-windows-x64 and bun-windows-x64-baseline reports 'w' from both links, and traces from the second were symbolized with the first one's PDB, producing plausible-looking nonsense. - lib/parser.ts: parse '4'; Parse.debug_id. - backend/debug-id.ts: read the same id out of the bun-profile executable in a profile zip (PE debug directory, ELF PT_NOTE, Mach-O LC_UUID), and the selection policy: check the trace's own arch artifact, fall back to the sibling x64 link that carries the id, flag a total mismatch. - backend/debug-store.ts, db.ts: record the artifact's id when it is downloaded (new nullable debug_file.debug_id column, added in place). - backend/remap.ts: on a mismatch leave the addresses unsymbolicated instead of remapping them against the wrong binary; Remap.arch is the link actually used; Remap.debug_id / debug_file carry the outcome. - backend/sentry.ts: arch/dist/baseline tags follow the link actually used; new debug_id and debug_file tags. - markdown, /remap response and the frontend footer say when a trace matched nothing. - lib/util.ts: the remap cache key includes the id so two links of one commit do not share entries; keys of older traces are unchanged. - tests: v4 roundtrips and field validation, synthetic PE/ELF/Mach-O files (the PE one pinned to a GUID llvm-readobj printed for a real bun-debug.exe), the selection policy, and two real v4 captures as parse fixtures. --- backend/db.ts | 44 ++- backend/debug-id.ts | 216 +++++++++++++ backend/debug-store.ts | 50 ++- backend/index.ts | 2 + backend/markdown.ts | 11 + backend/remap.ts | 25 +- backend/sentry.ts | 23 +- frontend/frontend.ts | 10 +- lib/parser.ts | 64 ++++ lib/util.ts | 4 + test/README.md | 4 +- test/__snapshots__/parse.test.ts.snap | 167 ++++++++++ test/debug-id.test.ts | 291 ++++++++++++++++++ .../parse/v4-linux-x86_64-panic-debug-id.json | 4 + .../v4-windows-x86_64-segfault-debug-id.json | 4 + test/helpers/encode.ts | 15 +- test/roundtrip.test.ts | 106 +++++++ 17 files changed, 1008 insertions(+), 32 deletions(-) create mode 100644 backend/debug-id.ts create mode 100644 test/debug-id.test.ts create mode 100644 test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json create mode 100644 test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json diff --git a/backend/db.ts b/backend/db.ts index 7ee9af8..00f5548 100644 --- a/backend/db.ts +++ b/backend/db.ts @@ -37,6 +37,18 @@ initTable( last_updated INTEGER NOT NULL `, ); +// Added after the table existed in production: the id read from the +// executable in the profile zip (see debug-id.ts), NULL for rows cached +// before this column existed. +if ( + !db + .query("PRAGMA table_info(debug_file)") + .all() + .some((c: any) => c.name === "debug_id") +) { + db.run("ALTER TABLE debug_file ADD COLUMN debug_id TEXT"); +} + initTable( "issues", ` @@ -57,9 +69,11 @@ const insert_remap_stmt = db.prepare( "INSERT OR REPLACE INTO remap (cache_key, remapped_data) VALUES (?, ?)", ); -const get_debug_file_stmt = db.prepare("SELECT file_path FROM debug_file WHERE cache_key = ?"); +const get_debug_file_stmt = db.prepare( + "SELECT file_path, debug_id FROM debug_file WHERE cache_key = ?", +); const insert_debug_file_stmt = db.prepare( - "INSERT INTO debug_file (cache_key, file_path, last_updated) VALUES (?, ?, ?)", + "INSERT INTO debug_file (cache_key, file_path, debug_id, last_updated) VALUES (?, ?, ?, ?)", ); const update_debug_file_stmt = db.prepare( "UPDATE debug_file SET last_updated = ? WHERE cache_key = ?", @@ -91,21 +105,37 @@ export function putCachedRemap(cache_key: string, remap: Remap) { insert_remap_stmt.run(cache_key, JSON.stringify(remap)); } -export function getCachedDebugFile(os: Platform, arch: Arch, commit: string): string | null { +export interface CachedDebugFile { + file_path: string; + /** undefined when the executable's id could not be read, or for rows older than the column. */ + debug_id: string | undefined; +} + +export function getCachedDebugFile( + os: Platform, + arch: Arch, + commit: string, +): CachedDebugFile | null { const cache_key = `${os}-${arch}-${commit}`; const result = get_debug_file_stmt.get(cache_key) as { file_path: string; - last_updated: string; + debug_id: string | null; } | null; if (result) { update_debug_file_stmt.run(Date.now(), cache_key); - return result.file_path; + return { file_path: result.file_path, debug_id: result.debug_id ?? undefined }; } return null; } -export function putCachedDebugFile(os: Platform, arch: Arch, commit: string, file_path: string) { - insert_debug_file_stmt.run(`${os}-${arch}-${commit}`, file_path, Date.now()); +export function putCachedDebugFile( + os: Platform, + arch: Arch, + commit: string, + file_path: string, + debug_id: string | undefined, +) { + insert_debug_file_stmt.run(`${os}-${arch}-${commit}`, file_path, debug_id ?? null, Date.now()); } export function getCachedFeatureData( diff --git a/backend/debug-id.ts b/backend/debug-id.ts new file mode 100644 index 0000000..123b925 --- /dev/null +++ b/backend/debug-id.ts @@ -0,0 +1,216 @@ +import { closeSync, openSync, readSync } from "node:fs"; +import type { Arch } from "../lib/util"; + +// Deliberately free of backend imports (db, git, ...) so the selection policy +// below is unit-testable; debug-store.ts supplies the downloading. + +export interface DebugFileCheck { + /** Set only when the trace carried a debug id to check against. See `Remap.debug_file`. */ + debug_file?: "match" | "mismatch" | "unverified"; +} + +/** + * The other links a commit may be published as for the same os. A v4 trace + * says `'w'` for both Windows x64 links of a commit when the build that made + * it did not know which zip it would ship in, so the debug id decides. + */ +const sibling_archs: Partial> = { + x86_64: ["x86_64_baseline"], + x86_64_baseline: ["x86_64"], +}; + +function isUnavailable(e: unknown): boolean { + return (e as any)?.code === "DebugInfoUnavailable"; +} + +/** + * Which of a commit's links to symbolize a trace with. `fetch(arch)` yields + * that arch's artifact (with the id read from its executable, if readable) + * or throws `DebugInfoUnavailable` when the commit was not published under + * that name. + * + * - no trace id (formats 1-3): the trace's own arch, unchecked, as before. + * - it matches the trace's arch artifact: use it. + * - that artifact's id is unreadable: use it, flagged "unverified" (there is + * no evidence either way, so behave as before). + * - otherwise try the sibling links; the one carrying the id wins and a + * missing sibling is skipped. When the trace's own artifact is missing + * altogether this is also how a trace published only under the other + * name gets symbolized at all. + * - nothing carries the id: "mismatch". The caller then leaves the addresses + * unsymbolicated; remapping them against another link's debug info is what + * produced confidently wrong reports before the id existed. + */ +export async function selectDebugFile( + arch: Arch, + debug_id: string | undefined, + fetch: (arch: Arch) => Promise, +): Promise { + let primary: T | undefined; + let primary_error: unknown; + try { + primary = await fetch(arch); + } catch (e) { + if (debug_id === undefined || !isUnavailable(e)) throw e; + primary_error = e; + } + if (debug_id === undefined) return primary!; + if (primary) { + if (primary.debug_id === debug_id) return { ...primary, debug_file: "match" }; + if (primary.debug_id === undefined) return { ...primary, debug_file: "unverified" }; + } + + for (const sibling of sibling_archs[arch] ?? []) { + let candidate: T; + try { + candidate = await fetch(sibling); + } catch (e) { + if (isUnavailable(e)) continue; + throw e; + } + if (candidate.debug_id === debug_id) return { ...candidate, debug_file: "match" }; + } + + if (primary === undefined) throw primary_error; + return { ...primary, debug_file: "mismatch" }; +} + +/** + * The id a linker stamps into an executable and its debug info, read from the + * executable shipped in a `*-profile.zip`, in the same form bun's crash + * handler puts in a v4 trace string (`src/crash_handler/debug_id.rs`): + * lowercase hex, bytes in the order the platform's tools print them. + * + * PE CodeView (RSDS) record referenced by the debug data directory: + * the PDB GUID, first three fields byte-swapped into textual order. + * ELF descriptor of the NT_GNU_BUILD_ID note in a PT_NOTE segment. + * Mach-O the LC_UUID load command. + * + * Returns undefined when the file has no id or is not one of those formats; + * the caller treats that as "cannot verify", never as a mismatch. + */ +export function readExecutableDebugId(path: string): string | undefined { + let fd: number; + try { + fd = openSync(path, "r"); + } catch { + return undefined; + } + try { + const magic = readAt(fd, 0, 4); + if (magic.toString("latin1") === "\x7fELF") return elfBuildId(fd); + if (magic.toString("latin1", 0, 2) === "MZ") return peCodeViewGuid(fd); + if (magic.readUInt32LE(0) === 0xfeedfacf) return machoUuid(fd); + return undefined; + } catch { + return undefined; + } finally { + closeSync(fd); + } +} + +function readAt(fd: number, position: number, length: number): Buffer { + const buffer = Buffer.alloc(length); + for (let done = 0; done < length; ) { + const n = readSync(fd, buffer, done, length - done, position + done); + if (n === 0) throw new Error(`short read at ${position}`); + done += n; + } + return buffer; +} + +function elfBuildId(fd: number): string | undefined { + const header = readAt(fd, 0, 64); + const phoff = Number(header.readBigUInt64LE(0x20)); + const phentsize = header.readUInt16LE(0x36); + const phnum = header.readUInt16LE(0x38); + const phdrs = readAt(fd, phoff, phentsize * phnum); + for (let n = 0; n < phnum; n++) { + const phdr = phdrs.subarray(n * phentsize); + if (phdr.readUInt32LE(0) !== 4 /* PT_NOTE */) continue; + const notes = readAt(fd, Number(phdr.readBigUInt64LE(8)), Number(phdr.readBigUInt64LE(32))); + // Elf64_Nhdr {namesz, descsz, type}, then name and descriptor, each padded to 4. + for (let off = 0; off + 12 <= notes.length; ) { + const name_size = notes.readUInt32LE(off); + const desc_size = notes.readUInt32LE(off + 4); + const type = notes.readUInt32LE(off + 8); + const desc_start = (off + 12 + name_size + 3) & ~3; + if (desc_start + desc_size > notes.length) return undefined; + if ( + type === 3 /* NT_GNU_BUILD_ID */ && + notes.toString("latin1", off + 12, off + 12 + name_size) === "GNU\0" + ) { + return desc_size > 0 + ? notes.toString("hex", desc_start, desc_start + desc_size) + : undefined; + } + off = (desc_start + desc_size + 3) & ~3; + } + } + return undefined; +} + +function peCodeViewGuid(fd: number): string | undefined { + const pe_offset = readAt(fd, 0x3c, 4).readUInt32LE(0); + const file_header = readAt(fd, pe_offset, 24); + if (file_header.toString("latin1", 0, 4) !== "PE\0\0") return undefined; + const section_count = file_header.readUInt16LE(6); + const optional_header_size = file_header.readUInt16LE(20); + const optional_header = readAt(fd, pe_offset + 24, optional_header_size); + if (optional_header.readUInt16LE(0) !== 0x20b /* PE32+ */) return undefined; + // IMAGE_OPTIONAL_HEADER64: NumberOfRvaAndSizes at 108, DataDirectory at 112, + // 8 bytes per entry, IMAGE_DIRECTORY_ENTRY_DEBUG = 6. + if (optional_header.readUInt32LE(108) <= 6) return undefined; + const debug_rva = optional_header.readUInt32LE(112 + 6 * 8); + const debug_size = optional_header.readUInt32LE(112 + 6 * 8 + 4); + if (debug_rva === 0 || debug_size === 0) return undefined; + + const sections = readAt(fd, pe_offset + 24 + optional_header_size, section_count * 40); + let debug_offset: number | undefined; + for (let n = 0; n < section_count; n++) { + const section = sections.subarray(n * 40); + const virtual_address = section.readUInt32LE(12); + if (debug_rva >= virtual_address && debug_rva < virtual_address + section.readUInt32LE(16)) { + debug_offset = debug_rva - virtual_address + section.readUInt32LE(20); + } + } + if (debug_offset === undefined) return undefined; + + // IMAGE_DEBUG_DIRECTORY (28 bytes): Type at 12, PointerToRawData at 24. + const entries = readAt(fd, debug_offset, debug_size); + for (let off = 0; off + 28 <= entries.length; off += 28) { + if (entries.readUInt32LE(off + 12) !== 2 /* IMAGE_DEBUG_TYPE_CODEVIEW */) continue; + const pointer = entries.readUInt32LE(off + 24); + if (pointer === 0) continue; + const code_view = readAt(fd, pointer, 20); + if (code_view.toString("latin1", 0, 4) !== "RSDS") continue; + const g = code_view.subarray(4, 20); + return Buffer.from([ + g[3], + g[2], + g[1], + g[0], + g[5], + g[4], + g[7], + g[6], + ...g.subarray(8, 16), + ]).toString("hex"); + } + return undefined; +} + +function machoUuid(fd: number): string | undefined { + const header = readAt(fd, 0, 32); + const ncmds = header.readUInt32LE(16); + const commands = readAt(fd, 32, header.readUInt32LE(20)); + for (let n = 0, off = 0; n < ncmds && off + 8 <= commands.length; n++) { + const size = commands.readUInt32LE(off + 4); + if (size < 8) return undefined; + if (commands.readUInt32LE(off) === 0x1b /* LC_UUID */ && size >= 24) { + return commands.toString("hex", off + 8, off + 24); + } + off += size; + } + return undefined; +} diff --git a/backend/debug-store.ts b/backend/debug-store.ts index 68b5827..f01928b 100644 --- a/backend/debug-store.ts +++ b/backend/debug-store.ts @@ -13,14 +13,21 @@ import type { ResolvedCommit } from "../lib"; import { octokit } from "./git"; import type { FeatureConfig } from "./feature"; import { AsyncMutexMap } from "./mutex"; +import { readExecutableDebugId, selectDebugFile, type DebugFileCheck } from "./debug-id"; export const cache_root = join(import.meta.dir, "..", ".cache"); -interface DebugInfo { +export interface DebugInfo { file_path: string; feature_config: FeatureConfig; + /** The arch whose artifact this is. Differs from the trace's arch when a sibling link matched. */ + arch: Arch; + /** Read from the executable in the artifact; undefined when it could not be. */ + debug_id: string | undefined; } +export type SelectedDebugInfo = DebugInfo & DebugFileCheck; + export function storeRoot(platform: Platform, arch: Arch, is_canary: boolean | undefined) { return join(cache_root, platform + "-" + arch + (is_canary ? "-canary" : "")); } @@ -49,11 +56,29 @@ const map_download_os = { freebsd: "freebsd", } as const; +/** + * The debug file to symbolize a trace with. Without a `debug_id` (trace + * formats 1-3) that is the artifact named by the trace's arch, as it always + * was. With one, the artifact is checked against it and the commit's sibling + * links are tried when it does not match; see `selectDebugFile`. + */ export async function fetchDebugFile( os: Platform, arch: Arch, commit: ResolvedCommit, is_canary: boolean | undefined, + debug_id?: string, +): Promise { + return selectDebugFile(arch, debug_id, (candidate) => + fetchArtifact(os, candidate, commit, is_canary), + ); +} + +async function fetchArtifact( + os: Platform, + arch: Arch, + commit: ResolvedCommit, + is_canary: boolean | undefined, ): Promise { const oid = commit.oid; assert(oid.length === 40); @@ -74,15 +99,17 @@ async function fetchDebugFileWithoutCache( is_canary: boolean | undefined, store_suffix: string, path: string, -) { +): Promise { const oid = commit.oid; - const cached_path = getCachedDebugFile(os, arch, oid); - if (cached_path) { + const cached = getCachedDebugFile(os, arch, oid); + if (cached) { const feature_config = getCachedFeatureData(oid, is_canary)!; return { - file_path: cached_path, + file_path: cached.file_path, feature_config: feature_config, + arch, + debug_id: cached.debug_id, }; } @@ -93,6 +120,7 @@ async function fetchDebugFileWithoutCache( } let feature_config: FeatureConfig; + let debug_id: string | undefined; try { if (process.env.NODE_ENV === "development") { @@ -172,6 +200,14 @@ async function fetchDebugFileWithoutCache( throw new Error(`Failed to find ${relative(tmp.path, desired_file)} in extraction`); } + // The zip also holds the executable the debug file belongs to (the same + // link users run, so it carries the same id a v4 trace reports). Read the + // id before the debug file is moved out; on Linux they are the same file. + const executable = entries.find( + (entry) => entry === "bun-profile" || entry === "bun-profile.exe", + ); + debug_id = executable ? readExecutableDebugId(join(tmp.path, dir, executable)) : undefined; + await mkdir(dirname(path), { recursive: true }); await rename(desired_file, path); @@ -188,7 +224,7 @@ async function fetchDebugFileWithoutCache( feature_config ??= getCachedFeatureData(oid, is_canary) ?? (await fetchFeatureData(oid, is_canary)); - putCachedDebugFile(os, arch, oid, path); + putCachedDebugFile(os, arch, oid, path, debug_id); } catch (e) { await rm(path, { force: true }); throw e; @@ -197,6 +233,8 @@ async function fetchDebugFileWithoutCache( return { file_path: path, feature_config, + arch, + debug_id, }; } diff --git a/backend/index.ts b/backend/index.ts index 2516f8f..e57ff44 100644 --- a/backend/index.ts +++ b/backend/index.ts @@ -250,6 +250,8 @@ async function postRemap(request: Request, server: Server) { command: remapped.command, version: remapped.version, features: remapped.features, + arch: remapped.arch, + ...(remapped.debug_file ? { debug_file: remapped.debug_file } : {}), } satisfies RemapAPIResponse); } catch (e) { return handleError(url, e, false); diff --git a/backend/markdown.ts b/backend/markdown.ts index 79b3bcd..4fb77cf 100644 --- a/backend/markdown.ts +++ b/backend/markdown.ts @@ -13,6 +13,7 @@ export async function formatMarkdown(remap: Remap, internal?: { source: string } "", remap.features.length > 0 ? `Features: ${remap.features.map(escmd).join(", ")}` : "", "", + ...debugFileNote(remap), ...(internal ? [`[(see trace)]()`] : []), @@ -22,6 +23,16 @@ export async function formatMarkdown(remap: Remap, internal?: { source: string } .replace(/\n\n+/g, "\n\n"); } +function debugFileNote(remap: Remap): string[] { + if (!remap.debug_id) return []; + const note = `Debug id: \`${remap.debug_id}\``; + if (remap.debug_file !== "mismatch") return [note, ""]; + return [ + `${note} (no published ${remap.os} ${remap.arch} build of this commit has it, so the addresses above are not symbolicated)`, + "", + ]; +} + function treeURLMD(commit: ResolvedCommit) { // if (commit.pr) { // return `[#${commit.pr.number}](https://github.com/oven-sh/bun/pull/${commit.pr.number})`; diff --git a/backend/remap.ts b/backend/remap.ts index 9c70e2f..abbe173 100644 --- a/backend/remap.ts +++ b/backend/remap.ts @@ -2,10 +2,10 @@ import type { Parse, Remap, ResolvedCommit } from "../lib/parser"; import { getCommit } from "./git"; import { fetchDebugFile } from "./debug-store"; import { getCachedRemap, putCachedRemap } from "./db"; -import { parseCacheKey } from "../lib/util"; +import { parseCacheKey, type Arch } from "../lib/util"; import { llvm_symbolizer, pdb_addr2line } from "./system-deps"; import { formatMarkdown } from "./markdown"; -import { decodeFeatures } from "./feature"; +import { decodeFeatures, type FeatureConfig } from "./feature"; import { AsyncMutexMap } from "./mutex"; import { adjustBunAddresses, processSymbolizerOutput, filterAddresses } from "./symbolize"; @@ -78,12 +78,17 @@ export async function remapUncached( throw e; } - const debug_info = opts.exe + const debug_info: { + file_path: string; + feature_config: FeatureConfig | null; + arch?: Arch; + debug_file?: Remap["debug_file"]; + } = opts.exe ? { file_path: opts.exe, feature_config: null, } - : await fetchDebugFile(parse.os, parse.arch, commit, parse.is_canary); + : await fetchDebugFile(parse.os, parse.arch, commit, parse.is_canary, parse.debug_id); if (!debug_info) { const e: any = new Error(`Could not find debug file for ${parse.os}-${parse.arch} for commit ${parse.commitish}`); @@ -94,7 +99,11 @@ export async function remapUncached( let stdout = ""; const bun_addrs = adjustBunAddresses(parse.addresses, parse.os); - if (bun_addrs.length > 0) { + // A mismatch means no published link of this commit is the binary that + // crashed; its debug info describes different code at these addresses, so + // the frames stay raw (processSymbolizerOutput with no output) instead of + // being remapped into plausible-looking nonsense. + if (bun_addrs.length > 0 && debug_info.debug_file !== "mismatch") { const cmd = [ parse.os === "windows" ? pdb_addr2line : llvm_symbolizer, "--exe", @@ -139,15 +148,17 @@ export async function remapUncached( ? "StandaloneExecutable" : (command_map[parse.command] ?? parse.command); - const remap = { + const remap: Remap = { version: display_version, message: parse.message, os: parse.os, - arch: parse.arch, + arch: debug_info.arch ?? parse.arch, commit: commit, addresses: mapped_addrs, command, features, + ...(parse.debug_id ? { debug_id: parse.debug_id } : {}), + ...(debug_info.debug_file ? { debug_file: debug_info.debug_file } : {}), }; putCachedRemap(key, remap); diff --git a/backend/sentry.ts b/backend/sentry.ts index be31016..9a1dc5d 100644 --- a/backend/sentry.ts +++ b/backend/sentry.ts @@ -31,7 +31,7 @@ async function remapToPayload(parse: Parse, remap: Remap, trace_str: string): Pr event_id, platform: "other", release: `bun@${remap.version}+${remap.commit.oid.slice(0, 9)}`, - dist: buildDist(parse), + dist: buildDist(remap), level: "fatal", transaction: remap.command, tags: getTags(parse, remap), @@ -43,7 +43,7 @@ async function remapToPayload(parse: Parse, remap: Remap, trace_str: string): Pr version: remap.version + "+" + remap.commit.oid.slice(0, 9), }, os: getOSContext(parse), - device: getOSDeviceContext(parse), + device: getOSDeviceContext(remap), }, timestamp: new Date().getTime() / 1000, environment: parse.is_canary ? "canary" : "production", @@ -62,7 +62,9 @@ function getTags(parse: Parse, remap: Remap): any { tags.version = remap.version; tags.commit = remap.commit.oid.slice(0, 9); - tags.arch = parse.arch.replace(/_baseline$/, ""); + // remap.arch, not parse.arch: for a v4 trace it is the link whose debug + // info actually carried the trace's debug id (see debug-store.ts). + tags.arch = remap.arch.replace(/_baseline$/, ""); // cache_key is SHA256(commitish_arch_os_canary_addresses). Before the // randomUUID switch, MD5(cache_key) was the event_id — so Sentry deduped // identical (stack, build) tuples to one event. Sending it as a tag lets @@ -77,7 +79,7 @@ function getTags(parse: Parse, remap: Remap): any { tags[feature] = true; } - if (parse.arch.endsWith("_baseline")) { + if (remap.arch.endsWith("_baseline")) { tags.baseline = true; } @@ -85,6 +87,11 @@ function getTags(parse: Parse, remap: Remap): any { if (parse.fault_address) tags.fault_address = "0x" + parse.fault_address; + // Which exact link crashed, and whether the frames above were symbolicated + // against it. `debug_file:mismatch` events have raw frames on purpose. + if (remap.debug_id) tags.debug_id = remap.debug_id; + if (remap.debug_file) tags.debug_file = remap.debug_file; + return tags; } @@ -93,8 +100,8 @@ function getTags(parse: Parse, remap: Remap): any { * different compile flags. For bun that's baseline (older-CPU target) and musl * (Alpine/musl libc). undefined means the standard build for this os/arch. */ -function buildDist(parse: Parse): string | undefined { - return parse.arch.endsWith("_baseline") ? "baseline" : undefined; +function buildDist(remap: Remap): string | undefined { + return remap.arch.endsWith("_baseline") ? "baseline" : undefined; } function getOSContext(parse: Parse): Sentry.OS { @@ -119,8 +126,8 @@ function buildExtra(remap: Remap, view_url: string): Record { return extra; } -function getOSDeviceContext(parse: Parse): Sentry.PayloadEventContexts["device"] { - return { arch: parse.arch }; +function getOSDeviceContext(remap: Remap): Sentry.PayloadEventContexts["device"] { + return { arch: remap.arch }; } /** diff --git a/frontend/frontend.ts b/frontend/frontend.ts index 3106a6a..ef4cf86 100644 --- a/frontend/frontend.ts +++ b/frontend/frontend.ts @@ -235,7 +235,14 @@ function cardFooter() { ? `${parsed.commitish}` : parsed.commitish; - const arch = parsed.arch.split("_baseline"); + const arch = (fetched?.arch ?? parsed.arch).split("_baseline"); + + const debug_file = + fetched?.debug_file === "mismatch" + ? /* html */ ` +

No published build of this commit has this binary's debug id (${parsed.debug_id}), so the addresses above are not symbolicated.

+ ` + : ""; const features = fetched?.features ? /* html */ ` @@ -249,6 +256,7 @@ function cardFooter() { on ${os_names[parsed.os[0]]} ${arch[0]} ${arch.length > 1 ? "(baseline)" : ""}

${features} + ${debug_file} `; } diff --git a/lib/parser.ts b/lib/parser.ts index 814f297..8dd633c 100644 --- a/lib/parser.ts +++ b/lib/parser.ts @@ -25,6 +25,13 @@ const platform_map: { [key: string]: [Platform, Arch] } = { F: ["freebsd", "aarch64"], }; +/** + * Real ids are 16 bytes (PDB GUID, Mach-O UUID) or 20 (sha1 build-id); bun + * itself caps at 20 (`debug_id::MAX_LEN`). The bound here only exists to + * reject a corrupt string instead of slicing a huge "id" out of it. + */ +const max_debug_id_bytes = 64; + const reasons: { [key: string]: (fault_address: string | undefined, rest: string) => string | Promise; } = { @@ -90,6 +97,16 @@ export interface Parse { * encoder had no arch layout (FreeBSD/unknown). */ fault_registers?: FaultRegisters; + /** + * v4+: the id the linker stamped into both the crashing executable and its + * debug info (PDB GUID on Windows, GNU build-id on ELF, LC_UUID on Mach-O), + * as lowercase hex in the byte order the platform's tools print it. Unlike + * `commitish` + `arch` it names one specific link: a commit can be published + * as several links of one platform (x64 and x64-baseline, or a re-run + * release step), and the addresses only remap against the matching one. + * Absent for older formats and for executables that carry no id. + */ + debug_id?: string; } export interface FaultRegisters { @@ -121,12 +138,26 @@ export interface Remap { message: string; version: string; os: Platform; + /** + * The arch whose debug file the addresses were remapped with. Normally the + * trace's own arch; for a v4 trace it is whichever x64 link of the commit + * carries the trace's debug id. + */ arch: Arch; commit: ResolvedCommit; addresses: Address[]; issue?: number; command: string; features: string[]; + /** See `Parse.debug_id`. */ + debug_id?: string; + /** + * v4 traces only. "match": the debug file carries the trace's debug id. + * "mismatch": no published link of this commit does, so `addresses` were + * deliberately left unsymbolicated rather than remapped against the wrong + * binary. "unverified": the debug file's own id could not be read. + */ + debug_file?: "match" | "mismatch" | "unverified"; } export type Address = RemappedAddress | UnknownAddress; @@ -163,6 +194,10 @@ export interface RemapAPIResponse { command: string; version: string; features: string[]; + /** See `Remap.arch`; absent from responses of older servers. */ + arch?: Arch; + /** See `Remap.debug_file`. */ + debug_file?: Remap["debug_file"]; } function validateSemver(version: string): boolean { @@ -188,6 +223,7 @@ export async function parse(str: string): Promise { let is_canary = false; let has_build_flags = false; + let has_debug_id = false; let has_regs = false; if (trace_version === "1") { // '1' - original. uses 7 char hash with VLQ encoded stack-frames @@ -200,6 +236,14 @@ export async function parse(str: string): Promise { // regs) for fault reasons '2'..'7' only. has_build_flags = true; has_regs = true; + } else if (trace_version === "4") { + // '4' - '1' plus, after the sha, the build-flags VLQ of '3' and then the + // executable's debug id: a VLQ byte count followed by that many + // bytes as lowercase hex (count 0 = the executable has no id). + // No register block. Emitted by `encode_trace_string` in bun's + // src/crash_handler/lib.rs. + has_build_flags = true; + has_debug_id = true; } else { DEBUG && debug("invalid version '%s'", trace_version); return null; @@ -221,6 +265,25 @@ export async function parse(str: string): Promise { is_canary = !!(flags & (1 << 0)); } + let debug_id: string | undefined; + if (has_debug_id) { + const [byte_count, adv] = decodePart(str.slice(i)); + if (byte_count == null || byte_count < 0 || byte_count > max_debug_id_bytes) { + DEBUG && debug("invalid debug id length %o", str.slice(i)); + return null; + } + i += adv; + if (byte_count > 0) { + const hex = str.slice(i, i + byte_count * 2); + if (hex.length !== byte_count * 2 || !/^[0-9a-f]+$/.test(hex)) { + DEBUG && debug("invalid debug id %o", hex); + return null; + } + i += hex.length; + debug_id = hex; + } + } + const [f0, a0] = decodePart(str.slice(i)); i += a0; const [f1, a1] = decodePart(str.slice(i)); @@ -316,6 +379,7 @@ export async function parse(str: string): Promise { is_canary, ...(fault_address ? { fault_address } : {}), ...(fault_registers ? { fault_registers } : {}), + ...(debug_id ? { debug_id } : {}), }; } catch (e) { DEBUG && debug(e); diff --git a/lib/util.ts b/lib/util.ts index 9ce43c4..e2e4802 100644 --- a/lib/util.ts +++ b/lib/util.ts @@ -58,6 +58,10 @@ export function parseCacheKey(parse: Parse) { parse.arch, parse.os, !!parse.is_canary, + // Two links of one commit have different code at the same addresses, so + // their remaps must not share an entry. Only v4 traces have one, so the + // keys of already-cached v1-v3 traces are unchanged. + ...(parse.debug_id ? ["id:" + parse.debug_id] : []), ...parse.addresses.map((a) => a.address.toString(16)), ].join("_"); if (typeof Bun !== "undefined") { diff --git a/test/README.md b/test/README.md index f51b0f4..6e20e2a 100644 --- a/test/README.md +++ b/test/README.md @@ -11,7 +11,9 @@ Expected output lives in `__snapshots__/`. Fixtures hold inputs only. ## Scope -v1 and v2 trace formats only. v3 has not shipped; do not add v3 fixtures. +v1, v2 and v4 trace formats, plus the one v3 capture. v3 (fault registers) is +decoded but no shipped bun emits it; v4 (build flags + debug id) is what bun +emits from oven-sh/bun#38838 on, so real v4 captures are welcome. ## Adding a fixture diff --git a/test/__snapshots__/parse.test.ts.snap b/test/__snapshots__/parse.test.ts.snap index 1816d93..7b7a379 100644 --- a/test/__snapshots__/parse.test.ts.snap +++ b/test/__snapshots__/parse.test.ts.snap @@ -8627,6 +8627,173 @@ exports[`parse fixtures v3-linux-x86_64-segfault-registers 1`] = ` } `; +exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (canary flag set; the 20-byte id is the binary's GNU build-id as printed by readelf -n) 1`] = ` +{ + "addresses": [ + { + "address": 228894516, + "object": "bun", + }, + { + "address": 237765371, + "object": "bun", + }, + { + "address": 231130632, + "object": "bun", + }, + { + "address": 222925089, + "object": "bun", + }, + { + "address": 230873704, + "object": "bun", + }, + { + "address": 228888207, + "object": "bun", + }, + { + "address": 0, + "object": "?", + }, + { + "address": 334254303, + "object": "bun", + }, + { + "address": 334102908, + "object": "bun", + }, + { + "address": 381823109, + "object": "bun", + }, + { + "address": 381824323, + "object": "bun", + }, + { + "address": 391953016, + "object": "bun", + }, + { + "address": 391954698, + "object": "bun", + }, + { + "address": 142537666, + "object": "bun", + }, + { + "address": 142792837, + "object": "bun", + }, + { + "address": 142790821, + "object": "bun", + }, + ], + "arch": "x86_64", + "command": "a", + "commitish": "2c2ef7c", + "debug_id": "caac16c6401beba3fdd7e29cafe9bd212a0a23f8", + "features": [ + 96, + 1048641, + ], + "is_canary": true, + "message": "panic: invoked crashByPanic() handler", + "os": "linux", + "version": "1.4.0", +} +`; + +exports[`parse fixtures v4-windows-x86_64-segfault-debug-id: real trace from the windows x64 release build of oven-sh/bun#38838 in CI (canary; the 16-byte id is the PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames) 1`] = ` +{ + "addresses": [ + { + "address": 31963997, + "object": "bun", + }, + { + "address": 36215811, + "object": "bun", + }, + { + "address": 36216419, + "object": "bun", + }, + { + "address": 27103929, + "object": "bun", + }, + { + "address": 2825038, + "object": "bun", + }, + { + "address": 8028986, + "object": "bun", + }, + { + "address": 5724688, + "object": "bun", + }, + { + "address": 4592537, + "object": "bun", + }, + { + "address": 5719392, + "object": "bun", + }, + { + "address": 5741847, + "object": "bun", + }, + { + "address": 8364267, + "object": "bun", + }, + { + "address": 8349662, + "object": "bun", + }, + { + "address": 17095433, + "object": "bun", + }, + { + "address": 64396784, + "object": "bun", + }, + { + "address": 96964, + "object": "KERNEL32.DLL", + }, + { + "address": 370881, + "object": "ntdll.dll", + }, + ], + "arch": "x86_64", + "command": "t", + "commitish": "605e221", + "debug_id": "e4509f66f3b4e4984c4c44205044422e", + "fault_address": "00000000", + "features": [ + 96, + 1613893705, + ], + "is_canary": true, + "message": "Segmentation fault at address 0x00000000", + "os": "windows", + "version": "1.4.0", +} +`; + exports[`parse fixtures windows-segfault-neg1 1`] = ` { "addresses": [ diff --git a/test/debug-id.test.ts b/test/debug-id.test.ts new file mode 100644 index 0000000..0d99999 --- /dev/null +++ b/test/debug-id.test.ts @@ -0,0 +1,291 @@ +import { afterAll, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { readExecutableDebugId, selectDebugFile } from "../backend/debug-id"; +import type { Arch } from "../lib/util"; + +const dir = mkdtempSync(join(tmpdir(), "bun-report-debug-id-")); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +function file(name: string, contents: Buffer): string { + const path = join(dir, name); + writeFileSync(path, contents); + return path; +} + +function u32(n: number): Buffer { + const b = Buffer.alloc(4); + b.writeUInt32LE(n); + return b; +} +function u16(n: number): Buffer { + const b = Buffer.alloc(2); + b.writeUInt16LE(n); + return b; +} +function u64(n: number): Buffer { + const b = Buffer.alloc(8); + b.writeBigUInt64LE(BigInt(n)); + return b; +} +/** Lays `parts` out at the given absolute offsets in a zero-filled buffer. */ +function layout(size: number, parts: [offset: number, bytes: Buffer][]): Buffer { + const out = Buffer.alloc(size); + for (const [offset, bytes] of parts) bytes.copy(out, offset); + return out; +} + +describe("readExecutableDebugId", () => { + test("ELF: descriptor of the NT_GNU_BUILD_ID note, skipping other notes", () => { + const build_id = "caac16c6401beba3fdd7e29cafe9bd212a0a23f8"; // readelf -n of a real bun-debug + const note = (type: number, desc: Buffer) => + Buffer.concat([u32(4), u32(desc.length), u32(type), Buffer.from("GNU\0", "latin1"), desc]); + const notes = Buffer.concat([ + note(1 /* NT_GNU_ABI_TAG */, Buffer.alloc(16, 0xaa)), + note(3 /* NT_GNU_BUILD_ID */, Buffer.from(build_id, "hex")), + ]); + const notes_offset = 64 + 2 * 56; + const elf = layout(notes_offset + notes.length, [ + [0, Buffer.from("\x7fELF", "latin1")], + [0x20, u64(64)], // e_phoff + [0x36, u16(56)], // e_phentsize + [0x38, u16(2)], // e_phnum + // phdr 0: PT_LOAD, must be ignored. + [64, u32(1)], + // phdr 1: PT_NOTE at notes_offset. + [64 + 56, u32(4)], + [64 + 56 + 8, u64(notes_offset)], + [64 + 56 + 32, u64(notes.length)], + [notes_offset, notes], + ]); + expect(readExecutableDebugId(file("a.elf", elf))).toBe(build_id); + }); + + test("ELF without a build-id note", () => { + const elf = layout(64, [ + [0, Buffer.from("\x7fELF", "latin1")], + [0x20, u64(64)], + [0x36, u16(56)], + [0x38, u16(0)], + ]); + expect(readExecutableDebugId(file("no-note.elf", elf))).toBeUndefined(); + }); + + test("PE: CodeView GUID in the order llvm-readobj and dumpbin print it", () => { + // llvm-readobj --coff-debug-directory on a real bun-debug.exe printed + // PDBGUID: {94466803-4EEA-D862-4C4C-44205044422E}; these are the bytes as + // they sit in the file (the first three fields little-endian). + const guid_in_file = Buffer.from("03684694" + "ea4e" + "62d8" + "4c4c44205044422e", "hex"); + const code_view = Buffer.concat([ + Buffer.from("RSDS", "latin1"), + guid_in_file, + u32(1), + Buffer.from("bun.pdb\0", "latin1"), + ]); + + const pe_offset = 64; + const optional_header_size = 112 + 16 * 8; + const optional_header = pe_offset + 24; + const section_table = optional_header + optional_header_size; + const section_rva = 0x1000; + const section_file_offset = 512; + const entry_size = 28; + const code_view_offset = section_file_offset + 2 * entry_size; + + const pe = layout(code_view_offset + code_view.length, [ + [0, Buffer.from("MZ", "latin1")], + [0x3c, u32(pe_offset)], + [pe_offset, Buffer.from("PE\0\0", "latin1")], + [pe_offset + 4 + 2, u16(1)], // NumberOfSections + [pe_offset + 4 + 16, u16(optional_header_size)], + [optional_header, u16(0x20b)], + [optional_header + 108, u32(16)], // NumberOfRvaAndSizes + [optional_header + 112 + 6 * 8, u32(section_rva)], // debug directory rva... + [optional_header + 112 + 6 * 8 + 4, u32(2 * entry_size)], // ...and size + [section_table + 12, u32(section_rva)], // VirtualAddress + [section_table + 16, u32(0x200)], // SizeOfRawData + [section_table + 20, u32(section_file_offset)], // PointerToRawData + // entry 0: IMAGE_DEBUG_TYPE_COFF, must be skipped. + [section_file_offset + 12, u32(1)], + // entry 1: CodeView. + [section_file_offset + entry_size + 12, u32(2)], + [section_file_offset + entry_size + 24, u32(code_view_offset)], + [code_view_offset, code_view], + ]); + expect(readExecutableDebugId(file("a.exe", pe))).toBe("944668034eead8624c4c44205044422e"); + }); + + test("PE without a debug directory", () => { + const pe = layout(64 + 24 + 240, [ + [0, Buffer.from("MZ", "latin1")], + [0x3c, u32(64)], + [64, Buffer.from("PE\0\0", "latin1")], + [64 + 4 + 16, u16(240)], + [88, u16(0x20b)], + [88 + 108, u32(16)], + ]); + expect(readExecutableDebugId(file("no-debug.exe", pe))).toBeUndefined(); + }); + + test("Mach-O: LC_UUID bytes in order, after other load commands", () => { + const uuid = "0123456789abcdef0123456789abcdef"; + const segment = layout(72, [ + [0, u32(0x19 /* LC_SEGMENT_64 */)], + [4, u32(72)], + ]); + const uuid_command = Buffer.concat([u32(0x1b), u32(24), Buffer.from(uuid, "hex")]); + const commands = Buffer.concat([segment, uuid_command]); + const macho = layout(32 + commands.length, [ + [0, u32(0xfeedfacf)], + [16, u32(2)], // ncmds + [20, u32(commands.length)], // sizeofcmds + [32, commands], + ]); + expect(readExecutableDebugId(file("a.macho", macho))).toBe(uuid); + }); + + test("not an executable, or not there at all", () => { + expect( + readExecutableDebugId(file("features.json", Buffer.from('{"features":[]}'))), + ).toBeUndefined(); + expect(readExecutableDebugId(file("tiny", Buffer.from("MZ")))).toBeUndefined(); + expect(readExecutableDebugId(join(dir, "does-not-exist"))).toBeUndefined(); + }); + + test("the binary running this test has an id of a plausible shape", () => { + // bun's own releases are linked with a build-id / PDB / LC_UUID, so this + // exercises the real-file path on whichever platform the tests run. + expect(readExecutableDebugId(process.execPath)).toMatch(/^[0-9a-f]{32}([0-9a-f]{8})?$/); + }); +}); + +describe("selectDebugFile", () => { + interface Info { + arch: Arch; + debug_id: string | undefined; + } + const A = "aa".repeat(16); + const B = "bb".repeat(16); + + function unavailable(arch: Arch): Error & { code: string } { + return Object.assign(new Error(`no artifact for ${arch}`), { code: "DebugInfoUnavailable" }); + } + + /** `store` maps each published arch to the id its executable carries (undefined = unreadable). */ + function bucket(store: Partial>) { + const fetched: Arch[] = []; + const fetch = async (arch: Arch): Promise => { + fetched.push(arch); + if (!(arch in store)) throw unavailable(arch); + const entry = store[arch]; + if (entry instanceof Error) throw entry; + return { arch, debug_id: entry }; + }; + return { fetch, fetched }; + } + + test("a trace without an id uses its own arch unchecked, as before", async () => { + const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", undefined, fetch)).toEqual({ + arch: "x86_64", + debug_id: A, + }); + expect(fetched).toEqual(["x86_64"]); + }); + + test("a trace without an id still fails when its own arch is missing", async () => { + const { fetch } = bucket({ x86_64_baseline: B }); + await expect(selectDebugFile("x86_64", undefined, fetch)).rejects.toMatchObject({ + code: "DebugInfoUnavailable", + }); + }); + + test("the trace's own arch carries the id", async () => { + const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", A, fetch)).toEqual({ + arch: "x86_64", + debug_id: A, + debug_file: "match", + }); + expect(fetched).toEqual(["x86_64"]); + }); + + test("the other x64 link carries the id (the bun-windows-x64 vs -baseline case)", async () => { + const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", B, fetch)).toEqual({ + arch: "x86_64_baseline", + debug_id: B, + debug_file: "match", + }); + expect(fetched).toEqual(["x86_64", "x86_64_baseline"]); + }); + + test("works in the other direction too", async () => { + const { fetch } = bucket({ x86_64: A, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64_baseline", A, fetch)).toMatchObject({ + arch: "x86_64", + debug_file: "match", + }); + }); + + test("no published link carries the id: the trace's own arch, flagged mismatch", async () => { + const { fetch } = bucket({ x86_64: A, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", "cc".repeat(16), fetch)).toEqual({ + arch: "x86_64", + debug_id: A, + debug_file: "mismatch", + }); + }); + + test("a missing sibling is skipped, not an error", async () => { + const { fetch } = bucket({ x86_64: A }); + expect(await selectDebugFile("x86_64", B, fetch)).toMatchObject({ + arch: "x86_64", + debug_file: "mismatch", + }); + }); + + test("an arch with no siblings goes straight to mismatch", async () => { + const { fetch, fetched } = bucket({ aarch64: A }); + expect(await selectDebugFile("aarch64", B, fetch)).toMatchObject({ + arch: "aarch64", + debug_file: "mismatch", + }); + expect(fetched).toEqual(["aarch64"]); + }); + + test("an artifact whose executable has no readable id is used unverified", async () => { + const { fetch, fetched } = bucket({ x86_64: undefined, x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", B, fetch)).toEqual({ + arch: "x86_64", + debug_id: undefined, + debug_file: "unverified", + }); + expect(fetched).toEqual(["x86_64"]); + }); + + test("the trace's own arch was never published but a sibling carrying the id was", async () => { + const { fetch } = bucket({ x86_64_baseline: B }); + expect(await selectDebugFile("x86_64", B, fetch)).toMatchObject({ + arch: "x86_64_baseline", + debug_file: "match", + }); + }); + + test("nothing published at all reports the trace's own arch as unavailable", async () => { + const { fetch } = bucket({}); + await expect(selectDebugFile("x86_64", B, fetch)).rejects.toMatchObject({ + code: "DebugInfoUnavailable", + message: "no artifact for x86_64", + }); + }); + + test("errors other than a missing artifact propagate", async () => { + const boom = new Error("unzip exploded"); + await expect(selectDebugFile("x86_64", B, bucket({ x86_64: boom }).fetch)).rejects.toBe(boom); + await expect( + selectDebugFile("x86_64", B, bucket({ x86_64: A, x86_64_baseline: boom }).fetch), + ).rejects.toBe(boom); + }); +}); diff --git a/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json b/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json new file mode 100644 index 0000000..2463edd --- /dev/null +++ b/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json @@ -0,0 +1,4 @@ +{ + "description": "v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (canary flag set; the 20-byte id is the binary's GNU build-id as printed by readelf -n)", + "input": "1.4.0/la42c2ef7cCoBcaac16c6401beba3fdd7e29cafe9bd212a0a23f8gGikggCozzy0N2vhwlOwgj74NiyompNwmtr4N+ony0N_+tox9T43go9Tqo0o4Wm02o4Wwn9yrX0wgzrXk857vIqosrwIqqorwIA0eNrLzCvLz05NUUguSizOcKoMSMzLTNbQVMhIzEvJSS0CAK/LCxc" +} diff --git a/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json b/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json new file mode 100644 index 0000000..d5ce235 --- /dev/null +++ b/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json @@ -0,0 +1,4 @@ +{ + "description": "v4-windows-x86_64-segfault-debug-id: real trace from the windows x64 release build of oven-sh/bun#38838 in CI (canary; the 16-byte id is the PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames)", + "input": "https://bun.report/1.4.0/wt4605e221CgBe4509f66f3b4e4984c4c44205044422egGykoomgD619+8BmguilCmmvilCyrp2zB80tsF0zhqPght9Ky5p4Ig2i9Kuxu+K2uw+P89z9PywtzgBg/u66DCYKERNEL32.DLLos9FCSntdll.dllis0WA2AA" +} diff --git a/test/helpers/encode.ts b/test/helpers/encode.ts index 5d52794..ccb8c2f 100644 --- a/test/helpers/encode.ts +++ b/test/helpers/encode.ts @@ -87,10 +87,15 @@ export interface BuildTraceOpts { os: Platform; arch: Arch; command: string; - trace_version: "1" | "2" | "3"; + trace_version: "1" | "2" | "3" | "4"; commitish: string; /** v3+ build-flags VLQ (bit0 = canary). */ build_flags?: number; + /** + * v4: the executable's debug id as lowercase hex; encoded as a VLQ byte count + * followed by the hex. Omit for an executable without one (count 0). + */ + debug_id?: string; features?: [number, number]; addresses: ParsedAddress[]; reason: ReasonSpec; @@ -107,7 +112,13 @@ export function buildTraceString(opts: BuildTraceOpts): string { s += opts.command; s += opts.trace_version; s += opts.commitish; - if (opts.trace_version === "3") s += encodeVlq(opts.build_flags ?? 0); + if (opts.trace_version === "3" || opts.trace_version === "4") + s += encodeVlq(opts.build_flags ?? 0); + if (opts.trace_version === "4") { + const debug_id = opts.debug_id ?? ""; + if (debug_id.length % 2 !== 0) throw new Error("debug_id must be whole bytes"); + s += encodeVlq(debug_id.length / 2) + debug_id; + } s += encodeVlq(f0) + encodeVlq(f1); for (const a of opts.addresses) s += encodeStackLine(a); s += encodeVlq(0); diff --git a/test/roundtrip.test.ts b/test/roundtrip.test.ts index 3e443fc..bc38662 100644 --- a/test/roundtrip.test.ts +++ b/test/roundtrip.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect } from "bun:test"; import { parse } from "../lib/parser"; import { buildTraceString, encodeVlq, type BuildTraceOpts } from "./helpers/encode"; import { decodePart } from "../lib/vlq"; +import { parseCacheKey } from "../lib/util"; describe("vlq roundtrip", () => { for (const v of [0, 1, 2, 31, 32, 0x1234, 0x10ab34, 0x7fffffff]) { @@ -182,6 +183,54 @@ describe("parse(buildTraceString(x)) recovers x", () => { values: Array.from({ length: 33 }, (_, i) => BigInt(i)), }, }, + // v4: build flags, then the debug id. A 20-byte sha1 build-id (ELF)... + { + version: "1.4.0", + os: "linux", + arch: "x86_64", + command: "a", + trace_version: "4", + build_flags: 1, + debug_id: "caac16c6401beba3fdd7e29cafe9bd212a0a23f8", + commitish: "2c2ef7c", + features: [96, 1048577], + addresses: [ + { address: 0x2f864f4, object: "bun" }, + { address: 0, object: "?" }, + { address: 0x1234, object: "/libc.so.6" }, + ], + reason: { kind: "panic", message: "invoked crashByPanic() handler" }, + }, + // ...a 16-byte PDB GUID (Windows), release build, with a foreign frame + // after the id to show the VLQ stream is still aligned... + { + version: "1.4.0", + os: "windows", + arch: "x86_64", + command: "t", + trace_version: "4", + build_flags: 0, + debug_id: "e4509f66f3b4e4984c4c44205044422e", + commitish: "605e221", + addresses: [ + { address: 0x1111, object: "bun" }, + { address: 0x17344, object: "KERNEL32.DLL" }, + ], + reason: { kind: "segfault", addr_hi: 0, addr_lo: 0 }, + }, + // ...and an executable without an id (count 0), which must parse like a + // v3 trace minus the register block. + { + version: "1.4.0", + os: "macos", + arch: "aarch64", + command: "r", + trace_version: "4", + build_flags: 0, + commitish: "605e221", + addresses: [{ address: 0x1063487, object: "bun" }], + reason: { kind: "segfault", addr_hi: 0, addr_lo: 0xdeadbeef | 0 }, + }, ]; for (const c of cases) { @@ -217,6 +266,13 @@ describe("parse(buildTraceString(x)) recovers x", () => { expect(p.fault_address).toBe("102F864F4"); } + if (c.trace_version === "4" && c.debug_id) { + expect(p.debug_id).toBe(c.debug_id); + } else { + expect(p.debug_id).toBeUndefined(); + } + if (c.trace_version === "4") expect(p.fault_registers).toBeUndefined(); + if (c.registers) { expect(p.fault_registers).toBeDefined(); expect(p.fault_registers!.pc).toEqual(c.registers.pc); @@ -229,3 +285,53 @@ describe("parse(buildTraceString(x)) recovers x", () => { }); } }); + +describe("v4 debug id field", () => { + const base = { + version: "1.4.0", + os: "linux", + arch: "x86_64", + command: "a", + trace_version: "4", + build_flags: 0, + commitish: "2c2ef7c", + addresses: [{ address: 0x42, object: "bun" }], + reason: { kind: "oom" }, + } as const satisfies BuildTraceOpts; + + test("a truncated id is rejected rather than read into the following fields", async () => { + const full = buildTraceString({ ...base, debug_id: "00112233445566778899aabbccddeeff" }); + // Drop two hex digits: the declared count (16 bytes) now overruns into the + // features VLQs, which are not hex. + const cut = full.replace("ccddeeff", "ccddee"); + expect(await parse(cut)).toBeNull(); + }); + + test("non-hex where the id should be is rejected", async () => { + const s = buildTraceString({ ...base, debug_id: "00112233445566778899aabbccddeeff" }).replace("aabb", "AABB"); + expect(await parse(s)).toBeNull(); + }); + + test("an absurd byte count is rejected", async () => { + // Same layout as the helper writes, but with a hand-written count. + const prefix = "1.4.0/la4" + base.commitish + encodeVlq(0); + expect(await parse(prefix + encodeVlq(100) + "00".repeat(100) + encodeVlq(0) + encodeVlq(0) + encodeVlq(0) + "9")).toBeNull(); + }); + + test("is_canary comes from the build flags", async () => { + expect((await parse(buildTraceString({ ...base, build_flags: 1 })))!.is_canary).toBe(true); + expect((await parse(buildTraceString({ ...base, build_flags: 0 })))!.is_canary).toBe(false); + }); + + test("traces that differ only in debug id get different remap cache keys", async () => { + const a = (await parse(buildTraceString({ ...base, debug_id: "00".repeat(16) })))!; + const b = (await parse(buildTraceString({ ...base, debug_id: "ff".repeat(16) })))!; + const none = (await parse(buildTraceString(base)))!; + expect(parseCacheKey(a)).not.toBe(parseCacheKey(b)); + expect(parseCacheKey(a)).not.toBe(parseCacheKey(none)); + // A v1 trace of the same commit and addresses keys exactly as it did + // before the field existed. + const v1 = (await parse(buildTraceString({ ...base, trace_version: "1" })))!; + expect(parseCacheKey(v1)).toBe(parseCacheKey(none)); + }); +}); From d2260edcc0a08a468ce2d4ec14b0f2eff56c5ed9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:25:27 +0000 Subject: [PATCH 2/2] Format 4 header is tagged fields; try every build that shares a platform char The header after the sha is now a VLQ field count followed by (tag, char count, chars) fields, as bun's encoder writes it: tag 0 is the build flags VLQ, tag 1 the debug id in hex. Unknown tags are skipped, so bun can add fields without another version char and without the decoder deploying first. Selection now iterates every build a commit is published as under the same platform char: on Linux the glibc, musl and android builds all report 'l'/'L' and were all symbolized against the glibc binary; x64 additionally tries the -baseline name for trees that still build one. The build that matched is reported as Remap.variant (musl / android / baseline) and becomes Sentry's dist and a tag; Remap.arch stays the trace's arch. Cache rows and dirs for the plain build keep their existing names. Fixtures: the Linux capture is a real trace from the final encoder; the Windows one keeps its real frames with the header re-encoded. --- backend/db.ts | 11 +- backend/debug-id.ts | 83 ++++--- backend/debug-store.ts | 71 +++--- backend/index.ts | 2 +- backend/markdown.ts | 4 +- backend/remap.ts | 7 +- backend/sentry.ts | 19 +- frontend/frontend.ts | 5 +- lib/parser.ts | 112 ++++++--- test/__snapshots__/parse.test.ts.snap | 20 +- test/debug-id.test.ts | 228 +++++++++++++----- .../parse/v4-linux-x86_64-panic-debug-id.json | 4 +- .../v4-windows-x86_64-segfault-debug-id.json | 4 +- test/helpers/encode.ts | 32 ++- test/roundtrip.test.ts | 69 ++++-- 15 files changed, 445 insertions(+), 226 deletions(-) diff --git a/backend/db.ts b/backend/db.ts index 00f5548..b1f5ab6 100644 --- a/backend/db.ts +++ b/backend/db.ts @@ -2,7 +2,7 @@ // This is used to avoid remapping the same address multiple times. import { Database } from "bun:sqlite"; import type { Remap } from "../lib/parser"; -import { remapCacheKey, type Arch, type Platform } from "../lib/util"; +import { remapCacheKey, type Platform } from "../lib/util"; import { rm } from "node:fs/promises"; import { relative } from "node:path"; import type { FeatureConfig } from "./feature"; @@ -111,12 +111,13 @@ export interface CachedDebugFile { debug_id: string | undefined; } +/** `name` is debug-store's `cacheName()`: the arch for a commit's plain build, arch plus link otherwise. */ export function getCachedDebugFile( os: Platform, - arch: Arch, + name: string, commit: string, ): CachedDebugFile | null { - const cache_key = `${os}-${arch}-${commit}`; + const cache_key = `${os}-${name}-${commit}`; const result = get_debug_file_stmt.get(cache_key) as { file_path: string; debug_id: string | null; @@ -130,12 +131,12 @@ export function getCachedDebugFile( export function putCachedDebugFile( os: Platform, - arch: Arch, + name: string, commit: string, file_path: string, debug_id: string | undefined, ) { - insert_debug_file_stmt.run(`${os}-${arch}-${commit}`, file_path, debug_id ?? null, Date.now()); + insert_debug_file_stmt.run(`${os}-${name}-${commit}`, file_path, debug_id ?? null, Date.now()); } export function getCachedFeatureData( diff --git a/backend/debug-id.ts b/backend/debug-id.ts index 123b925..7851d0b 100644 --- a/backend/debug-id.ts +++ b/backend/debug-id.ts @@ -1,5 +1,5 @@ import { closeSync, openSync, readSync } from "node:fs"; -import type { Arch } from "../lib/util"; +import type { Arch, Platform } from "../lib/util"; // Deliberately free of backend imports (db, git, ...) so the selection policy // below is unit-testable; debug-store.ts supplies the downloading. @@ -10,46 +10,75 @@ export interface DebugFileCheck { } /** - * The other links a commit may be published as for the same os. A v4 trace - * says `'w'` for both Windows x64 links of a commit when the build that made - * it did not know which zip it would ship in, so the debug id decides. + * One published build of a commit: the part of the artifact name between + * `bun--` and `-profile.zip`, e.g. "x64", "x64-musl", "aarch64-android". */ -const sibling_archs: Partial> = { - x86_64: ["x86_64_baseline"], - x86_64_baseline: ["x86_64"], -}; +export type Link = string; + +/** + * Every link a commit is published as whose crash handler reports the given + * platform char, the one traces have always been symbolized with first. The + * glibc, musl and android builds of an arch all report 'l'/'L', and a tree + * that still builds a separate baseline binary reports 'w'/'l'/'m' from it + * too, so for a trace that carries a debug id these are the candidates. + */ +export function publishedLinks(os: Platform, arch: Arch): Link[] { + const cpu = arch === "aarch64" ? "aarch64" : "x64"; + const links: Link[] = [arch === "x86_64_baseline" ? `${cpu}-baseline` : cpu]; + if (os === "linux") links.push(`${cpu}-musl`, `${cpu}-android`); + if (os !== "freebsd" && cpu === "x64") + links.push(arch === "x86_64_baseline" ? cpu : `${cpu}-baseline`); + return links; +} + +/** What distinguishes a link from the plain build of its arch: "musl", "android", "baseline", or undefined. */ +export function linkVariant(link: Link): string | undefined { + const dash = link.indexOf("-"); + return dash === -1 ? undefined : link.slice(dash + 1); +} + +/** + * Cache namespace (debug-store's db rows and on-disk dirs) for one build of a + * commit. The build traces have always been symbolized with keeps the name the + * cache has always used (`x86_64`), so existing entries stay valid; the others + * get their own (`x86_64-x64-musl`). + */ +export function cacheName(os: Platform, arch: Arch, link: Link): string { + return link === publishedLinks(os, arch)[0] ? arch : `${arch}-${link}`; +} function isUnavailable(e: unknown): boolean { return (e as any)?.code === "DebugInfoUnavailable"; } /** - * Which of a commit's links to symbolize a trace with. `fetch(arch)` yields - * that arch's artifact (with the id read from its executable, if readable) - * or throws `DebugInfoUnavailable` when the commit was not published under - * that name. + * Which of a commit's links to symbolize a trace with. `fetch(link)` yields + * that artifact (with the id read from its executable, if readable) or throws + * `DebugInfoUnavailable` when the commit was not published under that name. * - * - no trace id (formats 1-3): the trace's own arch, unchecked, as before. - * - it matches the trace's arch artifact: use it. - * - that artifact's id is unreadable: use it, flagged "unverified" (there is - * no evidence either way, so behave as before). - * - otherwise try the sibling links; the one carrying the id wins and a - * missing sibling is skipped. When the trace's own artifact is missing - * altogether this is also how a trace published only under the other - * name gets symbolized at all. - * - nothing carries the id: "mismatch". The caller then leaves the addresses - * unsymbolicated; remapping them against another link's debug info is what - * produced confidently wrong reports before the id existed. + * - no trace id (formats 1-3): the first link, unchecked, as before. + * - it carries the id: use it. + * - its id is unreadable: use it, flagged "unverified" (no evidence either + * way, so behave as before). + * - otherwise the remaining links in turn; the one carrying the id wins and a + * missing one is skipped. This is also how a trace from a build that was + * only published under one of the other names gets symbolized at all. + * - nothing carries the id: "mismatch", with the first link. The caller then + * leaves the addresses unsymbolicated; remapping them against another + * build's debug info is what produced confidently wrong reports before the + * id existed. */ export async function selectDebugFile( + os: Platform, arch: Arch, debug_id: string | undefined, - fetch: (arch: Arch) => Promise, + fetch: (link: Link) => Promise, ): Promise { + const [first, ...rest] = publishedLinks(os, arch); let primary: T | undefined; let primary_error: unknown; try { - primary = await fetch(arch); + primary = await fetch(first); } catch (e) { if (debug_id === undefined || !isUnavailable(e)) throw e; primary_error = e; @@ -60,10 +89,10 @@ export async function selectDebugFile(); -const map_download_arch = { - x86_64: "x64", - x86_64_baseline: "x64-baseline", - aarch64: "aarch64", -} as const; - const map_download_os = { windows: "windows", macos: "darwin", @@ -58,9 +59,10 @@ const map_download_os = { /** * The debug file to symbolize a trace with. Without a `debug_id` (trace - * formats 1-3) that is the artifact named by the trace's arch, as it always - * was. With one, the artifact is checked against it and the commit's sibling - * links are tried when it does not match; see `selectDebugFile`. + * formats 1-3) that is the plain build of the trace's arch, as it always was. + * With one, that build is checked against it and the commit's other builds + * with the same platform char are tried when it does not match; see + * `selectDebugFile`. */ export async function fetchDebugFile( os: Platform, @@ -69,14 +71,15 @@ export async function fetchDebugFile( is_canary: boolean | undefined, debug_id?: string, ): Promise { - return selectDebugFile(arch, debug_id, (candidate) => - fetchArtifact(os, candidate, commit, is_canary), + return selectDebugFile(os, arch, debug_id, (link) => + fetchArtifact(os, arch, link, commit, is_canary), ); } async function fetchArtifact( os: Platform, arch: Arch, + link: Link, commit: ResolvedCommit, is_canary: boolean | undefined, ): Promise { @@ -84,31 +87,33 @@ async function fetchArtifact( assert(oid.length === 40); const store_suffix = os === "windows" ? ".pdb" : ""; - const root = storeRoot(os, arch, is_canary); - const path = join(root, oid[0], oid + store_suffix); + const name = cacheName(os, arch, link); + const path = join(storeRoot(os, name, is_canary), oid[0], oid + store_suffix); return in_progress_downloads.get(path, () => - fetchDebugFileWithoutCache(os, arch, commit, is_canary, store_suffix, path), + fetchDebugFileWithoutCache(os, name, link, commit, is_canary, store_suffix, path), ); } async function fetchDebugFileWithoutCache( os: Platform, - arch: Arch, + name: string, + link: Link, commit: ResolvedCommit, is_canary: boolean | undefined, store_suffix: string, path: string, ): Promise { const oid = commit.oid; + const variant = linkVariant(link); - const cached = getCachedDebugFile(os, arch, oid); + const cached = getCachedDebugFile(os, name, oid); if (cached) { const feature_config = getCachedFeatureData(oid, is_canary)!; return { file_path: cached.file_path, feature_config: feature_config, - arch, + variant, debug_id: cached.debug_id, }; } @@ -124,14 +129,13 @@ async function fetchDebugFileWithoutCache( try { if (process.env.NODE_ENV === "development") { - console.log("fetching debug file for", os, arch, oid); + console.log("fetching debug file for", os, link, oid); } const download_os = map_download_os[os]; - const download_arch = map_download_arch[arch]; using tmp = await temp(); - const dir = `bun-${download_os}-${download_arch}-profile`; + const dir = `bun-${download_os}-${link}-profile`; const url = `${process.env.BUN_DOWNLOAD_BASE}/${commit.oid}${is_canary ? "-canary" : ""}/${dir}.zip`; console.log(url); @@ -140,13 +144,13 @@ async function fetchDebugFileWithoutCache( const pr = commit.pr; if (pr) { if (process.env.NODE_ENV === "development") { - console.log("fetching debug file for", os, arch, oid, "from PR", pr.number); + console.log("fetching debug file for", os, link, oid, "from PR", pr.number); } try { - let success = await tryFromPR(os, arch, commit, tmp.path, is_canary); + let success = await tryFromPR(os, link, commit, tmp.path, is_canary); if (!success) { const err: any = new Error( - `Failed to fetch debug file for ${os}-${arch} for PR ${pr.number}`, + `Failed to fetch debug file for ${os}-${link} for PR ${pr.number}`, ); err.code = "DebugInfoUnavailable"; throw err; @@ -156,7 +160,7 @@ async function fetchDebugFileWithoutCache( } } else { const err: any = new Error( - `Failed to fetch debug file for ${os}-${arch} for commit ${commit.oid}`, + `Failed to fetch debug file for ${os}-${link} for commit ${commit.oid}`, ); err.code = "DebugInfoUnavailable"; throw err; @@ -224,7 +228,7 @@ async function fetchDebugFileWithoutCache( feature_config ??= getCachedFeatureData(oid, is_canary) ?? (await fetchFeatureData(oid, is_canary)); - putCachedDebugFile(os, arch, oid, path, debug_id); + putCachedDebugFile(os, name, oid, path, debug_id); } catch (e) { await rm(path, { force: true }); throw e; @@ -233,14 +237,14 @@ async function fetchDebugFileWithoutCache( return { file_path: path, feature_config, - arch, + variant, debug_id, }; } export async function tryFromPR( os: Platform, - arch: Arch, + link: Link, commit: ResolvedCommit, temp: string, is_canary: boolean | undefined, @@ -251,7 +255,6 @@ export async function tryFromPR( assert(pr); const download_os = map_download_os[os]; - const download_arch = map_download_arch[arch]; const data_1 = await octokit.rest.actions.listWorkflowRunsForRepo({ owner: "oven-sh", @@ -287,7 +290,7 @@ export async function tryFromPR( per_page: 100, // Fetch up to 100 artifacts }); - const dir = `bun-${download_os}-${download_arch}-profile`; + const dir = `bun-${download_os}-${link}-profile`; { const artifact = artifacts.data.artifacts.find((artifact) => artifact.name === dir); diff --git a/backend/index.ts b/backend/index.ts index e57ff44..da14168 100644 --- a/backend/index.ts +++ b/backend/index.ts @@ -250,7 +250,7 @@ async function postRemap(request: Request, server: Server) { command: remapped.command, version: remapped.version, features: remapped.features, - arch: remapped.arch, + ...(remapped.variant ? { variant: remapped.variant } : {}), ...(remapped.debug_file ? { debug_file: remapped.debug_file } : {}), } satisfies RemapAPIResponse); } catch (e) { diff --git a/backend/markdown.ts b/backend/markdown.ts index 4fb77cf..ce33d9a 100644 --- a/backend/markdown.ts +++ b/backend/markdown.ts @@ -5,7 +5,7 @@ import { basename, escmd, escmdcode } from "../lib/util"; export async function formatMarkdown(remap: Remap, internal?: { source: string }): Promise { return [ - `Bun v${remap.version} (${treeURLMD(remap.commit)}) on ${remap.os} ${remap.arch} [${remap.command}]`, + `Bun v${remap.version} (${treeURLMD(remap.commit)}) on ${remap.os} ${remap.arch}${remap.variant ? ` (${remap.variant})` : ""} [${remap.command}]`, "", remap.message.replace(/^panic: /, "**panic**: "), "", @@ -28,7 +28,7 @@ function debugFileNote(remap: Remap): string[] { const note = `Debug id: \`${remap.debug_id}\``; if (remap.debug_file !== "mismatch") return [note, ""]; return [ - `${note} (no published ${remap.os} ${remap.arch} build of this commit has it, so the addresses above are not symbolicated)`, + `${note} (none of this commit's published ${remap.os} ${remap.arch} builds has it, so the addresses above are not symbolicated)`, "", ]; } diff --git a/backend/remap.ts b/backend/remap.ts index abbe173..78f146a 100644 --- a/backend/remap.ts +++ b/backend/remap.ts @@ -2,7 +2,7 @@ import type { Parse, Remap, ResolvedCommit } from "../lib/parser"; import { getCommit } from "./git"; import { fetchDebugFile } from "./debug-store"; import { getCachedRemap, putCachedRemap } from "./db"; -import { parseCacheKey, type Arch } from "../lib/util"; +import { parseCacheKey } from "../lib/util"; import { llvm_symbolizer, pdb_addr2line } from "./system-deps"; import { formatMarkdown } from "./markdown"; import { decodeFeatures, type FeatureConfig } from "./feature"; @@ -81,7 +81,7 @@ export async function remapUncached( const debug_info: { file_path: string; feature_config: FeatureConfig | null; - arch?: Arch; + variant?: string; debug_file?: Remap["debug_file"]; } = opts.exe ? { @@ -152,11 +152,12 @@ export async function remapUncached( version: display_version, message: parse.message, os: parse.os, - arch: debug_info.arch ?? parse.arch, + arch: parse.arch, commit: commit, addresses: mapped_addrs, command, features, + ...(debug_info.variant ? { variant: debug_info.variant } : {}), ...(parse.debug_id ? { debug_id: parse.debug_id } : {}), ...(debug_info.debug_file ? { debug_file: debug_info.debug_file } : {}), }; diff --git a/backend/sentry.ts b/backend/sentry.ts index 9a1dc5d..a2c46b7 100644 --- a/backend/sentry.ts +++ b/backend/sentry.ts @@ -62,8 +62,6 @@ function getTags(parse: Parse, remap: Remap): any { tags.version = remap.version; tags.commit = remap.commit.oid.slice(0, 9); - // remap.arch, not parse.arch: for a v4 trace it is the link whose debug - // info actually carried the trace's debug id (see debug-store.ts). tags.arch = remap.arch.replace(/_baseline$/, ""); // cache_key is SHA256(commitish_arch_os_canary_addresses). Before the // randomUUID switch, MD5(cache_key) was the event_id — so Sentry deduped @@ -79,9 +77,11 @@ function getTags(parse: Parse, remap: Remap): any { tags[feature] = true; } - if (remap.arch.endsWith("_baseline")) { - tags.baseline = true; - } + // Which of the commit's builds the frames were remapped with (musl, + // android, baseline); see Remap.variant. + const variant = buildDist(remap); + if (variant) tags.variant = variant; + if (variant === "baseline") tags.baseline = true; if (parse.is_canary) tags.canary = true; @@ -97,11 +97,14 @@ function getTags(parse: Parse, remap: Remap): any { /** * `dist` marks build variants of the same release — same version, same commit, - * different compile flags. For bun that's baseline (older-CPU target) and musl - * (Alpine/musl libc). undefined means the standard build for this os/arch. + * different compile flags. For bun that's musl (Alpine), android, and baseline + * (older-CPU target). undefined means the standard build for this os/arch. + * Until traces carried a debug id only baseline was knowable (it had its own + * platform chars); musl and android come from which build the id matched. */ function buildDist(remap: Remap): string | undefined { - return remap.arch.endsWith("_baseline") ? "baseline" : undefined; + // Remaps cached before `variant` existed only know about baseline, via the arch. + return remap.variant ?? (remap.arch.endsWith("_baseline") ? "baseline" : undefined); } function getOSContext(parse: Parse): Sentry.OS { diff --git a/frontend/frontend.ts b/frontend/frontend.ts index ef4cf86..8b2ae87 100644 --- a/frontend/frontend.ts +++ b/frontend/frontend.ts @@ -235,7 +235,8 @@ function cardFooter() { ? `${parsed.commitish}` : parsed.commitish; - const arch = (fetched?.arch ?? parsed.arch).split("_baseline"); + const arch = parsed.arch.split("_baseline"); + const variant = fetched?.variant ? `(${fetched.variant})` : arch.length > 1 ? "(baseline)" : ""; const debug_file = fetched?.debug_file === "mismatch" @@ -253,7 +254,7 @@ function cardFooter() { return /* html */ `

Bun v${addCanarySuffix(fetched ? fetched.version : parsed.version, parsed.is_canary)} (${commit}) - on ${os_names[parsed.os[0]]} ${arch[0]} ${arch.length > 1 ? "(baseline)" : ""} + on ${os_names[parsed.os[0]]} ${arch[0]} ${variant}

${features} ${debug_file} diff --git a/lib/parser.ts b/lib/parser.ts index 8dd633c..5eb0bb7 100644 --- a/lib/parser.ts +++ b/lib/parser.ts @@ -26,11 +26,24 @@ const platform_map: { [key: string]: [Platform, Arch] } = { }; /** - * Real ids are 16 bytes (PDB GUID, Mach-O UUID) or 20 (sha1 build-id); bun - * itself caps at 20 (`debug_id::MAX_LEN`). The bound here only exists to - * reject a corrupt string instead of slicing a huge "id" out of it. + * Tags of the format-4 header fields (`HeaderField` in bun's + * src/crash_handler/lib.rs). Unknown tags are skipped, so bun can add fields + * without a new version char; add the tag here once we want to read one. */ -const max_debug_id_bytes = 64; +const header_field = { + /** One VLQ; bit 0 = canary. */ + build_flags: 0, + /** The executable's debug id as lowercase hex. Absent when it has none. */ + debug_id: 1, +} as const; + +/** + * A header field's chars. A debug id is at most 40 chars (a 20-byte sha1 + * build-id); the bound only exists so a corrupt count cannot swallow the rest + * of the string as one field. + */ +const max_header_field_chars = 256; +const max_header_fields = 32; const reasons: { [key: string]: (fault_address: string | undefined, rest: string) => string | Promise; @@ -101,10 +114,10 @@ export interface Parse { * v4+: the id the linker stamped into both the crashing executable and its * debug info (PDB GUID on Windows, GNU build-id on ELF, LC_UUID on Mach-O), * as lowercase hex in the byte order the platform's tools print it. Unlike - * `commitish` + `arch` it names one specific link: a commit can be published - * as several links of one platform (x64 and x64-baseline, or a re-run - * release step), and the addresses only remap against the matching one. - * Absent for older formats and for executables that carry no id. + * `commitish` + `arch` it names one specific build: the glibc, musl and + * android builds of a commit all report the same platform char, and the + * addresses only remap against the one that was actually running. Absent + * for older formats and for executables that carry no id. */ debug_id?: string; } @@ -138,24 +151,26 @@ export interface Remap { message: string; version: string; os: Platform; - /** - * The arch whose debug file the addresses were remapped with. Normally the - * trace's own arch; for a v4 trace it is whichever x64 link of the commit - * carries the trace's debug id. - */ arch: Arch; commit: ResolvedCommit; addresses: Address[]; issue?: number; command: string; features: string[]; + /** + * Which of the commit's builds for this os/arch the addresses were remapped + * with: "musl", "android" or "baseline"; absent for the plain build. Traces + * without a debug id always use the plain build (or, for the old baseline + * platform chars, the baseline one), so this only varies for v4 traces. + */ + variant?: string; /** See `Parse.debug_id`. */ debug_id?: string; /** * v4 traces only. "match": the debug file carries the trace's debug id. - * "mismatch": no published link of this commit does, so `addresses` were - * deliberately left unsymbolicated rather than remapped against the wrong - * binary. "unverified": the debug file's own id could not be read. + * "mismatch": none of the commit's published builds does, so `addresses` + * were deliberately left unsymbolicated rather than remapped against the + * wrong binary. "unverified": the debug file's own id could not be read. */ debug_file?: "match" | "mismatch" | "unverified"; } @@ -194,8 +209,8 @@ export interface RemapAPIResponse { command: string; version: string; features: string[]; - /** See `Remap.arch`; absent from responses of older servers. */ - arch?: Arch; + /** See `Remap.variant`. */ + variant?: string; /** See `Remap.debug_file`. */ debug_file?: Remap["debug_file"]; } @@ -223,7 +238,7 @@ export async function parse(str: string): Promise { let is_canary = false; let has_build_flags = false; - let has_debug_id = false; + let has_header = false; let has_regs = false; if (trace_version === "1") { // '1' - original. uses 7 char hash with VLQ encoded stack-frames @@ -237,13 +252,11 @@ export async function parse(str: string): Promise { has_build_flags = true; has_regs = true; } else if (trace_version === "4") { - // '4' - '1' plus, after the sha, the build-flags VLQ of '3' and then the - // executable's debug id: a VLQ byte count followed by that many - // bytes as lowercase hex (count 0 = the executable has no id). - // No register block. Emitted by `encode_trace_string` in bun's - // src/crash_handler/lib.rs. - has_build_flags = true; - has_debug_id = true; + // '4' - '1' plus a header after the sha: a VLQ field count, then per + // field a VLQ tag (`header_field`), a VLQ char count and that many + // chars. No register block. Emitted by `encode_trace_string` in + // bun's src/crash_handler/lib.rs. + has_header = true; } else { DEBUG && debug("invalid version '%s'", trace_version); return null; @@ -266,21 +279,48 @@ export async function parse(str: string): Promise { } let debug_id: string | undefined; - if (has_debug_id) { - const [byte_count, adv] = decodePart(str.slice(i)); - if (byte_count == null || byte_count < 0 || byte_count > max_debug_id_bytes) { - DEBUG && debug("invalid debug id length %o", str.slice(i)); + if (has_header) { + const [field_count, adv] = decodePart(str.slice(i)); + if (field_count == null || field_count < 0 || field_count > max_header_fields) { + DEBUG && debug("invalid header field count %o", str.slice(i)); return null; } i += adv; - if (byte_count > 0) { - const hex = str.slice(i, i + byte_count * 2); - if (hex.length !== byte_count * 2 || !/^[0-9a-f]+$/.test(hex)) { - DEBUG && debug("invalid debug id %o", hex); + for (let n = 0; n < field_count; n++) { + const [tag, tag_adv] = decodePart(str.slice(i)); + if (tag == null || tag < 0) { + DEBUG && debug("invalid header field tag %o", str.slice(i)); return null; } - i += hex.length; - debug_id = hex; + i += tag_adv; + const [length, length_adv] = decodePart(str.slice(i)); + if (length == null || length < 0 || length > max_header_field_chars || i + length_adv + length > str.length) { + DEBUG && debug("invalid header field length %o", str.slice(i)); + return null; + } + i += length_adv; + const chars = str.slice(i, i + length); + i += length; + + switch (tag) { + case header_field.build_flags: { + const [flags, flags_adv] = decodePart(chars); + if (flags == null || flags_adv !== chars.length) { + DEBUG && debug("invalid build_flags field %o", chars); + return null; + } + is_canary = !!(flags & (1 << 0)); + break; + } + case header_field.debug_id: + if (chars.length === 0 || chars.length % 2 !== 0 || !/^[0-9a-f]+$/.test(chars)) { + DEBUG && debug("invalid debug_id field %o", chars); + return null; + } + debug_id = chars; + break; + // A field this decoder predates: skipped by its length. + } } } diff --git a/test/__snapshots__/parse.test.ts.snap b/test/__snapshots__/parse.test.ts.snap index 7b7a379..3bc7e83 100644 --- a/test/__snapshots__/parse.test.ts.snap +++ b/test/__snapshots__/parse.test.ts.snap @@ -8627,7 +8627,7 @@ exports[`parse fixtures v3-linux-x86_64-segfault-registers 1`] = ` } `; -exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (canary flag set; the 20-byte id is the binary's GNU build-id as printed by readelf -n) 1`] = ` +exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (header: build flags = canary, then the 20-byte GNU build-id exactly as readelf -n prints it) 1`] = ` { "addresses": [ { @@ -8659,27 +8659,27 @@ exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux "object": "?", }, { - "address": 334254303, + "address": 334255327, "object": "bun", }, { - "address": 334102908, + "address": 334103932, "object": "bun", }, { - "address": 381823109, + "address": 381824133, "object": "bun", }, { - "address": 381824323, + "address": 381825347, "object": "bun", }, { - "address": 391953016, + "address": 391954040, "object": "bun", }, { - "address": 391954698, + "address": 391955722, "object": "bun", }, { @@ -8697,8 +8697,8 @@ exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux ], "arch": "x86_64", "command": "a", - "commitish": "2c2ef7c", - "debug_id": "caac16c6401beba3fdd7e29cafe9bd212a0a23f8", + "commitish": "e65f750", + "debug_id": "e2217c82decf60268399d61b412999385c762757", "features": [ 96, 1048641, @@ -8710,7 +8710,7 @@ exports[`parse fixtures v4-linux-x86_64-panic-debug-id: real trace from a linux } `; -exports[`parse fixtures v4-windows-x86_64-segfault-debug-id: real trace from the windows x64 release build of oven-sh/bun#38838 in CI (canary; the 16-byte id is the PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames) 1`] = ` +exports[`parse fixtures v4-windows-x86_64-segfault-debug-id: frames/features/reason from a real crash of the windows x64 release build of oven-sh/bun#38838 in CI, header re-encoded in the final layout (canary; the 16-byte id is the binary PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames) 1`] = ` { "addresses": [ { diff --git a/test/debug-id.test.ts b/test/debug-id.test.ts index 0d99999..02c1690 100644 --- a/test/debug-id.test.ts +++ b/test/debug-id.test.ts @@ -2,8 +2,14 @@ import { afterAll, describe, expect, test } from "bun:test"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { readExecutableDebugId, selectDebugFile } from "../backend/debug-id"; -import type { Arch } from "../lib/util"; +import { + cacheName, + linkVariant, + publishedLinks, + readExecutableDebugId, + selectDebugFile, + type Link, +} from "../backend/debug-id"; const dir = mkdtempSync(join(tmpdir(), "bun-report-debug-id-")); afterAll(() => rmSync(dir, { recursive: true, force: true })); @@ -160,132 +166,220 @@ describe("readExecutableDebugId", () => { }); }); +describe("publishedLinks", () => { + test("the builds that share a platform char, plain one first", () => { + expect(publishedLinks("linux", "x86_64")).toEqual([ + "x64", + "x64-musl", + "x64-android", + "x64-baseline", + ]); + expect(publishedLinks("linux", "aarch64")).toEqual([ + "aarch64", + "aarch64-musl", + "aarch64-android", + ]); + expect(publishedLinks("windows", "x86_64")).toEqual(["x64", "x64-baseline"]); + expect(publishedLinks("macos", "x86_64")).toEqual(["x64", "x64-baseline"]); + expect(publishedLinks("macos", "aarch64")).toEqual(["aarch64"]); + expect(publishedLinks("windows", "aarch64")).toEqual(["aarch64"]); + expect(publishedLinks("freebsd", "x86_64")).toEqual(["x64"]); + // The old baseline platform chars: their own build first, as before. + expect(publishedLinks("linux", "x86_64_baseline")).toEqual([ + "x64-baseline", + "x64-musl", + "x64-android", + "x64", + ]); + expect(publishedLinks("windows", "x86_64_baseline")).toEqual(["x64-baseline", "x64"]); + }); + + test("linkVariant", () => { + expect(linkVariant("x64")).toBeUndefined(); + expect(linkVariant("aarch64")).toBeUndefined(); + expect(linkVariant("x64-musl")).toBe("musl"); + expect(linkVariant("aarch64-android")).toBe("android"); + expect(linkVariant("x64-baseline")).toBe("baseline"); + }); +}); + describe("selectDebugFile", () => { interface Info { - arch: Arch; + link: Link; debug_id: string | undefined; } - const A = "aa".repeat(16); - const B = "bb".repeat(16); + const GLIBC = "aa".repeat(20); + const MUSL = "bb".repeat(20); + const ANDROID = "cc".repeat(20); + const OTHER = "dd".repeat(20); - function unavailable(arch: Arch): Error & { code: string } { - return Object.assign(new Error(`no artifact for ${arch}`), { code: "DebugInfoUnavailable" }); + function unavailable(link: Link): Error & { code: string } { + return Object.assign(new Error(`no artifact for ${link}`), { code: "DebugInfoUnavailable" }); } - /** `store` maps each published arch to the id its executable carries (undefined = unreadable). */ - function bucket(store: Partial>) { - const fetched: Arch[] = []; - const fetch = async (arch: Arch): Promise => { - fetched.push(arch); - if (!(arch in store)) throw unavailable(arch); - const entry = store[arch]; + /** `published` maps each link the commit has to the id its executable carries (undefined = unreadable). */ + function bucket(published: Record) { + const fetched: Link[] = []; + const fetch = async (link: Link): Promise => { + fetched.push(link); + if (!(link in published)) throw unavailable(link); + const entry = published[link]; if (entry instanceof Error) throw entry; - return { arch, debug_id: entry }; + return { link, debug_id: entry }; }; return { fetch, fetched }; } + const upstream_linux = { + x64: GLIBC, + "x64-musl": MUSL, + "x64-android": ANDROID, + "x64-baseline": GLIBC, + }; - test("a trace without an id uses its own arch unchecked, as before", async () => { - const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", undefined, fetch)).toEqual({ - arch: "x86_64", - debug_id: A, + test("a trace without an id uses the plain build unchecked, as before", async () => { + const { fetch, fetched } = bucket(upstream_linux); + expect(await selectDebugFile("linux", "x86_64", undefined, fetch)).toEqual({ + link: "x64", + debug_id: GLIBC, }); - expect(fetched).toEqual(["x86_64"]); + expect(fetched).toEqual(["x64"]); }); - test("a trace without an id still fails when its own arch is missing", async () => { - const { fetch } = bucket({ x86_64_baseline: B }); - await expect(selectDebugFile("x86_64", undefined, fetch)).rejects.toMatchObject({ + test("a trace without an id still fails when the plain build is missing", async () => { + const { fetch } = bucket({ "x64-musl": MUSL }); + await expect(selectDebugFile("linux", "x86_64", undefined, fetch)).rejects.toMatchObject({ code: "DebugInfoUnavailable", }); }); - test("the trace's own arch carries the id", async () => { - const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", A, fetch)).toEqual({ - arch: "x86_64", - debug_id: A, + test("an old baseline platform char without an id uses the baseline build, as before", async () => { + const { fetch, fetched } = bucket(upstream_linux); + expect(await selectDebugFile("linux", "x86_64_baseline", undefined, fetch)).toEqual({ + link: "x64-baseline", + debug_id: GLIBC, + }); + expect(fetched).toEqual(["x64-baseline"]); + }); + + test("the plain build carries the id", async () => { + const { fetch, fetched } = bucket(upstream_linux); + expect(await selectDebugFile("linux", "x86_64", GLIBC, fetch)).toEqual({ + link: "x64", + debug_id: GLIBC, debug_file: "match", }); - expect(fetched).toEqual(["x86_64"]); + expect(fetched).toEqual(["x64"]); }); - test("the other x64 link carries the id (the bun-windows-x64 vs -baseline case)", async () => { - const { fetch, fetched } = bucket({ x86_64: A, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", B, fetch)).toEqual({ - arch: "x86_64_baseline", - debug_id: B, + test("a musl trace (reports 'l' like glibc) is matched to the musl build", async () => { + const { fetch, fetched } = bucket(upstream_linux); + expect(await selectDebugFile("linux", "x86_64", MUSL, fetch)).toEqual({ + link: "x64-musl", + debug_id: MUSL, debug_file: "match", }); - expect(fetched).toEqual(["x86_64", "x86_64_baseline"]); + expect(fetched).toEqual(["x64", "x64-musl"]); }); - test("works in the other direction too", async () => { - const { fetch } = bucket({ x86_64: A, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64_baseline", A, fetch)).toMatchObject({ - arch: "x86_64", + test("an android trace is matched to the android build", async () => { + const { fetch, fetched } = bucket({ + aarch64: GLIBC, + "aarch64-musl": MUSL, + "aarch64-android": ANDROID, + }); + expect(await selectDebugFile("linux", "aarch64", ANDROID, fetch)).toMatchObject({ + link: "aarch64-android", + debug_file: "match", + }); + expect(fetched).toEqual(["aarch64", "aarch64-musl", "aarch64-android"]); + }); + + test("a trace from a tree that builds a separate baseline binary (the bun-windows-x64 vs -baseline case)", async () => { + const baseline = "ee".repeat(16); + const { fetch, fetched } = bucket({ x64: "ff".repeat(16), "x64-baseline": baseline }); + expect(await selectDebugFile("windows", "x86_64", baseline, fetch)).toEqual({ + link: "x64-baseline", + debug_id: baseline, debug_file: "match", }); + expect(fetched).toEqual(["x64", "x64-baseline"]); }); - test("no published link carries the id: the trace's own arch, flagged mismatch", async () => { - const { fetch } = bucket({ x86_64: A, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", "cc".repeat(16), fetch)).toEqual({ - arch: "x86_64", - debug_id: A, + test("no published build carries the id: the plain build, flagged mismatch, after trying them all", async () => { + const { fetch, fetched } = bucket(upstream_linux); + expect(await selectDebugFile("linux", "x86_64", OTHER, fetch)).toEqual({ + link: "x64", + debug_id: GLIBC, debug_file: "mismatch", }); + expect(fetched).toEqual(["x64", "x64-musl", "x64-android", "x64-baseline"]); }); - test("a missing sibling is skipped, not an error", async () => { - const { fetch } = bucket({ x86_64: A }); - expect(await selectDebugFile("x86_64", B, fetch)).toMatchObject({ - arch: "x86_64", + test("builds the commit was not published as are skipped, not errors", async () => { + const { fetch } = bucket({ x64: GLIBC }); + expect(await selectDebugFile("linux", "x86_64", OTHER, fetch)).toMatchObject({ + link: "x64", debug_file: "mismatch", }); }); - test("an arch with no siblings goes straight to mismatch", async () => { - const { fetch, fetched } = bucket({ aarch64: A }); - expect(await selectDebugFile("aarch64", B, fetch)).toMatchObject({ - arch: "aarch64", + test("an arch with a single build goes straight to mismatch", async () => { + const { fetch, fetched } = bucket({ aarch64: GLIBC }); + expect(await selectDebugFile("macos", "aarch64", OTHER, fetch)).toMatchObject({ + link: "aarch64", debug_file: "mismatch", }); expect(fetched).toEqual(["aarch64"]); }); - test("an artifact whose executable has no readable id is used unverified", async () => { - const { fetch, fetched } = bucket({ x86_64: undefined, x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", B, fetch)).toEqual({ - arch: "x86_64", + test("a plain build whose executable has no readable id is used unverified", async () => { + const { fetch, fetched } = bucket({ x64: undefined, "x64-musl": MUSL }); + expect(await selectDebugFile("linux", "x86_64", MUSL, fetch)).toEqual({ + link: "x64", debug_id: undefined, debug_file: "unverified", }); - expect(fetched).toEqual(["x86_64"]); + expect(fetched).toEqual(["x64"]); }); - test("the trace's own arch was never published but a sibling carrying the id was", async () => { - const { fetch } = bucket({ x86_64_baseline: B }); - expect(await selectDebugFile("x86_64", B, fetch)).toMatchObject({ - arch: "x86_64_baseline", + test("the plain build was never published but another build carrying the id was", async () => { + const { fetch } = bucket({ "x64-musl": MUSL }); + expect(await selectDebugFile("linux", "x86_64", MUSL, fetch)).toMatchObject({ + link: "x64-musl", debug_file: "match", }); }); - test("nothing published at all reports the trace's own arch as unavailable", async () => { + test("nothing published at all reports the plain build as unavailable", async () => { const { fetch } = bucket({}); - await expect(selectDebugFile("x86_64", B, fetch)).rejects.toMatchObject({ + await expect(selectDebugFile("linux", "x86_64", MUSL, fetch)).rejects.toMatchObject({ code: "DebugInfoUnavailable", - message: "no artifact for x86_64", + message: "no artifact for x64", }); }); test("errors other than a missing artifact propagate", async () => { const boom = new Error("unzip exploded"); - await expect(selectDebugFile("x86_64", B, bucket({ x86_64: boom }).fetch)).rejects.toBe(boom); await expect( - selectDebugFile("x86_64", B, bucket({ x86_64: A, x86_64_baseline: boom }).fetch), + selectDebugFile("linux", "x86_64", MUSL, bucket({ x64: boom }).fetch), ).rejects.toBe(boom); + await expect( + selectDebugFile("linux", "x86_64", MUSL, bucket({ x64: GLIBC, "x64-musl": boom }).fetch), + ).rejects.toBe(boom); + }); +}); + +describe("cacheName", () => { + test("the plain build keeps the name the cache always used; other builds get their own", () => { + expect(cacheName("linux", "x86_64", "x64")).toBe("x86_64"); + expect(cacheName("linux", "aarch64", "aarch64")).toBe("aarch64"); + expect(cacheName("linux", "x86_64_baseline", "x64-baseline")).toBe("x86_64_baseline"); + expect(cacheName("linux", "x86_64", "x64-musl")).toBe("x86_64-x64-musl"); + expect(cacheName("windows", "x86_64", "x64-baseline")).toBe("x86_64-x64-baseline"); + // The two directions of the baseline pair must not share an entry. + expect(cacheName("linux", "x86_64_baseline", "x64")).toBe("x86_64_baseline-x64"); + expect( + new Set(publishedLinks("linux", "x86_64").map((l) => cacheName("linux", "x86_64", l))).size, + ).toBe(4); }); }); diff --git a/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json b/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json index 2463edd..6404d54 100644 --- a/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json +++ b/test/fixtures/parse/v4-linux-x86_64-panic-debug-id.json @@ -1,4 +1,4 @@ { - "description": "v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (canary flag set; the 20-byte id is the binary's GNU build-id as printed by readelf -n)", - "input": "1.4.0/la42c2ef7cCoBcaac16c6401beba3fdd7e29cafe9bd212a0a23f8gGikggCozzy0N2vhwlOwgj74NiyompNwmtr4N+ony0N_+tox9T43go9Tqo0o4Wm02o4Wwn9yrX0wgzrXk857vIqosrwIqqorwIA0eNrLzCvLz05NUUguSizOcKoMSMzLTNbQVMhIzEvJSS0CAK/LCxc" + "description": "v4-linux-x86_64-panic-debug-id: real trace from a linux x64 debug build of oven-sh/bun#38838 (header: build flags = canary, then the 20-byte GNU build-id exactly as readelf -n prints it)", + "input": "1.4.0/la4e65f750EACCCwCe2217c82decf60268399d61b412999385c762757gGikggCozzy0N2vhwlOwgj74NiyompNwmtr4N+ony0N_+tqx9T43io9Tqo2o4Wm04o4Wwn/yrX0wizrXk857vIqosrwIqqorwIA0eNrLzCvLz05NUUguSizOcKoMSMzLTNbQVMhIzEvJSS0CAK/LCxc" } diff --git a/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json b/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json index d5ce235..0620b56 100644 --- a/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json +++ b/test/fixtures/parse/v4-windows-x86_64-segfault-debug-id.json @@ -1,4 +1,4 @@ { - "description": "v4-windows-x86_64-segfault-debug-id: real trace from the windows x64 release build of oven-sh/bun#38838 in CI (canary; the 16-byte id is the PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames)", - "input": "https://bun.report/1.4.0/wt4605e221CgBe4509f66f3b4e4984c4c44205044422egGykoomgD619+8BmguilCmmvilCyrp2zB80tsF0zhqPght9Ky5p4Ig2i9Kuxu+K2uw+P89z9PywtzgBg/u66DCYKERNEL32.DLLos9FCSntdll.dllis0WA2AA" + "description": "v4-windows-x86_64-segfault-debug-id: frames/features/reason from a real crash of the windows x64 release build of oven-sh/bun#38838 in CI, header re-encoded in the final layout (canary; the 16-byte id is the binary PDB GUID {E4509F66-F3B4-E498-4C4C-44205044422E}; foreign KERNEL32/ntdll frames follow the bun frames)", + "input": "https://bun.report/1.4.0/wt4605e221EACCCgCe4509f66f3b4e4984c4c44205044422egGykoomgD619+8BmguilCmmvilCyrp2zB80tsF0zhqPght9Ky5p4Ig2i9Kuxu+K2uw+P89z9PywtzgBg/u66DCYKERNEL32.DLLos9FCSntdll.dllis0WA2AA" } diff --git a/test/helpers/encode.ts b/test/helpers/encode.ts index ccb8c2f..825242f 100644 --- a/test/helpers/encode.ts +++ b/test/helpers/encode.ts @@ -89,13 +89,15 @@ export interface BuildTraceOpts { command: string; trace_version: "1" | "2" | "3" | "4"; commitish: string; - /** v3+ build-flags VLQ (bit0 = canary). */ + /** v3: the bare VLQ after the sha; v4: header field 0. bit0 = canary. */ build_flags?: number; + /** v4: the executable's debug id as lowercase hex. Omit for an executable without one. */ + debug_id?: string; /** - * v4: the executable's debug id as lowercase hex; encoded as a VLQ byte count - * followed by the hex. Omit for an executable without one (count 0). + * v4: header fields appended after the ones bun emits today, e.g. a tag this + * decoder does not know, to exercise the skip path. */ - debug_id?: string; + extra_header_fields?: [tag: number, chars: string][]; features?: [number, number]; addresses: ParsedAddress[]; reason: ReasonSpec; @@ -103,6 +105,19 @@ export interface BuildTraceOpts { registers?: { pc: ParsedAddress | null; values: bigint[] }; } +/** The format-4 header, in the order bun's `encode_trace_string` writes it. */ +export function encodeHeader( + opts: Pick, +): string { + const fields: [tag: number, chars: string][] = [[0, encodeVlq(opts.build_flags ?? 0)]]; + if (opts.debug_id !== undefined) fields.push([1, opts.debug_id]); + fields.push(...(opts.extra_header_fields ?? [])); + return ( + encodeVlq(fields.length) + + fields.map(([tag, chars]) => encodeVlq(tag) + encodeVlq(chars.length) + chars).join("") + ); +} + export function buildTraceString(opts: BuildTraceOpts): string { if (opts.commitish.length !== 7) throw new Error("commitish must be 7 chars"); const [f0, f1] = opts.features ?? [0, 0]; @@ -112,13 +127,8 @@ export function buildTraceString(opts: BuildTraceOpts): string { s += opts.command; s += opts.trace_version; s += opts.commitish; - if (opts.trace_version === "3" || opts.trace_version === "4") - s += encodeVlq(opts.build_flags ?? 0); - if (opts.trace_version === "4") { - const debug_id = opts.debug_id ?? ""; - if (debug_id.length % 2 !== 0) throw new Error("debug_id must be whole bytes"); - s += encodeVlq(debug_id.length / 2) + debug_id; - } + if (opts.trace_version === "3") s += encodeVlq(opts.build_flags ?? 0); + if (opts.trace_version === "4") s += encodeHeader(opts); s += encodeVlq(f0) + encodeVlq(f1); for (const a of opts.addresses) s += encodeStackLine(a); s += encodeVlq(0); diff --git a/test/roundtrip.test.ts b/test/roundtrip.test.ts index bc38662..7dc71ce 100644 --- a/test/roundtrip.test.ts +++ b/test/roundtrip.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from "bun:test"; import { parse } from "../lib/parser"; -import { buildTraceString, encodeVlq, type BuildTraceOpts } from "./helpers/encode"; +import { buildTraceString, encodeHeader, encodeVlq, type BuildTraceOpts } from "./helpers/encode"; import { decodePart } from "../lib/vlq"; import { parseCacheKey } from "../lib/util"; @@ -286,7 +286,7 @@ describe("parse(buildTraceString(x)) recovers x", () => { } }); -describe("v4 debug id field", () => { +describe("v4 header", () => { const base = { version: "1.4.0", os: "linux", @@ -298,27 +298,64 @@ describe("v4 debug id field", () => { addresses: [{ address: 0x42, object: "bun" }], reason: { kind: "oom" }, } as const satisfies BuildTraceOpts; + const id = "00112233445566778899aabbccddeeff"; - test("a truncated id is rejected rather than read into the following fields", async () => { - const full = buildTraceString({ ...base, debug_id: "00112233445566778899aabbccddeeff" }); - // Drop two hex digits: the declared count (16 bytes) now overruns into the - // features VLQs, which are not hex. - const cut = full.replace("ccddeeff", "ccddee"); - expect(await parse(cut)).toBeNull(); + /** `base` with a hand-written header in place of the one the helper writes. */ + function withHeader(header: string): string { + const generated = encodeHeader(base); + const full = buildTraceString(base); + const at = full.indexOf(generated, full.indexOf("/") + 1); + return full.slice(0, at) + header + full.slice(at + generated.length); + } + + test("fields with tags this decoder does not know are skipped", async () => { + const p = await parse( + buildTraceString({ + ...base, + build_flags: 1, + debug_id: id, + extra_header_fields: [ + [7, "anything/goes+here_9"], + [300, ""], + ], + }), + ); + expect(p).toMatchObject({ is_canary: true, debug_id: id, commitish: base.commitish }); + expect(p!.addresses).toEqual([{ address: 0x42, object: "bun" }]); + expect(p!.message).toBe("Bun ran out of memory"); + }); + + test("field order does not matter", async () => { + const header = encodeVlq(2) + encodeVlq(1) + encodeVlq(id.length) + id + encodeVlq(0) + encodeVlq(1) + encodeVlq(1); + expect(await parse(withHeader(header))).toMatchObject({ is_canary: true, debug_id: id }); + }); + + test("a header with only build flags means the executable has no id", async () => { + const p = await parse(buildTraceString({ ...base, build_flags: 1 })); + expect(p).toMatchObject({ is_canary: true }); + expect(p!.debug_id).toBeUndefined(); + }); + + test("a field length running past the end of the string is rejected", async () => { + expect(await parse(withHeader(encodeVlq(1) + encodeVlq(1) + encodeVlq(5000) + "0011"))).toBeNull(); + }); + + test("an absurd field count is rejected", async () => { + expect(await parse(withHeader(encodeVlq(100000)))).toBeNull(); }); - test("non-hex where the id should be is rejected", async () => { - const s = buildTraceString({ ...base, debug_id: "00112233445566778899aabbccddeeff" }).replace("aabb", "AABB"); - expect(await parse(s)).toBeNull(); + test("a malformed debug id field is rejected", async () => { + for (const bad of ["abc", "AABBCCDD", "", "zz".repeat(8)]) { + expect(await parse(buildTraceString({ ...base, debug_id: bad }))).toBeNull(); + } }); - test("an absurd byte count is rejected", async () => { - // Same layout as the helper writes, but with a hand-written count. - const prefix = "1.4.0/la4" + base.commitish + encodeVlq(0); - expect(await parse(prefix + encodeVlq(100) + "00".repeat(100) + encodeVlq(0) + encodeVlq(0) + encodeVlq(0) + "9")).toBeNull(); + test("a build flags field that is not exactly one VLQ is rejected", async () => { + expect(await parse(withHeader(encodeVlq(1) + encodeVlq(0) + encodeVlq(2) + "AA"))).toBeNull(); + expect(await parse(withHeader(encodeVlq(1) + encodeVlq(0) + encodeVlq(0)))).toBeNull(); }); - test("is_canary comes from the build flags", async () => { + test("is_canary comes from the build flags field", async () => { expect((await parse(buildTraceString({ ...base, build_flags: 1 })))!.is_canary).toBe(true); expect((await parse(buildTraceString({ ...base, build_flags: 0 })))!.is_canary).toBe(false); });