From 7e3e35cbff9c5688eacb3cddc5045f872d744efd Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Sat, 8 Feb 2025 08:44:41 -0800 Subject: [PATCH] Detect and report late assignment of Module API elements We had some bug reports recently when we switched to using async/await in MODULUARIZE mode. One of the side effects was the `--post-js` code then runs after module instantiation and startup, which means assignment of properties like `onRuntimeInitialized` is too late if it happens in `--post-js`. This was always the case for sync instantiation too. We now detect this too-late-assignment at abort with a useful error. See #23626 and #23420 --- src/postamble.js | 6 ++++++ src/preamble.js | 6 ++++++ src/runtime_debug.js | 12 ++++++++++++ test/other/codesize/test_codesize_hello_O0.gzsize | 2 +- test/other/codesize/test_codesize_hello_O0.jssize | 2 +- test/other/test_unoptimized_code_size.js.size | 2 +- test/other/test_unoptimized_code_size_strict.js.size | 2 +- test/test_other.py | 9 +++++++++ 8 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/postamble.js b/src/postamble.js index 64608c8253cdf..fdda0014035f2 100644 --- a/src/postamble.js +++ b/src/postamble.js @@ -199,6 +199,9 @@ function run() { #endif #if expectToReceiveOnModule('onRuntimeInitialized') Module['onRuntimeInitialized']?.(); +#if ASSERTIONS + consumedModuleProp('onRuntimeInitialized'); +#endif #endif #if HAS_MAIN @@ -297,6 +300,9 @@ if (Module['preInit']) { Module['preInit'].pop()(); } } +#if ASSERTIONS +consumedModuleProp('preInit'); +#endif #endif run(); diff --git a/src/preamble.js b/src/preamble.js index 98e0af7358d03..c1a20b16515dc 100644 --- a/src/preamble.js +++ b/src/preamble.js @@ -181,6 +181,9 @@ function preRun() { addOnPreRun(Module['preRun'].shift()); } } +#if ASSERTIONS + consumedModuleProp('preRun'); +#endif #endif <<< ATPRERUNS >>> } @@ -276,6 +279,9 @@ function postRun() { addOnPostRun(Module['postRun'].shift()); } } +#if ASSERTIONS + consumedModuleProp('postRun'); +#endif #endif <<< ATPOSTRUNS >>> diff --git a/src/runtime_debug.js b/src/runtime_debug.js index 2cc1d464e8ea3..9e576295c85cb 100644 --- a/src/runtime_debug.js +++ b/src/runtime_debug.js @@ -33,6 +33,18 @@ function legacyModuleProp(prop, newName, incoming=true) { } } +function consumedModuleProp(prop) { + if (!Object.getOwnPropertyDescriptor(Module, prop)) { + Object.defineProperty(Module, prop, { + configurable: true, + set() { + abort(`Attempt to set \`Module.${prop}\` after it has already been processed. This can happen, for example, when code is injected via '--post-js' rather than '--pre-js'`); + + } + }); + } +} + function ignoredModuleProp(prop) { if (Object.getOwnPropertyDescriptor(Module, prop)) { abort(`\`Module.${prop}\` was supplied but \`${prop}\` not included in INCOMING_MODULE_JS_API`); diff --git a/test/other/codesize/test_codesize_hello_O0.gzsize b/test/other/codesize/test_codesize_hello_O0.gzsize index d66c3d33e6707..19a67a42b75e8 100644 --- a/test/other/codesize/test_codesize_hello_O0.gzsize +++ b/test/other/codesize/test_codesize_hello_O0.gzsize @@ -1 +1 @@ -7872 +7985 diff --git a/test/other/codesize/test_codesize_hello_O0.jssize b/test/other/codesize/test_codesize_hello_O0.jssize index 7fc6c64150ded..b4b134aa9fc2b 100644 --- a/test/other/codesize/test_codesize_hello_O0.jssize +++ b/test/other/codesize/test_codesize_hello_O0.jssize @@ -1 +1 @@ -20962 +21341 diff --git a/test/other/test_unoptimized_code_size.js.size b/test/other/test_unoptimized_code_size.js.size index 433572758b27c..c9f6039805b13 100644 --- a/test/other/test_unoptimized_code_size.js.size +++ b/test/other/test_unoptimized_code_size.js.size @@ -1 +1 @@ -51969 +52492 diff --git a/test/other/test_unoptimized_code_size_strict.js.size b/test/other/test_unoptimized_code_size_strict.js.size index defea51f0836d..683d54fd87150 100644 --- a/test/other/test_unoptimized_code_size_strict.js.size +++ b/test/other/test_unoptimized_code_size_strict.js.size @@ -1 +1 @@ -50401 +50780 diff --git a/test/test_other.py b/test/test_other.py index 5e36ca8dc4254..56a419a9179e2 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15607,3 +15607,12 @@ def test_instantiate_wasm(self): return {}; // Compiling asynchronously, no exports. }''') self.do_runf('test_manual_wasm_instantiate.c', emcc_args=['--pre-js=pre.js']) + + def test_late_module_api_assignment(self): + # When sync instantiation is used (or when async/await is used in MODULARIZE mode) certain + # Module properties cannot be assigned in `--post-js` code because its too late by the time + # it runs. + for prop in ('onRuntimeInitialized', 'postRun', 'preRun', 'preInit'): + create_file('post.js', f'Module.{prop} = () => console.log("will never fire since assigned too late")') + expected = f"Aborted(Attempt to set `Module.{prop}` after it has already been processed. This can happen, for example, when code is injected via '--post-js' rather than '--pre-js')" + self.do_runf(test_file('hello_world.c'), expected, emcc_args=['--post-js=post.js', '-sWASM_ASYNC_COMPILATION=0'], assert_returncode=NON_ZERO)