[lld][WebAssembly] Handle weakly defined symbols in shared libraries.

[sbc100]

Summary

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 DSOs 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

[sbc100]

comment

[sbc100]

• comment

lld/test/wasm/shared-weak-symbols.s

+# 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

lld/wasm/Symbols.h

 public:
   DefinedFunction(StringRef name, uint32_t flags, InputFile *f,
                   InputFunction *function);
 
   static bool classof(const Symbol *s) {
     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;
 };
 
 class UndefinedFunction : public FunctionSymbol {
 public:
   UndefinedFunction(StringRef name, llvm::Optional<StringRef> importName,
                     llvm::Optional<StringRef> importModule, uint32_t flags,
                     InputFile *file = nullptr,

lld/wasm/Symbols.cpp

void Symbol::setHidden(bool isHidden) {
   flags &= ~WASM_SYMBOL_VISIBILITY_MASK;
   if (isHidden)
     flags |= WASM_SYMBOL_VISIBILITY_HIDDEN;
   else
     flags |= WASM_SYMBOL_VISIBILITY_DEFAULT;
 }
 
 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;
 
   if (config->exportAll || (config->exportDynamic && !isHidden()))
     return true;
 
   return isExportedExplicit();
 }
 
 bool Symbol::isExportedExplicit() const {
   return forceExport || flags & WASM_SYMBOL_EXPORTED;
 }
 
 bool Symbol::isNoStrip() const {
   return flags & WASM_SYMBOL_NO_STRIP;
 }
 
 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);
+  if (functionIndex != INVALID_INDEX)
     return functionIndex;
+  auto *f = cast<DefinedFunction>(this);
+  return f->function->getFunctionIndex();
 }
 
 void FunctionSymbol::setFunctionIndex(uint32_t index) {
   LLVM_DEBUG(dbgs() << "setFunctionIndex " << name << " -> " << index << "\n");
   assert(functionIndex == INVALID_INDEX);
   functionIndex = index;
 }
 

 }
 
 DefinedFunction::DefinedFunction(StringRef name, uint32_t flags, InputFile *f,
                                  InputFunction *function)
     : FunctionSymbol(name, DefinedFunctionKind, flags, 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)
     return segment->getVA(value);
   return value;
 }
 
 void DefinedData::setVA(uint64_t value_) {

lld/wasm/SyntheticSections.cpp

for (const Symbol *sym : symtabEntries) {
       assert(sym->isDefined() || sym->isUndefined());
       WasmSymbolType kind = sym->getWasmType();
       uint32_t flags = sym->flags;
 
       writeU8(sub.os, kind, "sym kind");
       writeUleb128(sub.os, flags, "sym flags");
 
       if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
+        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)) {
         writeUleb128(sub.os, g->getGlobalIndex(), "index");
         if (sym->isDefined() || (flags & WASM_SYMBOL_EXPLICIT_NAME) != 0)
           writeStr(sub.os, sym->getName(), "sym name");
       } else if (auto *t = dyn_cast<TagSymbol>(sym)) {
         writeUleb128(sub.os, t->getTagIndex(), "index");

lld/wasm/Writer.cpp

for (auto &feature : required.keys()) {
         error(Twine("Missing target feature '") + feature + "' in " + fileName +
               ", required by " + required[feature] +
               ". Use --no-check-features to suppress.");
     }
   }
 }
 
 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.
   // Indirect usage all goes via GOT imports.
   if (config->isPic) {
     if (auto *f = dyn_cast<UndefinedFunction>(sym))
       if (!f->isCalledDirectly)
         return false;
if (!sym->isLive())
       continue;
 
     StringRef name = sym->getName();
     WasmExport export_;
     if (auto *f = dyn_cast<DefinedFunction>(sym)) {
       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.
         // __stack_pointer or __tls_base) unless they are explicitly exported
         // from the command line.
         // Without this check `--export-all` would cause any program using the
         // stack pointer to export a mutable global even if none of the input
         // files were built with the `mutable-globals` feature.