[lld][WebAssembly] Fix for weak undefined functions in -pie mode

[sbc100]

Summary

This fixes two related related issue. Firstly we were never generting
imports for weak functions (even with the import-functions policy
for undefined symbols). Adding a direct call to foo in the
weak-undefined-pic.s exposed a crash in the linker which this
change fixes.

Secondly we were failing to call handleWeakUndefines for the -pie
case which is PIC but doesn't set the undefined symbol policy to
import-functions. With this change -pie binaries will by default
call handleWeakUndefines which generates the undefined stub handlers
for any weakly undefined symbols.

Fixes: https://github.com/emscripten-core/emscripten/issues/13337

[sbc100]

typos

[dschuff]

lld/test/wasm/weak-undefined-pic.s

72–74

97

[tlively]

@sbc100 This revision is responsible for the sanitizer errors on the emscripten autorollers: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8855537012484712752/+/steps/Emscripten_testsuite__upstream__wasm3_/0/stdout

Would you be able to take a look?

[sbc100]

In D95914#2559003, @tlively wrote:

@sbc100 This revision is responsible for the sanitizer errors on the emscripten autorollers: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8855537012484712752/+/steps/Emscripten_testsuite__upstream__wasm3_/0/stdout

Would you be able to take a look?

I think I will revert for now and followup after the weekend..

lld/test/wasm/weak-undefined-pic.s

get_foo_addr:
   .functype get_foo_addr () -> (i32)
   global.get foo@GOT
   end_function
 
 .globl _start
 _start:
   .functype _start () -> ()
   call get_foo_addr
+  call foo
   end_function
 
 .weak foo
 .functype foo () -> (i32)
 
 # Verify that we do not generate dynamic relocations for the GOT entry.
 
 # CHECK-NOT: __wasm_apply_global_relocs

 # CHECK-NEXT:      - Index:           2
 # CHECK-NEXT:        Name:            _start
 # CHECK-NEXT:    GlobalNames:
 # CHECK-NEXT:      - Index:           0
 # CHECK-NEXT:        Name:            __stack_pointer
 # CHECK-NEXT:      - Index:           1
 # CHECK-NEXT:        Name:            'GOT.func.internal.undefined_weak:foo'
 
-# With `-pie` or `-shared` the resolution should be deferred to the dynamic
-# linker and the function address should be imported as GOT.func.foo.
+# With `-pie + + --unresolved-symbols=import-functions` or `-shared` the
+# resolution should be deferred to the dynamic linker and the function address
+# should be imported as GOT.func.foo.
 #
-# RUN: wasm-ld --experimental-pic -pie %t.o -o %t3.wasm
+# RUN: wasm-ld --experimental-pic -shared %t.o -o %t3.wasm
 # RUN: obj2yaml %t3.wasm | FileCheck %s --check-prefix=IMPORT
+#
+# RUN: wasm-ld --experimental-pic -pie --unresolved-symbols=import-functions %t.o -o %t4.wasm
+# RUN: obj2yaml %t4.wasm | FileCheck %s --check-prefix=IMPORT
 
 #      IMPORT:  - Type:            IMPORT
 #      IMPORT:      - Module:          GOT.func
 # IMPORT-NEXT:        Field:           foo
 # IMPORT-NEXT:        Kind:            GLOBAL
 # IMPORT-NEXT:        GlobalType:      I32
 # IMPORT-NEXT:        GlobalMutable:   true
 
 #      IMPORT:     GlobalNames:
 # IMPORT-NEXT:       - Index:           0
 # IMPORT-NEXT:         Name:            __memory_base
 # IMPORT-NEXT:       - Index:           1
 # IMPORT-NEXT:         Name:            __table_base
 # IMPORT-NEXT:       - Index:           2
 # IMPORT-NEXT:         Name:            foo
+
+# With just `-pie` (which does not default to import-functions) there shoule be
+# no import at all.
+#
+# RUN: wasm-ld --experimental-pic -pie %t.o -o %t5.wasm
+# RUN: obj2yaml %t5.wasm | FileCheck %s --check-prefix=NO-IMPORT
+
+# NO-IMPORT:      Name:            'undefined_weak:foo'
+# NO-IMPORT-NOT:  Name: foo

lld/wasm/Driver.cpp

if (!wrapped.empty())
     wrapSymbols(wrapped);
 
   for (auto *arg : args.filtered(OPT_export)) {
     Symbol *sym = symtab->find(arg->getValue());
     if (sym && sym->isDefined())
       sym->forceExport = true;
   }
 
-  if (!config->relocatable && !config->isPic) {
+  if (!config->relocatable &&
+      config->unresolvedSymbols != UnresolvedPolicy::ImportFuncs) {
     // Add synthetic dummies for weak undefined functions.  Must happen
     // after LTO otherwise functions may not yet have signatures.
     symtab->handleWeakUndefines();
   }
 
   if (entrySym)
     entrySym->setHidden(false);
 

lld/wasm/Writer.cpp

static bool shouldImport(Symbol *sym) {
 
   return false;
 }
 
 void Writer::calculateImports() {
   for (Symbol *sym : symtab->getSymbols()) {
     if (!sym->isUndefined())
       continue;
-    if (sym->isWeak() && !config->relocatable)
-      continue;
     if (!sym->isLive())
       continue;
     if (!sym->isUsedInRegularObj)
       continue;
     if (shouldImport(sym)) {
       LLVM_DEBUG(dbgs() << "import: " << sym->getName() << "\n");
       out.importSec->addImport(sym);
     }