Skip to content

Commit

Permalink
Use RuntimeError in abort() before module instantiation
Browse files Browse the repository at this point in the history
We decided use a trap for `abort` function in case of Wasm EH in order
to prevent infinite-looping (emscripten-core#16910).
https://github.com/emscripten-core/emscripten/blob/44b2c2a1ecad39c534e14179acb49419dfee528b/src/preamble.js#L472-L474

The short reason is, in Wasm EH `RuntimeError` is treated as a foreign
exception and caught by `catch_all`, so you try to abort the program and
throw a `RuntimeError`, but it is caught by some `catch_all` used in the
cleanup (=destructors, etc) code, causing the program to continue to
run.

`__trap` is defined in compiler-rt and exported when Wasm EH is enabled.

This has worked so far, but in case we fail to instantiate a Wasm
module and call `abort` because of it, we don't have access to the
imported `Module['asm']['__trap']`, because `Module['asm']` has not been
set:
https://github.com/emscripten-core/emscripten/blob/44b2c2a1ecad39c534e14179acb49419dfee528b/src/preamble.js#L848

So the `abort` call will ends like this:
```console
TypeError: Cannot read properties of undefined (reading '__trap')
    at ___trap (/usr/local/google/home/aheejin/test/gl84/exported_api.js:5152:34)
    at abort (/usr/local/google/home/aheejin/test/gl84/exported_api.js:892:5)
    at /usr/local/google/home/aheejin/test/gl84/exported_api.js:1172:5
```
which may not be the worst thing in the world given that we are crashing
anyway, but not the situation we intended.

This PR lets us throw `RuntimeError` in case we don't have the wasm
module instantiated and have access to `Module['asm']['__trap']`. This
is OK even with Wasm EH because we haven't been running the module,
there's no risk of infinite-looping we tried to prevent in emscripten-core#16910.

I'm not sure how to add a test for this, because the test would require
a module that fails to instantiate. This was found while I was debugging
something else.
  • Loading branch information
aheejin committed Feb 4, 2023
1 parent ad65236 commit 6e2d3d2
Showing 1 changed file with 28 additions and 11 deletions.
39 changes: 28 additions & 11 deletions src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -469,20 +469,37 @@ function abort(what) {
// defintion for WebAssembly.RuntimeError claims it takes no arguments even
// though it can.
// TODO(https://github.com/google/closure-compiler/pull/3913): Remove if/when upstream closure gets fixed.
#if WASM_EXCEPTIONS == 1
// See above, in the meantime, we resort to wasm code for trapping.
___trap();
#else
/** @suppress {checkTypes} */
var e = new WebAssembly.RuntimeError(what);
function throwError(what) {
/** @suppress {checkTypes} */
var e = new WebAssembly.RuntimeError(what);
#if MODULARIZE
readyPromiseReject(e);
readyPromiseReject(e);
#endif
// Throw the error whether or not MODULARIZE is set because abort is used
// in code paths apart from instantiation where an exception is expected
// to be thrown when abort is called.
throw e;
// Throw the error whether or not MODULARIZE is set because abort is used
// in code paths apart from instantiation where an exception is expected
// to be thrown when abort is called.
throw e;
}
#if WASM_EXCEPTIONS == 1
// See above, in the meantime, we resort to wasm code for trapping.
//
// In case abort() is called before the module is initialized, Module['asm']
// and its exported '__trap' function is not available, in which case we throw
// a RuntimeError.
//
// We trap instead of throwing RuntimeError to prevent infinite-looping in
// Wasm EH code (because RuntimeError is considered as a foreign exception and
// caught by 'catch_all'), but in case throwing RuntimeError is fine because
// the module has not even been instantiated, even less running.
if (typeof Module['asm'] !== 'undefined')
___trap();
else
throwError(what);
#else
throwError(what);
#endif
}
Expand Down

0 comments on commit 6e2d3d2

Please sign in to comment.