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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 created this revision.
Herald added subscribers: wingo, ecnelises, sunfish, jgravelle-google, dschuff.
sbc100 requested review of this revision.
Herald added subscribers: llvm-commits, aheejin.
Herald added a project: LLVM.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95914

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


Index: lld/wasm/Writer.cpp
===================================================================
--- lld/wasm/Writer.cpp
+++ lld/wasm/Writer.cpp
@@ -585,8 +585,6 @@
   for (Symbol *sym : symtab->getSymbols()) {
     if (!sym->isUndefined())
       continue;
-    if (sym->isWeak() && !config->relocatable)
-      continue;
     if (!sym->isLive())
       continue;
     if (!sym->isUsedInRegularObj)
Index: lld/wasm/Driver.cpp
===================================================================
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -1023,7 +1023,8 @@
       warn(Twine("symbol exported via --export not found: ") + arg->getValue());
   }
 
-  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();
Index: lld/test/wasm/weak-undefined-pic.s
===================================================================
--- lld/test/wasm/weak-undefined-pic.s
+++ lld/test/wasm/weak-undefined-pic.s
@@ -22,6 +22,7 @@
 _start:
   .functype _start () -> ()
   call get_foo_addr
+  call foo
   end_function
 
 .weak foo
@@ -68,11 +69,15 @@
 # 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` or `-shared + --unresolved-symbols=import-functions` 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 @@
 # IMPORT-NEXT:         Name:            __table_base
 # IMPORT-NEXT:       - Index:           2
 # IMPORT-NEXT:         Name:            foo
+
+# With just `-pie` (which doesn't default or 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95914.320968.patch
Type: text/x-patch
Size: 2678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210203/87fecc9a/attachment.bin>