[lld][WebAssembly] 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

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

diff --git a/lld/test/wasm/weak-undefined-pic.s b/lld/test/wasm/weak-undefined-pic.s
new file mode 100644 (file)
index 0000000..c12ef23
--- /dev/null
+++ b/lld/test/wasm/weak-undefined-pic.s
@@ -0,0 +1,90 @@
+# Checks handling of undefined weak external functions.  When the
+# static linker decides they are undefined, check GOT relocations
+# resolve to zero (i.e. a global that contains zero.).
+#
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld %t.o -o %t1.wasm
+# RUN: obj2yaml %t1.wasm | FileCheck %s
+#
+# With `--unresolved-symbols=ignore-all` the behaviour should be the same
+# as the default.>
+#
+# RUN: wasm-ld --unresolved-symbols=ignore-all %t.o -o %t2.wasm
+# RUN: obj2yaml %t2.wasm | FileCheck %s
+
+.globl get_foo_addr
+get_foo_addr:
+  .functype get_foo_addr () -> (i32)
+  global.get foo@GOT
+  end_function
+
+.globl _start
+_start:
+  .functype _start () -> ()
+  call get_foo_addr
+  end_function
+
+.weak foo
+.functype foo () -> (i32)
+
+# Verify that we do not generate dynamnic relocations for the GOT entry.
+
+# CHECK-NOT: __wasm_apply_relocs
+
+# Verify that we do not generate an import for foo
+
+# CHECK-NOT:  - Type:            IMPORT
+
+#      CHECK:   - Type:            GLOBAL
+# CHECK-NEXT:     Globals:
+# CHECK-NEXT:       - Index:           0
+# CHECK-NEXT:         Type:            I32
+# CHECK-NEXT:         Mutable:         true
+# CHECK-NEXT:         InitExpr:
+# CHECK-NEXT:           Opcode:          I32_CONST
+# CHECK-NEXT:           Value:           66560
+# Global 'undefined_weak:foo' representing the GOT entry for foo
+# Unlike other internal GOT entries that need to be mutable this one
+# is immutable and not updated by `__wasm_apply_relocs`
+# CHECK-NEXT:       - Index:           1
+# CHECK-NEXT:         Type:            I32
+# CHECK-NEXT:         Mutable:         false
+# CHECK-NEXT:         InitExpr:
+# CHECK-NEXT:           Opcode:          I32_CONST
+# CHECK-NEXT:           Value:           0
+
+#      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'
+
+# With `-pie` or `-shared` the resolution should is defered 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: obj2yaml %t3.wasm | FileCheck %s --check-prefix=IMPORT
+
+#      IMPORT:  - Type:            IMPORT
+#      IMPORT:      - Module:          GOT.func
+# IMPORT-NEXT:        Field:           foo
+# IMPORT-NEXT:        Kind:            GLOBAL
+# IMPORT-NEXT:        GlobalType:      I32
+# IMPORT-NEXT:        GlobalMutable:   true
+
+#      IMPORT:     GlobalNames:
+# IMPORT-NEXT:       - Index:           0
+# IMPORT-NEXT:         Name:            __memory_base
+# IMPORT-NEXT:       - Index:           1
+# IMPORT-NEXT:         Name:            __table_base
+# IMPORT-NEXT:       - Index:           2
+# IMPORT-NEXT:         Name:            foo

diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 8aa124795f6feadfd0f4bc777908727cc7376968..f26b190b6058a5391e9c08b18c1082a4dca6bed3 100644 (file)
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -973,7 +973,7 @@ void LinkerDriver::link(ArrayRef<const char *> argsArr) {
       warn(Twine("symbol exported via --export not found: ") + arg->getValue());
   }
 
-  if (!config->relocatable) {
+  if (!config->relocatable && !config->isPic) {
     // Add synthetic dummies for weak undefined functions.  Must happen
     // after LTO otherwise functions may not yet have signatures.
     symtab->handleWeakUndefines();

diff --git a/lld/wasm/MarkLive.cpp b/lld/wasm/MarkLive.cpp
index 4bce688770400f67749c6324fb5c198315edf45e..235936e4ef3eca840e31b040087bf6ab476a8b98 100644 (file)
--- 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 5b131d4f23f409ed537b04ba65229189b96e78eb..bd07c0410dc57e1cedae0a00533c3ca4b42f23ef 100644 (file)
--- 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 fc92020d07ddc7dd7faea6001eeaa111dbcb125b..d8a070c3c2f23a71793fa3efdb2c14b152d74a3c 100644 (file)
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -673,9 +673,9 @@ 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);
+  // Ensure the stub function doesn't get a table entry.  Its address
+  // should always compare equal to the null pointer.
+  sym->isStub = true;
   return func;
 }
 

diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index 865a9e7fee99fb0c6867a41c8923e9230bcf926c..90fb5194edcd97e2379027173e202e5737bc7aea 100644 (file)
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -124,7 +124,7 @@ protected:
   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 @@ public:
   // 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 3e0581cbcfc4d9f8c49c45ecbac398c6f00d1623..f141e37b4988150a0d50c1b04363999499dcf2f0 100644 (file)
--- 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 they 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,10 @@ void StartSection::writeBody() {
 }
 
 void ElemSection::addEntry(FunctionSymbol *sym) {
-  if (sym->hasTableIndex())
+  // Don't add stub functions to the wasm table.  The address of all stub
+  // functions should be zero and they should they don't appear in the table.
+  // They only exist so that the calls to missing functions can validate.
+  if (sym->hasTableIndex() || sym->isStub)
     return;
   sym->setTableIndex(config->tableBase + indirectFunctions.size());
   indirectFunctions.emplace_back(sym);