fix(wasi): the setjmp macro selection omitted __wasi__ - #1728
Merged
Conversation
aether_panic.c guards its sigaction crash handler with
!defined(__wasi__). aether_panic.h, which selects the setjmp/longjmp
macro pair, did not. WASI is hosted (__STDC_HOSTED__ == 1) and does not
define __EMSCRIPTEN__, so it landed in the POSIX arm and got
_setjmp/_longjmp -- which wasi-libc declares but never implements.
That is a LINK error, not a compile error, so it surfaced only at the
very end of a cross build:
wasm-ld: error: libaether.a(aether_panic.o): undefined symbol: _longjmp
and only for code that actually reaches the panic machinery. A wasi
library with no try/catch/panic never references aether_panic.o and
links fine, which is why this went unnoticed until a downstream tried
--emit=lib on real code.
Neither obvious fix works. Plain setjmp/longjmp is a hard #error in
wasi-libc directing you to `-mllvm -wasm-enable-sjlj` plus an engine
implementing the exception-handling proposal; measured on zig 0.16.0
that flag does not help, because the #error fires before the pass runs.
Real support needs the WebAssembly exception-handling proposal in both
toolchain and engine.
So the wasi arm does not unwind: SIGSETJMP always takes the
first-return arm and SIGLONGJMP calls abort().
THE CONSEQUENCE, PLAINLY: on wasi, panic/try/catch are fail-stop rather
than recoverable. A panic traps the instance instead of unwinding to the
nearest catch, and the catch never runs. That is a real semantic
reduction, documented in docs/build-system.md beside the target's other
caveats. It is still strictly better than the alternative, which is that
wasi cannot link at all -- abort() is a trap the host observes, not a
fallthrough into a half-unwound stack.
Verified end to end rather than by linking alone: the module instantiates
under node's WASI, aether_risky(21) and aether_safe(21) both return 42,
and aether_safe(-1) traps with a RuntimeError after printing the panic
reason. Native try/catch still recovers normally; make test 394/0.
The regression test uses try/catch/panic deliberately, since a fixture
without them passes even with the selection wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From an ask filed by the html-sanitizer downstream. Validated first — the report is accurate in every particular I could check, including the two claims that would have made a simpler fix possible.
The bug
aether_panic.cguards its sigaction crash handler with!defined(__wasi__).aether_panic.h, which selects the setjmp/longjmp macro pair, does not. WASI is hosted (__STDC_HOSTED__ == 1) and does not define__EMSCRIPTEN__, so it lands in the POSIX arm and gets_setjmp/_longjmp— which wasi-libc declares but never implements.Reproduced on
main@ad413dbb:Why it went unnoticed: it is a link error, and only for code that reaches the panic machinery. A wasi library with no
try/catch/panicnever referencesaether_panic.oand links fine — I confirmed that first, which is what sent me looking for a fixture that actually drags it in.Why the obvious fix does not work — checked, not assumed
The report claims the plain
setjmp/longjmparm is no better. Verified independently with a five-line C file andzig cc -target wasm32-wasi:It is a hard
#error, not a link failure. And passing-mllvm -wasm-enable-sjljas the message suggests does not help on zig 0.16.0 — the#errorfires before the pass ever runs. So there is no workingsetjmpon wasm32-wasi in either spelling, and a non-unwinding arm is the only option short of the WebAssembly exception-handling proposal landing in both toolchain and engine.The fix
A dedicated
__wasi__arm that does not unwind:AETHER_SIGSETJMPalways takes the first-return path,AETHER_SIGLONGJMPcallsabort(). Plus<stdlib.h>forabort().The consequence, plainly: on wasi,
panic/try/catchare fail-stop rather than recoverable. A panic traps the instance instead of unwinding to the nearestcatch, and the catch never runs. That is a real semantic reduction, now documented indocs/build-system.mdas a fourth entry in its existing "places where WASI had been forgotten beside Emscripten" list, with its own subsection spelling out the consequence for downstream code.It is still strictly better than the alternative, which is that wasi cannot link at all. Nothing silently mis-executes:
abort()is a trap the host observes, not a fallthrough into a half-unwound stack. A catch block that never runs is visible; one that runs on a corrupt stack is not.Verified behaviourally, not just by linking
The module instantiates under node's WASI and executes correctly:
Note the message says "outside any try/catch" even though there is one — because the frame never registers. That is the documented reduction being visible rather than silent.
Native builds are unaffected:
try/catchstill recovers (caught: negative, thenrisky(21) = 42), andmake testis 394 passed / 0 failed.The regression test
tests/integration/wasi_panic_link/— and its fixture deliberately usestry/catch/panic, because one without them passes even with the selection wrong. I verified the test earns its place by reverting the fix: it fails with the original link error and prints an actionable diagnosis pointing at the header. Added totests/ae_sweep_prune.txt(the fixture has nomain), and I confirmed the prune actually excludes it rather than assuming.Cost-capped at one cross link, matching the convention in the neighbouring
cross_emit_libtest.Not in scope
The ask also raises artifact size — 2.0 MB native vs 85 KB from a hand-rolled
zig ccscript, of which 82% is DWARF — and proposes either a--release/--sizemode or--cflags=/--ldflags=passthrough. That is genuinely independent (the setjmp fix alone makes the target usable; the size work makes it preferable), it is a design decision about the CLI surface rather than a bug, and it deserves its own issue and its own discussion. Deliberately not bundled here.🤖 Generated with Claude Code