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) {

‎llvm/lib/MC/MCSymbolELF.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ enum {
3030
ELF_IsSignature_Shift = 10,
3131

3232
// One bit.
33-
ELF_WeakrefUsedInReloc_Shift = 11,
33+
ELF_Weakref_Shift = 11,
3434

3535
// One bit.
3636
ELF_BindingSet_Shift = 12,
@@ -84,8 +84,6 @@ unsigned MCSymbolELF::getBinding() const {
8484
return ELF::STB_LOCAL;
8585
if (isUsedInReloc())
8686
return ELF::STB_GLOBAL;
87-
if (isWeakrefUsedInReloc())
88-
return ELF::STB_WEAK;
8987
if (isSignature())
9088
return ELF::STB_LOCAL;
9189
return ELF::STB_GLOBAL;
@@ -170,13 +168,13 @@ unsigned MCSymbolELF::getOther() const {
170168
return Other << 5;
171169
}
172170

173-
void MCSymbolELF::setIsWeakrefUsedInReloc() const {
174-
uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_WeakrefUsedInReloc_Shift);
175-
setFlags(OtherFlags | (1 << ELF_WeakrefUsedInReloc_Shift));
171+
void MCSymbolELF::setIsWeakref() const {
172+
uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_Weakref_Shift);
173+
setFlags(OtherFlags | (1 << ELF_Weakref_Shift));
176174
}
177175

178-
bool MCSymbolELF::isWeakrefUsedInReloc() const {
179-
return getFlags() & (0x1 << ELF_WeakrefUsedInReloc_Shift);
176+
bool MCSymbolELF::isWeakref() const {
177+
return getFlags() & (0x1 << ELF_Weakref_Shift);
180178
}
181179

182180
void MCSymbolELF::setIsSignature() const {

‎llvm/lib/MC/MCWasmStreamer.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,6 @@ void MCWasmStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
7070
Asm.registerSymbol(*Section->getBeginSymbol());
7171
}
7272

73-
void MCWasmStreamer::emitWeakReference(MCSymbol *Alias,
74-
const MCSymbol *Symbol) {
75-
getAssembler().registerSymbol(*Symbol);
76-
const MCExpr *Value = MCSymbolRefExpr::create(
77-
Symbol, MCSymbolRefExpr::VK_WEAKREF, getContext());
78-
Alias->setVariableValue(Value);
79-
}
80-
8173
bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
8274
assert(Attribute != MCSA_IndirectSymbol && "indirect symbols not supported");
8375

‎llvm/lib/MC/WasmObjectWriter.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -525,13 +525,6 @@ void WasmObjectWriter::recordRelocation(const MCFragment &F,
525525
return;
526526
}
527527

528-
if (SymA->isVariable()) {
529-
const MCExpr *Expr = SymA->getVariableValue();
530-
if (const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr))
531-
if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF)
532-
llvm_unreachable("weakref used in reloc not yet implemented");
533-
}
534-
535528
// Put any constant offset in an addend. Offsets can be negative, and
536529
// LLVM expects wrapping, in contrast to wasm's immediates which can't
537530
// be negative and don't wrap.

‎llvm/test/MC/ELF/weakref.s

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readelf -s - | FileCheck %s
2+
# 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:
23

34
// This is a long test that checks that the aliases created by weakref are
45
// never in the symbol table and that the only case it causes a symbol to
@@ -12,17 +13,18 @@
1213
# CHECK-NEXT: 3: 0000000000000018 0 NOTYPE LOCAL DEFAULT 2 bar7
1314
# CHECK-NEXT: 4: 000000000000001c 0 NOTYPE LOCAL DEFAULT 2 bar8
1415
# CHECK-NEXT: 5: 0000000000000020 0 NOTYPE LOCAL DEFAULT 2 bar9
15-
# CHECK-NEXT: 6: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar1
16-
# CHECK-NEXT: 7: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar2
17-
# CHECK-NEXT: 8: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar3
18-
# CHECK-NEXT: 9: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar4
19-
# CHECK-NEXT: 10: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar5
20-
# CHECK-NEXT: 11: 0000000000000028 0 NOTYPE GLOBAL DEFAULT 2 bar10
21-
# CHECK-NEXT: 12: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar11
22-
# CHECK-NEXT: 13: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar12
23-
# CHECK-NEXT: 14: 0000000000000034 0 NOTYPE GLOBAL DEFAULT 2 bar13
24-
# CHECK-NEXT: 15: 0000000000000038 0 NOTYPE GLOBAL DEFAULT 2 bar14
25-
# CHECK-NEXT: 16: 0000000000000040 0 NOTYPE GLOBAL DEFAULT 2 bar15
16+
# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar2
17+
# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar4
18+
# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND bar5
19+
# CHECK-NEXT: 0000000000000028 0 NOTYPE GLOBAL DEFAULT 2 bar10
20+
# CHECK-NEXT: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar11
21+
# CHECK-NEXT: 0000000000000030 0 NOTYPE GLOBAL DEFAULT 2 bar12
22+
# CHECK-NEXT: 0000000000000034 0 NOTYPE GLOBAL DEFAULT 2 bar13
23+
# CHECK-NEXT: 0000000000000038 0 NOTYPE GLOBAL DEFAULT 2 bar14
24+
# CHECK-NEXT: 0000000000000040 0 NOTYPE GLOBAL DEFAULT 2 bar15
25+
# CHECK-NEXT: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar3
26+
# CHECK-NEXT: 0000000000000000 0 NOTYPE WEAK DEFAULT UND bar16
27+
# CHECK-EMPTY:
2628

2729
.weakref foo1, bar1
2830

@@ -87,3 +89,20 @@ bar15:
8789
.weakref foo15, bar15
8890
.long bar15
8991
.long foo15
92+
93+
.long foo16
94+
.weakref foo16, bar16
95+
96+
.ifdef ERR
97+
alias:
98+
.weakref alias, target
99+
# ERR: [[#@LINE-1]]:1: error: symbol 'alias' is already defined
100+
101+
.set alias1, 1
102+
.weakref alias1, target
103+
# ERR: [[#@LINE-1]]:1: error: symbol 'alias1' is already defined
104+
105+
.weakref alias2, target
106+
.set alias2, 1
107+
108+
.endif

0 commit comments

Comments
 (0)