Skip to content

Commit bd3d4ff

Browse files
[BOLT] Skip flushing optional out-of-range pending relocs
When a pending relocation is created it is also marked whether it is optional or not. It can be optional when such relocation is added as part of an optimization (i.e., `scanExternalRefs`). When bolt tries to `flushPendingRelocations`, it safely skips any optional relocations that cannot be encoded. Background: BOLT, as part of scanExternalRefs, identifies external references from calls and creates some pending relocations for them. Those when flushed will update references to point to the optimized functions. This optimization can be disabled using `--no-scan`. BOLT can assert if any of these pending relocations cannot be encoded. This patch does not disable this optimization but instead selectively applies it given that a pending relocation is optional.
1 parent 4f805f0 commit bd3d4ff

File tree

6 files changed

+74
-18
lines changed

6 files changed

+74
-18
lines changed

bolt/include/bolt/Core/BinarySection.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,10 @@ class BinarySection {
6666
// from the original section address.
6767
RelocationSetType DynamicRelocations;
6868

69-
// Pending relocations for this section.
70-
std::vector<Relocation> PendingRelocations;
69+
/// Pending relocations for this section and whether they are optional, i.e.,
70+
/// added as part of an optimization. In that case they can be safely omitted
71+
/// if flushPendingRelocations discovers they cannot be encoded.
72+
std::vector<std::pair<Relocation, bool>> PendingRelocations;
7173

7274
struct BinaryPatch {
7375
uint64_t Offset;
@@ -375,9 +377,10 @@ class BinarySection {
375377
DynamicRelocations.emplace(Reloc);
376378
}
377379

378-
/// Add relocation against the original contents of this section.
379-
void addPendingRelocation(const Relocation &Rel) {
380-
PendingRelocations.push_back(Rel);
380+
/// Add relocation against the original contents of this section. When added
381+
/// as part of an optimization it is marked as \p Optional.
382+
void addPendingRelocation(const Relocation &Rel, bool Optional = false) {
383+
PendingRelocations.push_back({Rel, Optional});
381384
}
382385

383386
/// Add patch to the input contents of this section.

bolt/include/bolt/Core/Relocation.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,12 @@ struct Relocation {
6464
/// and \P Type mismatch occurred.
6565
static bool skipRelocationProcess(uint64_t &Type, uint64_t Contents);
6666

67-
// Adjust value depending on relocation type (make it PC relative or not)
67+
/// Adjust value depending on relocation type (make it PC relative or not).
6868
static uint64_t encodeValue(uint64_t Type, uint64_t Value, uint64_t PC);
6969

70+
/// Return true if there are enough bits to encode the relocation value.
71+
static bool canEncodeValue(uint64_t Type, uint64_t Value, uint64_t PC);
72+
7073
/// Extract current relocated value from binary contents. This is used for
7174
/// RISC architectures where values are encoded in specific bits depending
7275
/// on the relocation value. For X86, we limit to sign extending the value

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ bool BinaryFunction::scanExternalRefs() {
17121712
// Add relocations unless disassembly failed for this function.
17131713
if (!DisassemblyFailed)
17141714
for (Relocation &Rel : FunctionRelocations)
1715-
getOriginSection()->addPendingRelocation(Rel);
1715+
getOriginSection()->addPendingRelocation(Rel, /*Optional*/ true);
17161716

17171717
// Inform BinaryContext that this function symbols will not be defined and
17181718
// relocations should not be created against them.

bolt/lib/Core/BinarySection.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,19 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
174174
OS.pwrite(Patch.Bytes.data(), Patch.Bytes.size(),
175175
SectionFileOffset + Patch.Offset);
176176

177-
for (Relocation &Reloc : PendingRelocations) {
177+
uint64_t SkippedPendingRelocations = 0;
178+
for (auto &[Reloc, Optional] : PendingRelocations) {
178179
uint64_t Value = Reloc.Addend;
179180
if (Reloc.Symbol)
180181
Value += Resolver(Reloc.Symbol);
181182

183+
// Safely skip any pending relocation that cannot be encoded and was added
184+
// as part of an optimization.
185+
if (Optional && !Relocation::canEncodeValue(
186+
Reloc.Type, Value, SectionAddress + Reloc.Offset)) {
187+
++SkippedPendingRelocations;
188+
continue;
189+
}
182190
Value = Relocation::encodeValue(Reloc.Type, Value,
183191
SectionAddress + Reloc.Offset);
184192

@@ -197,6 +205,10 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
197205
}
198206

199207
clearList(PendingRelocations);
208+
209+
if (SkippedPendingRelocations > 0)
210+
LLVM_DEBUG(dbgs() << "BOLT-INFO: Skipped " << SkippedPendingRelocations
211+
<< " pending relocations as they were out of range\n");
200212
}
201213

202214
BinarySection::~BinarySection() { updateContents(nullptr, 0); }

bolt/lib/Core/Relocation.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,16 @@ static uint64_t encodeValueX86(uint64_t Type, uint64_t Value, uint64_t PC) {
365365
return Value;
366366
}
367367

368+
static bool canEncodeValueAArch64(uint64_t Type, uint64_t Value, uint64_t PC) {
369+
switch (Type) {
370+
default:
371+
llvm_unreachable("unsupported relocation");
372+
case ELF::R_AARCH64_CALL26:
373+
case ELF::R_AARCH64_JUMP26:
374+
return isInt<28>(Value - PC);
375+
}
376+
}
377+
368378
static uint64_t encodeValueAArch64(uint64_t Type, uint64_t Value, uint64_t PC) {
369379
switch (Type) {
370380
default:
@@ -397,6 +407,16 @@ static uint64_t encodeValueAArch64(uint64_t Type, uint64_t Value, uint64_t PC) {
397407
return Value;
398408
}
399409

410+
static uint64_t canEncodeValueRISCV(uint64_t Type, uint64_t Value,
411+
uint64_t PC) {
412+
switch (Type) {
413+
default:
414+
llvm_unreachable("unsupported relocation");
415+
case ELF::R_RISCV_64:
416+
return true;
417+
}
418+
}
419+
400420
static uint64_t encodeValueRISCV(uint64_t Type, uint64_t Value, uint64_t PC) {
401421
switch (Type) {
402422
default:
@@ -846,6 +866,19 @@ uint64_t Relocation::encodeValue(uint64_t Type, uint64_t Value, uint64_t PC) {
846866
}
847867
}
848868

869+
bool Relocation::canEncodeValue(uint64_t Type, uint64_t Value, uint64_t PC) {
870+
switch (Arch) {
871+
default:
872+
llvm_unreachable("Unsupported architecture");
873+
case Triple::aarch64:
874+
return canEncodeValueAArch64(Type, Value, PC);
875+
case Triple::riscv64:
876+
return canEncodeValueRISCV(Type, Value, PC);
877+
case Triple::x86_64:
878+
return true;
879+
}
880+
}
881+
849882
uint64_t Relocation::extractValue(uint64_t Type, uint64_t Contents,
850883
uint64_t PC) {
851884
switch (Arch) {

bolt/unittests/Core/BinaryContext.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -161,31 +161,36 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) {
161161
<< "Wrong forward branch value\n";
162162
}
163163

164-
// TODO: BOLT currently asserts when a relocation is out of range.
165164
TEST_P(BinaryContextTester, FlushOptionalOutOfRangePendingRelocCALL26) {
166165
if (GetParam() != Triple::aarch64)
167166
GTEST_SKIP();
168167

169168
// This test checks that flushPendingRelocations skips flushing any optional
170169
// pending relocations that cannot be encoded.
171170

171+
bool DebugFlagPrev = ::llvm::DebugFlag;
172+
::llvm::DebugFlag = true;
173+
testing::internal::CaptureStderr();
174+
172175
BinarySection &BS = BC->registerOrUpdateSection(
173176
".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
174177
MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
175178
ASSERT_TRUE(RelSymbol);
176-
BS.addPendingRelocation(
177-
Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0});
179+
BS.addPendingRelocation(Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0},
180+
/*Optional*/ true);
178181

179182
SmallVector<char> Vect;
180183
raw_svector_ostream OS(Vect);
181184

182-
// FIXME: bolt must be able to skip pending relocs that are out of range.
183-
EXPECT_DEBUG_DEATH(
184-
// Resolve relocation symbol to a high value so encoding will be out of
185-
// range.
186-
BS.flushPendingRelocations(OS,
187-
[&](const MCSymbol *S) { return 0x800000F; }),
188-
".*only PC \\+/- 128MB is allowed for direct call");
185+
// Resolve relocation symbol to a high value so encoding will be out of range.
186+
BS.flushPendingRelocations(OS, [&](const MCSymbol *S) { return 0x800000F; });
187+
188+
std::string CapturedStdErr = testing::internal::GetCapturedStderr();
189+
EXPECT_TRUE(CapturedStdErr.find("BOLT-INFO: Skipped 1 pending relocations as "
190+
"they were out of range") !=
191+
std::string::npos);
192+
193+
::llvm::DebugFlag = DebugFlagPrev;
189194
}
190195

191196
#endif

0 commit comments

Comments
 (0)