diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 8fd5af890e3..f281e8de10c 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2574,14 +2574,15 @@ pub fn process_fetch_log( } _ => { - // Spec caps at 256 (`var errors_stack: [256]JSValue`). PERF(port): - // was inline switch — Zig stack-allocated; we heap-allocate the - // exact `len` since `JSValue` is a thin u64 and 256 * 8 B = 2 KiB - // is fine either way, but `Vec` avoids the uninit-array dance. - let len = log.msgs.len().min(256); - let mut errors: alloc::vec::Vec = alloc::vec::Vec::with_capacity(len); - for msg in log.msgs.drain(..len) { - let v = match msg.metadata { + // Spec caps at 256 (`var errors_stack: [256]JSValue`). Must be a + // stack array so the conservative GC scan keeps each freshly + // allocated BuildMessage/ResolveMessage cell alive while we + // allocate the next one; a heap `Vec` leaves them + // unrooted and they can be swept mid-loop. + let mut errors = [JSValue::UNDEFINED; 256]; + let len = log.msgs.len().min(errors.len()); + for (i, msg) in log.msgs.drain(..len).enumerate() { + errors[i] = match msg.metadata { bun_ast::Metadata::Build => take(BuildMessage::create(global_this, msg)), bun_ast::Metadata::Resolve(_) => take(ResolveMessage::create( global_this, @@ -2589,8 +2590,8 @@ pub fn process_fetch_log( referrer_utf8.slice(), )), }; - errors.push(v); } + let errors = &errors[..len]; // C++ `Zig::toString` does `createWithoutCopying`, so the buffer // must outlive the AggregateError. Mark it global so JSC adopts it @@ -2604,7 +2605,7 @@ pub fn process_fetch_log( message.mark_global(); *ret = ErrorableResolvedSource::err( err, - take(global_this.create_aggregate_error(&errors, &message)), + take(global_this.create_aggregate_error(errors, &message)), ); } } diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index a23f5a105ec..8e40d3a0422 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -2138,17 +2138,20 @@ impl LogJsc for bun_ast::Log { fn to_js(&self, global: &JSGlobalObject, message: &str) -> JsResult { let msgs = &self.msgs; // Spec: `@min(msgs.len, errors_stack.len)` — errors_stack is `[256]JSValue`. - let count = msgs.len().min(256); + // Must be a stack array so the conservative GC scan keeps the freshly + // allocated BuildMessage/ResolveMessage cells alive while we allocate + // the next one; a heap `Vec` would leave them unrooted. + let mut errors_stack = [JSValue::UNDEFINED; 256]; + let count = msgs.len().min(errors_stack.len()); match count { 0 => Ok(JSValue::UNDEFINED), 1 => msg_to_js(&msgs[0], global), _ => { - let mut errors_stack: Vec = Vec::with_capacity(count); - for msg in &msgs[0..count] { - errors_stack.push(msg_to_js(msg, global)?); + for (i, msg) in msgs[0..count].iter().enumerate() { + errors_stack[i] = msg_to_js(msg, global)?; } let out = bun_core::ZigString::init(message.as_bytes()); - global.create_aggregate_error(&errors_stack, &out) + global.create_aggregate_error(&errors_stack[0..count], &out) } } } diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index a623aa505e9..22b9d31a806 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -1,4 +1,4 @@ -import { tempDir } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "node:path"; test("BuildError is modifiable", async () => { @@ -19,6 +19,44 @@ test("BuildError is modifiable", async () => { expect(error!.message).not.toBe(message); }); +test("importing a module with many build errors does not crash while reporting them", async () => { + // The AggregateError for a failed module build is assembled from one + // BuildMessage wrapper per log message (process_fetch_log). Those wrappers + // used to be collected only in a heap Vec (invisible to the conservative GC + // scan), so a GC during the loop could finalize earlier wrappers and free + // their native BuildMessage before the AggregateError was created, causing + // a use-after-free when the unhandled rejection was printed. + using dir = tempDir("build-error-many", { + // 40 declarations + 80 redeclarations -> ~80 build errors in one module + "bad.js": Array.from({ length: 120 }, (_, i) => `const x${i % 40} = 1;`).join("\n"), + "index.js": `import("./bad.js");`, + }); + + // Windows + collectContinuously is prohibitively slow in CI and the code + // path is platform-agnostic, so rely on zombie mode alone there. + const gcEnv: Record = { + ...bunEnv, + BUN_JSC_useZombieMode: "1", + }; + if (!isWindows) gcEnv.BUN_JSC_collectContinuously = "1"; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + cwd: String(dir), + env: gcEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Every error in the AggregateError should have been printed. + expect(stderr).toContain('"x0" has already been declared'); + expect(stderr).toContain('"x39" has already been declared'); + // Unhandled rejection -> clean exit with code 1, not a crash. + expect(exitCode).toBe(1); +}); + test("BuildMessage finalize frees with the same allocator it was created with", async () => { // BuildMessage.create() clones the message with the passed allocator // but finalize() was freeing it with bun.default_allocator and never diff --git a/test/js/bun/transpiler/transpiler-async-error-uaf.test.ts b/test/js/bun/transpiler/transpiler-async-error-uaf.test.ts new file mode 100644 index 00000000000..6f7973eba06 --- /dev/null +++ b/test/js/bun/transpiler/transpiler-async-error-uaf.test.ts @@ -0,0 +1,61 @@ +// Log.to_js (used by Bun.Transpiler().transform/transformSync when rejecting +// with parse errors, and by the module loader via process_fetch_log) builds +// an AggregateError by allocating one BuildMessage JS cell per log entry. The +// Rust port collected those cells in a heap Vec, which the +// conservative GC scan does not see, so an earlier cell could be swept while +// allocating a later one and the AggregateError would reference a zapped +// StructureID. +// +// useZombieMode scribbles 0xbadbeef0 over swept cells so the dangling access +// manifests deterministically; collectContinuously races the collector against +// the allocation loop so it reliably sweeps mid-loop. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; + +const fixture = ` +const src = Array.from({ length: 300 }, () => "a b").join("\\n"); +const t = new Bun.Transpiler(); +for (let i = 0; i < 20; i++) { + let err; + try { t.transformSync(src); } catch (e) { err = e; } + if (!(err instanceof AggregateError)) throw new Error("not AggregateError: " + err); + if (err.errors.length !== 256) throw new Error("wrong count: " + err.errors.length); + for (const m of err.errors) { + const msg = m.message; + if (msg !== 'Expected ";" but found "b"') { + throw new Error("corrupt BuildMessage: " + JSON.stringify(typeof msg) + " " + String(msg).slice(0, 80)); + } + } +} +console.log("OK"); +`; + +test("Log.to_js roots BuildMessage cells across allocation", async () => { + using dir = tempDir("log-to-js-gc-root", { + "fixture.js": fixture, + }); + + // Windows + collectContinuously is prohibitively slow in CI and the code + // path is platform-agnostic, so rely on zombie mode alone there. + const gcEnv: Record = { + ...bunEnv, + BUN_JSC_useZombieMode: "1", + }; + if (!isWindows) gcEnv.BUN_JSC_collectContinuously = "1"; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: gcEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(stdout.trim()).toBe("OK"); + expect(exitCode).toBe(0); +}, 60_000);