Skip to content

Commit 3563c51

Browse files
committed
Fix dynCall API after regression from PR emscripten-core#12059. Fixes issue emscripten-core#12733.
1 parent ee483fc commit 3563c51

10 files changed

+522
-60
lines changed

emcc.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,9 @@ def default_setting(name, new_default):
12821282
if shared.Settings.CLOSURE_WARNINGS not in ['quiet', 'warn', 'error']:
12831283
exit_with_error('Invalid option -s CLOSURE_WARNINGS=%s specified! Allowed values are "quiet", "warn" or "error".' % shared.Settings.CLOSURE_WARNINGS)
12841284

1285+
# Calling function pointers from JS libraries is default runtime functionality, so always include the functionality. (to be DCEd if not used)
1286+
shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$dynCall', '$bindDynCall', '$bindDynCallArray']
1287+
12851288
if shared.Settings.MAIN_MODULE:
12861289
assert not shared.Settings.SIDE_MODULE
12871290
if shared.Settings.MAIN_MODULE == 1:
@@ -2420,11 +2423,13 @@ def consume_arg_file():
24202423
options.llvm_opts = ['-Os']
24212424
options.requested_level = 2
24222425
shared.Settings.SHRINK_LEVEL = 1
2426+
shared.Settings.USE_LEGACY_DYNCALLS = 0 # In -Os builds, use a more size compact, but slower 'wasmTable.get()' method of accessing function pointers
24232427
settings_changes.append('INLINING_LIMIT=50')
24242428
elif options.requested_level == 'z':
24252429
options.llvm_opts = ['-Oz']
24262430
options.requested_level = 2
24272431
shared.Settings.SHRINK_LEVEL = 2
2432+
shared.Settings.USE_LEGACY_DYNCALLS = 0 # In -Oz builds, use a more size compact, but slower 'wasmTable.get()' method of accessing function pointers
24282433
settings_changes.append('INLINING_LIMIT=25')
24292434
shared.Settings.OPT_LEVEL = validate_arg_level(options.requested_level, 3, 'Invalid optimization level: ' + arg, clamp=True)
24302435
elif check_arg('--js-opts'):

emscripten.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,43 @@
3737
WASM_INIT_FUNC = '__wasm_call_ctors'
3838

3939

40+
def make_wasm_table_static_dyncaller(func):
41+
sig = func.replace('dynCall_', '')
42+
ret = '' if sig[0] == 'v' else 'return '
43+
args = '' # 'a0, a1, a2, ..., aN'
44+
ptr_args = '' # 'ptr, a0, a1, a2, ..., aN'
45+
i = 0
46+
while i < len(sig)-1:
47+
if i > 0: args += ', '
48+
args += 'a' + str(i)
49+
i += 1
50+
ptr_args = ('ptr, ' + args) if len(args) > 0 else 'ptr'
51+
52+
return 'function ' + func + '(' + ptr_args + ') { ' + ret + 'wasmTable.get(ptr)(' + args + '); }'
53+
54+
4055
def compute_minimal_runtime_initializer_and_exports(post, exports, receiving):
4156
# Declare all exports out to global JS scope so that JS library functions can access them in a
4257
# way that minifies well with Closure
4358
# e.g. var a,b,c,d,e,f;
4459
exports_that_are_not_initializers = [x for x in exports if x not in WASM_INIT_FUNC]
4560
# In Wasm backend the exports are still unmangled at this point, so mangle the names here
4661
exports_that_are_not_initializers = [asmjs_mangle(x) for x in exports_that_are_not_initializers]
62+
63+
static_dyncall_sig_functions = ''
64+
65+
if shared.Settings.USE_LEGACY_DYNCALLS:
66+
if len([x for x in exports_that_are_not_initializers if x.startswith('dynCall_')]) > 0:
67+
exports_that_are_not_initializers += ['dynCalls = {}']
68+
else:
69+
for x in exports_that_are_not_initializers:
70+
if x.startswith('dynCall_'):
71+
static_dyncall_sig_functions += make_wasm_table_static_dyncaller(x) + '\n'
72+
73+
exports_that_are_not_initializers = [x for x in exports_that_are_not_initializers if not x.startswith('dynCall_')]
74+
4775
post = post.replace('/*** ASM_MODULE_EXPORTS_DECLARES ***/', 'var ' + ',\n '.join(exports_that_are_not_initializers) + ';')
76+
post = post.replace('/*** STATIC_DYNCALL_SIG_FUNCTIONS ***/', static_dyncall_sig_functions)
4877

4978
# Generate assignments from all asm.js/wasm exports out to the JS variables above: e.g. a = asm['a']; b = asm['b'];
5079
post = post.replace('/*** ASM_MODULE_EXPORTS ***/', receiving)
@@ -401,7 +430,7 @@ def finalize_wasm(infile, outfile, memfile, DEBUG):
401430
args.append('-g')
402431
if shared.Settings.WASM_BIGINT:
403432
args.append('--bigint')
404-
if shared.Settings.USE_LEGACY_DYNCALLS:
433+
if True:##shared.Settings.USE_LEGACY_DYNCALLS:
405434
# we need to add all dyncalls to the wasm
406435
modify_wasm = True
407436
else:
@@ -697,6 +726,8 @@ def create_receiving(exports):
697726
return ''
698727

699728
exports_that_are_not_initializers = [x for x in exports if x != WASM_INIT_FUNC]
729+
if not shared.Settings.USE_LEGACY_DYNCALLS:
730+
exports_that_are_not_initializers = [x for x in exports_that_are_not_initializers if not x.startswith('dynCall_')]
700731

701732
receiving = []
702733

@@ -710,7 +741,10 @@ def create_receiving(exports):
710741
# WebAssembly.instantiate(Module["wasm"], imports).then((function(output) {
711742
# var asm = output.instance.exports;
712743
# _main = asm["_main"];
713-
receiving += [asmjs_mangle(s) + ' = asm["' + s + '"];' for s in exports_that_are_not_initializers]
744+
for s in exports_that_are_not_initializers:
745+
mangled = asmjs_mangle(s)
746+
dynCallAssignment = ('dynCalls["' + s.replace('dynCall_', '') + '"] = ') if shared.Settings.USE_LEGACY_DYNCALLS and mangled.startswith('dynCall_') else ''
747+
receiving += [dynCallAssignment + mangled + ' = asm["' + s + '"];']
714748
else:
715749
if shared.Settings.MINIMAL_RUNTIME:
716750
# In wasm2js exports can be directly processed at top level, i.e.

src/library.js

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3696,62 +3696,6 @@ LibraryManager.library = {
36963696
});
36973697
},
36983698

3699-
#if USE_LEGACY_DYNCALLS || !WASM_BIGINT
3700-
$dynCallLegacy: function(sig, ptr, args) {
3701-
#if ASSERTIONS
3702-
assert(('dynCall_' + sig) in Module, 'bad function pointer type - no table for sig \'' + sig + '\'');
3703-
if (args && args.length) {
3704-
// j (64-bit integer) must be passed in as two numbers [low 32, high 32].
3705-
assert(args.length === sig.substring(1).replace(/j/g, '--').length);
3706-
} else {
3707-
assert(sig.length == 1);
3708-
}
3709-
#endif
3710-
if (args && args.length) {
3711-
return Module['dynCall_' + sig].apply(null, [ptr].concat(args));
3712-
}
3713-
return Module['dynCall_' + sig].call(null, ptr);
3714-
},
3715-
$dynCall__deps: ['$dynCallLegacy'],
3716-
3717-
// Used in library code to get JS function from wasm function pointer.
3718-
// All callers should use direct table access where possible and only fall
3719-
// back to this function if needed.
3720-
$getDynCaller__deps: ['$dynCall'],
3721-
$getDynCaller: function(sig, ptr) {
3722-
#if !USE_LEGACY_DYNCALLS
3723-
assert(sig.indexOf('j') >= 0, 'getDynCaller should only be called with i64 sigs')
3724-
#endif
3725-
var argCache = [];
3726-
return function() {
3727-
argCache.length = arguments.length;
3728-
for (var i = 0; i < arguments.length; i++) {
3729-
argCache[i] = arguments[i];
3730-
}
3731-
return dynCall(sig, ptr, argCache);
3732-
};
3733-
},
3734-
#endif
3735-
3736-
$dynCall: function(sig, ptr, args) {
3737-
#if USE_LEGACY_DYNCALLS
3738-
return dynCallLegacy(sig, ptr, args);
3739-
#else
3740-
#if !WASM_BIGINT
3741-
// Without WASM_BIGINT support we cannot directly call function with i64 as
3742-
// part of thier signature, so we rely the dynCall functions generated by
3743-
// wasm-emscripten-finalize
3744-
if (sig.indexOf('j') != -1) {
3745-
return dynCallLegacy(sig, ptr, args);
3746-
}
3747-
#endif
3748-
#if ASSERTIONS
3749-
assert(wasmTable.get(ptr), 'missing table entry in dynCall: ' + ptr);
3750-
#endif
3751-
return wasmTable.get(ptr).apply(null, args)
3752-
#endif
3753-
},
3754-
37553699
$callRuntimeCallbacks: function(callbacks) {
37563700
while(callbacks.length > 0) {
37573701
var callback = callbacks.shift();

src/library_dyncall.js

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
mergeInto(LibraryManager.library, {
2+
$bindDynCall: function(funcPtr) {
3+
return wasmTable.get(funcPtr);
4+
},
5+
6+
$bindDynCallArray: function(funcPtr) {
7+
var func = wasmTable.get(funcPtr);
8+
return func.length ? function(args) {
9+
return func.apply(null, args);
10+
} : function() { return func(); }
11+
},
12+
13+
#if USE_LEGACY_DYNCALLS || !WASM_BIGINT
14+
$dynCallLegacy: function(sig, ptr, args) {
15+
#if ASSERTIONS
16+
assert(('dynCall_' + sig) in Module, 'bad function pointer type - no table for sig \'' + sig + '\'');
17+
if (args && args.length) {
18+
// j (64-bit integer) must be passed in as two numbers [low 32, high 32].
19+
assert(args.length === sig.substring(1).replace(/j/g, '--').length);
20+
} else {
21+
assert(sig.length == 1);
22+
}
23+
#endif
24+
if (args && args.length) {
25+
return Module['dynCall_' + sig].apply(null, [ptr].concat(args));
26+
}
27+
return Module['dynCall_' + sig].call(null, ptr);
28+
},
29+
30+
// Used in library code to get JS function from wasm function pointer.
31+
// All callers should use direct table access where possible and only fall
32+
// back to this function if needed.
33+
$getDynCaller__deps: ['$dynCall'],
34+
$getDynCaller: function(sig, ptr) {
35+
#if !USE_LEGACY_DYNCALLS
36+
assert(sig.indexOf('j') >= 0, 'getDynCaller should only be called with i64 sigs')
37+
#endif
38+
var argCache = [];
39+
return function() {
40+
argCache.length = arguments.length;
41+
for (var i = 0; i < arguments.length; i++) {
42+
argCache[i] = arguments[i];
43+
}
44+
return dynCall(sig, ptr, argCache);
45+
};
46+
},
47+
#endif
48+
49+
// $dynCall__deps: ['$dynCallLegacy'],
50+
$dynCall: function(sig, ptr, args) {
51+
#if USE_LEGACY_DYNCALLS
52+
#if MINIMAL_RUNTIME
53+
var func = dynCalls[sig];
54+
#else
55+
var func = Module['dynCall_'+sig];
56+
#endif
57+
return args ? func.apply(null, [ptr].concat(args)) : func(ptr);
58+
#else
59+
#if !WASM_BIGINT
60+
// Without WASM_BIGINT support we cannot directly call function with i64 as
61+
// part of thier signature, so we rely the dynCall functions generated by
62+
// wasm-emscripten-finalize
63+
if (sig.indexOf('j') != -1) {
64+
#if MINIMAL_RUNTIME
65+
var func = dynCalls[sig];
66+
#else
67+
var func = Module['dynCall_'+sig];
68+
#endif
69+
return args ? func.apply(null, [ptr].concat(args)) : func(ptr);
70+
}
71+
#endif
72+
#if ASSERTIONS
73+
assert(wasmTable.get(ptr), 'missing table entry in dynCall: ' + ptr);
74+
#endif
75+
return wasmTable.get(ptr).apply(null, args)
76+
#endif
77+
}
78+
});

src/modules.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ var LibraryManager = {
6262
// Core system libraries (always linked against)
6363
var libraries = [
6464
'library.js',
65+
'library_dyncall.js',
6566
'library_stack.js',
6667
'library_formatString.js',
6768
'library_math.js',

src/postamble_minimal.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ function loadWasmModuleToWorkers() {
123123
/*** ASM_MODULE_EXPORTS_DECLARES ***/
124124
#endif
125125

126+
/*** STATIC_DYNCALL_SIG_FUNCTIONS ***/
127+
128+
126129
#if MINIMAL_RUNTIME_STREAMING_WASM_INSTANTIATION
127130
// https://caniuse.com/#feat=wasm and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/instantiateStreaming
128131
// Firefox 52 added Wasm support, but only Firefox 58 added instantiateStreaming.
@@ -222,7 +225,7 @@ WebAssembly.instantiate(Module['wasm'], imports).then(function(output) {
222225
initRuntime(asm);
223226
#if USE_PTHREADS && PTHREAD_POOL_SIZE
224227
if (!ENVIRONMENT_IS_PTHREAD) loadWasmModuleToWorkers();
225-
#if !PTHREAD_POOL_DELAY_LOAD
228+
#if !PTHREAD_POOL_DELAY_LOAD
226229
else
227230
#endif
228231
ready();

src/settings_internal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ var EXPECT_MAIN = 1;
175175
// MODULARIZE, and returned from the factory function.
176176
var EXPORT_READY_PROMISE = 1;
177177

178-
var USE_LEGACY_DYNCALLS = 0;
178+
var USE_LEGACY_DYNCALLS = 1;
179179

180180
// struct_info that is either generated or cached
181181
var STRUCT_INFO = '';

0 commit comments

Comments
 (0)