[llvm-branch-commits] [lld] 465e0bf - [lld][WebAssebmly] Ensure stub symbols always get address 0

Sam Clegg via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Nov 24 08:20:06 PST 2020


Author: Sam Clegg
Date: 2020-11-24T08:13:04-08:00
New Revision: 465e0bf415e1449919ec2f62027dc5df0ebc338d

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

LOG: [lld][WebAssebmly] Ensure stub symbols always get address 0

Without this extra flag we can't distingish between stub functions and
functions that happen to have address 0 (relative to __table_base).

Adding this flag bit the base symbol class actually avoids growing the
SymbolUnion struct which would not be true if we added it to the
FunctionSymbol subclass (due to bitbacking).

The previous approach of setting it's table index to zero worked for
normal static relocations but not for `-fPIC` code.

See https://github.com/emscripten-core/emscripten/issues/12819

Added: 
    lld/test/wasm/weak-undefined-pic.s

Modified: 
    lld/wasm/MarkLive.cpp
    lld/wasm/Relocations.cpp
    lld/wasm/SymbolTable.cpp
    lld/wasm/Symbols.h
    lld/wasm/SyntheticSections.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/weak-undefined-pic.s b/lld/test/wasm/weak-undefined-pic.s
new file mode 100644
index 000000000000..f0c85268fe79
--- /dev/null
+++ b/lld/test/wasm/weak-undefined-pic.s
@@ -0,0 +1,36 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld %t.o -o %t.wasm
+# RUN: obj2yaml %t.wasm | FileCheck %s
+
+# Test that undefined weak external functions get resolved to address zero
+# under static linking, and also with `-pie --unresolved-symbols=ignore`
+
+.globl get_foo_addr
+get_foo_addr:
+  .functype get_foo_addr () -> (i32)
+  global.get foo at GOT
+  end_function
+
+.globl _start
+_start:
+  .functype _start () -> ()
+  call get_foo_addr
+  end_function
+
+.weak foo
+.functype foo () -> (i32)
+
+#      CHECK:  - Type:            CUSTOM
+# CHECK-NEXT:    Name:            name
+# CHECK-NEXT:    FunctionNames:
+# CHECK-NEXT:      - Index:           0
+# CHECK-NEXT:        Name:            'undefined_weak:foo'
+# CHECK-NEXT:      - Index:           1
+# CHECK-NEXT:        Name:            get_foo_addr
+# CHECK-NEXT:      - Index:           2
+# CHECK-NEXT:        Name:            _start
+# CHECK-NEXT:    GlobalNames:
+# CHECK-NEXT:      - Index:           0
+# CHECK-NEXT:        Name:            __stack_pointer
+# CHECK-NEXT:      - Index:           1
+# CHECK-NEXT:        Name:            'undefined_weak:foo'

diff  --git a/lld/wasm/MarkLive.cpp b/lld/wasm/MarkLive.cpp
index 4bce68877040..235936e4ef3e 100644
--- a/lld/wasm/MarkLive.cpp
+++ b/lld/wasm/MarkLive.cpp
@@ -142,7 +142,7 @@ void MarkLive::mark() {
           reloc.Type == R_WASM_TABLE_INDEX_I32 ||
           reloc.Type == R_WASM_TABLE_INDEX_I64) {
         auto *funcSym = cast<FunctionSymbol>(sym);
-        if (funcSym->hasTableIndex() && funcSym->getTableIndex() == 0)
+        if (funcSym->isStub)
           continue;
       }
 

diff  --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index 5b131d4f23f4..bd07c0410dc5 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -62,7 +62,9 @@ static void reportUndefined(Symbol *sym) {
                      << "ignoring undefined symbol: " + toString(*sym) + "\n");
           f->stubFunction = symtab->createUndefinedStub(*f->getSignature());
           f->stubFunction->markLive();
-          f->setTableIndex(0);
+          // Mark the function itself as a stub which prevents it from being
+          // assigned a table entry.
+          f->isStub = true;
         }
       }
       break;

diff  --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index fc92020d07dd..e469aca89ad7 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -673,9 +673,7 @@ InputFunction *SymbolTable::replaceWithUnreachable(Symbol *sym,
   // to be exported outside the object file.
   replaceSymbol<DefinedFunction>(sym, debugName, WASM_SYMBOL_BINDING_LOCAL,
                                  nullptr, func);
-  // Ensure it compares equal to the null pointer, and so that table relocs
-  // don't pull in the stub body (only call-operand relocs should do that).
-  func->setTableIndex(0);
+  sym->isStub = true;
   return func;
 }
 

diff  --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index 865a9e7fee99..90fb5194edcd 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -124,7 +124,7 @@ class Symbol {
   Symbol(StringRef name, Kind k, uint32_t flags, InputFile *f)
       : name(name), file(f), symbolKind(k), referenced(!config->gcSections),
         requiresGOT(false), isUsedInRegularObj(false), forceExport(false),
-        canInline(false), traced(false), flags(flags) {}
+        canInline(false), traced(false), isStub(false), flags(flags) {}
 
   StringRef name;
   InputFile *file;
@@ -157,6 +157,11 @@ class Symbol {
   // True if this symbol is specified by --trace-symbol option.
   bool traced : 1;
 
+  // True if this symbol is a linker-synthesized stub function (traps when
+  // called) and should otherwise be treated as missing/undefined.  See
+  // SymbolTable::replaceWithUndefined.
+  bool isStub : 1;
+
   uint32_t flags;
 };
 

diff  --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index 407eb8084d0e..083d1c0969c1 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -303,6 +303,8 @@ void GlobalSection::generateRelocationCode(raw_ostream &os) const {
       writeU8(os, opcode_ptr_const, "CONST");
       writeSleb128(os, d->getVirtualAddress(), "offset");
     } else if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
+      if (f->isStub)
+        continue;
       // Get __table_base
       writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
       writeUleb128(os, WasmSym::tableBase->getGlobalIndex(), "__table_base");
@@ -329,12 +331,16 @@ void GlobalSection::writeBody() {
   // TODO(wvo): when do these need I64_CONST?
   for (const Symbol *sym : internalGotSymbols) {
     WasmGlobal global;
-    global.Type = {WASM_TYPE_I32, config->isPic};
+    // In the case of dynamic linking, internal GOT entries
+    // need to be mutable since the get updated to the correct
+    // runtime value during `__wasm_apply_relocs`.
+    bool mutable_ = config->isPic & !sym->isStub;
+    global.Type = {WASM_TYPE_I32, mutable_};
     global.InitExpr.Opcode = WASM_OPCODE_I32_CONST;
     if (auto *d = dyn_cast<DefinedData>(sym))
       global.InitExpr.Value.Int32 = d->getVirtualAddress();
     else if (auto *f = dyn_cast<FunctionSymbol>(sym))
-      global.InitExpr.Value.Int32 = f->getTableIndex();
+      global.InitExpr.Value.Int32 = f->isStub ? 0 : f->getTableIndex();
     else {
       assert(isa<UndefinedData>(sym));
       global.InitExpr.Value.Int32 = 0;
@@ -375,7 +381,7 @@ void StartSection::writeBody() {
 }
 
 void ElemSection::addEntry(FunctionSymbol *sym) {
-  if (sym->hasTableIndex())
+  if (sym->hasTableIndex() || sym->isStub)
     return;
   sym->setTableIndex(config->tableBase + indirectFunctions.size());
   indirectFunctions.emplace_back(sym);


        


More information about the llvm-branch-commits mailing list