[llvm-branch-commits] [lld] 704e0ba - [lld][WebAssembly] Honor --allow-undefined for data symbols too

[Sam Clegg via llvm-branch-commits]

Author: Sam Clegg
Date: 2020-05-01T10:08:58-07:00
New Revision: 704e0ba53cb2d849fef08d1850175de8efd814c7

URL: https://github.com/llvm/llvm-project/commit/704e0ba53cb2d849fef08d1850175de8efd814c7
DIFF: https://github.com/llvm/llvm-project/commit/704e0ba53cb2d849fef08d1850175de8efd814c7.diff

LOG: [lld][WebAssembly] Honor --allow-undefined for data symbols too

This was originally the way this worked before before
https://reviews.llvm.org/D60882.

In retrospect it seems inconsistent that `--allow-undefined` doesn't
work for all symbols.  See:
https://groups.google.com/g/emscripten-discuss/c/HSRgQiIq1gI/m/Kt9oFWHiAwAJ

I'm also planning a followup change which implement the full
`--unresolved-symbols=..` flags supported by ELF linkers (both ld and
ld.lld) since it seems more standard.

Added: 
    

Modified: 
    lld/test/wasm/undefined-data.ll
    lld/wasm/Relocations.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/undefined-data.ll b/lld/test/wasm/undefined-data.ll
index e19086c2e0e0..59f7407bd610 100644
--- a/lld/test/wasm/undefined-data.ll
+++ b/lld/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=UNDEF
+; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
 ; RUN: not wasm-ld --shared -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=SHARED
 
 target triple = "wasm32-unknown-unknown"

diff  --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index 9475ff1c312e..e49acec244c1 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -21,13 +21,6 @@ static bool requiresGOTAccess(const Symbol *sym) {
 }
 
 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;
   // Undefined functions with explicit import name are allowed to be undefined
   // at link time.
   if (auto *F = dyn_cast<UndefinedFunction>(sym))