[lld][WebAssembly] Report undefined symbols during scanRelocations

This puts handling of undefined symbols in a single location.  Its
also more in line with the ELF backend which only reports undefined
symbols based on relocations.

One side effect is that we no longer report undefined symbols that are
only referenced in GC'd sections.

This also fixes a crash reported in the emscripten toolchain:
https://github.com/emscripten-core/emscripten/issues/8930.

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

git-svn-id: https://llvm.org/svn/llvm-project/lld/trunk@365553 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 30 insertions, 51 deletions

diff --git a/test/wasm/undefined-data.ll b/test/wasm/undefined-data.ll
index 4c8864854..e19086c2e 100644
--- a/test/wasm/undefined-data.ll
+++ b/test/wasm/undefined-data.ll
@@ -1,6 +1,6 @@
 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: not wasm-ld -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=UNDEF
-; RUN: not wasm-ld --allow-undefined -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=ALLOW
+; RUN: not wasm-ld --allow-undefined -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=UNDEF
 ; RUN: not wasm-ld --shared -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=SHARED
 
 target triple = "wasm32-unknown-unknown"
@@ -14,5 +14,4 @@ entry:
 }
 
 ; UNDEF: error: {{.*}}undefined-data.ll.tmp.o: undefined symbol: data_external
-; ALLOW: error: {{.*}}undefined-data.ll.tmp.o: cannot resolve relocation of type R_WASM_MEMORY_ADDR_LEB against undefined (non-weak) data symbol: data_external
 ; SHARED: error: {{.*}}undefined-data.ll.tmp.o: relocation R_WASM_MEMORY_ADDR_LEB cannot be used against symbol data_external; recompile with -fPIC
diff --git a/test/wasm/undefined.ll b/test/wasm/undefined.ll
index 1da979fc0..90f92f2bc 100644
--- a/test/wasm/undefined.ll
+++ b/test/wasm/undefined.ll
@@ -1,7 +1,7 @@
 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
 
-; Fails due to undefined 'foo' 
+; Fails due to undefined 'foo'
 ; RUN: not wasm-ld --undefined=baz -o %t.wasm %t.o 2>&1 | FileCheck %s
 ; CHECK: error: {{.*}}.o: undefined symbol: foo
 ; CHECK-NOT: undefined symbol: baz
@@ -16,7 +16,8 @@
 target triple = "wasm32-unknown-unknown"
 
 ; Takes the address of the external foo() resulting in undefined external
-@bar = hidden local_unnamed_addr global i8* bitcast (i32 ()* @foo to i8*), align 4
+@bar = global i8* bitcast (i32 ()* @foo to i8*), align 4
+@llvm.used = appending global [1 x i8**] [i8** @bar], section "llvm.metadata"
 
 declare i32 @foo() #0
 
diff --git a/wasm/InputFiles.cpp b/wasm/InputFiles.cpp
index c475a669f..b3e41a41b 100644
--- a/wasm/InputFiles.cpp
+++ b/wasm/InputFiles.cpp
@@ -160,22 +160,6 @@ uint32_t ObjFile::calcNewValue(const WasmRelocation &Reloc) const {
     // in debug sections.
     if ((isa<FunctionSymbol>(Sym) || isa<DataSymbol>(Sym)) && !Sym->isLive())
       return 0;
-
-    // Special handling for undefined data symbols.  Most relocations against
-    // such symbols cannot be resolved.
-    if (isa<DataSymbol>(Sym) && Sym->isUndefined()) {
-      if (Sym->isWeak() || Config->Relocatable)
-        return 0;
-      // R_WASM_MEMORY_ADDR_I32 relocations in PIC code are turned into runtime
-      // fixups in __wasm_apply_relocs
-      if (Config->Pic && Reloc.Type == R_WASM_MEMORY_ADDR_I32)
-        return 0;
-      if (Reloc.Type != R_WASM_GLOBAL_INDEX_LEB) {
-        llvm_unreachable(
-          ("invalid relocation against undefined data symbol: " + toString(*Sym))
-              .c_str());
-      }
-    }
   }
 
   switch (Reloc.Type) {
@@ -189,6 +173,8 @@ uint32_t ObjFile::calcNewValue(const WasmRelocation &Reloc) const {
   case R_WASM_MEMORY_ADDR_I32:
   case R_WASM_MEMORY_ADDR_LEB:
   case R_WASM_MEMORY_ADDR_REL_SLEB:
+    if (isa<UndefinedData>(Sym))
+      return 0;
     return cast<DefinedData>(Sym)->getVirtualAddress() + Reloc.Addend;
   case R_WASM_TYPE_INDEX_LEB:
     return TypeMap[Reloc.Index];
diff --git a/wasm/Relocations.cpp b/wasm/Relocations.cpp
index 00b542278..faedd0003 100644
--- a/wasm/Relocations.cpp
+++ b/wasm/Relocations.cpp
@@ -21,6 +21,25 @@ static bool requiresGOTAccess(const Symbol *Sym) {
   return Config->Pic && !Sym->isHidden() && !Sym->isLocal();
 }
 
+static bool allowUndefined(const Symbol* Sym) {
+  // Historically --allow-undefined doesn't work for data symbols since we don't
+  // have any way to represent these as imports in the final binary.  The idea
+  // behind allowing undefined symbols is to allow importing these symbols from
+  // the embedder and we can't do this for data symbols (at least not without
+  // compiling with -fPIC)
+  if (isa<DataSymbol>(Sym))
+    return false;
+  return (Config->AllowUndefined ||
+          Config->AllowUndefinedSymbols.count(Sym->getName()) != 0);
+}
+
+static void reportUndefined(const Symbol* Sym) {
+  assert(Sym->isUndefined());
+  assert(!Sym->isWeak());
+  if (!allowUndefined(Sym))
+    error(toString(Sym->getFile()) + ": undefined symbol: " + toString(*Sym));
+}
+
 void lld::wasm::scanRelocations(InputChunk *Chunk) {
   if (!Chunk->Live)
     return;
@@ -50,15 +69,6 @@ void lld::wasm::scanRelocations(InputChunk *Chunk) {
       if (!isa<GlobalSymbol>(Sym))
         Out.ImportSec->addGOTEntry(Sym);
       break;
-    case R_WASM_MEMORY_ADDR_SLEB:
-    case R_WASM_MEMORY_ADDR_LEB:
-    case R_WASM_MEMORY_ADDR_REL_SLEB:
-      if (!Config->Relocatable && Sym->isUndefined() && !Sym->isWeak()) {
-        error(toString(File) + ": cannot resolve relocation of type " +
-              relocTypeToString(Reloc.Type) +
-              " against undefined (non-weak) data symbol: " + toString(*Sym));
-      }
-      break;
     }
 
     if (Config->Pic) {
@@ -81,6 +91,11 @@ void lld::wasm::scanRelocations(InputChunk *Chunk) {
           Out.ImportSec->addGOTEntry(Sym);
         break;
       }
+    } else {
+      // Report undefined symbols
+      if (Sym->isUndefined() && !Config->Relocatable && !Sym->isWeak())
+        reportUndefined(Sym);
     }
+
   }
 }
diff --git a/wasm/SymbolTable.cpp b/wasm/SymbolTable.cpp
index aab372472..1a391eff7 100644
--- a/wasm/SymbolTable.cpp
+++ b/wasm/SymbolTable.cpp
@@ -80,19 +80,6 @@ void SymbolTable::addCombinedLTOObject() {
   }
 }
 
-void SymbolTable::reportRemainingUndefines() {
-  for (const auto& Pair : SymMap) {
-    const Symbol *Sym = SymVector[Pair.second];
-    if (!Sym->isUndefined() || Sym->isWeak())
-      continue;
-    if (Config->AllowUndefinedSymbols.count(Sym->getName()) != 0)
-      continue;
-    if (!Sym->IsUsedInRegularObj)
-      continue;
-    error(toString(Sym->getFile()) + ": undefined symbol: " + toString(*Sym));
-  }
-}
-
 Symbol *SymbolTable::find(StringRef Name) {
   auto It = SymMap.find(CachedHashStringRef(Name));
   if (It == SymMap.end() || It->second == -1)
diff --git a/wasm/SymbolTable.h b/wasm/SymbolTable.h
index 01f0d22d5..d98d3273c 100644
--- a/wasm/SymbolTable.h
+++ b/wasm/SymbolTable.h
@@ -41,8 +41,6 @@ public:
 
   void addCombinedLTOObject();
 
-  void reportRemainingUndefines();
-
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
 
   Symbol *find(StringRef Name);
diff --git a/wasm/Writer.cpp b/wasm/Writer.cpp
index e79d30131..cc8075aeb 100644
--- a/wasm/Writer.cpp
+++ b/wasm/Writer.cpp
@@ -827,13 +827,6 @@ void Writer::run() {
     if (Config->Pic)
       createApplyRelocationsFunction();
     createCallCtorsFunction();
-
-    // Make sure we have resolved all symbols.
-    if (!Config->AllowUndefined)
-      Symtab->reportRemainingUndefines();
-
-    if (errorCount())
-      return;
   }
 
   log("-- calculateTypes");