From 3d24046b3306cbc682aa1f17426169a942d8931a Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Fri, 4 Apr 2025 17:31:14 +0100 Subject: [PATCH] [BOLT] Skip out-of-range pending relocations (#116964) 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 due to being out of range. A pre-requisite to that is the usage of the `-force-patch` flag. Alternatrively, BOLT will bail out with a relevant message. 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 and `-force-patch` was enabled. --- bolt/include/bolt/Core/Relocation.h | 3 ++ bolt/lib/Core/BinaryFunction.cpp | 2 + bolt/lib/Core/BinarySection.cpp | 27 +++++++++++- bolt/lib/Core/Relocation.cpp | 33 ++++++++++++++ bolt/unittests/Core/BinaryContext.cpp | 62 +++++++++++++++++++++++++++ bolt/unittests/Core/CMakeLists.txt | 1 + 6 files changed, 127 insertions(+), 1 deletion(-) diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index 78e94cd63829..3fcf69d79dba 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -86,6 +86,9 @@ public: /// 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 d1b293ada5fd..c4f4d234b30c 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 Rel = BC.MIB->createRelocation(Fixup, *BC.MAB); + // Can be skipped in case of overlow during relocation value encoding. + Rel->setOptional(); if (!Rel) { Success = false; continue; diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp index b16e0a4333aa..e5def7547a18 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 PrintRelocations; extern cl::opt HotData; +extern cl::opt PrintRelocations; } // namespace opts uint64_t BinarySection::Count = 0; @@ -174,11 +175,30 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS, OS.pwrite(Patch.Bytes.data(), Patch.Bytes.size(), SectionFileOffset + Patch.Offset); + uint64_t SkippedPendingRelocations = 0; for (Relocation &Reloc : PendingRelocations) { uint64_t Value = Reloc.Addend; if (Reloc.Symbol) Value += Resolver(Reloc.Symbol); + // 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. + if (!opts::ForcePatch) { + BC.errs() + << "BOLT-ERROR: cannot encode relocation for symbol " + << Reloc.Symbol->getName() + << " as it is out-of-range. To proceed must use -force-patch\n"; + exit(1); + } + + ++SkippedPendingRelocations; + continue; + } Value = Relocation::encodeValue(Reloc.Type, Value, SectionAddress + Reloc.Offset); @@ -197,6 +217,11 @@ void BinarySection::flushPendingRelocations(raw_pwrite_stream &OS, } clearList(PendingRelocations); + + 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/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp index 1a142c7d9716..4696a1f1f040 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 09d16966334d..377517adf03d 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,6 +162,67 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) { << "Wrong forward branch value\n"; } +TEST_P(BinaryContextTester, + FlushOptionalOutOfRangePendingRelocCALL26_ForcePatchOff) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + + // 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); + Relocation Reloc{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0}; + Reloc.setOptional(); + BS.addPendingRelocation(Reloc); + + SmallVector 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 encode relocation for symbol Func0x4 as it is" + " out-of-range. To proceed must use -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; + + 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); + Relocation Reloc{8, RelSymbol, ELF::R_AARCH64_CALL26, 0, 0}; + Reloc.setOptional(); + BS.addPendingRelocation(Reloc); + + SmallVector 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; }); + outs().flush(); + std::string CapturedStdOut = testing::internal::GetCapturedStdout(); + EXPECT_EQ(CapturedStdOut, + "BOLT-INFO: skipped 1 out-of-range optional relocations\n"); +} + #endif TEST_P(BinaryContextTester, BaseAddress) { diff --git a/bolt/unittests/Core/CMakeLists.txt b/bolt/unittests/Core/CMakeLists.txt index 8ac88b701ea0..54e8ea10cda1 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 )