Skip to content

Commit

Permalink
Fix Closure compile of small builds that use legacy DYNCALLS mode (#1…
Browse files Browse the repository at this point in the history
…3883)

* 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 <alonzakai@gmail.com>
  • Loading branch information
juj and kripken committed Mar 6, 2022
1 parent a2d0b4e commit 48a1620
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions src/parseTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -832,13 +832,28 @@ 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}) })`;
}
}

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}]); })`;
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 48a1620

Please sign in to comment.