[lld][WebAssembly] Handle mixed strong and weak undefined symbols

When there are both strong and weak references to an undefined
symbol ensure that the strong reference prevails in the output symbol
generating the correct error.

Test case copied from lld/test/ELF/weak-and-strong-undef.s

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

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

@@ -0,0 +1,2 @@
+.functype foo () -> ()
+.weak foo

lld/test/wasm/weak-and-strong-undef.s

@@ -0,0 +1,18 @@
+# Test that when a symbol (foo) is both weakly and strongly referenced
+# the strong undefined symbol always generates an error, whichever object
+# file is seen first.
+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %S/Inputs/weak-undefined.s -o %t2.o
+# RUN: not wasm-ld %t1.o %t2.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not wasm-ld %t2.o %t1.o -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK: undefined symbol: foo
+
+_start:
+    .globl _start
+    .functype _start () -> ()
+    call foo
+    end_function
+
+.functype foo () -> ()

lld/wasm/SymbolTable.cpp

@@ -406,7 +406,7 @@ Symbol *SymbolTable::addDefinedEvent(StringRef name, uint32_t flags,
 template <typename T>
 static void setImportAttributes(T *existing, Optional<StringRef> importName,
                                 Optional<StringRef> importModule,
-                                InputFile *file) {
+                                uint32_t flags, InputFile *file) {
   if (importName) {
     if (!existing->importName)
       existing->importName = importName;
@@ -426,6 +426,12 @@ static void setImportAttributes(T *existing, Optional<StringRef> importName,
             toString(existing->getFile()) + "\n>>> defined as " +
             *importModule + " in " + toString(file));
   }
+
+  // Update symbol binding, if the existing symbol is weak
+  uint32_t binding = flags & WASM_SYMBOL_BINDING_MASK;
+  if (existing->isWeak() && binding != WASM_SYMBOL_BINDING_WEAK) {
+    existing->flags = (existing->flags & ~WASM_SYMBOL_BINDING_MASK) | binding;
+  }
 }
 
 Symbol *SymbolTable::addUndefinedFunction(StringRef name,
@@ -436,7 +442,8 @@ Symbol *SymbolTable::addUndefinedFunction(StringRef name,
                                           bool isCalledDirectly) {
   LLVM_DEBUG(dbgs() << "addUndefinedFunction: " << name << " ["
                     << (sig ? toString(*sig) : "none")
-                    << "] IsCalledDirectly:" << isCalledDirectly << "\n");
+                    << "] IsCalledDirectly:" << isCalledDirectly << " flags=0x"
+                    << utohexstr(flags) << "\n");
   assert(flags & WASM_SYMBOL_UNDEFINED);
 
   Symbol *s;
@@ -460,20 +467,21 @@ Symbol *SymbolTable::addUndefinedFunction(StringRef name,
       reportTypeError(s, file, WASM_SYMBOL_TYPE_FUNCTION);
       return s;
     }
-    auto *existingUndefined = dyn_cast<UndefinedFunction>(existingFunction);
     if (!existingFunction->signature && sig)
       existingFunction->signature = sig;
+    auto *existingUndefined = dyn_cast<UndefinedFunction>(existingFunction);
     if (isCalledDirectly && !signatureMatches(existingFunction, sig)) {
-      // If the existing undefined functions is not called direcltly then let
+      // If the existing undefined functions is not called directly then let
       // this one take precedence.  Otherwise the existing function is either
-      // direclty called or defined, in which case we need a function variant.
+      // directly called or defined, in which case we need a function variant.
       if (existingUndefined && !existingUndefined->isCalledDirectly)
         replaceSym();
       else if (getFunctionVariant(s, sig, file, &s))
         replaceSym();
     }
     if (existingUndefined)
-      setImportAttributes(existingUndefined, importName, importModule, file);
+      setImportAttributes(existingUndefined, importName, importModule, flags,
+                          file);
   }
 
   return s;
@@ -634,7 +642,7 @@ InputFunction *SymbolTable::replaceWithUnreachable(Symbol *sym,
   auto *func = make<SyntheticFunction>(sig, sym->getName(), debugName);
   func->setBody(unreachableFn);
   syntheticFunctions.emplace_back(func);
-  replaceSymbol<DefinedFunction>(sym, sym->getName(), sym->getFlags(), nullptr,
+  replaceSymbol<DefinedFunction>(sym, sym->getName(), sym->flags, nullptr,
                                  func);
   return func;
 }

lld/wasm/Symbols.h

@@ -86,8 +86,6 @@ public:
   // Returns the file from which this symbol was created.
   InputFile *getFile() const { return file; }
 
-  uint32_t getFlags() const { return flags; }
-
   InputChunk *getChunk() const;
 
   // Indicates that the section or import for this symbol will be included in
@@ -124,14 +122,12 @@ public:
 
 protected:
   Symbol(StringRef name, Kind k, uint32_t flags, InputFile *f)
-      : name(name), file(f), flags(flags), symbolKind(k),
-        referenced(!config->gcSections), requiresGOT(false),
-        isUsedInRegularObj(false), forceExport(false), canInline(false),
-        traced(false) {}
+      : name(name), file(f), symbolKind(k), referenced(!config->gcSections),
+        requiresGOT(false), isUsedInRegularObj(false), forceExport(false),
+        canInline(false), traced(false), flags(flags) {}
 
   StringRef name;
   InputFile *file;
-  uint32_t flags;
   uint32_t outputSymbolIndex = INVALID_INDEX;
   uint32_t gotIndex = INVALID_INDEX;
   Kind symbolKind;
@@ -160,6 +156,8 @@ public:
 
   // True if this symbol is specified by --trace-symbol option.
   bool traced : 1;
+
+  uint32_t flags;
 };
 
 class FunctionSymbol : public Symbol {

lld/wasm/SyntheticSections.cpp

@@ -391,7 +391,7 @@ void LinkingSection::writeBody() {
     for (const Symbol *sym : symtabEntries) {
       assert(sym->isDefined() || sym->isUndefined());
       WasmSymbolType kind = sym->getWasmType();
-      uint32_t flags = sym->getFlags();
+      uint32_t flags = sym->flags;
 
       writeU8(sub.os, kind, "sym kind");
       writeUleb128(sub.os, flags, "sym flags");