[lld][WebAssemby] Demote LazySymbols back to undefined symbols if they are not loaded

[sbc100]

Summary

A LazySymbol is one that lives in .a archive and gets pulled in by a
strong reference. However, weak references to such symbols do not
result in them be loaded from the archive. In this case we want to
treat such symbols at undefined rather then lazy, once symbols
resolution is complete.

This fixes a crash bug in the linker when weakly referenced symbol that
lives in an archive file is live at the end of the link. In the case of
dynamic linking this is expected to turn into an import with (in the
case of a function symbol) a function index.

[dschuff]

lld/wasm/Driver.cpp

577

if (s->signature)
is the way to differentiate function symbols from data symbols?

1048

[sbc100]

• feedback

[sbc100]

lld/wasm/Driver.cpp

577

Yes see the comment in LazySymbol:

// Lazy symbols can have a signature because they can replace an               
// UndefinedFunction in which case we need to be able to preserve the          
// signature.                                                                     
// TODO(sbc): This repetition of the signature field is inelegant.  Revisit    
// the use of class hierarchy to represent symbol taxonomy.                    
const WasmSignature *signature = nullptr;

[dschuff]

lld/wasm/Symbols.h

652

these 2 places that set referenced are there so that the replaceSymbol you added above works properly?

[sbc100]

lld/wasm/Symbols.h

652

I need to preserve the referenced of the symbol as it gets replaced.. which means I also need to initialize it correctly in insertName since all symbols start in that default state. There is some duplication between the defaults in insertName and the default ins the Symbol constructor.. its not ideal, but its the same logic that exists in the ELF linker.

lld/test/wasm/shared-weak-undefined.s

 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 # RUN: wasm-ld --experimental-pic -shared -o %t.wasm %t.o
 # RUN: obj2yaml %t.wasm | FileCheck %s
 # RUN: llvm-objdump -d %t.wasm | FileCheck %s -check-prefix=ASM
 
+# Run the same test but include a definition of ret32 in a library file.
+# This verifies that LazySymbols (those found in library archives) are correctly
+# demoted to undefined symbols in the final link when they are only weakly
+# referenced.
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.ret32.o %p/Inputs/ret32.s
+# RUN: rm -f %T/libret32.a
+# RUN: llvm-ar cru %T/libret32.a %t.ret32.o
+# RUN: wasm-ld --experimental-pic -shared -o %t.ret32.wasm %t.o %T/libret32.a
+# RUN: obj2yaml %t.wasm | FileCheck %s
+# RUN: llvm-objdump -d %t.wasm | FileCheck %s -check-prefix=ASM
+
 # Verify the weak undefined symbols are marked as such in the
 # dylink section.
 
 .weak weak_func
 .functype weak_func () -> (i32)
+.weak ret32
+.functype ret32 (f32) -> (i32)
 
 .globl call_weak
 call_weak:
 # ASM: <call_weak>:
   .functype call_weak () -> (i32)
   call weak_func
 # ASM:           10 80 80 80 80 00      call  0
   end_function
 # ASM-NEXT:      0b                     end
 
+# This function is defined in library archive, but since our reference to it
+# is weak we don't expect this definition to be used.  Instead we expect it to
+# act like an undefined reference and result in an imported function.
+.globl call_weak_libfunc
+call_weak_libfunc:
+# ASM: <call_weak_libfunc>:
+  .functype call_weak_libfunc () -> (i32)
+  f32.const 1.0
+  call ret32
+# ASM:           10 81 80 80 80 00 call 1
+  end_function
+# ASM-NEXT:      0b                     end
+
 #      CHECK: Sections:
 # CHECK-NEXT:   - Type:            CUSTOM
 # CHECK-NEXT:     Name:            dylink.0
 # CHECK-NEXT:     MemorySize:      0
 # CHECK-NEXT:     MemoryAlignment: 0
 # CHECK-NEXT:     TableSize:       0
 # CHECK-NEXT:     TableAlignment:  0
 # CHECK-NEXT:     Needed:          []
 # CHECK-NEXT:     ImportInfo:
 # CHECK-NEXT:       - Module:          env
 # CHECK-NEXT:         Field:           weak_func
 # CHECK-NEXT:         Flags:           [ BINDING_WEAK, UNDEFINED ]
+# CHECK-NEXT:       - Module:          env
+# CHECK-NEXT:         Field:           ret32
+# CHECK-NEXT:         Flags:           [ BINDING_WEAK, UNDEFINED ]

lld/wasm/Driver.cpp

static void handleLibcall(StringRef name) {
 
   if (auto *lazySym = dyn_cast<LazySymbol>(sym)) {
     MemoryBufferRef mb = lazySym->getMemberBuffer();
     if (isBitcode(mb))
       lazySym->fetch();
   }
 }
 
+// Equivalent of demote demoteSharedAndLazySymbols() in the ELF linker
+static void demoteLazySymbols() {
+  for (Symbol *sym : symtab->getSymbols()) {
+    if (auto* s = dyn_cast<LazySymbol>(sym)) {
+      if (s->signature) {
+        LLVM_DEBUG(llvm::dbgs()
+                   << "demoting lazy func: " << s->getName() << "\n");
+        replaceSymbol<UndefinedFunction>(s, s->getName(), None, None,
+                                         WASM_SYMBOL_BINDING_WEAK, s->getFile(),
+                                         s->signature);
+      }
+    }
+  }
+}
+
 static UndefinedGlobal *
 createUndefinedGlobal(StringRef name, llvm::wasm::WasmGlobalType *type) {
   auto *sym = cast<UndefinedGlobal>(symtab->addUndefinedGlobal(
       name, None, None, WASM_SYMBOL_UNDEFINED, nullptr, type));
   config->allowUndefinedSymbols.insert(sym->getName());
   sym->isUsedInRegularObj = true;
   return sym;
 }
void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   if (errorCount())
     return;
 
   // Split WASM_SEG_FLAG_STRINGS sections into pieces in preparation for garbage
   // collection.
   splitSections();
 
+  // Any remaining lazy symbols should be demoted to Undefined
+  demoteLazySymbols();
+
   // Do size optimizations: garbage collection
   markLive();
 
   // Provide the indirect function table if needed.
   WasmSym::indirectFunctionTable =
       symtab->resolveIndirectFunctionTable(/*required =*/false);
 
   if (errorCount())
     return;
 
   // Write the result to the file.
   writeResult();
 }
 
 } // namespace wasm
 } // namespace lld

lld/wasm/SymbolTable.cpp

std::pair<Symbol *, bool> SymbolTable::insertName(StringRef name) {
   if (!isNew)
     return {symVector[symIndex], false};
 
   Symbol *sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
   sym->isUsedInRegularObj = false;
   sym->canInline = true;
   sym->traced = trace;
   sym->forceExport = false;
+  sym->referenced = !config->gcSections;
   symVector.emplace_back(sym);
   return {sym, true};
 }
 
 std::pair<Symbol *, bool> SymbolTable::insert(StringRef name,
                                               const InputFile *file) {
   Symbol *s;
   bool wasInserted;

lld/wasm/Symbols.h

LazySymbol(StringRef name, uint32_t flags, InputFile *file,
       : Symbol(name, LazyKind, flags, file), archiveSymbol(sym) {}
 
   static bool classof(const Symbol *s) { return s->kind() == LazyKind; }
   void fetch();
   void setWeak();
   MemoryBufferRef getMemberBuffer();
 
   // Lazy symbols can have a signature because they can replace an
-  // UndefinedFunction which which case we need to be able to preserve the
+  // UndefinedFunction in which case we need to be able to preserve the
   // signature.
   // TODO(sbc): This repetition of the signature field is inelegant.  Revisit
   // the use of class hierarchy to represent symbol taxonomy.
   const WasmSignature *signature = nullptr;
 
 private:
   llvm::object::Archive::Symbol archiveSymbol;
 };
T *replaceSymbol(Symbol *s, ArgT &&... arg) {
 
   Symbol symCopy = *s;
 
   T *s2 = new (s) T(std::forward<ArgT>(arg)...);
   s2->isUsedInRegularObj = symCopy.isUsedInRegularObj;
   s2->forceExport = symCopy.forceExport;
   s2->canInline = symCopy.canInline;
   s2->traced = symCopy.traced;
+  s2->referenced = symCopy.referenced;
 
   // Print out a log message if --trace-symbol was specified.
   // This is for debugging.
   if (s2->traced)
     printTraceSymbol(s2);
 
   return s2;
 }