Fix for weak undefined symbols in dynamic linking (#17164)

With this change we maintain a global set of weak undefined symbols
(WeakSymbols), when modules are loaded the `dylink` section tells us
which imports are weak imports.

We then resolve weakly imported symbol addresses to zero without
generating any errors.

Sadly there is some asymmetry with the way the main module is loaded
and the way side modules are, which makes this patch a litte more
complex than I would like.  For example, the weak imports of the main
module are extracted at build time rather than runtime.

Fixes: #12819

Author: sbc100

emscripten.py

@@ -112,7 +112,15 @@ def to_nice_ident(ident): # limited version of the JS function toNiceIdent
   return ident.replace('%', '$').replace('@', '_').replace('.', '_')
 
 
-def update_settings_glue(metadata):
+def get_weak_imports(main_wasm):
+  dylink_sec = webassembly.parse_dylink_section(main_wasm)
+  for symbols in dylink_sec.import_info.values():
+    for symbol, flags in symbols.items():
+      if flags & webassembly.SYMBOL_BINDING_MASK == webassembly.SYMBOL_BINDING_WEAK:
+        settings.WEAK_IMPORTS.append(symbol)
+
+
+def update_settings_glue(wasm_file, metadata):
   optimize_syscalls(metadata['declares'])
 
   # Integrate info from backend
@@ -124,6 +132,8 @@ def update_settings_glue(metadata):
     syms = set(syms).difference(metadata['exports'])
     syms.update(metadata['globalImports'])
     settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE = sorted(syms)
+    if settings.MAIN_MODULE:
+      get_weak_imports(wasm_file)
 
   settings.WASM_EXPORTS = metadata['exports'] + list(metadata['namedGlobals'].keys())
   # Store function exports so that Closure and metadce can track these even in
@@ -296,7 +306,7 @@ def emscript(in_wasm, out_wasm, outfile_js, memfile):
 
   metadata = finalize_wasm(in_wasm, out_wasm, memfile)
 
-  update_settings_glue(metadata)
+  update_settings_glue(out_wasm, metadata)
 
   if not settings.WASM_BIGINT and metadata['emJsFuncs']:
     module = webassembly.Module(in_wasm)

src/compiler.js

@@ -52,6 +52,7 @@ EXPORTED_FUNCTIONS = new Set(EXPORTED_FUNCTIONS);
 WASM_EXPORTS = new Set(WASM_EXPORTS);
 SIDE_MODULE_EXPORTS = new Set(SIDE_MODULE_EXPORTS);
 INCOMING_MODULE_JS_API = new Set(INCOMING_MODULE_JS_API);
+WEAK_IMPORTS = new Set(WEAK_IMPORTS);
 
 // Side modules are pure wasm and have no JS
 assert(!SIDE_MODULE, 'JS compiler should not run on side modules');

src/jsifier.js

@@ -197,13 +197,17 @@ function ${name}(${args}) {
         return '';
       }
 
-      let noExport = false;
+      // This gets set to true in the case of dynamic linking for symbols that
+      // are undefined in the main module.  In this case we create a stub that
+      // will resolve the correct symbol at runtime, or assert if its missing.
+      let isStub = false;
 
       if (!LibraryManager.library.hasOwnProperty(ident)) {
         if (ONLY_CALC_JS_SYMBOLS) {
           return;
         }
-        if (!isDefined(ident)) {
+        const isWeakImport = WEAK_IMPORTS.has(ident);
+        if (!isDefined(ident) && !isWeakImport) {
           let undefinedSym = ident;
           if (ident === '__main_argc_argv') {
             undefinedSym = 'main';
@@ -239,7 +243,7 @@ function ${name}(${args}) {
           }
           const functionBody = assertion + `return ${target}.apply(null, arguments);`;
           LibraryManager.library[ident] = new Function(functionBody);
-          noExport = true;
+          isStub = true;
         }
       }
 
@@ -383,7 +387,7 @@ function ${name}(${args}) {
       const sig = LibraryManager.library[ident + '__sig'];
       // asm module exports are done in emscripten.py, after the asm module is ready. Here
       // we also export library methods as necessary.
-      if ((EXPORT_ALL || EXPORTED_FUNCTIONS.has(finalName)) && !noExport) {
+      if ((EXPORT_ALL || EXPORTED_FUNCTIONS.has(finalName)) && !isStub) {
         contentText += `\nModule["${finalName}"] = ${finalName};`;
       }
       // Relocatable code needs signatures to create proper wrappers. Stack
@@ -394,6 +398,9 @@ function ${name}(${args}) {
       if (sig && (RELOCATABLE || ASYNCIFY == 2)) {
         contentText += `\n${finalName}.sig = '${sig}';`;
       }
+      if (isStub) {
+        contentText += `\n${finalName}.stub = true;`;
+      }
 
       let commentText = '';
       if (LibraryManager.library[ident + '__docs']) {

src/library_dylink.js

@@ -20,6 +20,9 @@ var LibraryDylink = {
 #endif
     if (!sym) {
       sym = asmLibraryArg[symName];
+      // Ignore 'stub' symbols that are auto-generated as part of the original
+      // `asmLibraryArg` used to instantate the main module.
+      if (sym && sym.stub) sym = undefined;
     }
 
     // Check for the symbol on the Module object.  This is the only
@@ -44,20 +47,28 @@ var LibraryDylink = {
   },
 
   $GOT: {},
+  $CurrentModuleWeakSymbols: '=new Set({{{ JSON.stringify(Array.from(WEAK_IMPORTS)) }}})',
 
   // Create globals to each imported symbol.  These are all initialized to zero
   // and get assigned later in `updateGOT`
   $GOTHandler__internal: true,
-  $GOTHandler__deps: ['$GOT'],
+  $GOTHandler__deps: ['$GOT', '$CurrentModuleWeakSymbols'],
   $GOTHandler: {
     'get': function(obj, symName) {
-      if (!GOT[symName]) {
-        GOT[symName] = new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true});
+      var rtn = GOT[symName];
+      if (!rtn) {
+        rtn = GOT[symName] = new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true});
 #if DYLINK_DEBUG
         err("new GOT entry: " + symName);
 #endif
       }
-      return GOT[symName]
+      if (!CurrentModuleWeakSymbols.has(symName)) {
+        // Any non-weak reference to a symbol marks it as `required`, which
+        // enabled `reportUndefinedSymbols` to report undefeind symbol errors
+        // correctly.
+        rtn.required = true;
+      }
+      return rtn;
     }
   },
 
@@ -175,6 +186,13 @@ var LibraryDylink = {
     for (var symName in GOT) {
       if (GOT[symName].value == 0) {
         var value = resolveGlobalSymbol(symName, true)
+        if (!value && !GOT[symName].required) {
+          // Ignore undefined symbols that are imported as weak.
+#if DYLINK_DEBUG
+          err('ignoring undefined weak symbol: ' + symName);
+#endif
+          continue;
+        }
 #if ASSERTIONS
         assert(value, 'undefined symbol `' + symName + '`. perhaps a side module was not linked in? if this global was expected to arrive from a system library, try to build the MAIN_MODULE with EMCC_FORCE_STDLIBS=1 in the environment');
 #endif
@@ -349,7 +367,7 @@ var LibraryDylink = {
       name = getString();
     }
 
-    var customSection = { neededDynlibs: [], tlsExports: {} };
+    var customSection = { neededDynlibs: [], tlsExports: new Set(), weakImports: new Set() };
     if (name == 'dylink') {
       customSection.memorySize = getLEB();
       customSection.memoryAlign = getLEB();
@@ -368,7 +386,10 @@ var LibraryDylink = {
       var WASM_DYLINK_MEM_INFO = 0x1;
       var WASM_DYLINK_NEEDED = 0x2;
       var WASM_DYLINK_EXPORT_INFO = 0x3;
+      var WASM_DYLINK_IMPORT_INFO = 0x4;
       var WASM_SYMBOL_TLS = 0x100;
+      var WASM_SYMBOL_BINDING_MASK = 0x3;
+      var WASM_SYMBOL_BINDING_WEAK = 0x1;
       while (offset < end) {
         var subsectionType = getU8();
         var subsectionSize = getLEB();
@@ -389,7 +410,17 @@ var LibraryDylink = {
             var symname = getString();
             var flags = getLEB();
             if (flags & WASM_SYMBOL_TLS) {
-              customSection.tlsExports[symname] = 1;
+              customSection.tlsExports.add(symname);
+            }
+          }
+        } else if (subsectionType === WASM_DYLINK_IMPORT_INFO) {
+          var count = getLEB();
+          while (count--) {
+            var modname = getString();
+            var symname = getString();
+            var flags = getLEB();
+            if ((flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK) {
+              customSection.weakImports.add(symname);
             }
           }
         } else {
@@ -471,9 +502,12 @@ var LibraryDylink = {
     '$loadDynamicLibrary', '$createInvokeFunction', '$getMemory',
     '$relocateExports', '$resolveGlobalSymbol', '$GOTHandler',
     '$getDylinkMetadata', '$alignMemory', '$zeroMemory',
+    '$alignMemory', '$zeroMemory',
+    '$CurrentModuleWeakSymbols', '$alignMemory', '$zeroMemory',
   ],
   $loadWebAssemblyModule: function(binary, flags, handle) {
     var metadata = getDylinkMetadata(binary);
+    CurrentModuleWeakSymbols = metadata.weakImports;
 #if ASSERTIONS
     var originalTable = wasmTable;
 #endif

src/library_pthread.js

@@ -491,14 +491,13 @@ var LibraryPThread = {
       if (!__tls_base) {
 #if ASSERTIONS
         // __tls_base should never be zero if there are tls exports
-        assert(__tls_base || Object.keys(metadata.tlsExports).length == 0);
+        assert(__tls_base || metadata.tlsExports.size == 0);
 #endif
         return;
       }
-      for (var sym in metadata.tlsExports) {
-        metadata.tlsExports[sym] = moduleExports[sym];
-      }
-      relocateExports(metadata.tlsExports, __tls_base, /*replace=*/true);
+      var tlsExports = {};
+      metadata.tlsExports.forEach((s) => tlsExports[s] = moduleExports[s]);
+      relocateExports(tlsExports, __tls_base, /*replace=*/true);
     }
 
     // Register this function so that its gets called for each thread on

src/settings_internal.js

@@ -226,4 +226,10 @@ var TRANSPILE_TO_ES5 = false;
 
 // A copy of the default the default INCOMING_MODULE_JS_API. (Soon to
 // include additional items).
-var ALL_INCOMING_MODULE_JS_API = []
+var ALL_INCOMING_MODULE_JS_API = [];
+
+// List of all imports that are weak, and therefore allowed to be undefined at
+// runtime.  This is used by the JS compiler to avoid build-time warnings/errors
+// when weak symbols are undefined.  Only applies in the case of dyanmic linking
+// (MAIN_MODULE).
+var WEAK_IMPORTS = [];

tests/other/test_extern_weak.c

@@ -6,7 +6,11 @@ extern int __attribute__((weak)) externData;
 
 int main() {
   printf("externFunc: %p\n", externFunc);
-  printf("externData: %p\n", &externData); 
+  printf("externData: %p\n", &externData);
+  if (externFunc)
+    printf("externFunc value: %d\n", externFunc());
+  if (externData)
+    printf("externData value: %d\n", externData);
   return 0;
 }
 

tests/other/test_extern_weak.resolved.out

@@ -0,0 +1,2 @@
+externFunc value: 42
+externData value: 43

tests/other/test_extern_weak_side.c

@@ -0,0 +1,5 @@
+int externFunc() {
+  return 42;
+}
+
+int externData = 43;

tests/test_other.py

@@ -8274,9 +8274,12 @@ def test_fflush_fs_exit(self):
   def test_extern_weak(self):
     self.do_other_test('test_extern_weak.c')
 
-  @disabled('https://github.com/emscripten-core/emscripten/issues/12819')
   def test_extern_weak_dynamic(self):
+    # If the symbols are left undefined we should get the same expected output as with static
+    # linking (i.e. null symbol addresses);
     self.do_other_test('test_extern_weak.c', emcc_args=['-sMAIN_MODULE=2'])
+    self.run_process([EMCC, '-o', 'libside.wasm', test_file('other/test_extern_weak_side.c'), '-sSIDE_MODULE'])
+    self.do_run_in_out_file_test(test_file('other/test_extern_weak.c'), out_suffix='.resolved', emcc_args=['-sMAIN_MODULE=2', 'libside.wasm'])
 
   def test_main_module_without_main(self):
     create_file('pre.js', r'''