Skip to content

Commit d3589ed

Browse files
MaskRayMaskRay
committed
MCAsmBackend::applyFixup: Change Data to indicate the relocated location
`Data` now references the first byte of the fixup offset within the current fragment. MCAssembler::layout asserts that the fixup offset is within either the fixed-size content or the optional variable-size tail, as this is the most the generic code can validate without knowing the target-specific fixup size. Many backends applyFixup assert ``` assert(Offset + Size <= F.getSize() && "Invalid fixup offset!"); ``` This refactoring allows a subsequent change to move the fixed-size content outside of MCSection::ContentStorage, fixing the -fsanitize=pointer-overflow issue of #150846 Pull Request: #151724
1 parent 300e41d commit d3589ed

File tree

31 files changed

+154
-204
lines changed

31 files changed

+154
-204
lines changed

‎llvm/include/llvm/MC/MCAsmBackend.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,13 @@ class LLVM_ABI MCAsmBackend {
117117
void maybeAddReloc(const MCFragment &, const MCFixup &, const MCValue &,
118118
uint64_t &Value, bool IsResolved);
119119

120-
/// Determine if a relocation is required. In addition,
121-
/// Apply the \p Value for given \p Fixup into the provided data fragment, at
122-
/// the offset specified by the fixup and following the fixup kind as
123-
/// appropriate. Errors (such as an out of range fixup value) should be
124-
/// reported via \p Ctx.
120+
// Determine if a relocation is required. In addition, apply `Value` to the
121+
// `Data` fragment at the specified fixup offset if applicable. `Data` points
122+
// to the first byte of the fixup offset, which may be at the content's end if
123+
// the fixup is zero-sized.
125124
virtual void applyFixup(const MCFragment &, const MCFixup &,
126-
const MCValue &Target, MutableArrayRef<char> Data,
127-
uint64_t Value, bool IsResolved) = 0;
125+
const MCValue &Target, uint8_t *Data, uint64_t Value,
126+
bool IsResolved) = 0;
128127

129128
/// @}
130129

‎llvm/include/llvm/MC/MCAssembler.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ class MCAssembler {
9999
/// \param RecordReloc Record relocation if needed.
100100
/// relocation.
101101
bool evaluateFixup(const MCFragment &F, MCFixup &Fixup, MCValue &Target,
102-
uint64_t &Value, bool RecordReloc,
103-
MutableArrayRef<char> Contents) const;
102+
uint64_t &Value, bool RecordReloc, uint8_t *Data) const;
104103

105104
/// Check whether a fixup can be satisfied, or whether it needs to be relaxed
106105
/// (increased in size, in order to hold its value correctly).

‎llvm/lib/MC/MCAssembler.cpp‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ bool MCAssembler::isThumbFunc(const MCSymbol *Symbol) const {
141141

142142
bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
143143
MCValue &Target, uint64_t &Value,
144-
bool RecordReloc,
145-
MutableArrayRef<char> Contents) const {
144+
bool RecordReloc, uint8_t *Data) const {
146145
if (RecordReloc)
147146
++stats::Fixups;
148147

@@ -187,7 +186,7 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
187186

188187
if (IsResolved && mc::isRelocRelocation(Fixup.getKind()))
189188
IsResolved = false;
190-
getBackend().applyFixup(F, Fixup, Target, Contents, Value, IsResolved);
189+
getBackend().applyFixup(F, Fixup, Target, Data, Value, IsResolved);
191190
return true;
192191
}
193192

@@ -705,21 +704,25 @@ void MCAssembler::layout() {
705704
for (MCFixup &Fixup : F.getFixups()) {
706705
uint64_t FixedValue;
707706
MCValue Target;
707+
assert(mc::isRelocRelocation(Fixup.getKind()) ||
708+
Fixup.getOffset() <= F.getFixedSize());
709+
auto *Data =
710+
reinterpret_cast<uint8_t *>(Contents.data() + Fixup.getOffset());
708711
evaluateFixup(F, Fixup, Target, FixedValue,
709-
/*RecordReloc=*/true, Contents);
712+
/*RecordReloc=*/true, Data);
710713
}
711-
if (F.getVarFixups().size()) {
712-
// In the variable part, fixup offsets are relative to the fixed part's
713-
// start. Extend the variable contents to the left to account for the
714-
// fixed part size.
715-
Contents = MutableArrayRef(F.getParent()->ContentStorage)
716-
.slice(F.VarContentStart - Contents.size(), F.getSize());
717-
for (MCFixup &Fixup : F.getVarFixups()) {
718-
uint64_t FixedValue;
719-
MCValue Target;
720-
evaluateFixup(F, Fixup, Target, FixedValue,
721-
/*RecordReloc=*/true, Contents);
722-
}
714+
// In the variable part, fixup offsets are relative to the fixed part's
715+
// start.
716+
for (MCFixup &Fixup : F.getVarFixups()) {
717+
uint64_t FixedValue;
718+
MCValue Target;
719+
assert(mc::isRelocRelocation(Fixup.getKind()) ||
720+
(Fixup.getOffset() >= F.getFixedSize() &&
721+
Fixup.getOffset() <= F.getSize()));
722+
auto *Data = reinterpret_cast<uint8_t *>(
723+
F.getVarContents().data() + (Fixup.getOffset() - F.getFixedSize()));
724+
evaluateFixup(F, Fixup, Target, FixedValue,
725+
/*RecordReloc=*/true, Data);
723726
}
724727
}
725728
}

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ class AArch64AsmBackend : public MCAsmBackend {
7979
}
8080

8181
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
82-
MutableArrayRef<char> Data, uint64_t Value,
83-
bool IsResolved) override;
82+
uint8_t *Data, uint64_t Value, bool IsResolved) override;
8483

8584
bool fixupNeedsRelaxation(const MCFixup &Fixup,
8685
uint64_t Value) const override;
@@ -421,9 +420,8 @@ static bool shouldForceRelocation(const MCFixup &Fixup) {
421420
}
422421

423422
void AArch64AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
424-
const MCValue &Target,
425-
MutableArrayRef<char> Data, uint64_t Value,
426-
bool IsResolved) {
423+
const MCValue &Target, uint8_t *Data,
424+
uint64_t Value, bool IsResolved) {
427425
if (shouldForceRelocation(Fixup))
428426
IsResolved = false;
429427
maybeAddReloc(F, Fixup, Target, Value, IsResolved);
@@ -460,8 +458,8 @@ void AArch64AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
460458
// Shift the value into position.
461459
Value <<= Info.TargetOffset;
462460

463-
unsigned Offset = Fixup.getOffset();
464-
assert(Offset + NumBytes <= F.getSize() && "Invalid fixup offset!");
461+
assert(Fixup.getOffset() + NumBytes <= F.getSize() &&
462+
"Invalid fixup offset!");
465463

466464
// Used to point to big endian bytes.
467465
unsigned FulleSizeInBytes = getFixupKindContainereSizeInBytes(Fixup.getKind());
@@ -471,15 +469,16 @@ void AArch64AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
471469
if (FulleSizeInBytes == 0) {
472470
// Handle as little-endian
473471
for (unsigned i = 0; i != NumBytes; ++i) {
474-
Data[Offset + i] |= uint8_t((Value >> (i * 8)) & 0xff);
472+
Data[i] |= uint8_t((Value >> (i * 8)) & 0xff);
475473
}
476474
} else {
477475
// Handle as big-endian
478-
assert((Offset + FulleSizeInBytes) <= Data.size() && "Invalid fixup size!");
476+
assert(Fixup.getOffset() + FulleSizeInBytes <= F.getSize() &&
477+
"Invalid fixup size!");
479478
assert(NumBytes <= FulleSizeInBytes && "Invalid fixup size!");
480479
for (unsigned i = 0; i != NumBytes; ++i) {
481480
unsigned Idx = FulleSizeInBytes - 1 - i;
482-
Data[Offset + Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
481+
Data[Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
483482
}
484483
}
485484

@@ -492,9 +491,9 @@ void AArch64AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
492491
// If the immediate is negative, generate MOVN else MOVZ.
493492
// (Bit 30 = 0) ==> MOVN, (Bit 30 = 1) ==> MOVZ.
494493
if (SignedValue < 0)
495-
Data[Offset + 3] &= ~(1 << 6);
494+
Data[3] &= ~(1 << 6);
496495
else
497-
Data[Offset + 3] |= (1 << 6);
496+
Data[3] |= (1 << 6);
498497
}
499498
}
500499

‎llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ class AMDGPUAsmBackend : public MCAsmBackend {
3333
AMDGPUAsmBackend(const Target &T) : MCAsmBackend(llvm::endianness::little) {}
3434

3535
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
36-
MutableArrayRef<char> Data, uint64_t Value,
37-
bool IsResolved) override;
36+
uint8_t *Data, uint64_t Value, bool IsResolved) override;
3837
bool fixupNeedsRelaxation(const MCFixup &Fixup,
3938
uint64_t Value) const override;
4039

@@ -129,9 +128,8 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
129128
}
130129

131130
void AMDGPUAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
132-
const MCValue &Target,
133-
MutableArrayRef<char> Data, uint64_t Value,
134-
bool IsResolved) {
131+
const MCValue &Target, uint8_t *Data,
132+
uint64_t Value, bool IsResolved) {
135133
if (Target.getSpecifier())
136134
IsResolved = false;
137135
maybeAddReloc(F, Fixup, Target, Value, IsResolved);
@@ -148,13 +146,13 @@ void AMDGPUAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
148146
Value <<= Info.TargetOffset;
149147

150148
unsigned NumBytes = getFixupKindNumBytes(Fixup.getKind());
151-
uint32_t Offset = Fixup.getOffset();
152-
assert(Offset + NumBytes <= F.getSize() && "Invalid fixup offset!");
149+
assert(Fixup.getOffset() + NumBytes <= F.getSize() &&
150+
"Invalid fixup offset!");
153151

154152
// For each byte of the fragment that the fixup touches, mask in the bits from
155153
// the fixup value.
156154
for (unsigned i = 0; i != NumBytes; ++i)
157-
Data[Offset + i] |= static_cast<uint8_t>((Value >> (i * 8)) & 0xff);
155+
Data[i] |= static_cast<uint8_t>((Value >> (i * 8)) & 0xff);
158156
}
159157

160158
std::optional<MCFixupKind>

‎llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,9 +1108,8 @@ std::optional<bool> ARMAsmBackend::evaluateFixup(const MCFragment &F,
11081108
}
11091109

11101110
void ARMAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
1111-
const MCValue &Target,
1112-
MutableArrayRef<char> Data, uint64_t Value,
1113-
bool IsResolved) {
1111+
const MCValue &Target, uint8_t *Data,
1112+
uint64_t Value, bool IsResolved) {
11141113
if (IsResolved && shouldForceRelocation(Fixup, Target))
11151114
IsResolved = false;
11161115
maybeAddReloc(F, Fixup, Target, Value, IsResolved);
@@ -1124,14 +1123,15 @@ void ARMAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
11241123
return; // Doesn't change encoding.
11251124
const unsigned NumBytes = getFixupKindNumBytes(Kind);
11261125

1127-
unsigned Offset = Fixup.getOffset();
1128-
assert(Offset + NumBytes <= F.getSize() && "Invalid fixup offset!");
1126+
assert(Fixup.getOffset() + NumBytes <= F.getSize() &&
1127+
"Invalid fixup offset!");
11291128

11301129
// Used to point to big endian bytes.
11311130
unsigned FullSizeBytes;
11321131
if (Endian == llvm::endianness::big) {
11331132
FullSizeBytes = getFixupKindContainerSizeBytes(Kind);
1134-
assert((Offset + FullSizeBytes) <= Data.size() && "Invalid fixup size!");
1133+
assert(Fixup.getOffset() + FullSizeBytes <= F.getSize() &&
1134+
"Invalid fixup size!");
11351135
assert(NumBytes <= FullSizeBytes && "Invalid fixup size!");
11361136
}
11371137

@@ -1141,7 +1141,7 @@ void ARMAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
11411141
for (unsigned i = 0; i != NumBytes; ++i) {
11421142
unsigned Idx =
11431143
Endian == llvm::endianness::little ? i : (FullSizeBytes - 1 - i);
1144-
Data[Offset + Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
1144+
Data[Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
11451145
}
11461146
}
11471147

‎llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ class ARMAsmBackend : public MCAsmBackend {
4040
std::optional<bool> evaluateFixup(const MCFragment &, MCFixup &, MCValue &,
4141
uint64_t &) override;
4242
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
43-
MutableArrayRef<char> Data, uint64_t Value,
44-
bool IsResolved) override;
43+
uint8_t *Data, uint64_t Value, bool IsResolved) override;
4544

4645
unsigned getRelaxedOpcode(unsigned Op, const MCSubtargetInfo &STI) const;
4746

‎llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,8 @@ AVRAsmBackend::createObjectTargetWriter() const {
368368
}
369369

370370
void AVRAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
371-
const MCValue &Target,
372-
MutableArrayRef<char> Data, uint64_t Value,
373-
bool IsResolved) {
371+
const MCValue &Target, uint8_t *Data,
372+
uint64_t Value, bool IsResolved) {
374373
// AVR sets the fixup value to bypass the assembly time overflow with a
375374
// relocation.
376375
if (IsResolved) {
@@ -397,14 +396,14 @@ void AVRAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
397396
// Shift the value into position.
398397
Value <<= Info.TargetOffset;
399398

400-
unsigned Offset = Fixup.getOffset();
401-
assert(Offset + NumBytes <= F.getSize() && "Invalid fixup offset!");
399+
assert(Fixup.getOffset() + NumBytes <= F.getSize() &&
400+
"Invalid fixup offset!");
402401

403402
// For each byte of the fragment that the fixup touches, mask in the
404403
// bits from the fixup value.
405404
for (unsigned i = 0; i < NumBytes; ++i) {
406405
uint8_t mask = (((Value >> (i * 8)) & 0xff));
407-
Data[Offset + i] |= mask;
406+
Data[i] |= mask;
408407
}
409408
}
410409

‎llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ class AVRAsmBackend : public MCAsmBackend {
3838
createObjectTargetWriter() const override;
3939

4040
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
41-
MutableArrayRef<char> Data, uint64_t Value,
42-
bool IsResolved) override;
41+
uint8_t *Data, uint64_t Value, bool IsResolved) override;
4342

4443
std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
4544
MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;

‎llvm/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ class BPFAsmBackend : public MCAsmBackend {
2727
~BPFAsmBackend() override = default;
2828

2929
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
30-
MutableArrayRef<char> Data, uint64_t Value,
31-
bool IsResolved) override;
30+
uint8_t *Data, uint64_t Value, bool IsResolved) override;
3231

3332
std::unique_ptr<MCObjectTargetWriter>
3433
createObjectTargetWriter() const override;
@@ -66,35 +65,32 @@ bool BPFAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
6665
}
6766

6867
void BPFAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
69-
const MCValue &Target,
70-
MutableArrayRef<char> Data, uint64_t Value,
71-
bool IsResolved) {
68+
const MCValue &Target, uint8_t *Data,
69+
uint64_t Value, bool IsResolved) {
7270
maybeAddReloc(F, Fixup, Target, Value, IsResolved);
7371
if (Fixup.getKind() == FK_SecRel_8) {
7472
// The Value is 0 for global variables, and the in-section offset
7573
// for static variables. Write to the immediate field of the inst.
7674
assert(Value <= UINT32_MAX);
77-
support::endian::write<uint32_t>(&Data[Fixup.getOffset() + 4],
78-
static_cast<uint32_t>(Value),
75+
support::endian::write<uint32_t>(Data + 4, static_cast<uint32_t>(Value),
7976
Endian);
8077
} else if (Fixup.getKind() == FK_Data_4 && !Fixup.isPCRel()) {
81-
support::endian::write<uint32_t>(&Data[Fixup.getOffset()], Value, Endian);
78+
support::endian::write<uint32_t>(Data, Value, Endian);
8279
} else if (Fixup.getKind() == FK_Data_8) {
83-
support::endian::write<uint64_t>(&Data[Fixup.getOffset()], Value, Endian);
80+
support::endian::write<uint64_t>(Data, Value, Endian);
8481
} else if (Fixup.getKind() == FK_Data_4 && Fixup.isPCRel()) {
8582
Value = (uint32_t)((Value - 8) / 8);
8683
if (Endian == llvm::endianness::little) {
87-
Data[Fixup.getOffset() + 1] = 0x10;
88-
support::endian::write32le(&Data[Fixup.getOffset() + 4], Value);
84+
Data[1] = 0x10;
85+
support::endian::write32le(Data + 4, Value);
8986
} else {
90-
Data[Fixup.getOffset() + 1] = 0x1;
91-
support::endian::write32be(&Data[Fixup.getOffset() + 4], Value);
87+
Data[1] = 0x1;
88+
support::endian::write32be(Data + 4, Value);
9289
}
9390
} else if (Fixup.getKind() == BPF::FK_BPF_PCRel_4) {
9491
// The input Value represents the number of bytes.
9592
Value = (uint32_t)((Value - 8) / 8);
96-
support::endian::write<uint32_t>(&Data[Fixup.getOffset() + 4], Value,
97-
Endian);
93+
support::endian::write<uint32_t>(Data + 4, Value, Endian);
9894
} else {
9995
assert(Fixup.getKind() == FK_Data_2 && Fixup.isPCRel());
10096

@@ -103,8 +99,7 @@ void BPFAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
10399
report_fatal_error("Branch target out of insn range");
104100

105101
Value = (uint16_t)((Value - 8) / 8);
106-
support::endian::write<uint16_t>(&Data[Fixup.getOffset() + 2], Value,
107-
Endian);
102+
support::endian::write<uint16_t>(Data + 2, Value, Endian);
108103
}
109104
}
110105

0 commit comments

Comments
 (0)