Use RuntimeError in abort() before module instantiation

[aheejin]

We decided use a trap for abort function in case of Wasm EH in order to prevent infinite-looping (#16910).

emscripten/src/preamble.js

Lines 472 to 474 in 44b2c2a

	#if WASM_EXCEPTIONS == 1
	// See above, in the meantime, we resort to wasm code for trapping.
	___trap();

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:

emscripten/src/preamble.js

Line 848 in 44b2c2a

abort(reason);

And we haven't had the opportunity to run this code:

emscripten/src/preamble.js

Line 975 in 44b2c2a

Module['asm'] = exports;

So the abort call will end like this:

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 #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.

[kripken]

Can this check if (runtimeInitialized) ? I think that should work but I'm not sure. It's generally the right condition to check before calling into the wasm, like ___trap() here, but maybe this place is special somehow.

[aheejin]

if (runtimeInitialized) seems to work, thanks. Didn't know that variable exited.

[sbc100]

Do you know where the offending call to abort is coming from exactly? I'm happy to land this fix but it also seems wrong to be a calling abort if the program isn't even running yet. I'd like to look into fixing that too.

[kripken]

lgtm but I'm also curious where the problem happens. Is it in a global ctor or other startup code?

[kripken]

Oh, I think it can't be the wasm module at all, because then runtimeInitialized would be true..?

Then I think it has to be what you said before @aheejin , that it errors during a failure to instantiate the wasm in the JS code. @sbc100 I think such a call to abort() is reasonable? I'm not sure what we'd replace it with.

[aheejin]

The case I was debugging was this: #18592 (comment)

And the callsite involved in this case was here, in instantiateArrayBuffer (click to see the code around the line):

emscripten/src/preamble.js

Line 848 in 30ba0ae

abort(reason);

Actually, it is not the initialization itself that fails, even though this can happen when intitialization fails too. What fails in this example is this line:

emscripten/src/preamble.js

Line 818 in 30ba0ae

wasmOffsetConverter = new WasmOffsetConverter(savedBinary, instance.module);

which transfers the control flow here:

emscripten/src/preamble.js

Lines 821 to 849 in 30ba0ae

	}).then(receiver, function(reason) {
	err('failed to asynchronously prepare wasm: ' + reason);
	#if WASM == 2
	#if ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL
	if (typeof location != 'undefined') {
	#endif
	// WebAssembly compilation failed, try running the JS fallback instead.
	var search = location.search;
	if (search.indexOf('_rwasm=0') < 0) {
	location.href += (search ? search + '&' : '?') + '_rwasm=0';
	// Return here to avoid calling abort() below. The application
	// still has a chance to start sucessfully do we don't want to
	// trigger onAbort or onExit handlers.
	return;
	}
	#if ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL
	}
	#endif
	#endif // WASM == 2
	#if ASSERTIONS
	// Warn on some common problems.
	if (isFileURI(wasmBinaryFile)) {
	err('warning: Loading from a file URI (' + wasmBinaryFile + ') is not supported in most browsers. See https://emscripten.org/docs/getting_started/FAQ.html#how-do-i-run-a-local-webserver-for-testing-why-does-my-program-stall-in-downloading-or-preparing');
	}
	#endif
	abort(reason);
	});

which ends with the abort call.

[kripken]

Makes sense, thanks for the details @aheejin