[lld][WebAssembly] Handle weakly defined symbols in shared libraries.
authorSam Clegg <sbc@chromium.org>
Wed, 18 Aug 2021 16:57:34 +0000 (12:57 -0400)
committerSam Clegg <sbc@chromium.org>
Thu, 19 Aug 2021 23:25:49 +0000 (19:25 -0400)
In the case of weakly defined symbols in shared libraries we now
generate both an import and an export.  The dynamic linker can then
choose how a winner from among all the shared libraries that define a
given symbol.

Previously any direct usage of a weakly defined symbol would use the
DSO-local definition (For example, even through there would be single
address for a weakly defined function, each DSO could end up directly
calling its local version).

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

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

lld/test/wasm/shared-weak-symbols.s [new file with mode: 0644]
lld/wasm/Symbols.cpp
lld/wasm/Symbols.h
lld/wasm/SyntheticSections.cpp
lld/wasm/Writer.cpp

diff --git a/lld/test/wasm/shared-weak-symbols.s b/lld/test/wasm/shared-weak-symbols.s
new file mode 100644 (file)
index 0000000..00b5d3a
--- /dev/null
@@ -0,0 +1,59 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld --experimental-pic -shared -o %t.wasm %t.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+# RUN: llvm-objdump -d %t.wasm | FileCheck %s -check-prefix=ASM
+
+# Verify the weakly defined fuctions (weak_func) are both
+# imported and exported, and that internal usage (direct call)
+# always uses the imported version.
+
+
+.globl weak_func
+.weak weak_func
+weak_func:
+  .functype weak_func () -> (i32)
+  i32.const 0
+  end_function
+
+.globl call_weak
+call_weak:
+# ASM: <call_weak>:
+  .functype call_weak () -> (i32)
+  call weak_func
+# ASM:           10 80 80 80 80 00      call  0
+  end_function
+# ASM-NEXT:      0b                     end
+
+# CHECK:       - Type:            IMPORT
+# CHECK-NEXT:    Imports:
+# CHECK-NEXT:      - Module:          env
+# CHECK-NEXT:        Field:           memory
+# CHECK-NEXT:        Kind:            MEMORY
+# CHECK-NEXT:        Memory:
+# CHECK-NEXT:          Minimum:         0x0
+# CHECK-NEXT:      - Module:          env
+# CHECK-NEXT:        Field:           __memory_base
+# CHECK-NEXT:        Kind:            GLOBAL
+# CHECK-NEXT:        GlobalType:      I32
+# CHECK-NEXT:        GlobalMutable:   false
+# CHECK-NEXT:      - Module:          env
+# CHECK-NEXT:        Field:           __table_base
+# CHECK-NEXT:        Kind:            GLOBAL
+# CHECK-NEXT:        GlobalType:      I32
+# CHECK-NEXT:        GlobalMutable:   false
+# CHECK-NEXT:      - Module:          env
+# CHECK-NEXT:        Field:           weak_func
+# CHECK-NEXT:        Kind:            FUNCTION
+# CHECK-NEXT:        SigIndex:        0
+
+#      CHECK:   - Type:            CUSTOM
+# CHECK-NEXT:     Name:            name
+# CHECK-NEXT:     FunctionNames:
+# CHECK-NEXT:       - Index:           0
+# CHECK-NEXT:         Name:            weak_func
+# CHECK-NEXT:       - Index:           1
+# CHECK-NEXT:         Name:            __wasm_call_ctors
+# CHECK-NEXT:       - Index:           2
+# CHECK-NEXT:         Name:            __wasm_apply_data_relocs
+# CHECK-NEXT:       - Index:           3
+# CHECK-NEXT:         Name:            weak_func
index 37a26b3..09b5bad 100644 (file)
@@ -215,6 +215,12 @@ void Symbol::setHidden(bool isHidden) {
 }
 
 bool Symbol::isExported() const {
+  // Shared libraries must export all weakly defined symbols
+  // in case they contain the version that will be chosed by
+  // the dynamic linker.
+  if (config->shared && isLive() && isDefined() && isWeak())
+    return true;
+
   if (!isDefined() || isLocal())
     return false;
 
@@ -233,15 +239,13 @@ bool Symbol::isNoStrip() const {
 }
 
 uint32_t FunctionSymbol::getFunctionIndex() const {
-  if (auto *f = dyn_cast<DefinedFunction>(this))
-    return f->function->getFunctionIndex();
-  if (const auto *u = dyn_cast<UndefinedFunction>(this)) {
-    if (u->stubFunction) {
+  if (const auto *u = dyn_cast<UndefinedFunction>(this))
+    if (u->stubFunction)
       return u->stubFunction->getFunctionIndex();
-    }
-  }
-  assert(functionIndex != INVALID_INDEX);
-  return functionIndex;
+  if (functionIndex != INVALID_INDEX)
+    return functionIndex;
+  auto *f = cast<DefinedFunction>(this);
+  return f->function->getFunctionIndex();
 }
 
 void FunctionSymbol::setFunctionIndex(uint32_t index) {
@@ -288,6 +292,10 @@ DefinedFunction::DefinedFunction(StringRef name, uint32_t flags, InputFile *f,
                      function ? &function->signature : nullptr),
       function(function) {}
 
+uint32_t DefinedFunction::getExportedFunctionIndex() const {
+  return function->getFunctionIndex();
+}
+
 uint64_t DefinedData::getVA() const {
   LLVM_DEBUG(dbgs() << "getVA: " << getName() << "\n");
   if (segment)
index a883b8a..ef2ae7f 100644 (file)
@@ -214,6 +214,12 @@ public:
     return s->kind() == DefinedFunctionKind;
   }
 
+  // Get the function index to be used when exporting.  This only applies to
+  // defined functions and can be differ from the regular function index for
+  // weakly defined functions (that are imported and used via one index but
+  // defined and exported via another).
+  uint32_t getExportedFunctionIndex() const;
+
   InputFunction *function;
 };
 
index fc37115..bb04396 100644 (file)
@@ -516,7 +516,11 @@ void LinkingSection::writeBody() {
       writeUleb128(sub.os, flags, "sym flags");
 
       if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
-        writeUleb128(sub.os, f->getFunctionIndex(), "index");
+        if (auto *d = dyn_cast<DefinedFunction>(sym)) {
+          writeUleb128(sub.os, d->getExportedFunctionIndex(), "index");
+        } else {
+          writeUleb128(sub.os, f->getFunctionIndex(), "index");
+        }
         if (sym->isDefined() || (flags & WASM_SYMBOL_EXPLICIT_NAME) != 0)
           writeStr(sub.os, sym->getName(), "sym name");
       } else if (auto *g = dyn_cast<GlobalSymbol>(sym)) {
index efe64e6..2d4dc47 100644 (file)
@@ -548,18 +548,23 @@ void Writer::populateTargetFeatures() {
 }
 
 static bool shouldImport(Symbol *sym) {
-  if (!sym->isUndefined())
-    return false;
-  if (sym->isWeak() && !config->relocatable && !config->isPic)
+  // We don't generate imports for data symbols. They however can be imported
+  // as GOT entries.
+  if (isa<DataSymbol>(sym))
     return false;
   if (!sym->isLive())
     return false;
   if (!sym->isUsedInRegularObj)
     return false;
 
-  // We don't generate imports for data symbols. They however can be imported
-  // as GOT entries.
-  if (isa<DataSymbol>(sym))
+  // When a symbol is weakly defined in a shared library we need to allow
+  // it to be overridden by another module so need to both import
+  // and export the symbol.
+  if (config->shared && sym->isDefined() && sym->isWeak())
+    return true;
+  if (!sym->isUndefined())
+    return false;
+  if (sym->isWeak() && !config->relocatable && !config->isPic)
     return false;
 
   // In PIC mode we only need to import functions when they are called directly.
@@ -619,7 +624,7 @@ void Writer::calculateExports() {
       if (Optional<StringRef> exportName = f->function->getExportName()) {
         name = *exportName;
       }
-      export_ = {name, WASM_EXTERNAL_FUNCTION, f->getFunctionIndex()};
+      export_ = {name, WASM_EXTERNAL_FUNCTION, f->getExportedFunctionIndex()};
     } else if (auto *g = dyn_cast<DefinedGlobal>(sym)) {
       if (g->getGlobalType()->Mutable && !g->getFile() && !g->forceExport) {
         // Avoid exporting mutable globals are linker synthesized (e.g.