[WebAssembly] Don't error when --undefined symbols are not found

[sbc100]

Summary

This matches the behavior of the ELF linker where -u/--undefined
means symbols will get pulled in from archives but won't result
in link error if they are missing.

This results in nice simplification of the code too.

[sbc100]

• Update comment

[ruiu]

In ELF, we took a different approach. What we did in the lld/ELF driver is this:

1. Add all symbols to the symbol table
2. For each symbol that is given by --undefined, look up the symbol table, and if it is a lazy symbol, fetch it.

I don't think there's particular reason to implement it in a different way, so could you do it that way?

[ruiu]

Is this the right patch? Looks like this is the same as the another one.

[sbc100]

• Handle --undefined like the ELF linker

[sbc100]

• revert part

[ruiu]

Generally looking good.

wasm/Driver.cpp

487–489 ↗

(On Diff #159130)

Not sure if this has to be inside a guard for --relocatable. Any reason for that?

[sbc100]

• --undefined should work with -r

[sbc100]

wasm/Driver.cpp

487–489 ↗

(On Diff #159130)

Its was that way before. but I think you are probably right. Fixed.

[ruiu]

LGTM

lld/trunk/test/wasm/call-indirect.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:       - Name:            bar
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            call_bar_indirect
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           3
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:       - Name:            indirect_func
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            call_ptr
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           5
 ; CHECK-NEXT:   - Type:            ELEM
 ; CHECK-NEXT:     Segments:

lld/trunk/test/wasm/cxx-mangling.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            _Z3fooi
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            0B
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            000B

lld/trunk/test/wasm/demangle.ll

 ; RUN: llc -filetype=obj %s -o %t.o
-; RUN: not wasm-ld --undefined _Z3fooi \
-; RUN:     -o %t.wasm %t.o 2>&1 | FileCheck %s
+; RUN: not wasm-ld -o %t.wasm %t.o 2>&1 | FileCheck %s
 
-; CHECK: error: undefined symbol: foo(int)
+; CHECK: error: {{.*}}.o: undefined symbol: foo(int)
 
-; RUN: not wasm-ld --no-demangle --undefined _Z3fooi \
-; RUN:     -o %t.wasm %t.o 2>&1 |  FileCheck -check-prefix=CHECK-NODEMANGLE %s
+; RUN: not wasm-ld --no-demangle \
+; RUN:     -o %t.wasm %t.o 2>&1 | FileCheck -check-prefix=CHECK-NODEMANGLE %s
 
-; CHECK-NODEMANGLE: error: undefined symbol: _Z3fooi
+; CHECK-NODEMANGLE: error: {{.*}}.o: undefined symbol: _Z3fooi
 
 target triple = "wasm32-unknown-unknown"
 
+declare void @_Z3fooi(i32);
+
 define hidden void @_start() local_unnamed_addr {
 entry:
+    call void @_Z3fooi(i32 1)
     ret void
 }

lld/trunk/test/wasm/export-all.ll

 
 ; CHECK:      - Type:            EXPORT
 ; CHECK:         - Name:            _start
 ; CHECK-NOT:     - Name:            bar
 ; CHECK-NOT:     - Name:            foo
 ; CHECK-NOT:     - Name:            internal_func
 
 ; EXPORT:     - Type:            EXPORT
-; EXPORT:        - Name:            _start
 ; EXPORT:        - Name:            bar
 ; EXPORT:        - Name:            foo
+; EXPORT:        - Name:            _start
 ; EXPORT-NOT:    - Name:            internal_func

lld/trunk/test/wasm/export.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            hidden_function
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:   - Type:            CODE

lld/trunk/test/wasm/load-undefined.test

 
 ; CHECK:         Name:            name
 ; CHECK-NEXT:    FunctionNames:   
 ; CHECK-NEXT:      - Index:           0
 ; CHECK-NEXT:        Name:            __wasm_call_ctors
 ; CHECK-NEXT:      - Index:           1
 ; CHECK-NEXT:        Name:            _start
 ; CHECK-NEXT:      - Index:           2
-; CHECK-NEXT:        Name:            ret32
-; CHECK-NEXT:      - Index:           3
 ; CHECK-NEXT:        Name:            ret64
+; CHECK-NEXT:      - Index:           3
+; CHECK-NEXT:        Name:            ret32
 ; CHECK-NEXT: ...
 
 ; NO-LOAD:         Name:            name
 ; NO-LOAD-NEXT:    FunctionNames:   
 ; NO-LOAD-NEXT:      - Index:           0
 ; NO-LOAD-NEXT:        Name:            __wasm_call_ctors
 ; NO-LOAD-NEXT:      - Index:           1
 ; NO-LOAD-NEXT:        Name:            _start
 ; NO-LOAD-NEXT:      - Index:           2
 ; NO-LOAD-NEXT:        Name:            ret64
 ; NO-LOAD-NEXT: ...
 
-; Verify that referencing a symbol that doesn't exist won't work
-; RUN: not wasm-ld %t.start.o -o %t.wasm -u symboldoesnotexist 2>&1 | FileCheck -check-prefix=CHECK-UNDEFINED1 %s
-; CHECK-UNDEFINED1: error: undefined symbol: symboldoesnotexist
-
-; RUN: not wasm-ld %t.start.o -o %t.wasm --undefined symboldoesnotexist --allow-undefined 2>&1 | FileCheck -check-prefix=CHECK-UNDEFINED2 %s
-; CHECK-UNDEFINED2: symbol forced with --undefined not found: symboldoesnotexist
+; Verify that referencing a symbol that is not found doesn't result in a link
+; failure.  This matches the behaviour of the ELF linker.
+; RUN: wasm-ld %t.start.o -o %t.wasm -u symboldoesnotexist

lld/trunk/test/wasm/lto/export.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            hidden_function
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:   - Type:            CODE

lld/trunk/test/wasm/undefined-entry.test

 RUN: llc -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
 RUN: not wasm-ld -o %t.wasm %t.ret32.o 2>&1 | FileCheck %s
+RUN: not wasm-ld --allow-undefined -o %t.wasm %t.ret32.o 2>&1 | FileCheck %s
 RUN: not wasm-ld -entry=foo -o %t.wasm %t.ret32.o 2>&1 | FileCheck %s -check-prefix=CHECK-CUSTOM
-RUN: not wasm-ld --allow-undefined -o %t.wasm %t.ret32.o 2>&1 | FileCheck %s -check-prefix=CHECK-ALLOW
 
-CHECK: error: undefined symbol: _start
-CHECK-CUSTOM: error: undefined symbol: foo
-CHECK-ALLOW: error: entry symbol not defined (pass --no-entry to supress):
-_start
+CHECK: error: entry symbol not defined (pass --no-entry to supress): _start
+CHECK-CUSTOM: error: entry symbol not defined (pass --no-entry to supress): foo
 
 RUN: wasm-ld --no-entry -o %t.wasm %t.ret32.o

lld/trunk/test/wasm/undefined.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
 
-; Fails due to undefined 'foo' and also 'baz'
+; 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: error: undefined symbol: baz
+; CHECK-NOT: undefined symbol: baz
 
 ; Succeeds if we pass a file containing 'foo' as --allow-undefined-file.
 ; RUN: echo 'foo' > %t.txt
 ; RUN: wasm-ld --allow-undefined-file=%t.txt -o %t.wasm %t.o
 
 ; Succeeds even if a missing symbol is added via --export
 ; RUN: wasm-ld --allow-undefined --export=xxx -o %t.wasm %t.o
 

lld/trunk/test/wasm/visibility-hidden.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            objectDefault
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            archiveDefault
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           5
 ; CHECK-NEXT:   - Type:

lld/trunk/test/wasm/weak-alias-overide.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            alias_fn
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            direct_fn
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            call_direct
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:       - Name:            call_alias
 ; CHECK-NEXT:         Kind:            FUNCTION

lld/trunk/test/wasm/weak-undefined.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __data_end
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            get_address_of_foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            get_address_of_global_var
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            0B
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            4180808080000B
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            4180808080000B
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            1082808080001A0B
 ; CHECK-NEXT: ...

lld/trunk/wasm/Driver.cpp

for (Symbol *Sym : Symtab->getSymbols()) {
     Symtab->SyntheticFunctions.emplace_back(Func);
     // Hide our dummy to prevent export.
     uint32_t Flags = WASM_SYMBOL_VISIBILITY_HIDDEN;
     replaceSymbol<DefinedFunction>(Sym, Sym->getName(), Flags, nullptr, Func);
   }
 }
 
 // Force Sym to be entered in the output. Used for -u or equivalent.
-static Symbol *addUndefined(StringRef Name) {
-  Symbol *S = Symtab->addUndefinedFunction(Name, 0, nullptr, nullptr);
+static Symbol *handleUndefined(StringRef Name) {
+  Symbol *Sym = Symtab->find(Name);
+  if (!Sym)
+    return nullptr;
 
   // Since symbol S may not be used inside the program, LTO may
   // eliminate it. Mark the symbol as "used" to prevent it.
-  S->IsUsedInRegularObj = true;
+  Sym->IsUsedInRegularObj = true;
+
+  if (auto *LazySym = dyn_cast<LazySymbol>(Sym))
+    LazySym->fetch();
 
-  return S;
+  return Sym;
 }
 
 void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   WasmOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
 
   // Handle --help
   if (Args.hasArg(OPT_help)) {
if (!Config->Relocatable) {
     // spec proposal is implemented in all major browsers.
     // See: https://github.com/WebAssembly/mutable-global
     WasmSym::StackPointer = Symtab->addSyntheticGlobal(
         "__stack_pointer", WASM_SYMBOL_VISIBILITY_HIDDEN, StackPointer);
     WasmSym::HeapBase = Symtab->addSyntheticDataSymbol("__heap_base", 0);
     WasmSym::DsoHandle = Symtab->addSyntheticDataSymbol(
         "__dso_handle", WASM_SYMBOL_VISIBILITY_HIDDEN);
     WasmSym::DataEnd = Symtab->addSyntheticDataSymbol("__data_end", 0);
-
-    // For now, since we don't actually use the start function as the
-    // wasm start symbol, we don't need to care about it signature.
-    if (!Config->Entry.empty())
-      EntrySym = addUndefined(Config->Entry);
-
-    // Handle the `--undefined <sym>` options.
-    for (auto *Arg : Args.filtered(OPT_undefined))
-      addUndefined(Arg->getValue());
   }
 
   createFiles(Args);
   if (errorCount())
     return;
 
   // Add all files to the symbol table. This will add almost all
   // symbols that we need to the symbol table.
   for (InputFile *F : Files)
     Symtab->addFile(F);
   if (errorCount())
     return;
 
+  // Handle the `--undefined <sym>` options.
+  for (auto *Arg : Args.filtered(OPT_undefined))
+    handleUndefined(Arg->getValue());
+
+  if (!Config->Relocatable) {
     // Add synthetic dummies for weak undefined functions.
-  if (!Config->Relocatable)
     handleWeakUndefines();
 
+    if (!Config->Entry.empty()) {
+      EntrySym = handleUndefined(Config->Entry);
+      if (!EntrySym)
+        error("entry symbol not defined (pass --no-entry to supress): " +
+              Config->Entry);
+    }
+
+    // Make sure we have resolved all symbols.
+    if (!Config->AllowUndefined)
+      Symtab->reportRemainingUndefines();
+  }
+
+  if (errorCount())
+    return;
+
   // Handle --export.
   for (auto *Arg : Args.filtered(OPT_export)) {
     StringRef Name = Arg->getValue();
     Symbol *Sym = Symtab->find(Name);
     if (Sym && Sym->isDefined())
       Sym->ForceExport = true;
     else if (!Config->AllowUndefined)
       error("symbol exported via --export not found: " + Name);
   }
 
   // Do link-time optimization if given files are LLVM bitcode files.
   // This compiles bitcode files into real object files.
   Symtab->addCombinedLTOObject();
   if (errorCount())
     return;
 
-  // Make sure we have resolved all symbols.
-  if (!Config->Relocatable && !Config->AllowUndefined) {
-    Symtab->reportRemainingUndefines();
-  } else {
-    // Even when using --allow-undefined we still want to report the absence of
-    // our initial set of undefined symbols (i.e. the entry point and symbols
-    // specified via --undefined).
-    // Part of the reason for this is that these function don't have signatures
-    // so which means they cannot be written as wasm function imports.
-    for (auto *Arg : Args.filtered(OPT_undefined)) {
-      Symbol *Sym = Symtab->find(Arg->getValue());
-      if (!Sym->isDefined())
-        error("symbol forced with --undefined not found: " + Sym->getName());
-    }
-    if (EntrySym && !EntrySym->isDefined())
-      error("entry symbol not defined (pass --no-entry to supress): " +
-            EntrySym->getName());
-  }
-  if (errorCount())
-    return;
-
   if (EntrySym)
     EntrySym->setHidden(false);
 
   if (errorCount())
     return;
 
   // Do size optimizations: garbage collection
   markLive();
 
   // Write the result to the file.
   writeResult();
 }

lld/trunk/wasm/SymbolTable.cpp

void SymbolTable::addCombinedLTOObject() {
   for (StringRef Filename : LTO->compile()) {
     auto *Obj = make<ObjFile>(MemoryBufferRef(Filename, "lto.tmp"));
     Obj->parse();
     ObjectFiles.push_back(Obj);
   }
 }
 
 void SymbolTable::reportRemainingUndefines() {
-  SetVector<Symbol *> Undefs;
   for (Symbol *Sym : SymVector) {
     if (!Sym->isUndefined() || Sym->isWeak())
       continue;
     if (Config->AllowUndefinedSymbols.count(Sym->getName()) != 0)
       continue;
     if (!Sym->IsUsedInRegularObj)
       continue;
-    Undefs.insert(Sym);
+    error(toString(Sym->getFile()) + ": undefined symbol: " + toString(*Sym));
   }
-
-  if (Undefs.empty())
-    return;
-
-  for (ObjFile *File : ObjectFiles)
-    for (Symbol *Sym : File->getSymbols())
-      if (Undefs.count(Sym))
-        error(toString(File) + ": undefined symbol: " + toString(*Sym));
-
-  for (Symbol *Sym : Undefs)
-    if (!Sym->getFile())
-      error("undefined symbol: " + toString(*Sym));
 }
 
 Symbol *SymbolTable::find(StringRef Name) {
   return SymMap.lookup(CachedHashStringRef(Name));
 }
 
 std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name, InputFile *File) {
   bool Inserted = false;