Skip to content

Commit 95756e6

Browse files
MaskRayMaskRay
committed
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.
1 parent eda3e96 commit 95756e6

14 files changed

+76
-75
lines changed

‎lld/test/ELF/amdgpu-relocs.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ foo:
110110
# CHECK-NEXT: R_AMDGPU_ABS64 weak_var0 0x0
111111
# CHECK-NEXT: R_AMDGPU_ABS64 weak_var1 0x0
112112
# CHECK-NEXT: R_AMDGPU_ABS64 weak_var2 0x0
113+
# CHECK-NEXT: R_AMDGPU_ABS64 temp 0x0
113114
# CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var0 0x0
114115
# CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var1 0x0
115116
# CHECK-NEXT: R_AMDGPU_ABS64 weakref_alias_var2 0x0
116-
# CHECK-NEXT: R_AMDGPU_ABS64 temp 0x0
117117
# CHECK-NEXT: }
118118
# CHECK-NEXT: ]
119119

‎llvm/include/llvm/MC/MCELFObjectWriter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ class ELFObjectWriter final : public MCObjectWriter {
149149

150150
DenseMap<const MCSectionELF *, std::vector<ELFRelocationEntry>> Relocations;
151151
DenseMap<const MCSymbolELF *, const MCSymbolELF *> Renames;
152+
// .weakref aliases
153+
SmallVector<const MCSymbolELF *, 0> Weakrefs;
152154
bool IsLittleEndian = false;
153155
bool SeenGnuAbi = false;
154156
std::optional<uint8_t> OverrideABIVersion;

‎llvm/include/llvm/MC/MCELFStreamer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class MCELFStreamer : public MCObjectStreamer {
5353
void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
5454
void emitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCDataFragment &F,
5555
uint64_t Offset) override;
56-
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
56+
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) override;
5757
bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
5858
void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
5959
Align ByteAlignment) override;

‎llvm/include/llvm/MC/MCObjectStreamer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class MCObjectStreamer : public MCStreamer {
119119
SMLoc Loc = SMLoc()) override;
120120
void emitULEB128Value(const MCExpr *Value) override;
121121
void emitSLEB128Value(const MCExpr *Value) override;
122-
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
122+
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) override;
123123
void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
124124
void switchSectionNoPrint(MCSection *Section) override;
125125
void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;

‎llvm/include/llvm/MC/MCSymbolELF.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ class MCSymbolELF : public MCSymbol {
3838

3939
bool isBindingSet() const;
4040

41-
void setIsWeakrefUsedInReloc() const;
42-
bool isWeakrefUsedInReloc() const;
41+
void setIsWeakref() const;
42+
bool isWeakref() const;
4343

4444
void setIsSignature() const;
4545
bool isSignature() const;

‎llvm/include/llvm/MC/MCWasmStreamer.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ class MCWasmStreamer : public MCObjectStreamer {
4444
void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
4545
void emitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCDataFragment &F,
4646
uint64_t Offset) override;
47-
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
4847
bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
4948
void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
5049
Align ByteAlignment) override;

‎llvm/lib/MC/ELFObjectWriter.cpp

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -476,10 +476,9 @@ bool ELFWriter::isInSymtab(const MCSymbolELF &Symbol, bool Used, bool Renamed) {
476476
if (const auto *T = dyn_cast<MCTargetExpr>(Expr))
477477
if (T->inlineAssignedExpr())
478478
return false;
479-
if (const MCSymbolRefExpr *Ref = dyn_cast<MCSymbolRefExpr>(Expr)) {
480-
if (Ref->getKind() == MCSymbolRefExpr::VK_WEAKREF)
481-
return false;
482-
}
479+
// The .weakref alias does not appear in the symtab.
480+
if (Symbol.isWeakref())
481+
return false;
483482
}
484483

485484
if (Used)
@@ -531,10 +530,8 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
531530
for (auto It : llvm::enumerate(Asm.symbols())) {
532531
const auto &Symbol = cast<MCSymbolELF>(It.value());
533532
bool Used = Symbol.isUsedInReloc();
534-
bool WeakrefUsed = Symbol.isWeakrefUsedInReloc();
535533
bool isSignature = Symbol.isSignature();
536-
537-
if (!isInSymtab(Symbol, Used || WeakrefUsed || isSignature,
534+
if (!isInSymtab(Symbol, Used || isSignature,
538535
OWriter.Renames.count(&Symbol)))
539536
continue;
540537

@@ -1245,6 +1242,21 @@ void ELFObjectWriter::executePostLayoutBinding() {
12451242
Sym = Sym->getSection().getBeginSymbol();
12461243
Sym->setUsedInReloc();
12471244
}
1245+
1246+
// For each `.weakref alias, target`, if the variable `alias` is registered
1247+
// (typically through MCObjectStreamer::visitUsedSymbol), register `target`.
1248+
// If `target` was unregistered before (not directly referenced or defined),
1249+
// make it weak.
1250+
for (const MCSymbol *Alias : Weakrefs) {
1251+
if (!Alias->isRegistered())
1252+
continue;
1253+
auto *Expr = Alias->getVariableValue();
1254+
if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr)) {
1255+
auto &Sym = cast<MCSymbolELF>(Inner->getSymbol());
1256+
if (Asm->registerSymbol(Sym))
1257+
Sym.setBinding(ELF::STB_WEAK);
1258+
}
1259+
}
12481260
}
12491261

12501262
// It is always valid to create a relocation with a symbol. It is preferable
@@ -1323,17 +1335,6 @@ void ELFObjectWriter::recordRelocation(const MCFragment &F,
13231335
MCContext &Ctx = getContext();
13241336

13251337
const auto *SymA = cast_or_null<MCSymbolELF>(Target.getAddSym());
1326-
bool ViaWeakRef = false;
1327-
if (SymA && SymA->isVariable()) {
1328-
const MCExpr *Expr = SymA->getVariableValue();
1329-
if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr)) {
1330-
if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF) {
1331-
SymA = cast<MCSymbolELF>(&Inner->getSymbol());
1332-
ViaWeakRef = true;
1333-
}
1334-
}
1335-
}
1336-
13371338
const MCSectionELF *SecA = (SymA && SymA->isInSection())
13381339
? cast<MCSectionELF>(&SymA->getSection())
13391340
: nullptr;
@@ -1393,10 +1394,7 @@ void ELFObjectWriter::recordRelocation(const MCFragment &F,
13931394
if (const MCSymbolELF *R = Renames.lookup(SymA))
13941395
SymA = R;
13951396

1396-
if (ViaWeakRef)
1397-
SymA->setIsWeakrefUsedInReloc();
1398-
else
1399-
SymA->setUsedInReloc();
1397+
SymA->setUsedInReloc();
14001398
}
14011399
}
14021400
Relocations[&FixupSection].emplace_back(FixupOffset, SymA, Type, Addend);

‎llvm/lib/MC/MCELFStreamer.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,16 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
112112
Asm.registerSymbol(*Section->getBeginSymbol());
113113
}
114114

115-
void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {
116-
getAssembler().registerSymbol(*Symbol);
117-
const MCExpr *Value = MCSymbolRefExpr::create(
118-
Symbol, MCSymbolRefExpr::VK_WEAKREF, getContext());
119-
Alias->setVariableValue(Value);
115+
void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {
116+
auto *A = cast<MCSymbolELF>(Alias);
117+
if (A->isDefined()) {
118+
getContext().reportError(getStartTokLoc(), "symbol '" + A->getName() +
119+
"' is already defined");
120+
return;
121+
}
122+
A->setVariableValue(MCSymbolRefExpr::create(Target, getContext()));
123+
A->setIsWeakref();
124+
getWriter().Weakrefs.push_back(A);
120125
}
121126

122127
// When GNU as encounters more than one .type declaration for an object it seems

‎llvm/lib/MC/MCExpr.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,12 +478,7 @@ static bool canExpand(const MCSymbol &Sym, bool InSet) {
478478
if (Sym.isWeakExternal())
479479
return false;
480480

481-
const MCExpr *Expr = Sym.getVariableValue();
482-
const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr);
483-
if (Inner) {
484-
if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF)
485-
return false;
486-
}
481+
Sym.getVariableValue(true);
487482

488483
if (InSet)
489484
return true;

‎llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,8 @@ void MCObjectStreamer::emitSLEB128Value(const MCExpr *Value) {
278278
}
279279

280280
void MCObjectStreamer::emitWeakReference(MCSymbol *Alias,
281-
const MCSymbol *Symbol) {
282-
report_fatal_error("This file format doesn't support weak aliases.");
281+
const MCSymbol *Target) {
282+
reportFatalUsageError("this file format doesn't support weak aliases");
283283
}
284284

285285
void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {

0 commit comments

Comments
 (0)