Restore the dynCall() and dynCall_sig() API into the build (#13296)

* Restore the dynCall() and dynCall_sig() API into the build when -s USE_LEGACY_DYNCALLS=1 is passed. Add support for dynCall() in MINIMAL_RUNTIME builds (that was not implemented before).

* Add ChangeLog entry.

* Rename USE_LEGACY_DYNCALLS to DYNCALLS.

* Inline getDynCaller.

* Address review.

* Merge new dynCall test with old.

* Reformat parseTools.js

Author: juj

ChangeLog.md

@@ -26,6 +26,11 @@ Current Trunk
 - Emscripten now builds a complete sysroot inside the EM_CACHE directory.
   This includes the system headers which get copied into place there rather
   than adding a sequence of extra include directories.
+- Add back support for calling the legacy dynCall_sig() API to invoke function
+  pointers to wasm functions from JavaScript. Pass -s DYNCALLS=1
+  to include that functionality in the build. This fixes a regression that
+  started in Aug 31st 2020 (Emscripten 2.0.2) in #12059. Also implement
+  support for dynCall() in MINIMAL_RUNTIME builds. (#13296)
 - `SDL2_ttf` now uses upstream compiled with `TTF_USE_HARFBUZ` flag.
 - The system for linking native libraries on demand (based on the symbols
   present in input object files) has been removed.  Libraries such as libgl,
@@ -218,6 +223,7 @@ Current Trunk
 
       {{{ makeDynCall('sig', 'ptr') }}} (arg1, arg2);
 
+  See PR #12059 for details.
 - The native optimizer and the corresponding config setting
   (`EMSCRIPTEN_NATIVE_OPTIMIZER`) have been removed (it was only relevant to
   asmjs/fastcomp backend).

emcc.py

@@ -1283,6 +1283,10 @@ def default_setting(name, new_default):
     if shared.Settings.CLOSURE_WARNINGS not in ['quiet', 'warn', 'error']:
       exit_with_error('Invalid option -s CLOSURE_WARNINGS=%s specified! Allowed values are "quiet", "warn" or "error".' % shared.Settings.CLOSURE_WARNINGS)
 
+    # Include dynCall() function by default in DYNCALLS builds in classic runtime; in MINIMAL_RUNTIME, must add this explicitly.
+    if shared.Settings.DYNCALLS and not shared.Settings.MINIMAL_RUNTIME:
+      shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$dynCall']
+
     if shared.Settings.MAIN_MODULE:
       assert not shared.Settings.SIDE_MODULE
       if shared.Settings.MAIN_MODULE == 1:
@@ -1331,7 +1335,7 @@ def default_setting(name, new_default):
     if shared.Settings.ASYNCIFY:
       # See: https://github.com/emscripten-core/emscripten/issues/12065
       # See: https://github.com/emscripten-core/emscripten/issues/12066
-      shared.Settings.USE_LEGACY_DYNCALLS = 1
+      shared.Settings.DYNCALLS = 1
       shared.Settings.EXPORTED_FUNCTIONS += ['_emscripten_stack_get_base',
                                              '_emscripten_stack_get_end',
                                              '_emscripten_stack_set_limits']

emscripten.py

@@ -44,6 +44,11 @@ def compute_minimal_runtime_initializer_and_exports(post, exports, receiving):
   exports_that_are_not_initializers = [x for x in exports if x not in WASM_INIT_FUNC]
   # In Wasm backend the exports are still unmangled at this point, so mangle the names here
   exports_that_are_not_initializers = [asmjs_mangle(x) for x in exports_that_are_not_initializers]
+
+  # Decide whether we should generate the global dynCalls dictionary for the dynCall() function?
+  if shared.Settings.DYNCALLS and '$dynCall' in shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE and len([x for x in exports_that_are_not_initializers if x.startswith('dynCall_')]) > 0:
+    exports_that_are_not_initializers += ['dynCalls = {}']
+
   post = post.replace('/*** ASM_MODULE_EXPORTS_DECLARES ***/', 'var ' + ',\n  '.join(exports_that_are_not_initializers) + ';')
 
   # Generate assignments from all asm.js/wasm exports out to the JS variables above: e.g. a = asm['a']; b = asm['b'];
@@ -401,7 +406,7 @@ def finalize_wasm(infile, outfile, memfile, DEBUG):
     args.append('-g')
   if shared.Settings.WASM_BIGINT:
     args.append('--bigint')
-  if shared.Settings.USE_LEGACY_DYNCALLS:
+  if shared.Settings.DYNCALLS:
     # we need to add all dyncalls to the wasm
     modify_wasm = True
   else:
@@ -710,7 +715,11 @@ def create_receiving(exports):
       # WebAssembly.instantiate(Module["wasm"], imports).then((function(output) {
       # var asm = output.instance.exports;
       # _main = asm["_main"];
-      receiving += [asmjs_mangle(s) + ' = asm["' + s + '"];' for s in exports_that_are_not_initializers]
+      generate_dyncall_assignment = shared.Settings.DYNCALLS and '$dynCall' in shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE
+      for s in exports_that_are_not_initializers:
+        mangled = asmjs_mangle(s)
+        dynCallAssignment = ('dynCalls["' + s.replace('dynCall_', '') + '"] = ') if generate_dyncall_assignment and mangled.startswith('dynCall_') else ''
+        receiving += [dynCallAssignment + mangled + ' = asm["' + s + '"];']
     else:
       if shared.Settings.MINIMAL_RUNTIME:
         # In wasm2js exports can be directly processed at top level, i.e.

src/embind/embind.js

@@ -1063,15 +1063,15 @@ var LibraryEmbind = {
   },
 
   $embind__requireFunction__deps: ['$readLatin1String', '$throwBindingError'
-#if USE_LEGACY_DYNCALLS || !WASM_BIGINT
+#if DYNCALLS || !WASM_BIGINT
     , '$getDynCaller'
 #endif
   ],
   $embind__requireFunction: function(signature, rawFunction) {
     signature = readLatin1String(signature);
 
     function makeDynCaller() {
-#if USE_LEGACY_DYNCALLS
+#if DYNCALLS
       return getDynCaller(signature, rawFunction);
 #else
 #if !WASM_BIGINT

src/library.js

@@ -3699,21 +3699,28 @@ LibraryManager.library = {
     });
   },
 
-#if USE_LEGACY_DYNCALLS || !WASM_BIGINT
+#if DYNCALLS || !WASM_BIGINT
   $dynCallLegacy: function(sig, ptr, args) {
 #if ASSERTIONS
+#if MINIMAL_RUNTIME
+    assert(typeof dynCalls !== 'undefined', 'Global dynCalls dictionary was not generated in the build! Pass -s DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=["$dynCall"] linker flag to include it!');
+    assert(sig in dynCalls, 'bad function pointer type - no table for sig \'' + sig + '\'');
+#else
     assert(('dynCall_' + sig) in Module, 'bad function pointer type - no table for sig \'' + sig + '\'');
+#endif
     if (args && args.length) {
       // j (64-bit integer) must be passed in as two numbers [low 32, high 32].
       assert(args.length === sig.substring(1).replace(/j/g, '--').length);
     } else {
       assert(sig.length == 1);
     }
 #endif
-    if (args && args.length) {
-      return Module['dynCall_' + sig].apply(null, [ptr].concat(args));
-    }
-    return Module['dynCall_' + sig].call(null, ptr);
+#if MINIMAL_RUNTIME
+    var f = dynCalls[sig];
+#else
+    var f = Module["dynCall_" + sig];
+#endif
+    return args && args.length ? f.apply(null, [ptr].concat(args)) : f.call(null, ptr);
   },
   $dynCall__deps: ['$dynCallLegacy'],
 
@@ -3722,7 +3729,7 @@ LibraryManager.library = {
   // back to this function if needed.
   $getDynCaller__deps: ['$dynCall'],
   $getDynCaller: function(sig, ptr) {
-#if !USE_LEGACY_DYNCALLS
+#if ASSERTIONS && !DYNCALLS
     assert(sig.indexOf('j') >= 0, 'getDynCaller should only be called with i64 sigs')
 #endif
     var argCache = [];
@@ -3737,7 +3744,7 @@ LibraryManager.library = {
 #endif
 
   $dynCall: function(sig, ptr, args) {
-#if USE_LEGACY_DYNCALLS
+#if DYNCALLS
     return dynCallLegacy(sig, ptr, args);
 #else
 #if !WASM_BIGINT

src/parseTools.js

@@ -1006,13 +1006,13 @@ Please update to new syntax.`);
     }
     args = args.join(', ');
 
-    if (USE_LEGACY_DYNCALLS) {
+    if (DYNCALLS) {
       return `(function(cb, ${args}) { return getDynCaller("${sig}", cb)(${args}) })`;
     } else {
       return `(function(cb, ${args}) { return wasmTable.get(cb)(${args}) })`;
     }
   }
-  if (USE_LEGACY_DYNCALLS) {
+  if (DYNCALLS) {
     const dyncall = exportedAsmFunc(`dynCall_${sig}`);
     if (sig.length > 1) {
       let args = [];

src/settings.js

@@ -1146,6 +1146,10 @@ var BINARYEN_EXTRA_PASSES = "";
 // (This option was formerly called BINARYEN_ASYNC_COMPILATION)
 var WASM_ASYNC_COMPILATION = 1;
 
+// If set to 1, the dynCall() and dynCall_sig() API is made available
+// to caller.
+var DYNCALLS = 0;
+
 // WebAssembly integration with JavaScript BigInt. When enabled we don't need
 // to legalize i64s into pairs of i32s, as the wasm VM will use a BigInt where
 // an i64 is used.

src/settings_internal.js

@@ -175,8 +175,6 @@ var EXPECT_MAIN = 1;
 // MODULARIZE, and returned from the factory function.
 var EXPORT_READY_PROMISE = 1;
 
-var USE_LEGACY_DYNCALLS = 0;
-
 // struct_info that is either generated or cached
 var STRUCT_INFO = '';
 

tests/core/test_dyncalls.c

@@ -0,0 +1,24 @@
+#include <emscripten.h>
+#include <stdint.h>
+#include <stdio.h>
+
+void vijdf(int i, int64_t j, double d, float f)
+{
+	EM_ASM(console.log('vijdf: i='+$0+',jlo='+$1+',jhi='+$2+',d='+$3+',f='+$4), i, (uint32_t)j, (uint32_t)(((uint64_t)j) >> 32), d, f);
+}
+
+int iii(int i, int j)
+{
+	EM_ASM(console.log('iii: i='+$0+',j='+$1), i, j);
+	return 42;
+}
+
+
+void test_dyncalls_vijdf(void(*)(int, int64_t, double, float));
+void test_dyncalls_iii(int(*)(int, int));
+
+int main()
+{
+	test_dyncalls_vijdf(&vijdf);
+	test_dyncalls_iii(&iii);
+}

tests/core/test_dyncalls.js

@@ -0,0 +1,23 @@
+mergeInto(LibraryManager.library, {
+  test_dyncalls_vijdf: function(funcPtr) {
+    // 1. Directly access a function pointer via a static signature (32-bit ABI)
+    //    (this is the fastest way to call a function pointer when the signature is statically known in WASM_BIGINT==0 builds)
+    dynCall_vijdf(funcPtr, 1, /*lo=*/2, /*hi=*/3, 4, 5); // Available only in WASM_BIGINT != 2 builds
+
+    // 2. Access a function pointer using the convenience/legacy 'dynCall' function (32-bit ABI)
+    //    (this form should never be used, it is suboptimal for performance, but provided for legacy compatibility)
+    dynCall('vijdf', funcPtr, [2, /*lo=*/3, /*hi=*/4, 5, 6]); // Available only in WASM_BIGINT != 2 builds
+  },
+
+  test_dyncalls_iii: function(funcPtr) {
+    // 1. Directly access a function pointer via a static signature (32-bit ABI)
+    //    (this is the fastest way to call a function pointer when the signature is statically known in WASM_BIGINT==0 builds)
+    var ret = dynCall_iii(funcPtr, 1, 2); // Available only in WASM_BIGINT != 2 builds
+    console.log('iii returned ' + ret);
+
+    // 2. Access a function pointer using the convenience/legacy 'dynCall' function (32-bit ABI)
+    //    (this form should never be used, it is suboptimal for performance, but provided for legacy compatibility)
+    var ret = dynCall('iii', funcPtr, [2, 3]); // Available only in WASM_BIGINT != 2 builds
+    console.log('iii returned ' + ret);
+  }
+});

tests/core/test_dyncalls.out

@@ -0,0 +1,6 @@
+vijdf: i=1,jlo=2,jhi=3,d=4,f=5
+vijdf: i=2,jlo=3,jhi=4,d=5,f=6
+iii: i=1,j=2
+iii returned 42
+iii: i=2,j=3
+iii returned 42

tests/test_core.py

@@ -6233,7 +6233,6 @@ def test_EXTRA_EXPORTED_RUNTIME_METHODS(self):
     self.set_setting('EXTRA_EXPORTED_RUNTIME_METHODS', ['dynCall', 'addFunction', 'lengthBytesUTF8', 'getTempRet0', 'setTempRet0'])
     self.do_run_in_out_file_test('tests', 'core', 'EXTRA_EXPORTED_RUNTIME_METHODS.c')
 
-  @no_minimal_runtime('MINIMAL_RUNTIME does not blindly export all symbols to Module to save code size')
   def test_dyncall_specific(self):
     emcc_args = self.emcc_args[:]
     for which, exported_runtime_methods in [
@@ -6246,6 +6245,13 @@ def test_dyncall_specific(self):
       self.set_setting('EXTRA_EXPORTED_RUNTIME_METHODS', exported_runtime_methods)
       self.do_run_in_out_file_test('tests', 'core', 'dyncall_specific.c')
 
+      self.set_setting('EXTRA_EXPORTED_RUNTIME_METHODS', [])
+      self.emcc_args = emcc_args + ['-s', 'DYNCALLS=1', '--js-library', path_from_root('tests', 'core', 'test_dyncalls.js')]
+      self.do_run_in_out_file_test('tests', 'core', 'test_dyncalls.c')
+
+      self.emcc_args += ['-s', 'MINIMAL_RUNTIME=1', '-s', 'DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=["$dynCall"]']
+      self.do_run_in_out_file_test('tests', 'core', 'test_dyncalls.c')
+
   def test_getValue_setValue(self):
     # these used to be exported, but no longer are by default
     def test(output_prefix='', args=[], assert_returncode=0):
@@ -8273,6 +8279,14 @@ def test_gl_main_module(self):
     self.set_setting('MAIN_MODULE')
     self.do_runf(path_from_root('tests', 'core', 'test_gl_get_proc_address.c'))
 
+  @no_asan('asan does not yet work in MINIMAL_RUNTIME')
+  def test_dyncalls_minimal_runtime(self):
+    self.set_setting('DYNCALLS')
+    self.set_setting('MINIMAL_RUNTIME')
+    self.emcc_args += ['-s', 'DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[$dynCall]']
+    self.emcc_args += ['--js-library', path_from_root('tests', 'core', 'test_dyncalls.js')]
+    self.do_run_in_out_file_test('tests', 'core', 'test_dyncalls.c')
+
 
 # Generate tests for everything
 def make_run(name, emcc_args, settings=None, env=None):

tools/shared.py

@@ -804,7 +804,7 @@ def is_legal_sig(sig):
   @staticmethod
   def make_dynCall(sig, args):
     # wasm2c and asyncify are not yet compatible with direct wasm table calls
-    if Settings.USE_LEGACY_DYNCALLS or not JS.is_legal_sig(sig):
+    if Settings.DYNCALLS or not JS.is_legal_sig(sig):
       args = ','.join(args)
       if not Settings.MAIN_MODULE and not Settings.SIDE_MODULE:
         # Optimize dynCall accesses in the case when not building with dynamic