[llvm] [BOLT] Skip out-of-range pending relocations (PR #116964)
Paschalis Mpeis via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 3 08:45:10 PDT 2025
https://github.com/paschalis-mpeis updated https://github.com/llvm/llvm-project/pull/116964
>From a6dbcb860407921c4fac7c25930d917eb3408ed3 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Mon, 20 Jan 2025 16:08:02 +0000
Subject: [PATCH 1/5] BOLT asserts on out-of-range pending relocs
---
bolt/unittests/Core/BinaryContext.cpp | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 09d16966334da..a2f073e9b2e26 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -161,6 +161,33 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) {
<< "Wrong forward branch value\n";
}
+// TODO: BOLT currently asserts when a relocation is out of range.
+TEST_P(BinaryContextTester, FlushOptionalOutOfRangePendingRelocCALL26) {
+ if (GetParam() != Triple::aarch64)
+ GTEST_SKIP();
+
+ // This test checks that flushPendingRelocations skips flushing any optional
+ // pending relocations that cannot be encoded.
+
+ BinarySection &BS = BC->registerOrUpdateSection(
+ ".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
+ MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
+ ASSERT_TRUE(RelSymbol);
+ BS.addPendingRelocation(
+ Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0});
+
+ SmallVector<char> Vect;
+ raw_svector_ostream OS(Vect);
+
+ // FIXME: bolt must be able to skip pending relocs that are out of range.
+ EXPECT_DEBUG_DEATH(
+ // Resolve relocation symbol to a high value so encoding will be out of
+ // range.
+ BS.flushPendingRelocations(OS,
+ [&](const MCSymbol *S) { return 0x800000F; }),
+ ".*only PC \\+/- 128MB is allowed for direct call");
+}
+
#endif
TEST_P(BinaryContextTester, BaseAddress) {
>From e69ce41c8c2e32341850d12b9af7cd6df675d9e0 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Mon, 20 Jan 2025 16:16:58 +0000
Subject: [PATCH 2/5] [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.
---
bolt/include/bolt/Core/BinarySection.h | 13 ++++++----
bolt/include/bolt/Core/Relocation.h | 3 +++
bolt/lib/Core/BinaryFunction.cpp | 2 +-
bolt/lib/Core/BinarySection.cpp | 14 ++++++++++-
bolt/lib/Core/Relocation.cpp | 33 ++++++++++++++++++++++++++
bolt/unittests/Core/BinaryContext.cpp | 25 +++++++++++--------
6 files changed, 73 insertions(+), 17 deletions(-)
diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h
index ad2fed2cf27eb..71f13d9857d38 100644
--- a/bolt/include/bolt/Core/BinarySection.h
+++ b/bolt/include/bolt/Core/BinarySection.h
@@ -66,8 +66,10 @@ class BinarySection {
// from the original section address.
RelocationSetType DynamicRelocations;
- // Pending relocations for this section.
- std::vector<Relocation> PendingRelocations;
+ /// Pending relocations for this section and whether they are optional, i.e.,
+ /// added as part of an optimization. In that case they can be safely omitted
+ /// if flushPendingRelocations discovers they cannot be encoded.
+ std::vector<std::pair<Relocation, bool>> PendingRelocations;
struct BinaryPatch {
uint64_t Offset;
@@ -375,9 +377,10 @@ class BinarySection {
DynamicRelocations.emplace(Reloc);
}
- /// Add relocation against the original contents of this section.
- void addPendingRelocation(const Relocation &Rel) {
- PendingRelocations.push_back(Rel);
+ /// Add relocation against the original contents of this section. When added
+ /// as part of an optimization it is marked as \p Optional.
+ void addPendingRelocation(const Relocation &Rel, bool Optional = false) {
+ PendingRelocations.push_back({Rel, Optional});
}
/// Add patch to the input contents of this section.
diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h
index 78e94cd63829d..3fcf69d79dba2 100644
--- a/bolt/include/bolt/Core/Relocation.h
+++ b/bolt/include/bolt/Core/Relocation.h
@@ -86,6 +86,9 @@ class Relocation {
/// Adjust value depending on relocation type (make it PC relative or not).
static uint64_t encodeValue(uint32_t Type, uint64_t Value, uint64_t PC);
+ /// Return true if there are enough bits to encode the relocation value.
+ static bool canEncodeValue(uint32_t Type, uint64_t Value, uint64_t PC);
+
/// Extract current relocated value from binary contents. This is used for
/// RISC architectures where values are encoded in specific bits depending
/// on the relocation value. For X86, we limit to sign extending the value
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index d1b293ada5fdc..8cba003355a73 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1824,7 +1824,7 @@ bool BinaryFunction::scanExternalRefs() {
// Add relocations unless disassembly failed for this function.
if (!DisassemblyFailed)
for (Relocation &Rel : FunctionRelocations)
- getOriginSection()->addPendingRelocation(Rel);
+ getOriginSection()->addPendingRelocation(Rel, /*Optional*/ true);
// Add patches grouping them together.
if (!InstructionPatches.empty()) {
diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp
index b16e0a4333aa2..2e970ddf68f08 100644
--- a/bolt/lib/Core/BinarySection.cpp
+++ b/bolt/lib/Core/BinarySection.cpp
@@ -174,11 +174,19 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
OS.pwrite(Patch.Bytes.data(), Patch.Bytes.size(),
SectionFileOffset + Patch.Offset);
- for (Relocation &Reloc : PendingRelocations) {
+ uint64_t SkippedPendingRelocations = 0;
+ for (auto &[Reloc, Optional] : PendingRelocations) {
uint64_t Value = Reloc.Addend;
if (Reloc.Symbol)
Value += Resolver(Reloc.Symbol);
+ // Safely skip any pending relocation that cannot be encoded and was added
+ // as part of an optimization.
+ if (Optional && !Relocation::canEncodeValue(
+ Reloc.Type, Value, SectionAddress + Reloc.Offset)) {
+ ++SkippedPendingRelocations;
+ continue;
+ }
Value = Relocation::encodeValue(Reloc.Type, Value,
SectionAddress + Reloc.Offset);
@@ -197,6 +205,10 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
}
clearList(PendingRelocations);
+
+ if (SkippedPendingRelocations > 0)
+ LLVM_DEBUG(dbgs() << "BOLT-INFO: Skipped " << SkippedPendingRelocations
+ << " pending relocations as they were out of range\n");
}
BinarySection::~BinarySection() { updateContents(nullptr, 0); }
diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp
index 1a142c7d9716c..4696a1f1f0402 100644
--- a/bolt/lib/Core/Relocation.cpp
+++ b/bolt/lib/Core/Relocation.cpp
@@ -271,6 +271,16 @@ static uint64_t encodeValueX86(uint32_t Type, uint64_t Value, uint64_t PC) {
return Value;
}
+static bool canEncodeValueAArch64(uint32_t Type, uint64_t Value, uint64_t PC) {
+ switch (Type) {
+ default:
+ llvm_unreachable("unsupported relocation");
+ case ELF::R_AARCH64_CALL26:
+ case ELF::R_AARCH64_JUMP26:
+ return isInt<28>(Value - PC);
+ }
+}
+
static uint64_t encodeValueAArch64(uint32_t Type, uint64_t Value, uint64_t PC) {
switch (Type) {
default:
@@ -303,6 +313,16 @@ static uint64_t encodeValueAArch64(uint32_t Type, uint64_t Value, uint64_t PC) {
return Value;
}
+static uint64_t canEncodeValueRISCV(uint32_t Type, uint64_t Value,
+ uint64_t PC) {
+ switch (Type) {
+ default:
+ llvm_unreachable("unsupported relocation");
+ case ELF::R_RISCV_64:
+ return true;
+ }
+}
+
static uint64_t encodeValueRISCV(uint32_t Type, uint64_t Value, uint64_t PC) {
switch (Type) {
default:
@@ -739,6 +759,19 @@ uint64_t Relocation::encodeValue(uint32_t Type, uint64_t Value, uint64_t PC) {
}
}
+bool Relocation::canEncodeValue(uint32_t Type, uint64_t Value, uint64_t PC) {
+ switch (Arch) {
+ default:
+ llvm_unreachable("Unsupported architecture");
+ case Triple::aarch64:
+ return canEncodeValueAArch64(Type, Value, PC);
+ case Triple::riscv64:
+ return canEncodeValueRISCV(Type, Value, PC);
+ case Triple::x86_64:
+ return true;
+ }
+}
+
uint64_t Relocation::extractValue(uint32_t Type, uint64_t Contents,
uint64_t PC) {
switch (Arch) {
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index a2f073e9b2e26..423d9ac37fd16 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -161,7 +161,6 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) {
<< "Wrong forward branch value\n";
}
-// TODO: BOLT currently asserts when a relocation is out of range.
TEST_P(BinaryContextTester, FlushOptionalOutOfRangePendingRelocCALL26) {
if (GetParam() != Triple::aarch64)
GTEST_SKIP();
@@ -169,23 +168,29 @@ TEST_P(BinaryContextTester, FlushOptionalOutOfRangePendingRelocCALL26) {
// This test checks that flushPendingRelocations skips flushing any optional
// pending relocations that cannot be encoded.
+ bool DebugFlagPrev = ::llvm::DebugFlag;
+ ::llvm::DebugFlag = true;
+ testing::internal::CaptureStderr();
+
BinarySection &BS = BC->registerOrUpdateSection(
".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
ASSERT_TRUE(RelSymbol);
- BS.addPendingRelocation(
- Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0});
+ BS.addPendingRelocation(Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0},
+ /*Optional*/ true);
SmallVector<char> Vect;
raw_svector_ostream OS(Vect);
- // FIXME: bolt must be able to skip pending relocs that are out of range.
- EXPECT_DEBUG_DEATH(
- // Resolve relocation symbol to a high value so encoding will be out of
- // range.
- BS.flushPendingRelocations(OS,
- [&](const MCSymbol *S) { return 0x800000F; }),
- ".*only PC \\+/- 128MB is allowed for direct call");
+ // Resolve relocation symbol to a high value so encoding will be out of range.
+ BS.flushPendingRelocations(OS, [&](const MCSymbol *S) { return 0x800000F; });
+
+ std::string CapturedStdErr = testing::internal::GetCapturedStderr();
+ EXPECT_TRUE(CapturedStdErr.find("BOLT-INFO: Skipped 1 pending relocations as "
+ "they were out of range") !=
+ std::string::npos);
+
+ ::llvm::DebugFlag = DebugFlagPrev;
}
#endif
>From 1388020545aa80fcac52a39fec64a66fc1fe9589 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Thu, 20 Feb 2025 16:34:54 +0000
Subject: [PATCH 3/5] Ensure PatchEntries runs
Allow skipping pending relocations only when `-force-patch` is set.
Otherwise, exit with a relevant message.
---
bolt/lib/Core/BinarySection.cpp | 15 ++++++++++-
bolt/unittests/Core/BinaryContext.cpp | 39 ++++++++++++++++++++++++---
bolt/unittests/Core/CMakeLists.txt | 1 +
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp
index 2e970ddf68f08..126fe888da5bc 100644
--- a/bolt/lib/Core/BinarySection.cpp
+++ b/bolt/lib/Core/BinarySection.cpp
@@ -12,6 +12,7 @@
#include "bolt/Core/BinarySection.h"
#include "bolt/Core/BinaryContext.h"
+#include "bolt/Utils/CommandLineOpts.h"
#include "bolt/Utils/Utils.h"
#include "llvm/MC/MCStreamer.h"
#include "llvm/Support/CommandLine.h"
@@ -22,8 +23,8 @@ using namespace llvm;
using namespace bolt;
namespace opts {
-extern cl::opt<bool> PrintRelocations;
extern cl::opt<bool> HotData;
+extern cl::opt<bool> PrintRelocations;
} // namespace opts
uint64_t BinarySection::Count = 0;
@@ -184,6 +185,18 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
// as part of an optimization.
if (Optional && !Relocation::canEncodeValue(
Reloc.Type, Value, SectionAddress + Reloc.Offset)) {
+
+ // A successful run of 'scanExternalRefs' means that all pending
+ // relocations are flushed. Otherwise, PatchEntries should run.
+ if (!opts::ForcePatch) {
+ BC.errs()
+ << "BOLT-ERROR: Cannot fully run scanExternalRefs as pending "
+ "relocation for symbol "
+ << Reloc.Symbol->getName()
+ << " is out-of-range. Cannot proceed without using -force-patch\n";
+ exit(1);
+ }
+
++SkippedPendingRelocations;
continue;
}
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 423d9ac37fd16..e9e2db2154651 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "bolt/Core/BinaryContext.h"
+#include "bolt/Utils/CommandLineOpts.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
#include "llvm/Support/TargetSelect.h"
@@ -161,12 +162,44 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) {
<< "Wrong forward branch value\n";
}
-TEST_P(BinaryContextTester, FlushOptionalOutOfRangePendingRelocCALL26) {
+TEST_P(BinaryContextTester,
+ FlushOptionalOutOfRangePendingRelocCALL26_ForcePatchOff) {
if (GetParam() != Triple::aarch64)
GTEST_SKIP();
- // This test checks that flushPendingRelocations skips flushing any optional
- // pending relocations that cannot be encoded.
+ // Tests that flushPendingRelocations exits if any pending relocation is out
+ // of range and PatchEntries hasn't run. Pending relocations are added by
+ // scanExternalRefs, so this ensures that either all scanExternalRefs
+ // relocations were flushed or PatchEntries ran.
+
+ BinarySection &BS = BC->registerOrUpdateSection(
+ ".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
+ // Create symbol 'Func0x4'
+ MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
+ ASSERT_TRUE(RelSymbol);
+ BS.addPendingRelocation(Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0},
+ /*Optional*/ true);
+
+ SmallVector<char> Vect;
+ raw_svector_ostream OS(Vect);
+
+ // Resolve relocation symbol to a high value so encoding will be out of range.
+ EXPECT_EXIT(BS.flushPendingRelocations(
+ OS, [&](const MCSymbol *S) { return 0x800000F; }),
+ ::testing::ExitedWithCode(1),
+ "BOLT-ERROR: Cannot fully run scanExternalRefs as pending "
+ "relocation for symbol Func0x4 is out-of-range. Cannot proceed "
+ "without using -force-patch");
+}
+
+TEST_P(BinaryContextTester,
+ FlushOptionalOutOfRangePendingRelocCALL26_ForcePatchOn) {
+ if (GetParam() != Triple::aarch64)
+ GTEST_SKIP();
+
+ // Tests that flushPendingRelocations can skip flushing any optional pending
+ // relocations that cannot be encoded, given that PatchEntries runs.
+ opts::ForcePatch = true;
bool DebugFlagPrev = ::llvm::DebugFlag;
::llvm::DebugFlag = true;
diff --git a/bolt/unittests/Core/CMakeLists.txt b/bolt/unittests/Core/CMakeLists.txt
index 8ac88b701ea05..54e8ea10cda12 100644
--- a/bolt/unittests/Core/CMakeLists.txt
+++ b/bolt/unittests/Core/CMakeLists.txt
@@ -19,6 +19,7 @@ target_link_libraries(CoreTests
LLVMBOLTCore
LLVMBOLTRewrite
LLVMBOLTProfile
+ LLVMBOLTUtils
LLVMTestingSupport
)
>From f0b43e9417c0f097b5bbd33842a3f49856a9d9db Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Wed, 26 Mar 2025 16:11:45 +0000
Subject: [PATCH 4/5] Addressing reviewers
---
bolt/include/bolt/Core/BinarySection.h | 13 +++++--------
bolt/lib/Core/BinaryFunction.cpp | 4 +++-
bolt/lib/Core/BinarySection.cpp | 17 +++++++++--------
bolt/unittests/Core/BinaryContext.cpp | 26 ++++++++++++--------------
4 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h
index 71f13d9857d38..ad2fed2cf27eb 100644
--- a/bolt/include/bolt/Core/BinarySection.h
+++ b/bolt/include/bolt/Core/BinarySection.h
@@ -66,10 +66,8 @@ class BinarySection {
// from the original section address.
RelocationSetType DynamicRelocations;
- /// Pending relocations for this section and whether they are optional, i.e.,
- /// added as part of an optimization. In that case they can be safely omitted
- /// if flushPendingRelocations discovers they cannot be encoded.
- std::vector<std::pair<Relocation, bool>> PendingRelocations;
+ // Pending relocations for this section.
+ std::vector<Relocation> PendingRelocations;
struct BinaryPatch {
uint64_t Offset;
@@ -377,10 +375,9 @@ class BinarySection {
DynamicRelocations.emplace(Reloc);
}
- /// Add relocation against the original contents of this section. When added
- /// as part of an optimization it is marked as \p Optional.
- void addPendingRelocation(const Relocation &Rel, bool Optional = false) {
- PendingRelocations.push_back({Rel, Optional});
+ /// Add relocation against the original contents of this section.
+ void addPendingRelocation(const Relocation &Rel) {
+ PendingRelocations.push_back(Rel);
}
/// Add patch to the input contents of this section.
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 8cba003355a73..edae785f5b3dc 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1795,6 +1795,8 @@ bool BinaryFunction::scanExternalRefs() {
// Create relocation for every fixup.
for (const MCFixup &Fixup : Fixups) {
std::optional<Relocation> Rel = BC.MIB->createRelocation(Fixup, *BC.MAB);
+ // Can be skipped under the right circumstances.
+ Rel->setOptional();
if (!Rel) {
Success = false;
continue;
@@ -1824,7 +1826,7 @@ bool BinaryFunction::scanExternalRefs() {
// Add relocations unless disassembly failed for this function.
if (!DisassemblyFailed)
for (Relocation &Rel : FunctionRelocations)
- getOriginSection()->addPendingRelocation(Rel, /*Optional*/ true);
+ getOriginSection()->addPendingRelocation(Rel);
// Add patches grouping them together.
if (!InstructionPatches.empty()) {
diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp
index 126fe888da5bc..1a73aef319272 100644
--- a/bolt/lib/Core/BinarySection.cpp
+++ b/bolt/lib/Core/BinarySection.cpp
@@ -176,15 +176,15 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
SectionFileOffset + Patch.Offset);
uint64_t SkippedPendingRelocations = 0;
- for (auto &[Reloc, Optional] : PendingRelocations) {
+ for (Relocation &Reloc : PendingRelocations) {
uint64_t Value = Reloc.Addend;
if (Reloc.Symbol)
Value += Resolver(Reloc.Symbol);
- // Safely skip any pending relocation that cannot be encoded and was added
- // as part of an optimization.
- if (Optional && !Relocation::canEncodeValue(
- Reloc.Type, Value, SectionAddress + Reloc.Offset)) {
+ // Safely skip any optional pending relocation that cannot be encoded.
+ if (Reloc.isOptional() &&
+ !Relocation::canEncodeValue(Reloc.Type, Value,
+ SectionAddress + Reloc.Offset)) {
// A successful run of 'scanExternalRefs' means that all pending
// relocations are flushed. Otherwise, PatchEntries should run.
@@ -219,9 +219,10 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
clearList(PendingRelocations);
- if (SkippedPendingRelocations > 0)
- LLVM_DEBUG(dbgs() << "BOLT-INFO: Skipped " << SkippedPendingRelocations
- << " pending relocations as they were out of range\n");
+ if (SkippedPendingRelocations > 0 && opts::Verbosity >= 1) {
+ BC.outs() << "BOLT-INFO: skipped " << SkippedPendingRelocations
+ << " out-of-range optional relocations\n";
+ }
}
BinarySection::~BinarySection() { updateContents(nullptr, 0); }
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index e9e2db2154651..0a0f9eff5e86d 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -177,8 +177,9 @@ TEST_P(BinaryContextTester,
// Create symbol 'Func0x4'
MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
ASSERT_TRUE(RelSymbol);
- BS.addPendingRelocation(Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0},
- /*Optional*/ true);
+ Relocation Reloc{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0};
+ Reloc.setOptional();
+ BS.addPendingRelocation(Reloc);
SmallVector<char> Vect;
raw_svector_ostream OS(Vect);
@@ -201,29 +202,26 @@ TEST_P(BinaryContextTester,
// relocations that cannot be encoded, given that PatchEntries runs.
opts::ForcePatch = true;
- bool DebugFlagPrev = ::llvm::DebugFlag;
- ::llvm::DebugFlag = true;
- testing::internal::CaptureStderr();
+ opts::Verbosity = 1;
+ testing::internal::CaptureStdout();
BinarySection &BS = BC->registerOrUpdateSection(
".text", ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
MCSymbol *RelSymbol = BC->getOrCreateGlobalSymbol(4, "Func");
ASSERT_TRUE(RelSymbol);
- BS.addPendingRelocation(Relocation{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0},
- /*Optional*/ true);
+ Relocation Reloc{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0};
+ Reloc.setOptional();
+ BS.addPendingRelocation(Reloc);
SmallVector<char> Vect;
raw_svector_ostream OS(Vect);
// Resolve relocation symbol to a high value so encoding will be out of range.
BS.flushPendingRelocations(OS, [&](const MCSymbol *S) { return 0x800000F; });
-
- std::string CapturedStdErr = testing::internal::GetCapturedStderr();
- EXPECT_TRUE(CapturedStdErr.find("BOLT-INFO: Skipped 1 pending relocations as "
- "they were out of range") !=
- std::string::npos);
-
- ::llvm::DebugFlag = DebugFlagPrev;
+ outs().flush();
+ std::string CapturedStdOut = testing::internal::GetCapturedStdout();
+ EXPECT_EQ(CapturedStdOut,
+ "BOLT-INFO: skipped 1 out-of-range optional relocations\n");
}
#endif
>From 1bb9af5d128c7491e494a0c2dc63365c340275d1 Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Thu, 3 Apr 2025 14:37:53 +0100
Subject: [PATCH 5/5] Addressing reviewers (2)
---
bolt/lib/Core/BinaryFunction.cpp | 2 +-
bolt/lib/Core/BinarySection.cpp | 5 ++---
bolt/unittests/Core/BinaryContext.cpp | 5 ++---
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index edae785f5b3dc..c4f4d234b30c0 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1795,7 +1795,7 @@ bool BinaryFunction::scanExternalRefs() {
// Create relocation for every fixup.
for (const MCFixup &Fixup : Fixups) {
std::optional<Relocation> Rel = BC.MIB->createRelocation(Fixup, *BC.MAB);
- // Can be skipped under the right circumstances.
+ // Can be skipped in case of overlow during relocation value encoding.
Rel->setOptional();
if (!Rel) {
Success = false;
diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp
index 1a73aef319272..ae796d95a0807 100644
--- a/bolt/lib/Core/BinarySection.cpp
+++ b/bolt/lib/Core/BinarySection.cpp
@@ -190,10 +190,9 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS,
// relocations are flushed. Otherwise, PatchEntries should run.
if (!opts::ForcePatch) {
BC.errs()
- << "BOLT-ERROR: Cannot fully run scanExternalRefs as pending "
- "relocation for symbol "
+ << "BOLT-ERROR: Cannot encode relocation for symbol "
<< Reloc.Symbol->getName()
- << " is out-of-range. Cannot proceed without using -force-patch\n";
+ << " as it is out-of-range. To proceed must use -force-patch\n";
exit(1);
}
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 0a0f9eff5e86d..00aba622838e4 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -188,9 +188,8 @@ TEST_P(BinaryContextTester,
EXPECT_EXIT(BS.flushPendingRelocations(
OS, [&](const MCSymbol *S) { return 0x800000F; }),
::testing::ExitedWithCode(1),
- "BOLT-ERROR: Cannot fully run scanExternalRefs as pending "
- "relocation for symbol Func0x4 is out-of-range. Cannot proceed "
- "without using -force-patch");
+ "BOLT-ERROR: Cannot encode relocation for symbol Func0x4 as it is"
+ " out-of-range. To proceed must use -force-patch");
}
TEST_P(BinaryContextTester,
More information about the llvm-commits
mailing list