Skip to content

Commit 94821ce

Browse files
MaskRayMaskRay
committed
MCValue: Store SymA specifier at Specifier
The relocation specifier should be accessed via MCValue::Specifier. However, some targets encode the relocation specifier within SymA using MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though this method is now deprecated. This change stores the SymA specifier at Specifier as well, unifying the two code paths. * CSKY: GOT- and PLT- relocations now suppress the STT_SECTION conversion. * AArch64: https://reviews.llvm.org/D156505 added `getRefkind` check to prevent folding. This is a hack and is now removed. MCValue: Unify relocation specifier storage by storing SymA specifier at Specifier The relocation specifier is accessed via MCValue::Specifier, but some targets encoded it within SymA using MCSymbolRefExpr::SubclassData and retrieved it through the now-deprecated getAccessVariant() method. This commit unifies the two approaches by storing the SymA specifier at `Specifier` as well. Additional changes: - CSKY: GOT- and PLT- relocations now suppress STT_SECTION conversion. - AArch64: Removed the `getRefkind` check hack (introduced in https://reviews.llvm.org/D156505) that prevented folding. Removed the assertion from `getRelocType`. - RISCV: Removed the assertion from `getRelocType`. Future plans: - Replace MCSymbolRefExpr members with MCSymbol within MCValue. - Remove `getSymSpecifier` (added for migration).
1 parent aaaeb86 commit 94821ce

File tree

11 files changed

+49
-56
lines changed

11 files changed

+49
-56
lines changed

‎llvm/include/llvm/MC/MCExpr.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ class MCExpr {
135135
static bool evaluateSymbolicAdd(const MCAssembler *, const SectionAddrMap *,
136136
bool, const MCValue &,
137137
const MCSymbolRefExpr *,
138-
const MCSymbolRefExpr *, int64_t, MCValue &);
138+
const MCSymbolRefExpr *, int64_t, uint32_t,
139+
MCValue &);
139140
};
140141

141142
inline raw_ostream &operator<<(raw_ostream &OS, const MCExpr &E) {

‎llvm/include/llvm/MC/MCValue.h

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ class MCValue {
3636
int64_t Cst = 0;
3737
uint32_t Specifier = 0;
3838

39+
// SymA has a relocation specifier. This is a workaround for targets
40+
// that encode specifiers within MCSymbolRefExpr::SubclassData.
41+
bool SymSpecifier = false;
42+
3943
// SymB cannot have a specifier. Use getSubSym instead.
4044
const MCSymbolRefExpr *getSymB() const { return SymB; }
4145

@@ -67,18 +71,23 @@ class MCValue {
6771

6872
// Get the relocation specifier from SymA. This is a workaround for targets
6973
// that do not use MCValue::Specifier.
70-
uint16_t getSymSpecifier() const { return SymA->getSpecifier(); }
71-
// Get the relocation specifier from SymA, or 0 when SymA is null.
72-
uint16_t getAccessVariant() const;
74+
uint16_t getSymSpecifier() const { return Specifier; }
75+
uint16_t getAccessVariant() const { return Specifier; }
7376

7477
static MCValue get(const MCSymbolRefExpr *SymA,
75-
const MCSymbolRefExpr *SymB = nullptr,
76-
int64_t Val = 0, uint32_t RefKind = 0) {
78+
const MCSymbolRefExpr *SymB = nullptr, int64_t Val = 0,
79+
uint32_t Specifier = 0) {
7780
MCValue R;
7881
R.Cst = Val;
7982
R.SymA = SymA;
8083
R.SymB = SymB;
81-
R.Specifier = RefKind;
84+
R.Specifier = Specifier;
85+
assert(!(Specifier && SymA && SymA->getSpecifier()) &&
86+
"Specifier cannot be used with legacy SymSpecifier");
87+
if (!Specifier && SymA && SymA->getSpecifier()) {
88+
R.Specifier = SymA->getSpecifier();
89+
R.SymSpecifier = true;
90+
}
8291
return R;
8392
}
8493

‎llvm/lib/MC/MCAssembler.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
168168
IsResolved = false;
169169
} else {
170170
auto &SA = *Target.getAddSym();
171-
if (Target.getSymSpecifier() || SA.isUndefined()) {
171+
if (Target.SymSpecifier || SA.isUndefined()) {
172172
IsResolved = false;
173173
} else {
174174
IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||

‎llvm/lib/MC/MCExpr.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
439439
const MCValue &LHS,
440440
const MCSymbolRefExpr *RhsAdd,
441441
const MCSymbolRefExpr *RhsSub, int64_t RHS_Cst,
442-
MCValue &Res) {
442+
uint32_t RhsSpec, MCValue &Res) {
443443
const MCSymbol *LHS_A = LHS.getAddSym();
444444
const MCSymbol *LHS_B = LHS.getSubSym();
445445
int64_t LHS_Cst = LHS.getConstant();
@@ -451,16 +451,16 @@ bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
451451
int64_t Result_Cst = LHS_Cst + RHS_Cst;
452452

453453
// If we have a layout, we can fold resolved differences.
454-
if (Asm) {
454+
if (Asm && !LHS.getSpecifier() && !RhsSpec) {
455455
// While LHS_A-LHS_B and RHS_A-RHS_B from recursive calls have already been
456456
// folded, reassociating terms in
457457
// Result = (LHS_A - LHS_B + LHS_Cst) + (RHS_A - RHS_B + RHS_Cst).
458458
// might bring more opportunities.
459-
if (LHS_A && RHS_B && !LHS.getSymA()->getSpecifier()) {
459+
if (LHS_A && RHS_B) {
460460
attemptToFoldSymbolOffsetDifference(Asm, Addrs, InSet, LHS_A, RHS_B,
461461
Result_Cst);
462462
}
463-
if (RHS_A && LHS_B && !RhsAdd->getSpecifier()) {
463+
if (RHS_A && LHS_B) {
464464
attemptToFoldSymbolOffsetDifference(Asm, Addrs, InSet, RHS_A, LHS_B,
465465
Result_Cst);
466466
}
@@ -476,7 +476,12 @@ bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
476476
auto *B = LHS_B ? LHS.getSymB() : RHS_B ? RhsSub : nullptr;
477477
if (B && B->getKind() != MCSymbolRefExpr::VK_None)
478478
return false;
479+
auto Spec = LHS.getSpecifier();
480+
if (!Spec)
481+
Spec = RhsSpec;
479482
Res = MCValue::get(A, B, Result_Cst);
483+
Res.Specifier = Spec;
484+
Res.SymSpecifier = 0;
480485
return true;
481486
}
482487

@@ -634,9 +639,6 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
634639
return false;
635640
case MCBinaryExpr::Add:
636641
case MCBinaryExpr::Sub:
637-
// TODO: Prevent folding for AArch64 @AUTH operands.
638-
if (LHSValue.getSpecifier() || RHSValue.getSpecifier())
639-
return false;
640642
if (Op == MCBinaryExpr::Sub) {
641643
std::swap(RHSValue.SymA, RHSValue.SymB);
642644
RHSValue.Cst = -(uint64_t)RHSValue.Cst;
@@ -652,7 +654,8 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
652654
return true;
653655
}
654656
return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue.SymA,
655-
RHSValue.SymB, RHSValue.Cst, Res);
657+
RHSValue.SymB, RHSValue.Cst,
658+
RHSValue.SymSpecifier, Res);
656659
}
657660
}
658661

‎llvm/lib/MC/MCValue.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,3 @@ LLVM_DUMP_METHOD void MCValue::dump() const {
4343
print(dbgs());
4444
}
4545
#endif
46-
47-
uint16_t MCValue::getAccessVariant() const {
48-
if (!SymA)
49-
return 0;
50-
return SymA->getSpecifier();
51-
}

‎llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,8 +426,13 @@ void AArch64AsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
426426
AArch64MCExpr::Specifier SymLoc = AArch64MCExpr::getSymbolLoc(RefKind);
427427
if (SymLoc == AArch64AuthMCExpr::VK_AUTH ||
428428
SymLoc == AArch64AuthMCExpr::VK_AUTHADDR) {
429+
const auto *Expr = dyn_cast<AArch64AuthMCExpr>(Fixup.getValue());
430+
if (!Expr) {
431+
Asm.getContext().reportError(Fixup.getValue()->getLoc(),
432+
"expected relocatable expression");
433+
return;
434+
}
429435
assert(Value == 0);
430-
const auto *Expr = cast<AArch64AuthMCExpr>(Fixup.getValue());
431436
Value = (uint64_t(Expr->getDiscriminator()) << 32) |
432437
(uint64_t(Expr->getKey()) << 60) |
433438
(uint64_t(Expr->hasAddressDiversity()) << 63);

‎llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,6 @@ unsigned AArch64ELFObjectWriter::getRelocType(MCContext &Ctx,
116116
AArch64MCExpr::Specifier SymLoc = AArch64MCExpr::getSymbolLoc(RefKind);
117117
bool IsNC = AArch64MCExpr::isNotChecked(RefKind);
118118

119-
assert((!Target.getAddSym() ||
120-
Target.getSymSpecifier() == AArch64MCExpr::None ||
121-
Target.getSymSpecifier() == AArch64MCExpr::VK_PLT ||
122-
Target.getSymSpecifier() == AArch64MCExpr::VK_GOTPCREL) &&
123-
"Should only be expression-level modifiers here");
124-
125119
switch (SymLoc) {
126120
case AArch64MCExpr::VK_DTPREL:
127121
case AArch64MCExpr::VK_GOTTPREL:

‎llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,9 @@ unsigned PPCELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
196196
Type = ELF::R_PPC_ADDR14; // XXX: or BRNTAKEN?_
197197
break;
198198
case PPC::fixup_ppc_half16:
199-
switch (RefKind) {
199+
switch (Modifier) {
200200
default:
201-
break;
201+
llvm_unreachable("Unsupported specifier");
202202
case PPCMCExpr::VK_LO:
203203
return ELF::R_PPC_ADDR16_LO;
204204
case PPCMCExpr::VK_HI:
@@ -217,9 +217,7 @@ unsigned PPCELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
217217
return ELF::R_PPC64_ADDR16_HIGHEST;
218218
case PPCMCExpr::VK_HIGHESTA:
219219
return ELF::R_PPC64_ADDR16_HIGHESTA;
220-
}
221-
switch (Modifier) {
222-
default: llvm_unreachable("Unsupported Modifier");
220+
223221
case PPCMCExpr::VK_None:
224222
Type = ELF::R_PPC_ADDR16;
225223
break;
@@ -373,17 +371,12 @@ unsigned PPCELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
373371
break;
374372
case PPC::fixup_ppc_half16ds:
375373
case PPC::fixup_ppc_half16dq:
376-
switch (RefKind) {
374+
switch (Modifier) {
377375
default:
378376
Ctx.reportError(Fixup.getLoc(), "invalid VariantKind");
379377
return ELF::R_PPC64_NONE;
380-
case PPCMCExpr::VK_None:
381-
break;
382378
case PPCMCExpr::VK_LO:
383379
return ELF::R_PPC64_ADDR16_LO_DS;
384-
}
385-
switch (Modifier) {
386-
default: llvm_unreachable("Unsupported Modifier");
387380
case PPCMCExpr::VK_None:
388381
Type = ELF::R_PPC64_ADDR16_DS;
389382
break;

‎llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
5050
const MCValue &Target,
5151
const MCFixup &Fixup,
5252
bool IsPCRel) const {
53-
assert((!Target.getAddSym() ||
54-
Target.getSymSpecifier() == MCSymbolRefExpr::VK_None) &&
55-
"sym@specifier should have been rejected");
5653
const MCExpr *Expr = Fixup.getValue();
5754
// Determine the type of the relocation
5855
unsigned Kind = Fixup.getTargetKind();

‎llvm/test/MC/AArch64/elf-reloc-ptrauth.s

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,6 @@ _g9:
164164
// RUN: not llvm-mc -triple=aarch64 -filetype=obj --defsym=ERROBJ=1 %s -o /dev/null 2>&1 | \
165165
// RUN: FileCheck %s --check-prefix=ERROBJ
166166

167-
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
168-
.quad sym@AUTH(ia,42) + 1
169-
170-
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
171-
.quad 1 + sym@AUTH(ia,42)
172-
173-
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
174-
.quad 1 + sym@AUTH(ia,42) + 1
175-
176167
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
177168
.quad sym@AUTH(ia,42) + sym@AUTH(ia,42)
178169

@@ -181,11 +172,17 @@ _g9:
181172
// distance remains the same. Leave it in such state as for now since it
182173
// makes code simpler: subtraction of a non-AUTH symbol and of a constant
183174
// are handled identically.
184-
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
175+
// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
185176
.quad _g9@AUTH(ia,42) - _g8
186177

187-
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
178+
// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
188179
.quad _g9@AUTH(ia,42) - _g8@AUTH(ia,42)
189180
.quad 0
190181

182+
// ERROBJ: :[[#@LINE+1]]:23: error: expected relocatable expression
183+
.quad sym@AUTH(ia,42) + 1
184+
185+
// ERROBJ: :[[#@LINE+1]]:9: error: expected relocatable expression
186+
.quad 1 + sym@AUTH(ia,42)
187+
191188
.endif // ERROBJ

‎llvm/test/MC/CSKY/relocation-specifier.s

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
# RUN: llvm-readelf -rs %t | FileCheck %s --check-prefix=READELF
33

44
# READELF: '.rela.data'
5-
# READELF: R_CKCORE_GOT32 00000000 .data + 0
6-
# READELF: R_CKCORE_PLT32 00000000 .data + 0
5+
# READELF: R_CKCORE_GOT32 00000000 local + 0
6+
# READELF: R_CKCORE_PLT32 00000000 local + 0
77

88
# READELF: TLS GLOBAL DEFAULT UND gd
99
# READELF: TLS GLOBAL DEFAULT UND ld

0 commit comments

Comments
 (0)