fuzzilli: chain crash signal handlers to JSC/ASAN instead of SIG_DFL - #30092
fuzzilli: chain crash signal handlers to JSC/ASAN instead of SIG_DFL#30092robobun wants to merge 4 commits into
Conversation
Bun__REPRL__registerFuzzilliFunctions() installed handlers for SIGSEGV,
SIGILL, SIGFPE and SIGABRT via signal() that flushed stdio and then
re-raised with SIG_DFL. Because this runs after VM construction (and
therefore after Config::finalize() has installed WTF::jscSignalHandler
for SIGSEGV/SIGBUS), it clobbered JSC's handler and, through it, ASAN's.
Two consequences:
1. Every SIGSEGV-based crash the fuzzer found surfaced as a bare
"TERMSIG: 11" with empty stderr — ASAN's report never ran. This is
why fingerprint a488dd11600c00b1 (require("compile")+Bun.gc(true))
and similar flaky crashes have no stack trace.
2. JSC's signal-based VMTraps (Options::usePollingTraps defaults to
false) and WASM fault handling were broken in the REPRL process.
VMTraps::SignalSender installs trap breakpoints in JIT code that
raise SIGSEGV; jscSignalHandler normally catches it, jettisons the
CodeBlock, and returns. With the old handler that SIGSEGV killed the
process instead. Any prior REPRL iteration that armed VMTraps (e.g.
Worker#terminate → notifyNeedTermination) could make a *later*
iteration die in unrelated JIT code.
Save the previous sigaction and forward to it after flushing. The chain
becomes fuzzilliSignalHandler → jscSignalHandler → ASAN (→ SIG_DFL),
verified with fuzzilli('FUZZILLI_CRASH', 5):
==…==ERROR: AddressSanitizer: SEGV on unknown address 0x0
#0 functionFuzzilli …/FuzzilliREPRL.cpp:132
#6 WTF::jscSignalHandler …/Signals.cpp:540
All FUZZILLI_CRASH test modes still terminate the process so crash
detection is unchanged.
Fingerprint: a488dd11600c00b1
|
Updated 8:08 PM PT - May 1st, 2026
❌ @robobun, your commit 696a1f4 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30092That installs a local version of the PR into your bun-30092 --bun |
WalkthroughInstalls a sigaction-based handler for crash signals that flushes and fsyncs stdout/stderr and chains to previously installed handlers or SIG_DFL; registration uses a std::once_flag to avoid re-installation. Adds fuzzilli-gated subprocess tests (including a Worker case) that verify ASAN/handler chaining for SIGSEGV and abort behavior for SIGABRT, and configures ASAN options for fast, non-symbolized reports. ChangesSignal Handler Chain Preservation
Fuzzilli Signal-Chain Tests
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 0/5 reviews remaining, refill in 54 minutes and 27 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/FuzzilliREPRL.cpp`:
- Around line 31-35: Replace the non-async-signal-safe fflush(stdout) and
fflush(stderr) calls with async-signal-safe write() calls: write(STDOUT_FILENO,
<buffer>, <len>) and write(STDERR_FILENO, <buffer>, <len>) using the appropriate
output buffers or a small newline/flush string; keep the existing
fsync(STDOUT_FILENO) and fsync(STDERR_FILENO) calls. Locate the block containing
fflush(stdout); fflush(stderr); fsync(STDOUT_FILENO); fsync(STDERR_FILENO) in
FuzzilliREPRL.cpp and change the two fflush calls to write calls (using
STDOUT_FILENO and STDERR_FILENO) so the handler remains async-signal-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1d7d86bb-9b09-428d-b9a3-323ac9365f4a
📒 Files selected for processing (1)
src/bun.js/bindings/FuzzilliREPRL.cpp
| // Flush all output | ||
| fflush(stdout); | ||
| fflush(stderr); | ||
| fsync(STDOUT_FILENO); | ||
| fsync(STDERR_FILENO); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is fflush async-signal-safe according to POSIX, and what is the recommended way to ensure crash output is visible from a signal handler?
💡 Result:
No, fflush is not async-signal-safe according to POSIX. The official POSIX list of async-signal-safe functions does not include fflush or any stdio functions like printf. Instead, use low-level write to STDERR_FILENO (fd 2) or STDOUT_FILENO (fd 1) for output in signal handlers, followed by fsync if needed to ensure visibility, then _exit. This avoids stdio buffering issues and ensures the output is flushed to the kernel without relying on process termination cleanup, which may not occur reliably on crashes like SIGSEGV.
Citations:
- 1: https://www.man7.org/linux/man-pages/man7/signal-safety.7.html
- 2: https://pubs.opengroup.org/onlinepubs/9799919799/functions/fflush.html
- 3: https://man7.org/linux/man-pages/man7/signal-safety.7.html
- 4: https://pubs.opengroup.org/onlinepubs/9699959299/functions/xsh_chap02_04.html
- 5: https://stackoverflow.com/questions/103280/portable-way-to-catch-signals-and-report-problem-to-the-user
Replace fflush() calls with async-signal-safe write() to avoid deadlock in signal handler.
The fflush(stdout) and fflush(stderr) calls at lines 31–35 are not async-signal-safe. If the signal interrupts code that holds the stdio lock, the handler will deadlock before the chained JSC/ASAN handler runs, defeating the purpose of this patch. Use write(STDOUT_FILENO, ...) and write(STDERR_FILENO, ...) instead; the fsync() calls that follow are async-signal-safe and can remain.
Current code
// Flush all output
fflush(stdout);
fflush(stderr);
fsync(STDOUT_FILENO);
fsync(STDERR_FILENO);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/bindings/FuzzilliREPRL.cpp` around lines 31 - 35, Replace the
non-async-signal-safe fflush(stdout) and fflush(stderr) calls with
async-signal-safe write() calls: write(STDOUT_FILENO, <buffer>, <len>) and
write(STDERR_FILENO, <buffer>, <len>) using the appropriate output buffers or a
small newline/flush string; keep the existing fsync(STDOUT_FILENO) and
fsync(STDERR_FILENO) calls. Locate the block containing fflush(stdout);
fflush(stderr); fsync(STDOUT_FILENO); fsync(STDERR_FILENO) in FuzzilliREPRL.cpp
and change the two fflush calls to write calls (using STDOUT_FILENO and
STDERR_FILENO) so the handler remains async-signal-safe.
There was a problem hiding this comment.
The fflush() calls are pre-existing (present before this PR) and intentional — the whole point of this handler is to drain stdio's userspace buffers so Fuzzilli captures buffered output before the process dies. write(STDOUT_FILENO, …) can only push bytes we already hold; it cannot flush FILE* buffers, so swapping them in would make the handler a no-op for its stated purpose.
The async-signal-safety risk (deadlock if we crash while holding stdio's lock) is accepted here: the process is terminating regardless, and the next handler in the chain (WTF::jscSignalHandler, ASAN's reporter) is already non-signal-safe — they allocate, walk stacks, and write through stdio themselves. This is the standard crash-reporting trade-off.
There was a problem hiding this comment.
@robobun, that's a fair and well-reasoned explanation. You're right that write() cannot drain FILE* userspace buffers, so substituting it would silently drop exactly the buffered output Fuzzilli needs to see. And the signal-safety concern is moot in practice here because the process is already on its way out and the downstream handlers (WTF::jscSignalHandler, ASAN) are themselves non-signal-safe by necessity. This is the standard crash-reporter trade-off. Happy to dismiss this comment.
(・ω・)っ🐇
✏️ Learnings added
Learnt from: robobun
Repo: oven-sh/bun PR: 30092
File: src/bun.js/bindings/FuzzilliREPRL.cpp:31-35
Timestamp: 2026-05-02T02:52:13.970Z
Learning: In `src/bun.js/bindings/FuzzilliREPRL.cpp` (oven-sh/bun), the `fuzzilliSignalHandler` intentionally calls `fflush(stdout)` and `fflush(stderr)` before forwarding to the chained handler. This is by design: the purpose is to drain `FILE*` userspace buffers so Fuzzilli captures buffered output before the process dies. `write(STDOUT_FILENO, …)` cannot flush `FILE*` buffers and would be a no-op for this purpose. The async-signal-safety risk (deadlock if the signal fires while the stdio lock is held) is an accepted trade-off because (a) the process is terminating regardless, and (b) the downstream chained handlers (`WTF::jscSignalHandler`, ASAN's reporter) are themselves non-async-signal-safe. Do not flag these `fflush()` calls as unsafe in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: robobun
Repo: oven-sh/bun PR: 29389
File: src/bun.js/bindings/V8HeapSnapshotBuilder.cpp:592-594
Timestamp: 2026-04-17T04:59:15.762Z
Learning: When reviewing C++ code that uses WebKit Template Framework (WTF)/JSC types, do not treat `if (str.isEmpty()) str = String();` as dead code or a no-op. In WTF, `WTF::String::isEmpty()` returns true for both null strings and empty non-null strings, while `String()` constructs a null string. This conditional therefore normalizes empty non-null strings into null strings and may change behavior for downstream callers that distinguish null vs empty—so it should not be flagged as redundant solely on that basis.
Learnt from: robobun
Repo: oven-sh/bun PR: 29867
File: packages/bun-uws/src/App.h:284-285
Timestamp: 2026-04-28T22:49:27.619Z
Learning: When reviewing C++ move constructors/assignments, do not require calling `.clear()` on objects that are being moved-from when they are `std::vector` (or other standard containers) members. Moving a `std::vector` transfers ownership safely; the moved-from vector remains in a valid state and should not cause double-free by itself. Only flag issues if there are non-standard ownership/raw-pointer/double-destruction patterns beyond the default `std::vector` move behavior.
Skipped unless the binary under test exposes the fuzzilli() global (FUZZILLI_ENABLED builds only).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| installFuzzilliSignalHandler(SIGABRT); | ||
| installFuzzilliSignalHandler(SIGSEGV); | ||
| installFuzzilliSignalHandler(SIGILL); | ||
| installFuzzilliSignalHandler(SIGFPE); |
There was a problem hiding this comment.
🔴 Bun__REPRL__registerFuzzilliFunctions() is called from Zig__GlobalObject__create (ZigGlobalObject.cpp:518) for every global object — including Workers — so the first new Worker(...) re-runs installFuzzilliSignalHandler(), and sigaction() returns fuzzilliSignalHandler itself as the previous handler, overwriting fuzzilliOldActions[sig]. This permanently loses the saved JSC/ASAN chain (back to bare TERMSIG: 11) and makes any subsequent signal recurse infinitely into itself. Guard installation with a static once-flag, or skip the save when the returned sa_sigaction == fuzzilliSignalHandler.
Extended reasoning...
What the bug is
installFuzzilliSignalHandler() saves the previous handler into the static fuzzilliOldActions[sig] so that fuzzilliSignalHandler can chain to it. This is correct the first time it runs. But the function is not idempotent: if it runs a second time, sigaction() returns the current handler — which is now fuzzilliSignalHandler itself, with SA_SIGINFO set — and overwrites the saved JSC/ASAN handler with a self-reference.
Code path that triggers it
Bun__REPRL__registerFuzzilliFunctions() is invoked unconditionally at ZigGlobalObject.cpp:518 inside Zig__GlobalObject__create, which per its own header comment (lines 427–429) runs for the main thread (executionContextId == -1), macros (maxInt32), and Workers (> -1). The Worker-init branch at line 534 confirms Worker globals flow through this same function and reach line 518 first. Signal dispositions are process-wide, and fuzzilliOldActions[] is a file-scope static, so the Worker thread's call mutates the same state the main thread relies on.
JSC's Config::finalize() / WTF signal-handler installation uses call_once and runs once per process, not per-VM, so the Worker's VM creation does not re-install jscSignalHandler between the two calls. The handler in place when the Worker calls sigaction() is therefore fuzzilliSignalHandler.
Why nothing prevents it
There is no once-guard around the installation, and installFuzzilliSignalHandler() does not check whether the returned old action is already its own handler. The old signal()-based code was harmlessly idempotent under repeated installation (it just wrote the same function pointer); the new chaining code is stateful and is actively corrupted by re-installation — a regression introduced by this PR.
Step-by-step proof
- Main global is created →
installFuzzilliSignalHandler(SIGSEGV)runs.sigaction()installsfuzzilliSignalHandlerand writes the previous handler (WTF::jscSignalHandler,SA_SIGINFOset) intofuzzilliOldActions[SIGSEGV]. ✅ - Fuzzilli input executes
new Worker(...)→Zig__GlobalObject__create(executionContextId > -1)→ line 518 →installFuzzilliSignalHandler(SIGSEGV)runs again. sigaction()returns the current handler —{ .sa_sigaction = fuzzilliSignalHandler, .sa_flags = SA_SIGINFO }— into&fuzzilliOldActions[SIGSEGV], overwriting the savedjscSignalHandler.- Later, a SIGSEGV fires.
fuzzilliSignalHandlerflushes, readsold = fuzzilliOldActions[SIGSEGV], seesold.sa_flags & SA_SIGINFOandold.sa_sigaction != nullptr, and callsold.sa_sigaction(sig, info, ucontext)— i.e. calls itself. - This is a direct function call, not signal redelivery, so
sa_maskdoes not block it. The handler recurses until the stack overflows.
Impact
After the first Worker is spawned in a REPRL session: (a) the JSC→ASAN chain is permanently lost, so crashes go back to bare TERMSIG: 11 with no ASAN report — defeating the purpose of this PR; and (b) JSC's signal-based VMTraps/WASM fault handling is broken again; and (c) any crash signal triggers infinite self-recursion. The PR description's "100 iterations interleaving new Worker(...).terminate()" test only verified that no spurious crash occurred — it didn't deliver a real crash signal after a Worker existed, so the corrupted chain was never exercised.
Fix
Either guard the installation block with a static once-flag (signal handlers are process-wide, so installing once is correct), e.g.:
static bool installed = false;
if (!std::exchange(installed, true)) {
installFuzzilliSignalHandler(SIGABRT);
installFuzzilliSignalHandler(SIGSEGV);
installFuzzilliSignalHandler(SIGILL);
installFuzzilliSignalHandler(SIGFPE);
}or, inside installFuzzilliSignalHandler, capture the old action into a temporary and skip the save when tmp.sa_sigaction == fuzzilliSignalHandler.
There was a problem hiding this comment.
Good catch — confirmed: before the fix, new Worker(...) followed by fuzzilli("FUZZILLI_CRASH", 5) died with bare SIGSEGV (exit 139, no ASAN output). Wrapped the install block in std::call_once in 696a1f4 and added a test that creates a Worker before triggering the null deref.
| struct sigaction action; | ||
| action.sa_sigaction = fuzzilliSignalHandler; | ||
| sigfillset(&action.sa_mask); | ||
| action.sa_flags = SA_SIGINFO; |
There was a problem hiding this comment.
🟡 Consider adding SA_ONSTACK here (action.sa_flags = SA_SIGINFO | SA_ONSTACK). ASAN/JSC set up a sigaltstack and install their SIGSEGV handlers with SA_ONSTACK so they can run when the fault is a stack overflow; without it, fuzzilliSignalHandler is delivered on the already-exhausted stack, immediately re-faults, and the process dies with a bare TERMSIG: 11 — the symptom this PR is eliminating. Not a regression (the old signal() path also lacked it), but it's a one-token fix in code being rewritten here.
Extended reasoning...
What the bug is
installFuzzilliSignalHandler() installs the new chaining handler with action.sa_flags = SA_SIGINFO, omitting SA_ONSTACK. This means the kernel will deliver SIGSEGV by pushing a signal frame onto the current thread stack rather than the alternate signal stack.
For most faults that's fine. But when the SIGSEGV is caused by stack exhaustion (the guard page below the thread stack is hit), there is no room left on the current stack to push the signal frame. The kernel's attempt to set up the handler invocation itself faults, the kernel gives up and applies the default disposition, and the process dies with a bare SIGSEGV — no fflush, no chain to jscSignalHandler, no ASAN report. That's exactly the "TERMSIG: 11 with nothing on stderr" symptom this PR exists to eliminate.
Why the alt stack is already there
Both layers this PR is chaining to already account for this:
- ASAN's
MaybeInstallSigactioninstalls its deadly-signal handler withSA_SIGINFO | SA_NODEFER | SA_ONSTACKand callsSetAlternateSignalStack()per thread (defaultuse_sigaltstack=1). This file already#include <sanitizer/asan_interface.h>, so the fuzzilli build is running under ASAN with the alt stack available. - JSC's
WTF::SignalHandlers::addinstallsjscSignalHandlerwithSA_ONSTACKfor the same reason. - Bun's own
crash_handler.zigdoes the same (sigaltstack+SA.ONSTACK).
So the alternate stack already exists at the point installFuzzilliSignalHandler runs — the new outermost handler simply isn't opting into it. By installing without SA_ONSTACK, the fuzzilli wrapper effectively removes stack-overflow handling that the inner layers had.
Step-by-step proof
- Fuzzer generates deeply recursive JS (or native code recurses past JSC's soft stack check — JSC's check catches most JS recursion as
RangeError, but native frames under a JS call can blow through it). - Thread stack grows into the guard page → hardware fault → kernel raises
SIGSEGV. - Kernel looks up the disposition:
fuzzilliSignalHandler, flagsSA_SIGINFO(noSA_ONSTACK). - Kernel tries to push
siginfo_t+ucontext_t+ return trampoline onto the current (exhausted) stack → hits the guard page again. - Fault during signal delivery → kernel force-applies
SIG_DFL→ process killed withTERMSIG: 11. fuzzilliSignalHandlernever ran → no flush.jscSignalHandlernever ran.AsanOnDeadlySignalnever ran → no report.
Result: Fuzzilli logs a crash fingerprint with no output — indistinguishable from the bug class this PR is fixing.
Why this isn't a regression
The pre-PR code used signal(SIGSEGV, ...), which on glibc maps to sigaction with SA_RESTART and also lacks SA_ONSTACK. So stack-overflow crashes were equally undiagnosable before. This is a pre-existing limitation in code the PR is actively rewriting, not new breakage.
Fix
One token:
action.sa_flags = SA_SIGINFO | SA_ONSTACK;(Optionally also | SA_NODEFER to match ASAN, but SA_ONSTACK is the load-bearing one for this case.)
Given the PR's stated goal is "if this fingerprint re-fires it will now come with an ASAN stack", and stack overflow is a common fuzzer finding, it seems worth folding in here.
Bun__REPRL__registerFuzzilliFunctions runs from Zig__GlobalObject__create for every GlobalObject — main thread, macros, and every Worker. The first Worker re-ran installFuzzilliSignalHandler(), and sigaction() returned fuzzilliSignalHandler itself as the previous handler, overwriting fuzzilliOldActions[sig]. The next SIGSEGV then recursed into itself until the stack blew → bare TERMSIG 11. Guard with std::call_once. Also set SA_ONSTACK so stack-overflow faults are delivered on the sigaltstack ASAN/JSC already set up rather than the exhausted stack. Add a test that spawns a Worker before triggering FUZZILLI_CRASH 5 and verifies ASAN still reports the SEGV.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/FuzzilliREPRL.cpp`:
- Around line 54-64: The installFuzzilliSignalHandler function leaves its local
struct sigaction 'action' uninitialized and ignores the return value of
sigaction(), which can cause UB and silent failures; fix it by zero-initializing
the struct (e.g., aggregate-init or memset) before setting fields, ensure
sa_mask is initialized (you can keep sigfillset), set
sa_sigaction=fuzzilliSignalHandler and sa_flags=SA_SIGINFO|SA_ONSTACK as done
now, then call sigaction(sig, &action, &fuzzilliOldActions[sig]) and check its
return value, logging or handling errors on failure (consistent with other files
like c-bindings.cpp) so handler installation failures aren’t ignored.
In `@test/internal/fuzzilli-signal-chain.test.ts`:
- Around line 85-89: Reorder the assertions in the test so the subprocess signal
assertion runs before the exit-code check: move the line containing
expect(exitCode).not.toBe(0) to after the
expect(proc.signalCode).toBe("SIGABRT") call so the test asserts stdout, then
proc.signalCode, then exitCode (using the existing variables stdout, proc,
signalCode, exitCode).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f8c9b5f4-5dc0-4ace-be84-470ff2ba7001
📒 Files selected for processing (2)
src/bun.js/bindings/FuzzilliREPRL.cpptest/internal/fuzzilli-signal-chain.test.ts
| static void installFuzzilliSignalHandler(int sig) | ||
| { | ||
| struct sigaction action; | ||
| action.sa_sigaction = fuzzilliSignalHandler; | ||
| sigfillset(&action.sa_mask); | ||
| // SA_ONSTACK so stack-overflow faults are delivered on the sigaltstack | ||
| // ASAN/JSC already set up; otherwise the kernel can't push the frame and | ||
| // the process dies with a bare SIGSEGV before we reach the chain. | ||
| action.sa_flags = SA_SIGINFO | SA_ONSTACK; | ||
| sigaction(sig, &action, &fuzzilliOldActions[sig]); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and read the relevant section
head -70 src/bun.js/bindings/FuzzilliREPRL.cpp | tail -25Repository: oven-sh/bun
Length of output: 1030
🏁 Script executed:
# Search for sigaction usage in the codebase to see error handling patterns
rg "sigaction" --type cpp -B2 -A5Repository: oven-sh/bun
Length of output: 10135
🏁 Script executed:
# Check for similar error handling patterns with system calls in C++ files
rg "if.*!=.*0\)" src/bun.js/bindings --type cpp -B2 -A2 | head -40Repository: oven-sh/bun
Length of output: 2960
🏁 Script executed:
# Look for struct initialization patterns in the file
rg "struct.*=" src/bun.js/bindings/FuzzilliREPRL.cpp --type cppRepository: oven-sh/bun
Length of output: 153
Initialize struct sigaction and handle sigaction() errors.
The function leaves struct sigaction uninitialized and ignores sigaction() return values, creating two issues: uninitialized struct fields can cause undefined behavior when chained in fuzzilliSignalHandler, and silent failures disable handler installation. Other signal handling code in the codebase (c-bindings.cpp, SigintWatcher.cpp, BunProcess.cpp) consistently zero-initializes sigaction structs using either aggregate initialization or memset(), and c-bindings.cpp checks the sigaction() return value.
Proposed fix
static void installFuzzilliSignalHandler(int sig)
{
- struct sigaction action;
+ struct sigaction action = {};
action.sa_sigaction = fuzzilliSignalHandler;
sigfillset(&action.sa_mask);
// SA_ONSTACK so stack-overflow faults are delivered on the sigaltstack
// ASAN/JSC already set up; otherwise the kernel can't push the frame and
// the process dies with a bare SIGSEGV before we reach the chain.
action.sa_flags = SA_SIGINFO | SA_ONSTACK;
- sigaction(sig, &action, &fuzzilliOldActions[sig]);
+ if (sigaction(sig, &action, &fuzzilliOldActions[sig]) != 0) {
+ fprintf(stderr, "[fuzzilli] sigaction(%d) failed: %s\n", sig, strerror(errno));
+ std::abort();
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/bindings/FuzzilliREPRL.cpp` around lines 54 - 64, The
installFuzzilliSignalHandler function leaves its local struct sigaction 'action'
uninitialized and ignores the return value of sigaction(), which can cause UB
and silent failures; fix it by zero-initializing the struct (e.g.,
aggregate-init or memset) before setting fields, ensure sa_mask is initialized
(you can keep sigfillset), set sa_sigaction=fuzzilliSignalHandler and
sa_flags=SA_SIGINFO|SA_ONSTACK as done now, then call sigaction(sig, &action,
&fuzzilliOldActions[sig]) and check its return value, logging or handling errors
on failure (consistent with other files like c-bindings.cpp) so handler
installation failures aren’t ignored.
| const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stdout).toContain("FUZZILLI_CRASH: 0"); | ||
| expect(exitCode).not.toBe(0); | ||
| expect(proc.signalCode).toBe("SIGABRT"); |
There was a problem hiding this comment.
Make the exit-code assertion last in this subprocess test.
Please move expect(exitCode).not.toBe(0) after the signalCode assertion to match the repo’s failure-diagnostics style.
Proposed fix
expect(stdout).toContain("FUZZILLI_CRASH: 0");
- expect(exitCode).not.toBe(0);
expect(proc.signalCode).toBe("SIGABRT");
+ expect(exitCode).not.toBe(0);
});Based on learnings: in test/**/*.test.ts, when asserting subprocess results, keep the exit-code assertion last to preserve more useful failure output.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | |
| expect(stdout).toContain("FUZZILLI_CRASH: 0"); | |
| expect(exitCode).not.toBe(0); | |
| expect(proc.signalCode).toBe("SIGABRT"); | |
| const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | |
| expect(stdout).toContain("FUZZILLI_CRASH: 0"); | |
| expect(proc.signalCode).toBe("SIGABRT"); | |
| expect(exitCode).not.toBe(0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/internal/fuzzilli-signal-chain.test.ts` around lines 85 - 89, Reorder
the assertions in the test so the subprocess signal assertion runs before the
exit-code check: move the line containing expect(exitCode).not.toBe(0) to after
the expect(proc.signalCode).toBe("SIGABRT") call so the test asserts stdout,
then proc.signalCode, then exitCode (using the existing variables stdout, proc,
signalCode, exitCode).
|
Closing per the mechgate test requirement — this change is entirely inside The tests in and the Worker case (which guards against the once-flag regression caught in review) likewise fails without the fix. The fix is still worth landing: without it, every SIGSEGV the fuzzer hits is reported as a bare |
1 participant
Problem
Fuzzilli hit a flaky SIGSEGV (fingerprint
a488dd11600c00b1) from:The crash report is just
TERMSIG: 11with nothing on stderr — no ASAN output, no stack, unreproducible across thousands of direct and REPRL runs.The reason there's nothing to go on:
Bun__REPRL__registerFuzzilliFunctions()installs SIGSEGV/SIGILL/SIGFPE/SIGABRT handlers viasignal()that flush stdio and thensignal(sig, SIG_DFL); raise(sig). This runs after VM construction — i.e. afterConfig::finalize()has already installedWTF::jscSignalHandlerfor SIGSEGV/SIGBUS — so it overwrites JSC's handler (and, transitively, ASAN's).That has two effects in the REPRL process:
Every SIGSEGV is reported as a bare TERMSIG 11. ASAN's
AsanOnDeadlySignalis never reached, so null derefs / wild reads produce no report. This is why this fingerprint (and resolver: skip auto-install for invalid npm package names #29255's2519cad1804eace1) arrive with no stack.JSC's signal-based VMTraps and WASM fault handling are broken.
Options::usePollingTrapsdefaults tofalse, soVMTraps::SignalSenderinstalls trap breakpoints in JIT code that raise SIGSEGV on purpose;jscSignalHandlernormally catches it, jettisons the CodeBlock, and returns. With the old handler that SIGSEGV kills the process instead. Any earlier REPRL iteration that armed VMTraps (e.g.Worker#terminate()→notifyNeedTermination()→fireTrap) can make a later iteration die in unrelated JIT code — which matches "flaky, 609 ms, no output, involvesBun.gc(true)after auto-install spins the event loop".Fix
Save the previous
struct sigactionwithsigaction()and forward to it after flushing, falling back toSIG_DFL + raiseonly when there is no previous handler. The chain for SIGSEGV becomes:Verification
Before:
After:
All
FUZZILLI_CRASHmodes (0–7) still terminate the process, so Fuzzilli's crash detection is unchanged. 150 REPRL iterations of the original repro, plus 100 iterations interleavingnew Worker(...).terminate()(arms VMTraps) with the repro, run clean.(No regression test: this code is only compiled under
FUZZILLI_ENABLED, which isn't a CI target.)If this fingerprint re-fires after this lands it will now come with an ASAN stack.