Skip to content

Commit 6e2d3d2

Browse files
committed
Use RuntimeError in abort() before module instantiation
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.
1 parent ad65236 commit 6e2d3d2

File tree

1 file changed

+28
-11
lines changed

1 file changed

+28
-11
lines changed

src/preamble.js

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -469,20 +469,37 @@ function abort(what) {
469469
// defintion for WebAssembly.RuntimeError claims it takes no arguments even
470470
// though it can.
471471
// TODO(https://github.com/google/closure-compiler/pull/3913): Remove if/when upstream closure gets fixed.
472-
#if WASM_EXCEPTIONS == 1
473-
// See above, in the meantime, we resort to wasm code for trapping.
474-
___trap();
475-
#else
476-
/** @suppress {checkTypes} */
477-
var e = new WebAssembly.RuntimeError(what);
472+
473+
function throwError(what) {
474+
/** @suppress {checkTypes} */
475+
var e = new WebAssembly.RuntimeError(what);
478476

479477
#if MODULARIZE
480-
readyPromiseReject(e);
478+
readyPromiseReject(e);
481479
#endif
482-
// Throw the error whether or not MODULARIZE is set because abort is used
483-
// in code paths apart from instantiation where an exception is expected
484-
// to be thrown when abort is called.
485-
throw e;
480+
// Throw the error whether or not MODULARIZE is set because abort is used
481+
// in code paths apart from instantiation where an exception is expected
482+
// to be thrown when abort is called.
483+
throw e;
484+
}
485+
486+
#if WASM_EXCEPTIONS == 1
487+
// See above, in the meantime, we resort to wasm code for trapping.
488+
//
489+
// In case abort() is called before the module is initialized, Module['asm']
490+
// and its exported '__trap' function is not available, in which case we throw
491+
// a RuntimeError.
492+
//
493+
// We trap instead of throwing RuntimeError to prevent infinite-looping in
494+
// Wasm EH code (because RuntimeError is considered as a foreign exception and
495+
// caught by 'catch_all'), but in case throwing RuntimeError is fine because
496+
// the module has not even been instantiated, even less running.
497+
if (typeof Module['asm'] !== 'undefined')
498+
___trap();
499+
else
500+
throwError(what);
501+
#else
502+
throwError(what);
486503
#endif
487504
}
488505

0 commit comments

Comments
 (0)