[llvm] 95756e6 - MC: Rework .weakref
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun May 25 21:10:01 PDT 2025
Author: Fangrui Song
Date: 2025-05-25T21:09:55-07:00
New Revision: 95756e67c230c231c616a9aeabc2eea1e2831829
URL: https://github.com/llvm/llvm-project/commit/95756e67c230c231c616a9aeabc2eea1e2831829
DIFF: https://github.com/llvm/llvm-project/commit/95756e67c230c231c616a9aeabc2eea1e2831829.diff
LOG: 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.
Added:
Modified:
lld/test/ELF/amdgpu-relocs.s
llvm/include/llvm/MC/MCELFObjectWriter.h
llvm/include/llvm/MC/MCELFStreamer.h
llvm/include/llvm/MC/MCObjectStreamer.h
llvm/include/llvm/MC/MCSymbolELF.h
llvm/include/llvm/MC/MCWasmStreamer.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/MC/MCELFStreamer.cpp
llvm/lib/MC/MCExpr.cpp
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MCSymbolELF.cpp
llvm/lib/MC/MCWasmStreamer.cpp
llvm/lib/MC/WasmObjectWriter.cpp
llvm/test/MC/ELF/weakref.s
Removed:
################################################################################
diff --git a/lld/test/ELF/amdgpu-relocs.s b/lld/test/ELF/amdgpu-relocs.s
index f0a75130d69dc..447e60b4e7af6 100644
--- a/lld/test/ELF/amdgpu-relocs.s
+++ b/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: ]
diff --git a/llvm/include/llvm/MC/MCELFObjectWriter.h b/llvm/include/llvm/MC/MCELFObjectWriter.h
index 143979fa189df..a8b00aa2b45ac 100644
--- a/llvm/include/llvm/MC/MCELFObjectWriter.h
+++ b/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;
diff --git a/llvm/include/llvm/MC/MCELFStreamer.h b/llvm/include/llvm/MC/MCELFStreamer.h
index f00b1645347f1..aba87cb19b21c 100644
--- a/llvm/include/llvm/MC/MCELFStreamer.h
+++ b/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;
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index ee747253fd869..c987bc2426e9f 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/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;
diff --git a/llvm/include/llvm/MC/MCSymbolELF.h b/llvm/include/llvm/MC/MCSymbolELF.h
index 13c2c6b13f8e1..eba99647de939 100644
--- a/llvm/include/llvm/MC/MCSymbolELF.h
+++ b/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;
diff --git a/llvm/include/llvm/MC/MCWasmStreamer.h b/llvm/include/llvm/MC/MCWasmStreamer.h
index 1b97e765fdf48..bf5b5f6c3447d 100644
--- a/llvm/include/llvm/MC/MCWasmStreamer.h
+++ b/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;
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 77b3d074f0208..a5cd0b6c2a733 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/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);
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 17c8266289217..5a514f12ce549 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/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
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index bb2c7a98ed67c..e0948de387077 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/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;
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 20938c0268fec..cbf3a84cc1f02 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/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) {
diff --git a/llvm/lib/MC/MCSymbolELF.cpp b/llvm/lib/MC/MCSymbolELF.cpp
index 5a3814867e3d8..79774db0cc8e6 100644
--- a/llvm/lib/MC/MCSymbolELF.cpp
+++ b/llvm/lib/MC/MCSymbolELF.cpp
@@ -30,7 +30,7 @@ enum {
ELF_IsSignature_Shift = 10,
// One bit.
- ELF_WeakrefUsedInReloc_Shift = 11,
+ ELF_Weakref_Shift = 11,
// One bit.
ELF_BindingSet_Shift = 12,
@@ -84,8 +84,6 @@ unsigned MCSymbolELF::getBinding() const {
return ELF::STB_LOCAL;
if (isUsedInReloc())
return ELF::STB_GLOBAL;
- if (isWeakrefUsedInReloc())
- return ELF::STB_WEAK;
if (isSignature())
return ELF::STB_LOCAL;
return ELF::STB_GLOBAL;
@@ -170,13 +168,13 @@ unsigned MCSymbolELF::getOther() const {
return Other << 5;
}
-void MCSymbolELF::setIsWeakrefUsedInReloc() const {
- uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_WeakrefUsedInReloc_Shift);
- setFlags(OtherFlags | (1 << ELF_WeakrefUsedInReloc_Shift));
+void MCSymbolELF::setIsWeakref() const {
+ uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_Weakref_Shift);
+ setFlags(OtherFlags | (1 << ELF_Weakref_Shift));
}
-bool MCSymbolELF::isWeakrefUsedInReloc() const {
- return getFlags() & (0x1 << ELF_WeakrefUsedInReloc_Shift);
+bool MCSymbolELF::isWeakref() const {
+ return getFlags() & (0x1 << ELF_Weakref_Shift);
}
void MCSymbolELF::setIsSignature() const {
diff --git a/llvm/lib/MC/MCWasmStreamer.cpp b/llvm/lib/MC/MCWasmStreamer.cpp
index 9920ff9106fca..cc731fe02f7d3 100644
--- a/llvm/lib/MC/MCWasmStreamer.cpp
+++ b/llvm/lib/MC/MCWasmStreamer.cpp
@@ -70,14 +70,6 @@ void MCWasmStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
Asm.registerSymbol(*Section->getBeginSymbol());
}
-void MCWasmStreamer::emitWeakReference(MCSymbol *Alias,
- const MCSymbol *Symbol) {
- getAssembler().registerSymbol(*Symbol);
- const MCExpr *Value = MCSymbolRefExpr::create(
- Symbol, MCSymbolRefExpr::VK_WEAKREF, getContext());
- Alias->setVariableValue(Value);
-}
-
bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
assert(Attribute != MCSA_IndirectSymbol && "indirect symbols not supported");
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 4680408dae0c5..3b82fb782f888 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -525,13 +525,6 @@ void WasmObjectWriter::recordRelocation(const MCFragment &F,
return;
}
- if (SymA->isVariable()) {
- const MCExpr *Expr = SymA->getVariableValue();
- if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr))
- if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF)
- llvm_unreachable("weakref used in reloc not yet implemented");
- }
-
// Put any constant offset in an addend. Offsets can be negative, and
// LLVM expects wrapping, in contrast to wasm's immediates which can't
// be negative and don't wrap.
diff --git a/llvm/test/MC/ELF/weakref.s b/llvm/test/MC/ELF/weakref.s
index d23d0fd715dfa..aae049f4b9f19 100644
--- a/llvm/test/MC/ELF/weakref.s
+++ b/llvm/test/MC/ELF/weakref.s
@@ -1,4 +1,5 @@
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readelf -s - | FileCheck %s
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
// This is a long test that checks that the aliases created by weakref are
// never in the symbol table and that the only case it causes a symbol to
@@ -12,17 +13,18 @@
# CHECK-NEXT: 3: 0000000000000018 0 NOTYPE LOCAL DEFAULT 2 bar7
# CHECK-NEXT: 4: 000000000000001c 0 NOTYPE LOCAL DEFAULT 2 bar8
# CHECK-NEXT: 5: 0000000000000020 0 NOTYPE LOCAL DEFAULT 2 bar9
-# CHECK-NEXT: 6: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar1
-# CHECK-NEXT: 7: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar2
-# CHECK-NEXT: 8: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar3
-# CHECK-NEXT: 9: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar4
-# CHECK-NEXT: 10: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar5
-# CHECK-NEXT: 11: 0000000000000028 0 NOTYPE GLOBAL DEFAULT 2 bar10
-# CHECK-NEXT: 12: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar11
-# CHECK-NEXT: 13: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar12
-# CHECK-NEXT: 14: 0000000000000034 0 NOTYPE GLOBAL DEFAULT 2 bar13
-# CHECK-NEXT: 15: 0000000000000038 0 NOTYPE GLOBAL DEFAULT 2 bar14
-# CHECK-NEXT: 16: 0000000000000040 0 NOTYPE GLOBAL DEFAULT 2 bar15
+# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar2
+# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar4
+# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar5
+# CHECK-NEXT: 0000000000000028 0 NOTYPE GLOBAL DEFAULT 2 bar10
+# CHECK-NEXT: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar11
+# CHECK-NEXT: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar12
+# CHECK-NEXT: 0000000000000034 0 NOTYPE GLOBAL DEFAULT 2 bar13
+# CHECK-NEXT: 0000000000000038 0 NOTYPE GLOBAL DEFAULT 2 bar14
+# CHECK-NEXT: 0000000000000040 0 NOTYPE GLOBAL DEFAULT 2 bar15
+# CHECK-NEXT: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar3
+# CHECK-NEXT: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar16
+# CHECK-EMPTY:
.weakref foo1, bar1
@@ -87,3 +89,20 @@ bar15:
.weakref foo15, bar15
.long bar15
.long foo15
+
+.long foo16
+.weakref foo16, bar16
+
+.ifdef ERR
+alias:
+.weakref alias, target
+# ERR: [[#@LINE-1]]:1: error: symbol 'alias' is already defined
+
+.set alias1, 1
+.weakref alias1, target
+# ERR: [[#@LINE-1]]:1: error: symbol 'alias1' is already defined
+
+.weakref alias2, target
+.set alias2, 1
+
+.endif
More information about the llvm-commits
mailing list