From 48a16209b1a0de5efd8112ce6430415730008d18 Mon Sep 17 00:00:00 2001 From: juj Date: Sun, 6 Mar 2022 16:27:16 +0200 Subject: [PATCH] Fix Closure compile of small builds that use legacy DYNCALLS mode (#13883) * Fix Closure compile of small builds that use legacy DYNCALLS mode but do not export any wasm function with signature 'v' or 'vi'. * Address review * Update tests/test_other.py * Address review * Remove duplicate hasExportedFunction * Use hasExportedFunction() * Generalize closure optimization to makeDynCall macro * Clean up Closure warnings Co-authored-by: Alon Zakai --- src/library.js | 5 +++++ src/parseTools.js | 18 ++++++++++++++++++ tests/test_other.py | 8 ++++++++ 3 files changed, 31 insertions(+) diff --git a/src/library.js b/src/library.js index d9e5ba01f08c..e2529814cd0f 100644 --- a/src/library.js +++ b/src/library.js @@ -3276,8 +3276,13 @@ LibraryManager.library = { var func = callback.func; if (typeof func == 'number') { if (callback.arg === undefined) { + // Run the wasm function ptr with signature 'v'. If no function + // with such signature was exported, this call does not need + // to be emitted (and would confuse Closure) {{{ makeDynCall('v', 'func') }}}(); } else { + // If any function with signature 'vi' was exported, run + // the callback with that signature. {{{ makeDynCall('vi', 'func') }}}(callback.arg); } } else { diff --git a/src/parseTools.js b/src/parseTools.js index a7eeedf86999..c6b8ed57b0d4 100644 --- a/src/parseTools.js +++ b/src/parseTools.js @@ -832,6 +832,13 @@ New syntax is {{{ makeDynCall("${sig}", "funcPtr") }}}(arg1, arg2, ...). \ Please update to new syntax.`); if (DYNCALLS) { + if (!hasExportedFunction(`dynCall_${sig}`)) { + if (ASSERTIONS) { + return `(function(${args}) { throw 'Internal Error! Attempted to invoke wasm function pointer with signature "${sig}", but no such functions have gotten exported!'; })`; + } else { + return `(function(${args}) { /* a dynamic function call to signature ${sig}, but there are no exported function pointers with that signature, so this path should never be taken. Build with ASSERTIONS enabled to validate. */ })`; + } + } return `(function(cb, ${args}) { ${returnExpr} getDynCaller("${sig}", cb)(${args}) })`; } else { return `(function(cb, ${args}) { ${returnExpr} getWasmTableEntry(cb)(${args}) })`; @@ -839,6 +846,14 @@ Please update to new syntax.`); } if (DYNCALLS) { + if (!hasExportedFunction(`dynCall_${sig}`)) { + if (ASSERTIONS) { + return `(function(${args}) { throw 'Internal Error! Attempted to invoke wasm function pointer with signature "${sig}", but no such functions have gotten exported!'; })`; + } else { + return `(function(${args}) { /* a dynamic function call to signature ${sig}, but there are no exported function pointers with that signature, so this path should never be taken. Build with ASSERTIONS enabled to validate. */ })`; + } + } + const dyncall = exportedAsmFunc(`dynCall_${sig}`); if (sig.length > 1) { return `(function(${args}) { ${returnExpr} ${dyncall}.apply(null, [${funcPtr}, ${args}]); })`; @@ -1058,6 +1073,9 @@ function _asmjsDemangle(symbol) { if (symbol in WASM_SYSTEM_EXPORTS) { return symbol; } + if (symbol.startsWith('dynCall_')) { + return symbol; + } // Strip leading "_" assert(symbol.startsWith('_')); return symbol.substr(1); diff --git a/tests/test_other.py b/tests/test_other.py index 4503f0a6462f..ada6c6cf6e77 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -10815,6 +10815,14 @@ def test_old_makeDynCall_syntax(self): err = self.run_process([EMCC, test_file('test_old_dyncall_format.c'), '--js-library', test_file('library_test_old_dyncall_format.js')], stderr=PIPE).stderr self.assertContained('syntax for makeDynCall has changed', err) + # Tests that dynCalls are produced in Closure-safe way in DYNCALLS mode when no actual dynCalls are used + @parameterized({ + 'plain': [[]], + 'asyncify': [['-sASYNCIFY']], + 'asyncify_bigint': [['-sASYNCIFY', '-sWASM_BIGINT']]}) + def test_closure_safe(self, args): + self.run_process([EMCC, test_file('hello_world.c'), '--closure=1'] + args) + def test_post_link(self): err = self.run_process([EMCC, test_file('hello_world.c'), '--oformat=bare', '-o', 'bare.wasm'], stderr=PIPE).stderr self.assertContained('--oformat=bare/--post-link are experimental and subject to change', err)