From c468dc1b12d8db2c2f881db5b0166f73345631c2 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 18 Aug 2021 12:57:34 -0400 Subject: [PATCH] [lld][WebAssembly] Handle weakly defined symbols in shared libraries. 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 | 59 +++++++++++++++++++++++++++++++++++++ lld/wasm/Symbols.cpp | 24 ++++++++++----- lld/wasm/Symbols.h | 6 ++++ lld/wasm/SyntheticSections.cpp | 6 +++- lld/wasm/Writer.cpp | 19 +++++++----- 5 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 lld/test/wasm/shared-weak-symbols.s diff --git a/lld/test/wasm/shared-weak-symbols.s b/lld/test/wasm/shared-weak-symbols.s new file mode 100644 index 0000000..00b5d3a --- /dev/null +++ b/lld/test/wasm/shared-weak-symbols.s @@ -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: : + .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 diff --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp index 37a26b3..09b5bad 100644 --- a/lld/wasm/Symbols.cpp +++ b/lld/wasm/Symbols.cpp @@ -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(this)) - return f->function->getFunctionIndex(); - if (const auto *u = dyn_cast(this)) { - if (u->stubFunction) { + if (const auto *u = dyn_cast(this)) + if (u->stubFunction) return u->stubFunction->getFunctionIndex(); - } - } - assert(functionIndex != INVALID_INDEX); - return functionIndex; + if (functionIndex != INVALID_INDEX) + return functionIndex; + auto *f = cast(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) diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h index a883b8a..ef2ae7f 100644 --- a/lld/wasm/Symbols.h +++ b/lld/wasm/Symbols.h @@ -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; }; diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp index fc371158..bb04396 100644 --- a/lld/wasm/SyntheticSections.cpp +++ b/lld/wasm/SyntheticSections.cpp @@ -516,7 +516,11 @@ void LinkingSection::writeBody() { writeUleb128(sub.os, flags, "sym flags"); if (auto *f = dyn_cast(sym)) { - writeUleb128(sub.os, f->getFunctionIndex(), "index"); + if (auto *d = dyn_cast(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(sym)) { diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index efe64e6..2d4dc47 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -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(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(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 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(sym)) { if (g->getGlobalType()->Mutable && !g->getFile() && !g->forceExport) { // Avoid exporting mutable globals are linker synthesized (e.g. -- 2.7.4