[lld][WebAssembly] Fix handling of mixed strong and weak references

[sbc100]

Summary

When adding a undefined symbols to the symbol table, if the existing
reference is weak replace the symbol flags with (potentially) non-weak
binding.

Fixes: https://github.com/llvm/llvm-project/issues/60829

[sbc100]

ptal

[dschuff]

lld/test/wasm/weak-undefined.s

48

Should these RUN lines all be at the top? that seems to be the convention in most test files AFAIK?

[sbc100]

• feedback

lld/test/wasm/Inputs/strong-refs.s

+.globl  f2
+f2:
+  .functype f2 () -> (i32)
+  i32.const global_var
+  end_function

lld/test/wasm/weak-undefined.s

+# Test that undefined weak externals (global_var) and (foo) don't cause
+# link failures and resolve to zero.
+
 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 # RUN: wasm-ld -strip-all %t.o -o %t.wasm
 # RUN: obj2yaml %t.wasm | FileCheck %s
 
-# Test that undefined weak externals (global_var) and (foo) don't cause
-# link failures and resolve to zero.
+# Also verify test that strong references in another file do cause link
+# failure (See https://github.com/llvm/llvm-project/issues/60806)
+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/strong-refs.s -o %t-strong.o
+# RUN: not wasm-ld -strip-all %t.o %t-strong.o -o %t.wasm 2>&1 | FileCheck --check-prefix=ERROR %s
+# RUN: not wasm-ld -strip-all %t-strong.o %t.o -o %t.wasm 2>&1 | FileCheck --check-prefix=ERROR %s
+
+# ERROR: undefined symbol: global_var
 
 .functype foo () -> (i32)
 
 .globl  get_address_of_foo
 get_address_of_foo:
   .functype get_address_of_foo () -> (i32)
   i32.const foo
   end_function
_start:
   drop
   call  foo
   drop
   end_function
 
 .weak foo
 .weak global_var
 
-
 # CHECK:      --- !WASM
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Version:         0x1
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Type:            TYPE
 # CHECK-NEXT:     Signatures:
 # CHECK-NEXT:       - Index:           0
 # CHECK-NEXT:         ParamTypes:      []
 # CHECK-NEXT:         ReturnTypes:
 # CHECK-NEXT:           - I32
 # CHECK-NEXT:       - Index:           1
 # CHECK-NEXT:         ParamTypes:      []

lld/wasm/SymbolTable.cpp

if (isCalledDirectly && !signatureMatches(existingFunction, sig)) {
       else if (getFunctionVariant(s, sig, file, &s))
         replaceSym();
     }
     if (existingUndefined) {
       setImportAttributes(existingUndefined, importName, importModule, flags,
                           file);
       if (isCalledDirectly)
         existingUndefined->isCalledDirectly = true;
+      if (s->isWeak())
+        s->flags = flags;
     }
   }
 
   return s;
 }
 
 Symbol *SymbolTable::addUndefinedData(StringRef name, uint32_t flags,
                                       InputFile *file) {
if (wasInserted) {
     replaceSymbol<UndefinedData>(s, name, flags, file);
   } 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()) {
     checkDataType(s, file);
+  } else if (s->isWeak()) {
+    s->flags = flags;
   }
   return s;
 }
 
 Symbol *SymbolTable::addUndefinedGlobal(StringRef name,
                                         std::optional<StringRef> importName,
                                         std::optional<StringRef> importModule,
                                         uint32_t flags, InputFile *file,
Symbol *SymbolTable::addUndefinedGlobal(StringRef name,
 
   if (wasInserted)
     replaceSymbol<UndefinedGlobal>(s, name, importName, importModule, flags,
                                    file, type);
   else if (auto *lazy = dyn_cast<LazySymbol>(s))
     lazy->fetch();
   else if (s->isDefined())
     checkGlobalType(s, file, type);
+  else if (s->isWeak())
+    s->flags = flags;
   return s;
 }
 
 Symbol *SymbolTable::addUndefinedTable(StringRef name,
                                        std::optional<StringRef> importName,
                                        std::optional<StringRef> importModule,
                                        uint32_t flags, InputFile *file,
                                        const WasmTableType *type) {
   LLVM_DEBUG(dbgs() << "addUndefinedTable: " << name << "\n");
   assert(flags & WASM_SYMBOL_UNDEFINED);
 
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
   if (s->traced)
     printTraceSymbolUndefined(name, file);
 
   if (wasInserted)
     replaceSymbol<UndefinedTable>(s, name, importName, importModule, flags,
                                   file, type);
   else if (auto *lazy = dyn_cast<LazySymbol>(s))
     lazy->fetch();
   else if (s->isDefined())
     checkTableType(s, file, type);
+  else if (s->isWeak())
+    s->flags = flags;
   return s;
 }
 
 Symbol *SymbolTable::addUndefinedTag(StringRef name,
                                      std::optional<StringRef> importName,
                                      std::optional<StringRef> importModule,
                                      uint32_t flags, InputFile *file,
                                      const WasmSignature *sig) {
   LLVM_DEBUG(dbgs() << "addUndefinedTag: " << name << "\n");
   assert(flags & WASM_SYMBOL_UNDEFINED);
 
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
   if (s->traced)
     printTraceSymbolUndefined(name, file);
 
   if (wasInserted)
     replaceSymbol<UndefinedTag>(s, name, importName, importModule, flags, file,
                                 sig);
   else if (auto *lazy = dyn_cast<LazySymbol>(s))
     lazy->fetch();
   else if (s->isDefined())
     checkTagType(s, file, sig);
+  else if (s->isWeak())
+    s->flags = flags;
   return s;
 }
 
 TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) {
   WasmLimits limits{0, 0, 0}; // Set by the writer.
   WasmTableType *type = make<WasmTableType>();
   type->ElemType = uint8_t(ValType::FUNCREF);
   type->Limits = limits;

llvm/lib/Object/WasmObjectFile.cpp

 #define DEBUG_TYPE "wasm-object"
 
 using namespace llvm;
 using namespace object;
 
 void WasmSymbol::print(raw_ostream &Out) const {
   Out << "Name=" << Info.Name
       << ", Kind=" << toString(wasm::WasmSymbolType(Info.Kind)) << ", Flags=0x"
-      << Twine::utohexstr(Info.Flags);
+      << Twine::utohexstr(Info.Flags) << " [";
+  switch (getBinding()) {
+    case wasm::WASM_SYMBOL_BINDING_GLOBAL: Out << "global"; break;
+    case wasm::WASM_SYMBOL_BINDING_LOCAL: Out << "local"; break;
+    case wasm::WASM_SYMBOL_BINDING_WEAK: Out << "weak"; break;
+  }
+  if (isHidden()) {
+    Out << ", hidden";
+  } else {
+    Out << ", default";
+  }
+  Out << "]";
   if (!isTypeData()) {
     Out << ", ElemIndex=" << Info.ElementIndex;
   } else if (isDefined()) {
     Out << ", Segment=" << Info.DataRef.Segment;
     Out << ", Offset=" << Info.DataRef.Offset;
     Out << ", Size=" << Info.DataRef.Size;
   }
 }