Fix regression with Safari and EXPORT_NAME (#19656)

Fixes: #18357

Author: sbc100

.circleci/config.yml

@@ -601,7 +601,7 @@ jobs:
             echo "JSC_ENGINE = [os.path.expanduser('~/.jsvu/bin/javascriptcore')]" >> ~/emsdk/.emscripten
             echo "JS_ENGINES = [JSC_ENGINE]" >> ~/emsdk/.emscripten
       - run-tests:
-          test_targets: "core0.test_hello_world"
+          test_targets: "core0.test_hello_world other.test_modularize_incoming other.test_modularize_incoming_export_name"
   test-spidermonkey:
     executor: linux-python
     steps:

emcc.py

@@ -3229,8 +3229,7 @@ def phase_final_emitting(options, state, target, wasm_target, memfile):
 
   if settings.MODULARIZE:
     modularize()
-
-  if settings.USE_CLOSURE_COMPILER:
+  elif settings.USE_CLOSURE_COMPILER:
     module_export_name_substitution()
 
   # Run a final optimization pass to clean up items that were not possible to
@@ -3863,7 +3862,10 @@ def modularize():
      shared.target_environment_may_be('web'):
     async_emit = 'async '
 
-  return_value = settings.EXPORT_NAME
+  # Return the incoming `moduleArg`.  This is is equeivielt to the `Module` var within the
+  # generated code but its not run through closure minifiection so we can reference it in
+  # the the return statement.
+  return_value = 'moduleArg'
   if settings.WASM_ASYNC_COMPILATION:
     return_value += '.ready'
   if not settings.EXPORT_READY_PROMISE:
@@ -3874,7 +3876,7 @@ def modularize():
     diagnostics.warning('emcc', 'EXPORT_NAME should not be named "config" when targeting Safari')
 
   src = '''
-%(maybe_async)sfunction(%(EXPORT_NAME)s = {})  {
+%(maybe_async)sfunction(moduleArg = {}) {
 
 %(src)s
 
@@ -3883,7 +3885,6 @@ def modularize():
 %(capture_module_function_for_audio_worklet)s
 ''' % {
     'maybe_async': async_emit,
-    'EXPORT_NAME': settings.EXPORT_NAME,
     'src': src,
     'return_value': return_value,
     # Given the async nature of how the Module function and Module object come into existence in AudioWorkletGlobalScope,
@@ -3945,6 +3946,7 @@ def modularize():
 
 
 def module_export_name_substitution():
+  assert not settings.MODULARIZE
   global final_js
   logger.debug(f'Private module export name substitution with {settings.EXPORT_NAME}')
   src = read_file(final_js)

src/closure-externs/closure-externs.js

@@ -261,3 +261,5 @@ var sampleRate;
  * Avoid closure minifying anything to "id". See #13965
  */
 var id;
+
+var moduleArg;

src/preamble.js

@@ -630,6 +630,11 @@ if (Module['locateFile']) {
   }
 #if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE // in single-file mode, repeating WASM_BINARY_FILE would emit the contents again
 } else {
+#if ENVIRONMENT_MAY_BE_SHELL
+  if (ENVIRONMENT_IS_SHELL)
+    wasmBinaryFile = '{{{ WASM_BINARY_FILE }}}';
+  else
+#endif
   // Use bundler-friendly `new URL(..., import.meta.url)` pattern; works in browsers too.
   wasmBinaryFile = new URL('{{{ WASM_BINARY_FILE }}}', import.meta.url).href;
 }

src/shell.js

@@ -20,7 +20,9 @@
 // after the generated code, you will need to define   var Module = {};
 // before the code. Then that object will be used in the code, and you
 // can continue to use Module afterwards as well.
-#if USE_CLOSURE_COMPILER
+#if MODULARIZE
+var Module = moduleArg;
+#elif USE_CLOSURE_COMPILER
 // if (!Module)` is crucial for Closure Compiler here as it will otherwise replace every `Module` occurrence with a string
 var /** @type {{
   noImageDecoding: boolean,

src/shell_minimal.js

@@ -4,8 +4,9 @@
  * SPDX-License-Identifier: MIT
  */
 
-#if USE_CLOSURE_COMPILER
-
+#if MODULARIZE
+var Module = moduleArg;
+#elif USE_CLOSURE_COMPILER
 // if (!Module)` is crucial for Closure Compiler here as it will
 // otherwise replace every `Module` occurrence with the object below
 var /** @type{Object} */ Module;
@@ -15,7 +16,7 @@ if (!Module) /** @suppress{checkTypes}*/Module =
 #endif
   {"__EMSCRIPTEN_PRIVATE_MODULE_EXPORT_NAME_SUBSTITUTION__":1};
 
-#elif !MODULARIZE && (ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL)
+#elif ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL
 
 // When running on the web we expect Module to be defined externally, in the
 // HTML.  Otherwise we must define it here before its first use

test/code_size/hello_webgl2_wasm2js.json

@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 18209,
-  "a.js.gz": 8055,
+  "a.js": 18218,
+  "a.js.gz": 8059,
   "a.mem": 3171,
   "a.mem.gz": 2713,
-  "total": 21947,
-  "total_gz": 11147
+  "total": 21956,
+  "total_gz": 11151
 }

test/code_size/hello_webgl_wasm2js.json

@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 17681,
-  "a.js.gz": 7872,
+  "a.js": 17689,
+  "a.js.gz": 7874,
   "a.mem": 3171,
   "a.mem.gz": 2713,
-  "total": 21419,
-  "total_gz": 10964
+  "total": 21427,
+  "total_gz": 10966
 }

test/test_core.py

@@ -7770,6 +7770,7 @@ def test_webidl(self, mode, allow_memory_growth):
     if self.maybe_closure():
       # avoid closure minified names competing with our test code in the global name space
       self.set_setting('MODULARIZE')
+      self.set_setting('EXPORT_NAME', 'createModule')
     else:
       self.set_setting('WASM_ASYNC_COMPILATION', 0)
 
@@ -7781,7 +7782,7 @@ def test_webidl(self, mode, allow_memory_growth):
 
     post_js = '\n\n'
     if self.get_setting('MODULARIZE'):
-      post_js += 'var TheModule = Module();\n'
+      post_js += 'var TheModule = createModule();\n'
     else:
       post_js += 'var TheModule = Module;\n'
     post_js += '\n\n'
@@ -9129,7 +9130,7 @@ def test_asan_api(self):
   def test_asan_modularized_with_closure(self):
     # the bug is that createModule() returns undefined, instead of the
     # proper Promise object.
-    create_file('post.js', 'if (!(createModule() instanceof Promise)) throw "Promise was not returned :(";\n')
+    create_file('post.js', 'if (!(createModule() instanceof Promise)) throw `Promise was not returned (${typeof createModule()})`;\n')
     self.emcc_args += ['-fsanitize=address', '--extern-post-js=post.js']
     self.set_setting('MODULARIZE')
     self.set_setting('EXPORT_NAME', 'createModule')

test/test_other.py

@@ -5928,6 +5928,22 @@ def test_modularize_strict(self):
     output = self.run_js('run.js')
     self.assertEqual(output, 'hello, world!\n')
 
+  @parameterized({
+    '': ([],),
+    'export_name': (['-sEXPORT_NAME=Foo'],),
+    'closure': (['-sEXPORT_NAME=Foo', '--closure=1'],),
+  })
+  @crossplatform
+  def test_modularize_incoming(self, args):
+    self.run_process([EMCC, test_file('hello_world.c'), '-o', 'out.mjs'] + self.get_emcc_args() + args)
+    create_file('run.mjs', '''
+    import Module from './out.mjs';
+    await Module({onRuntimeInitialized: () => console.log('done init')})
+      .then(() => console.log('got module'));
+    ''')
+    output = self.run_js('run.mjs')
+    self.assertContained('done init\nhello, world!\ngot module\n', output)
+
   @crossplatform
   @node_pthreads
   @no_mac('https://github.com/emscripten-core/emscripten/issues/19683')