Fix use-after-free in AggregateError creation for multi-error module loads

src/jsc/VirtualMachine.rs

@@ -2558,23 +2558,27 @@
         }
 
         _ => {
-            // 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<JSValue> = 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`). The array
+            // must stay on the stack like the Zig spec: until
+            // `create_aggregate_error` runs, these freshly created
+            // BuildMessage/ResolveMessage wrappers have no other GC-visible
+            // reference, and `create()` can trigger a collection mid-loop.
+            // JSC's conservative scan only covers the stack/registers, so a
+            // heap `Vec<JSValue>` would let earlier wrappers be swept (and
+            // their native payloads freed) before the AggregateError exists.
+            let mut errors_stack = [JSValue::ZERO; 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,
                         &msg,
                         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
@@ -2588,7 +2592,7 @@
             message.mark_global();
             *ret = ErrorableResolvedSource::err(
                 err,
-                take(global_this.create_aggregate_error(&errors, &message)),
+                take(global_this.create_aggregate_error(errors, &message)),
             );
         }
     }

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,58 @@
   expect(error!.message).not.toBe(message);
 });
 
+test("AggregateError from a module with many build errors survives GC during its creation", async () => {
+  // A module that fails to load with multiple errors produces an
+  // AggregateError of BuildMessage objects. The wrappers used to be collected
+  // in a heap Vec the conservative GC scan cannot see, so a collection in the
+  // middle of creating them freed the native payloads and printing or
+  // inspecting the AggregateError afterwards was a use-after-free.
+  using dir = tempDir("aggregate-build-errors", {
+    "bad.js": Array.from({ length: 250 }, (_, i) => `let dup${i} = 1; let dup${i} = 2;`).join("\n"),
+    "index.js": `
+      let aggregate;
+      try {
+        await import("./bad.js");
+        throw new Error("expected import to fail");
+      } catch (e) {
+        aggregate = e;
+      }
+      Bun.gc(true);
+      if (aggregate.constructor.name !== "AggregateError") {
+        throw new Error("expected AggregateError, got " + aggregate.constructor.name);
+      }
+      if (aggregate.errors.length < 2) {
+        throw new Error("expected multiple build errors, got " + aggregate.errors.length);
+      }
+      for (const sub of aggregate.errors) {
+        if (typeof sub.message !== "string") throw new Error("bad message");
+        void sub.position;
+      }
+      console.error(aggregate);
+      console.log("OK", aggregate.errors.length);
+    `,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "index.js"],
+    cwd: String(dir),
+    env: {
+      ...bunEnv,
+      // Make a collection land while the per-message error wrappers are being created.
+      BUN_JSC_collectContinuously: "true",
+      BUN_JSC_forceRAMSize: "1000000",
+    },
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stderr).not.toContain("AddressSanitizer");
+  expect(stdout).toContain("OK 250");
+  expect(exitCode).toBe(0);
+});
+
 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