Add jsc-exception-lint and fix the missing exception checks it finds

Merged
RO
2mo ago
+2836-373

Problem

  • Native code must check for a pending JS exception before the next JS-observable call and before it returns from a ThrowScope. A missing check asserts on debug builds (ERROR: Unchecked JS exception from VM::verifyExceptionCheckNeedIsSatisfied). On release builds the code runs on a dummy result, a later unrelated check misattributes the error, or a second throw overwrites it.
  • Coverage was dynamic only (BUN_JSC_validateExceptionChecks=1 on the ASAN lane), so a path no test executes was never checked.

Fix

  • scripts/jsc-exception-lint: a clang LibTooling checker that models the validator's state machine over the CFG of every function in src/**/*.cpp. Callees are classified from visible bodies, then from summary passes over the JavaScriptCore sources and Bun's bindings, then from the JSGlobalObject* / ThrowScope& convention. run.ts drives it; rust-externs.ts cross-checks hand-declared Rust externs.
  • Fixes its findings: RETURN_IF_EXCEPTION after the throwing call, RELEASE_AND_RETURN for tail calls, asNumber()/asInt32() after a type check instead of the throwing coercion, one nested DECLARE_THROW_SCOPE removed. Rust externs whose C++ body throws go through the scope helpers and return JsResult. No termination special cases, no clearException.
  • Not touched: the files and functions #40068 and #40249 cover (napi, v8, JSMockFunction, ErrorCode, MIME, asymmetric matchers). The JSC-side sites are in oven-sh/WebKit#514; their skip-list entries stay until that bump.
  • Verified: test/js/bun/jsc/exception-checks.test.ts (new; each snippet aborted the validator before), the affected suites under the validator (sqlite, process, ffi, headers, streams, workers, vm, buffer, crypto), and bun run rust:check for linux, windows and macOS.

Background

  • The validator: every ThrowScope destructor sets VM::m_needExceptionCheck. The next ThrowScope constructor or non-released destructor asserts if it is still set. Only exception() (what RETURN_IF_EXCEPTION expands to), clearException() and assertNoException clear it. The tool reports the states in which those asserts fire, plus a call made after the function already threw.
  • A summary is a callee's exit state set: clean, check pending, thrown, or conditional thrower (the caller tests the return value; reported only with --kind maybe-thrown-call). Each pass resolves one more level of cross-file calls, so JSC gets three passes (cached per WebKit version).
  • Rust-implemented extern "C" functions run under their own scope and signal a throw with a sentinel, so the C++ side sees them as conditional throwers.
Notes

Numbers. First pass over src/jsc/bindings with only the signature convention: 2194 findings. With JSC and Bun summaries, the TopExceptionScope model, template-aware carrier detection, and the Rust-extern rule: 667 findings in 103 files (480 pending-call, 164 unchecked-exit, 13 nested scope, 10 call-after-throw). 603 were in scope for this PR after excluding the files above. After the fixes: 113 in 21 files, of which 72 are in the excluded files and the rest were reviewed as false positives (generated JSSink and ZigGeneratedClasses code, JSSetIterator::next in Keys mode, JSFunction::name, getCalculatedDisplayName, rejectWithCaughtException right after a throw, global object construction). Those need entries in nothrow.txt or the summary pass over build/*/codegen the driver now does.

What the tool does not see: a throwing call whose result is passed straight into another call and then RELEASE_AND_RETURN (the JSMockFunction pattern #40068 fixes) is legal for the validator and not reported. That needs value tracking. Exceptions observed only through a return value (if (!result) return {} after a helper that throws into the caller's scope) are reported as maybe-thrown-call and hidden by default.

Running it: bun scripts/jsc-exception-lint/run.ts needs a configured debug build (build/debug/compile_commands.json) and the LLVM 21 development package (libclang-cpp, headers; CI's llvm.sh 21 all installs them). The first run parses the JSC sources three times (about 30 minutes, cached in build/debug/jsc-exception-lint/); later runs take about 15 minutes. A CI step on the linux debug lane after the C++ build is the natural next step; this PR does not add it.

How the fixes were made: the findings were split by file and fixed in parallel under one written rule set (RETURN_IF_EXCEPTION only, no termination special cases, report false positives instead of editing), then the tree was rebuilt, re-analyzed, and a second pass handled the remainder. I reverted two of the resulting hunks by hand (a scope added to rsisDetachNativeTransform, a no-op branch in JSCTaskScheduler.cpp) because they rested on a stale classification of callees that cannot throw.

Dynamic runs: test/js/bun/util/BunObject.test.ts and test/js/bun/jsonl/jsonl-parse.test.ts still abort under the validator on the JSC-side sites (oven-sh/WebKit#514). test/js/node/test/parallel/test-repl-inspect-defaults.js is the JSONP doGet case in the same PR. Tests that failed in my container (worker message flood, ffi FTL warm-up, node:util parseArgs stress, stdin fixtures, IPv6 fetch, root-permission checks) fail identically on an unmodified main build there.

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

fails on main (without fix) 
ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/jsc/exception-checks.test.ts
bun test v1.4.1 (4448a2e21)

test/js/bun/jsc/exception-checks.test.ts:
(pass) process.exitCode assigned a rope string [272.39ms]
(pass) process.kill with an unknown rope signal name [268.19ms]
(pass) process.umask with a rope string [340.13ms]
42 |     // them in the comparison so a failure names the call site.
43 |     const unchecked = stderr
44 |       .split("\n")
45 |       .map(line => line.trim())
46 |       .filter(line => line.startsWith("This scope can throw") || line.startsWith("But the exception was unchecked"));
47 |     expect({ stdout: stdout.trim(), unchecked, exitCode }).toEqual({ stdout: expected, unchecked: [], exitCode: 0 });
                                                                ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
-   "stdout": "TypeError: Expected 2 values to compare",
-   "unchecked": [],
+   "exitCode": 134,
+   "stdout": "",
+   "unchecked": [
+     "This scope can throw a JS exception: functionBunDeepEquals @ ../../src/jsc/bindin
... (truncated)

release without fix: all passed
bun test v1.4.1-canary.1 (a95369a05)

test/js/bun/jsc/exception-checks.test.ts:
(pass) process.umask with a rope string [4.80ms]
(pass) Bun.deepEquals with one argument [5.74ms]
(pass) process.exitCode assigned a rope string [4.57ms]
(pass) process.kill with an unknown rope signal name [4.33ms]

 4 pass
 0 fail
 4 expect() calls
Ran 4 tests across 1 file. [91.00ms]
__F:0:S:0
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/jsc/exception-checks.test.ts
bun test v1.4.1 (4448a2e21)

test/js/bun/jsc/exception-checks.test.ts:
(pass) Bun.deepEquals with one argument [313.84ms]
(pass) process.umask with a rope string [277.49ms]
(pass) process.exitCode assigned a rope string [276.65ms]
(pass) process.kill with an unknown rope signal name [272.38ms]

 4 pass
 0 fail
 4 expect() calls
Ran 4 tests across 1 file. [2.32s]
__F:0:S:0

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/60] gen ProcessBindingHTTPParser.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingHTTPParser.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingHTTPParser.cpp
[2/60] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[3/60] gen cpp.rs (cppbind)
[4/60] gen BunProcess.lut.h
Generating /workspace/bun/build/release/codegen/BunProcess.lut.h from /workspace/bun/src/jsc/bindings/BunProcess.cpp
[5/60] gen generated_host_exports.rs
generated_host_exports.rs: 120 exports (host=5, lazy=10, generic=105, rust=0); 242 extern-C blocks audited
[6/60] gen BunObject.lut.h
Generating /workspace/bun/build/release/codegen/BunObject.lut.h from /workspace/bun/src/jsc/bindings/BunObject.cpp
[7/60] gen JS modules (bundle-modules)
Preprocess modules (7492ms)
Bundle modules (48ms)
Postprocesss modules (96ms)
Bundle Functions (491ms)
Generate Code (32ms)

[8.17s] Bundled "src/js" for production
  2594 kb
  197 internal modules
... (truncated)
diff hotspot 
scripts/jsc-exception-lint/README.md               |   83 ++
 scripts/jsc-exception-lint/jsc-exception-lint.cpp  | 1184 ++++++++++++++++++++
 scripts/jsc-exception-lint/nothrow.txt             |  112 ++
 scripts/jsc-exception-lint/run.ts                  |  450 ++++++++
 scripts/jsc-exception-lint/rust-externs.ts         |  142 +++
 src/http_jsc/headers_jsc.rs                        |    2 -
 src/jsc/ConsoleObject.rs                           |    4 +-
 src/jsc/FetchHeaders.rs                            |   18 +-
 src/jsc/JSGlobalObject.rs                          |   34 +-
 src/jsc/JSObject.rs                                |   18 +-
 src/jsc/JSUint8Array.rs                            |   23 +-
 src/jsc/JSValue.rs                                 |   43 +-
 src/jsc/VirtualMachine.rs                          |    7 +-
 src/jsc/array_buffer.rs                            |   16 +-
 src/jsc/bindings/BunDebugger.cpp                   |    7 +-
 src/jsc/bindings/BunInjectedScriptHost.cpp         |   34 +-
 src/jsc/bindings/BunObject.cpp                     |   13 +-
 src/jsc/bindings/BunPlugin.cpp                     |   20 +-
 src/jsc/bindings/BunProcess.cpp                    |   72 +-
 src/jsc/bindings/BunProcessReportObjectWindows.cpp |    1 +
 src/jsc/bindings/BunString.cpp                     |    2 +
 src/jsc/bindings/CallSite.cpp                      |    5 +
 src/jsc/bindings/CallSitePrototype.cpp             |    1 +
 src/jsc/bindings/ConsoleObject.cpp                 |    7 +-
 src/jsc/bindings/ErrorStackTrace.cpp               |   39 +-
 src/jsc/bindings/FormatStackTraceForJS.cpp         |    9 +-
 src/jsc/bindings/HTMLEntryPoint.cpp                |    2 +-
 src/jsc/bindings/ImportMetaObject.cpp              |    1 -
 src/jsc/bindings/InspectorLifecycleAgent.cpp       |    1 +
 src/jsc/bindings/InternalModuleRegistry.cpp        |    1 +
 src/jsc/bindings/JSBuffer.cpp                      |   12 +-
 src/jsc/bindings/JSCTestingHelpers.cpp       
... (truncated)

gate history · 2 passed · 0 rejected · iteration 0

evidence per changed file 
file                                               reads  edits  tests
scripts/jsc-exception-lint/README.md                   0      1      0
scripts/jsc-exception-lint/jsc-exception-lint.cpp      2      4      0
scripts/jsc-exception-lint/nothrow.txt                 0      1      0
scripts/jsc-exception-lint/run.ts                      0      1      0
scripts/jsc-exception-lint/rust-externs.ts             0      1      0
src/http_jsc/headers_jsc.rs                            0      0      0
src/jsc/ConsoleObject.rs                               0      0      0
src/jsc/FetchHeaders.rs                                0      0      0
src/jsc/JSGlobalObject.rs                              0      0      0
src/jsc/JSObject.rs                                    0      0      0
src/jsc/JSUint8Array.rs                                0      0      0
src/jsc/JSValue.rs                                     0      0      0
src/jsc/VirtualMachine.rs                              0      0      0
src/jsc/array_buffer.rs                                0      0      0
src/jsc/bindings/BunDebugger.cpp                       0      0      0
src/jsc/bindings/BunInjectedScriptHost.cpp             0      0      0
(+ 99 more files)