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

[Sam Clegg via llvm-commits]

Author: Sam Clegg
Date: 2021-02-11T17:16:03-08:00
New Revision: ac2be2b6a366c05c01b8228fd804ba6ed52d320b

URL: https://github.com/llvm/llvm-project/commit/ac2be2b6a366c05c01b8228fd804ba6ed52d320b
DIFF: https://github.com/llvm/llvm-project/commit/ac2be2b6a366c05c01b8228fd804ba6ed52d320b.diff

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

This fixes two somewhat related issues.  Firstly we were never
generating 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

Differential Revision: https://reviews.llvm.org/D95914

Added: 
    

Modified: 
    lld/test/wasm/weak-undefined-pic.s
    lld/wasm/Driver.cpp
    lld/wasm/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/weak-undefined-pic.s b/lld/test/wasm/weak-undefined-pic.s
index b0f31e1a12d9..34f9ed1e935e 100644
--- a/lld/test/wasm/weak-undefined-pic.s
+++ b/lld/test/wasm/weak-undefined-pic.s
@@ -22,6 +22,7 @@ get_foo_addr:
 _start:
   .functype _start () -> ()
   call get_foo_addr
+  call foo
   end_function
 
 .weak foo
@@ -68,11 +69,15 @@ _start:
 # 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
@@ -88,3 +93,12 @@ _start:
 # 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

diff  --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 3e2786d2b59d..91315c5fc743 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -1016,7 +1016,8 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
       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();

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index d91b6b811081..29fa99e50c31 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -569,8 +569,6 @@ 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)