MC: Rework .weakref

Use a variable symbol without any specifier instead of VK_WEAKREF.
Add code in ELFObjectWriter::executePostLayoutBinding to check
whether the target should be made an undefined weak symbol.

This change fixes several issues:

* Unreferenced `.weakref alias, target` no longer creates an undefined `target`.
* When `alias` is already defined, report an error instead of crashing.

.weakref is specific to ELF. llvm-ml has reused the VK_WEAKREF name for
a different concept. wasm incorrectly copied the ELF implementation.
Remove it.

Author: MaskRay

lld/test/ELF/amdgpu-relocs.s

@@ -110,10 +110,10 @@ foo:
 # CHECK-NEXT: R_AMDGPU_ABS64 weak_var0 0x0
 # CHECK-NEXT: R_AMDGPU_ABS64 weak_var1 0x0
 # CHECK-NEXT: R_AMDGPU_ABS64 weak_var2 0x0
+# CHECK-NEXT: R_AMDGPU_ABS64 temp 0x0
 # CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var0 0x0
 # CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var1 0x0
 # CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var2 0x0
-# CHECK-NEXT: R_AMDGPU_ABS64 temp 0x0
 # CHECK-NEXT: }
 # CHECK-NEXT: ]
 

llvm/include/llvm/MC/MCELFObjectWriter.h

@@ -149,6 +149,8 @@ class ELFObjectWriter final : public MCObjectWriter {
 
   DenseMap<const MCSectionELF *, std::vector<ELFRelocationEntry>> Relocations;
   DenseMap<const MCSymbolELF *, const MCSymbolELF *> Renames;
+  // .weakref aliases
+  SmallVector<const MCSymbolELF *, 0> Weakrefs;
   bool IsLittleEndian = false;
   bool SeenGnuAbi = false;
   std::optional<uint8_t> OverrideABIVersion;

llvm/include/llvm/MC/MCELFStreamer.h

@@ -53,7 +53,7 @@ class MCELFStreamer : public MCObjectStreamer {
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void emitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCDataFragment &F,
                       uint64_t Offset) override;
-  void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
+  void emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) override;
   bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         Align ByteAlignment) override;

llvm/include/llvm/MC/MCObjectStreamer.h

@@ -119,7 +119,7 @@ class MCObjectStreamer : public MCStreamer {
                      SMLoc Loc = SMLoc()) override;
   void emitULEB128Value(const MCExpr *Value) override;
   void emitSLEB128Value(const MCExpr *Value) override;
-  void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
+  void emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) override;
   void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
   void switchSectionNoPrint(MCSection *Section) override;
   void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;

llvm/include/llvm/MC/MCSymbolELF.h

@@ -38,8 +38,8 @@ class MCSymbolELF : public MCSymbol {
 
   bool isBindingSet() const;
 
-  void setIsWeakrefUsedInReloc() const;
-  bool isWeakrefUsedInReloc() const;
+  void setIsWeakref() const;
+  bool isWeakref() const;
 
   void setIsSignature() const;
   bool isSignature() const;

llvm/include/llvm/MC/MCWasmStreamer.h

@@ -44,7 +44,6 @@ class MCWasmStreamer : public MCObjectStreamer {
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void emitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCDataFragment &F,
                       uint64_t Offset) override;
-  void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
   bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         Align ByteAlignment) override;

llvm/lib/MC/ELFObjectWriter.cpp

@@ -476,10 +476,9 @@ bool ELFWriter::isInSymtab(const MCSymbolELF &Symbol, bool Used, bool Renamed) {
     if (const auto *T = dyn_cast<MCTargetExpr>(Expr))
       if (T->inlineAssignedExpr())
         return false;
-    if (const MCSymbolRefExpr *Ref = dyn_cast<MCSymbolRefExpr>(Expr)) {
-      if (Ref->getKind() == MCSymbolRefExpr::VK_WEAKREF)
-        return false;
-    }
+    // The .weakref alias does not appear in the symtab.
+    if (Symbol.isWeakref())
+      return false;
   }
 
   if (Used)
@@ -531,10 +530,8 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
   for (auto It : llvm::enumerate(Asm.symbols())) {
     const auto &Symbol = cast<MCSymbolELF>(It.value());
     bool Used = Symbol.isUsedInReloc();
-    bool WeakrefUsed = Symbol.isWeakrefUsedInReloc();
     bool isSignature = Symbol.isSignature();
-
-    if (!isInSymtab(Symbol, Used || WeakrefUsed || isSignature,
+    if (!isInSymtab(Symbol, Used || isSignature,
                     OWriter.Renames.count(&Symbol)))
       continue;
 
@@ -1245,6 +1242,21 @@ void ELFObjectWriter::executePostLayoutBinding() {
       Sym = Sym->getSection().getBeginSymbol();
     Sym->setUsedInReloc();
   }
+
+  // For each `.weakref alias, target`, if the variable `alias` is registered
+  // (typically through MCObjectStreamer::visitUsedSymbol), register `target`.
+  // If `target` was unregistered before (not directly referenced or defined),
+  // make it weak.
+  for (const MCSymbol *Alias : Weakrefs) {
+    if (!Alias->isRegistered())
+      continue;
+    auto *Expr = Alias->getVariableValue();
+    if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr)) {
+      auto &Sym = cast<MCSymbolELF>(Inner->getSymbol());
+      if (Asm->registerSymbol(Sym))
+        Sym.setBinding(ELF::STB_WEAK);
+    }
+  }
 }
 
 // It is always valid to create a relocation with a symbol. It is preferable
@@ -1323,17 +1335,6 @@ void ELFObjectWriter::recordRelocation(const MCFragment &F,
   MCContext &Ctx = getContext();
 
   const auto *SymA = cast_or_null<MCSymbolELF>(Target.getAddSym());
-  bool ViaWeakRef = false;
-  if (SymA && SymA->isVariable()) {
-    const MCExpr *Expr = SymA->getVariableValue();
-    if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr)) {
-      if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF) {
-        SymA = cast<MCSymbolELF>(&Inner->getSymbol());
-        ViaWeakRef = true;
-      }
-    }
-  }
-
   const MCSectionELF *SecA = (SymA && SymA->isInSection())
                                  ? cast<MCSectionELF>(&SymA->getSection())
                                  : nullptr;
@@ -1393,10 +1394,7 @@ void ELFObjectWriter::recordRelocation(const MCFragment &F,
       if (const MCSymbolELF *R = Renames.lookup(SymA))
         SymA = R;
 
-      if (ViaWeakRef)
-        SymA->setIsWeakrefUsedInReloc();
-      else
-        SymA->setUsedInReloc();
+      SymA->setUsedInReloc();
     }
   }
   Relocations[&FixupSection].emplace_back(FixupOffset, SymA, Type, Addend);

llvm/lib/MC/MCELFStreamer.cpp

@@ -112,11 +112,16 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   Asm.registerSymbol(*Section->getBeginSymbol());
 }
 
-void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {
-  getAssembler().registerSymbol(*Symbol);
-  const MCExpr *Value = MCSymbolRefExpr::create(
-      Symbol, MCSymbolRefExpr::VK_WEAKREF, getContext());
-  Alias->setVariableValue(Value);
+void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {
+  auto *A = cast<MCSymbolELF>(Alias);
+  if (A->isDefined()) {
+    getContext().reportError(getStartTokLoc(), "symbol '" + A->getName() +
+                                                   "' is already defined");
+    return;
+  }
+  A->setVariableValue(MCSymbolRefExpr::create(Target, getContext()));
+  A->setIsWeakref();
+  getWriter().Weakrefs.push_back(A);
 }
 
 // When GNU as encounters more than one .type declaration for an object it seems

llvm/lib/MC/MCExpr.cpp

@@ -478,12 +478,7 @@ static bool canExpand(const MCSymbol &Sym, bool InSet) {
   if (Sym.isWeakExternal())
     return false;
 
-  const MCExpr *Expr = Sym.getVariableValue();
-  const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr);
-  if (Inner) {
-    if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF)
-      return false;
-  }
+  Sym.getVariableValue(true);
 
   if (InSet)
     return true;

llvm/lib/MC/MCObjectStreamer.cpp

@@ -278,8 +278,8 @@ void MCObjectStreamer::emitSLEB128Value(const MCExpr *Value) {
 }
 
 void MCObjectStreamer::emitWeakReference(MCSymbol *Alias,
-                                         const MCSymbol *Symbol) {
-  report_fatal_error("This file format doesn't support weak aliases.");
+                                         const MCSymbol *Target) {
+  reportFatalUsageError("this file format doesn't support weak aliases");
 }
 
 void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {