Fix segfault when printing an AggregateError without an errors property

Merged
RO
3mo ago
+165-36

Problem

  • Printing an AggregateError with no own errors property kills the process: panic(main thread): Segmentation fault at address 0x5 in JSC::forEachInIterable (Sentry BUN-40MQ, 92 events on the 1.4.0 canary, 62 in bun test, and BUN-47FF, the same stack as grouped on Windows, 9 events in bun test, 1 of them on the public 1.4.0).
  • Cause: on main, JSC__JSValue__getErrorsProperty (src/jsc/bindings/bindings.cpp:4860) returns getDirect() as is, which is the empty value for a missing property. print_errorlike_object (src/jsc/VirtualMachine.rs:5221) passes it to for_each, which reads it as a cell at address 0.
  • JSC creates such errors itself: bun test crashed when a second file imported a module with two or more build errors (#36963).

Fix

  • The binding and get_errors_property are deleted. The printer reads errors with fast_get and a new BuiltinName::errors.
  • When it printed no member (errors missing, empty, or not iterable) it falls through and prints the AggregateError like any other error. The normal output, members only, is unchanged.
  • An exception from the read or the walk is cleared, as the normal path below already does. Before, it stayed pending and console.error threw for a non-iterable errors.
  • Verified: test/js/bun/util/inspect-error.test.js, 17 new tests, 16 fail on bun 1.4.0. Also the build error, inspect, plugin, ipc, cluster and source lint tests.

Background

  • print_errorlike_object is the native error printer behind uncaught errors, unhandled rejections, console.error and Bun.inspect. For an AggregateError it prints the members and returns.
  • getDirect reads an own property slot with no getters. A missing slot gives the empty JSValue, which passes JSC's isCell(). An accessor slot gives a GetterSetter cell.
  • fast_get is an ordinary [[Get]] keyed by a preallocated identifier (BuiltinNamesMap in bindings.cpp, same order as BuiltinName in src/jsc/lib.rs). Missing gives None, a throwing getter gives Err.
  • JSModuleLoader::duplicateError (JSModuleLoader.cpp:116) settles every later load of a module that failed to load with a copy of the stored error, rebuilt from its type and message only.
Notes

Repro on bun 1.4.0, user code:

const e = new AggregateError([new Error("a")], "agg");
delete e.errors;
console.error(e); // segfault at 0x5. Same for Bun.inspect(e) and throw e.

Repro without user code (#36963): lib.ts with a syntax error that produces two or more messages, two test files that import it, bun test. The second file crashed. It now reports AggregateError: 4 errors building ".../lib.ts". A single build error produces a BuildMessage, not an AggregateError, and is not affected. Loading the broken module twice with import() in one file and printing the second rejection crashes the same way. That is one of the new tests, next to the bun test case.

Shapes covered by the tests, each through console.error and as an uncaught error: errors deleted, [], {}, null, a number, a getter that throws. Before the fix, the deleted shape crashed (exit 139), [] printed nothing (same for Promise.any([])), {}, null and the number made console.error throw a TypeError, and an accessor (its GetterSetter cell was passed to the walk) threw a TypeError in release builds and failed ASSERT(isSymbol()) in JSValue::synthesizePrototype in debug builds. All of these now print AggregateError: <message> with the source preview and stack. A separate test defines errors as an accessor that returns an array: since the read is an ordinary [[Get]] now, the getter runs and its members are printed, with no header.

fast_get and for_each return Err with the exception still pending. Both sites call clear_exception(), the same call print_error_from_maybe_private_data makes when printing an error throws. The removed let _ = errors.for_each(...) lowers the count for VirtualMachine.rs in test/internal/source-lints/jsresult-swallow.inventory.json by one.

BuiltinName::errors is inserted before the two private names in both enums, so internal and sharedFd move by one on both sides. The tests that go through them still pass: test/js/bun/udp/dgram.test.ts, test/js/bun/spawn/spawn.ipc.test.ts, bun-ipc-inherit.test.ts, and the node test-cluster-basic, test-cluster-dgram-1, -2 and -reuse tests.

Review history: the first revision kept the binding and returned undefined unless the slot held an object, the second dropped its null check and a termination special case, and the third (current) replaced it with fast_get as asked in review.

Overlapping open PRs, each broader than this one: #36602 reworks the whole AggregateError output and guards this walk. #35816 guards the console formatters and adds an is_object check here, but still prints nothing for an errors-less AggregateError. #39204 (blocked on a WebKit bump) makes the loader replay the original error with errors intact, and #39292 prints replayed build errors again. This PR only makes the printer safe for whatever object it gets, and keeps the normal output unchanged, so it does not change what those PRs do.

Also run: test/js/bun/resolve/build-error.test.ts, test/js/bun/util/reportError.test.ts, test/js/bun/util/inspect.test.js, test/js/bun/plugin/plugins.test.ts, test/js/bun/transpiler/transpiler-error-gc-uaf.test.ts, test/regression/issue/hashbang-still-works.test.ts (130 pass), rustfmt --check, cargo clippy -p bun_jsc, clang-format on bindings.cpp, and the repros under BUN_JSC_validateExceptionChecks=1. The new tests spawn one process per case because the unfixed binary dies. The file takes about 4 s under the debug build.

Fixes #36963


[review] gate passed · iteration 0 · 5 files touched

fails on main (without fix)
ASAN without fix: 14 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/inspect-error.test.js
bun test v1.4.0 (4c689909e)

test/js/bun/util/inspect-error.test.js:
(pass) error.cause [6.93ms]
(pass) Error [3.37ms]
(pass) BuildMessage [11.73ms]
(pass) Error inside minified file (no color)  [80.80ms]
(pass) Error inside minified file (color)  [46.97ms]
(pass) Inserted originalLine and originalColumn do not appear in node:util.inspect [123.41ms]
(pass) observable properties > sourceURL is observable [5.71ms]
(pass) observable properties > line is observable [2.61ms]
(pass) observable properties > column is observable [1.85ms]
(pass) error.stack throwing an error doesn't lead to a crash [3.51ms]
(pass) source map remapping of the printed stack > external map whose original source is unavailable [321.92ms]
361 |     test(`console.error: ${name}`, async () => {
362 |       const { stdout, stderr, exitCode } = await run([
363 |         "-e",
364 |         `${make} ${shape} console.error(e); console.log("after");`,
365 |       ]);
366 |       expect(stderr).toContain(header);
                      
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (36c2bd937)

test/js/bun/util/inspect-error.test.js:
(pass) error.cause [0.22ms]
(pass) Error [0.08ms]
(pass) BuildMessage [0.93ms]
(pass) Error inside minified file (no color)  [2.25ms]
(pass) Error inside minified file (color)  [0.37ms]
(pass) Inserted originalLine and originalColumn do not appear in node:util.inspect [2.10ms]
(pass) observable properties > sourceURL is observable [0.15ms]
(pass) observable properties > line is observable [0.06ms]
(pass) observable properties > column is observable [0.05ms]
(pass) error.stack throwing an error doesn't lead to a crash [0.07ms]
(pass) source map remapping of the printed stack > external map whose original source is unavailable [15.36ms]
(pass) AggregateError whose errors cannot be walked > console.error: errors was deleted [12.28ms]
(pass) AggregateError whose errors cannot be walked > uncaught: errors was deleted [11.45ms]
(pass) AggregateError whose errors cannot be walked > console.error: errors is an empty array [11.04ms]
(pass) AggregateError whose errors cannot be walked > uncaught: errors is an empty array [10.43ms]
(pass) AggregateError whose errors cannot be walked > console.error: 
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/inspect-error.test.js
bun test v1.4.0 (4c689909e)

test/js/bun/util/inspect-error.test.js:
(pass) error.cause [6.87ms]
(pass) Error [3.22ms]
(pass) BuildMessage [11.69ms]
(pass) Error inside minified file (no color)  [82.14ms]
(pass) Error inside minified file (color)  [48.09ms]
(pass) Inserted originalLine and originalColumn do not appear in node:util.inspect [123.36ms]
(pass) observable properties > sourceURL is observable [5.96ms]
(pass) observable properties > line is observable [2.81ms]
(pass) observable properties > column is observable [1.88ms]
(pass) error.stack throwing an error doesn't lead to a crash [3.89ms]
(pass) source map remapping of the printed stack > external map whose original source is unavailable [324.62ms]
(pass) AggregateError whose errors cannot be walked > console.error: errors was deleted [285.43ms]
(pass) AggregateError whose errors cannot be walked > console.error: errors is an empty array [269.00ms]
(pass) AggregateError whose errors cannot be walked > uncaught: errors was deleted [286.79
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 628ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/9] cc obj/packages/bun-usockets/src/loop.c.o
[2/9] gen generated_host_exports.rs
generated_host_exports.rs: 92 exports (host=3, lazy=10, generic=79, rust=0); 241 extern-C blocks audited
[3/9] gen cpp.rs (cppbind)
[3/9] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

^[[1m^[[92m   Compiling^[[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
^[[1m^[[92m   Compiling^[[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
^[[1m^[[92m   Compiling^[[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
^[[1m^[[92m   Compiling^[[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
^[[1m^[[92m   Compiling^[[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
^[[1m^[[92m   Compiling^[[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
^[[1m^[[92m   Compiling^[[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
^[[1m^[[92m   Compiling^[[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
^[[1m^[[92m   Compiling^[[0m bun_picohttp 
... (truncated)
diff hotspot
src/jsc/JSValue.rs                                 |   6 +-
 src/jsc/VirtualMachine.rs                          |  38 +++---
 src/jsc/bindings/bindings.cpp                      |  11 +-
 .../source-lints/jsresult-swallow.inventory.json   |   2 +-
 test/js/bun/util/inspect-error.test.js             | 127 +++++++++++++++++++++
 5 files changed, 165 insertions(+), 19 deletions(-)

gate history · 2 passed · 0 rejected · iteration 0

evidence per changed file
file                                                      reads  edits  tests
src/jsc/JSValue.rs                                            6      3      0
src/jsc/VirtualMachine.rs                                    12     12      0
src/jsc/bindings/bindings.cpp                                 4      3      0
…t/internal/source-lints/jsresult-swallow.inventory.json      1      1      0
test/js/bun/util/inspect-error.test.js                        2      3      0