[lld][WebAssembly] Handle weakly referenced symbols when lazy (archive) version is see first

[sbc100]

Summary

When a weak reference of a lazy synmbol occurs we were not correctly
updating the lazy symbol. We need to tag the existing lazy symbol
as weak and, in the case of a function symbol, give it a signature.

Without the signature we can't then create the dummy function which
is needed when an weakly undefined function is called.

We had tests for weakly referenced lazy symbols but we were only
tests in the case where the reference was seen before the lazy
symbol.

See: https://github.com/WebAssembly/wasi-libc/pull/214

[alexcrichton]

Nice find!

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

-; Test that weak undefined symbols do not fetch members from archive files.
+;; Test that weak undefined symbols do not fetch members from archive files.
 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/ret32.s -o %t.ret32.o
 ; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/hello.s -o %t.hello.o
 ; RUN: rm -f %t.a
 ; RUN: llvm-ar rcs %t.a %t.ret32.o %t.hello.o
 
 ; RUN: wasm-ld %t.o %t.a -o %t.wasm
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
+;; Also test with the library symbols being read first
+; RUN: wasm-ld %t.a %t.o -o %t2.wasm
+; RUN: obj2yaml %t2.wasm | FileCheck %s
+
 ; RUN: wasm-ld -u hello_str %t.o %t.a -o %t2.wasm
 ; RUN: obj2yaml %t2.wasm | FileCheck %s -check-prefix=CHECK-DATA
 
 target triple = "wasm32-unknown-unknown"
 
 ; Weak external function symbol
 declare extern_weak i32 @ret32()
 

lld/wasm/SymbolTable.cpp

Symbol *SymbolTable::addUndefinedFunction(StringRef name,
   if (s->traced)
     printTraceSymbolUndefined(name, file);
 
   auto replaceSym = [&]() {
     replaceSymbol<UndefinedFunction>(s, name, importName, importModule, flags,
                                      file, sig, isCalledDirectly);
   };
 
-  if (wasInserted)
+  if (wasInserted) {
     replaceSym();
-  else if (auto *lazy = dyn_cast<LazySymbol>(s))
+  } else if (auto *lazy = dyn_cast<LazySymbol>(s)) {
+    if ((flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK) {
+      lazy->setWeak();
+      lazy->signature = sig;
+    } else {
       lazy->fetch();
-  else {
+    }
+  } else {
     auto existingFunction = dyn_cast<FunctionSymbol>(s);
     if (!existingFunction) {
       reportTypeError(s, file, WASM_SYMBOL_TYPE_FUNCTION);
       return s;
     }
     if (!existingFunction->signature && sig)
       existingFunction->signature = sig;
     auto *existingUndefined = dyn_cast<UndefinedFunction>(existingFunction);
Symbol *SymbolTable::addUndefinedData(StringRef name, uint32_t flags,
   assert(flags & WASM_SYMBOL_UNDEFINED);
 
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
   if (s->traced)
     printTraceSymbolUndefined(name, file);
 
-  if (wasInserted)
+  if (wasInserted) {
     replaceSymbol<UndefinedData>(s, name, flags, file);
-  else if (auto *lazy = dyn_cast<LazySymbol>(s))
+  } else if (auto *lazy = dyn_cast<LazySymbol>(s)) {
+    if ((flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK)
+      lazy->setWeak();
+    else
       lazy->fetch();
-  else if (s->isDefined())
+  } else if (s->isDefined()) {
     checkDataType(s, file);
+  }
   return s;
 }
 
 Symbol *SymbolTable::addUndefinedGlobal(StringRef name,
                                         Optional<StringRef> importName,
                                         Optional<StringRef> importModule,
                                         uint32_t flags, InputFile *file,
                                         const WasmGlobalType *type) {

lld/wasm/Symbols.h

 class LazySymbol : public Symbol {
 public:
   LazySymbol(StringRef name, uint32_t flags, InputFile *file,
              const llvm::object::Archive::Symbol &sym)
       : 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
   // 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;

lld/wasm/Symbols.cpp

 
 const OutputSectionSymbol *SectionSymbol::getOutputSectionSymbol() const {
   assert(section->outputSec && section->outputSec->sectionSym);
   return section->outputSec->sectionSym;
 }
 
 void LazySymbol::fetch() { cast<ArchiveFile>(file)->addMember(&archiveSymbol); }
 
+void LazySymbol::setWeak() {
+  flags |= (flags & ~WASM_SYMBOL_BINDING_MASK) | WASM_SYMBOL_BINDING_WEAK;
+}
+
 MemoryBufferRef LazySymbol::getMemberBuffer() {
   Archive::Child c =
       CHECK(archiveSymbol.getMember(),
             "could not get the member for symbol " + toString(*this));
 
   return CHECK(c.getMemoryBufferRef(),
                "could not get the buffer for the member defining symbol " +
                    toString(*this));