In MODULARIZE mode avoid modifying the incoming moduleArg. NFC

This avoids leaking of the partially constructed module and means
that only way to get access the module instance is via waiting on the
promise.

Previously we were attaching all our module properties to the incoming
object, but not, as far as I can tell for any good reason.

In case anybody was actually depending on this, inject thunks into the
moduleArg that abort on access with an actionable error.

src/closure-externs/closure-externs.js

@@ -245,9 +245,8 @@ var moduleArg;
  * Used in MODULARIZE mode.
  * We need to access this after the code we pass to closure so from closure's
  * POV this is "extern".
- * @suppress {duplicate}
  */
-var readyPromise;
+var moduleRtn;
 
 /**
  * This was removed from upstream closure compiler in

src/jsifier.mjs

@@ -744,6 +744,10 @@ var proxiedFunctionTable = [
       includeFile(fileName, shouldPreprocess(fileName));
     }
 
+    if (MODULARIZE) {
+      includeFile('postamble_modularize.js');
+    }
+
     print(
       '//FORWARDED_DATA:' +
         JSON.stringify({

src/postamble_modularize.js

@@ -0,0 +1,41 @@
+// In MODULARIZE mode we wrap the generated code in a factory function
+// and return either the Module itself, or a promise of the module.
+//
+// We assign to the `moduleRtn` global here and configure closure to see
+// this as and extern so it won't get minified.
+
+#if WASM_ASYNC_COMPILATION
+
+#if USE_READY_PROMISE
+moduleRtn = readyPromise;
+#else
+moduleRtn = {};
+#endif
+
+#else  // WASM_ASYNC_COMPILATION
+
+moduleRtn = Module;
+
+#endif // WASM_ASYNC_COMPILATION
+
+#if ASSERTIONS
+// Assertion for attempting to use access modulue properties on the incoming
+// moduleArg.  In the past we used this object as the prototype of the module
+// and assigned properties to it, but now we return a distinct object.  This
+// keeps the instance private until it is ready (i.e the promise has been
+// resolved).
+for (const prop of Object.keys(Module)) {
+  if (!(prop in moduleArg)) {
+    Object.defineProperty(moduleArg, prop, {
+      configurable: true,
+      get() {
+#if WASM_ASYNC_COMPILATION
+        abort(`Access to module property ('${prop}') is no longer possible via the incoming module contructor argument; Instead, use the result of the module promise.`)
+#else
+        abort(`Access to module property ('${prop}') is no longer possible via the module input argument; Instead, use the module constructor return value.`)
+#endif
+      }
+    });
+  }
+}
+#endif

src/shell.js

@@ -10,7 +10,7 @@
 // The Module object: Our interface to the outside world. We import
 // and export values on it. There are various ways Module can be used:
 // 1. Not defined. We create it here
-// 2. A function parameter, function(Module) { ..generated code.. }
+// 2. A function parameter, function(moduleArg) => Promise<Module>
 // 3. pre-run appended it, var Module = {}; ..generated code..
 // 4. External script tag defines var Module.
 // We need to check if Module already exists (e.g. case 3 above).
@@ -21,7 +21,7 @@
 // before the code. Then that object will be used in the code, and you
 // can continue to use Module afterwards as well.
 #if MODULARIZE
-var Module = moduleArg;
+var Module = Object.assign({}, 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 {{

test/code_size/hello_webgl2_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 569,
   "a.html.gz": 379,
-  "a.js": 4539,
-  "a.js.gz": 2315,
+  "a.js": 4550,
+  "a.js.gz": 2322,
   "a.wasm": 10440,
   "a.wasm.gz": 6715,
-  "total": 15548,
-  "total_gz": 9409
+  "total": 15559,
+  "total_gz": 9416
 }

test/code_size/hello_webgl2_wasm2js.json

@@ -1,8 +1,8 @@
 {
   "a.html": 354,
   "a.html.gz": 266,
-  "a.js": 22178,
-  "a.js.gz": 11582,
-  "total": 22532,
-  "total_gz": 11848
+  "a.js": 22189,
+  "a.js.gz": 11588,
+  "total": 22543,
+  "total_gz": 11854
 }

test/code_size/hello_webgl_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 569,
   "a.html.gz": 379,
-  "a.js": 4056,
-  "a.js.gz": 2152,
+  "a.js": 4067,
+  "a.js.gz": 2158,
   "a.wasm": 10440,
   "a.wasm.gz": 6715,
-  "total": 15065,
-  "total_gz": 9246
+  "total": 15076,
+  "total_gz": 9252
 }

test/code_size/hello_webgl_wasm2js.json

@@ -1,8 +1,8 @@
 {
   "a.html": 354,
   "a.html.gz": 266,
-  "a.js": 21680,
-  "a.js.gz": 11412,
-  "total": 22034,
-  "total_gz": 11678
+  "a.js": 21691,
+  "a.js.gz": 11420,
+  "total": 22045,
+  "total_gz": 11686
 }

test/other/test_unoptimized_code_size.js.size

@@ -1 +1 @@
-55580
+55579

test/other/test_unoptimized_code_size_no_asserts.js.size

@@ -1 +1 @@
-31465
+31464

test/other/test_unoptimized_code_size_strict.js.size

@@ -1 +1 @@
-54468
+54467

test/test_other.py

@@ -6416,11 +6416,11 @@ def test_modularize_sync_compilation(self):
 console.log('before');
 var result = Module();
 // It should be an object.
-console.log(typeof result);
+console.log('typeof result: ' + typeof result);
 // And it should have the exports that Module has, showing it is Module in fact.
-console.log(typeof result._main);
+console.log('typeof _main: ' + typeof result._main);
 // And it should not be a Promise.
-console.log(typeof result.then);
+console.log('typeof result.then: ' + typeof result.then);
 console.log('after');
 ''')
     self.run_process([EMCC, test_file('hello_world.c'),
@@ -6430,12 +6430,25 @@ def test_modularize_sync_compilation(self):
     self.assertContained('''\
 before
 hello, world!
-object
-function
-undefined
+typeof result: object
+typeof _main: function
+typeof result.then: undefined
 after
 ''', self.run_js('a.out.js'))
 
+  def test_modularize_argument_misuse(self):
+    create_file('test.c', '''
+      #include <emscripten.h>
+      EMSCRIPTEN_KEEPALIVE int foo() { return 42; }''')
+
+    create_file('post.js', r'''
+      var arg = { bar: 1 };
+      var promise = Module(arg);
+      arg._foo();''')
+
+    expected = "Aborted(Access to module property ('_foo') is no longer possible via the incoming module contructor argument; Instead, use the result of the module promise"
+    self.do_runf('test.c', expected, assert_returncode=NON_ZERO, emcc_args=['--no-entry', '-sMODULARIZE', '--extern-post-js=post.js'])
+
   def test_export_all_3142(self):
     create_file('src.cpp', r'''
 typedef unsigned int Bit32u;

tools/link.py

@@ -2350,16 +2350,6 @@ def modularize(options):
      shared.target_environment_may_be('web'):
     async_emit = 'async '
 
-  # Return the incoming `moduleArg`.  This is is equivalent to the `Module` var within the
-  # generated code but its not run through closure minification so we can reference it in
-  # the return statement.
-  return_value = 'moduleArg'
-  if settings.WASM_ASYNC_COMPILATION:
-    if settings.USE_READY_PROMISE:
-      return_value = 'readyPromise'
-    else:
-      return_value = '{}'
-
   # TODO: Remove when https://bugs.webkit.org/show_bug.cgi?id=223533 is resolved.
   if async_emit != '' and settings.EXPORT_NAME == 'config':
     diagnostics.warning('emcc', 'EXPORT_NAME should not be named "config" when targeting Safari')
@@ -2369,12 +2359,11 @@ def modularize(options):
 
 %(src)s
 
-  return %(return_value)s
+  return moduleRtn;
 }
 ''' % {
     'maybe_async': async_emit,
     'src': src,
-    'return_value': return_value,
   }
 
   if settings.MINIMAL_RUNTIME and not settings.PTHREADS: