diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 8fd5af890e3..632dc17495a 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2574,14 +2574,14 @@ 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`). On-stack + // array: conservative GC stack scan keeps these JSValues alive + // (see PORTING.md §JSC) — creating later messages can trigger a + // GC, and a heap `Vec` is invisible to the scan. + let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256]; + let len = log.msgs.len().min(errors_stack.len()); + for (i, msg) in log.msgs.drain(..len).enumerate() { + errors_stack[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 +2589,8 @@ pub fn process_fetch_log( referrer_utf8.slice(), )), }; - errors.push(v); } + let errors = &errors_stack[..len]; // C++ `Zig::toString` does `createWithoutCopying`, so the buffer // must outlive the AggregateError. Mark it global so JSC adopts it @@ -2604,7 +2604,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..03e1f5653eb 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -2143,12 +2143,16 @@ impl LogJsc for bun_ast::Log { 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)?); + // On-stack array: conservative GC stack scan keeps these + // JSValues alive (see PORTING.md §JSC) — creating later + // messages can trigger a GC, and a heap `Vec` is invisible + // to the scan. + let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256]; + 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[..count], &out) } } } diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index a623aa505e9..2b607381554 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, tempDir } from "harness"; import { join } from "node:path"; test("BuildError is modifiable", async () => { @@ -19,6 +19,36 @@ 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. 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");`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + cwd: String(dir), + env: { ...bunEnv, BUN_JSC_collectContinuously: "1" }, + 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