From 4e88d962bc477ceb03b4ac3c937568066bd96f27 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Mon, 17 Mar 2025 15:28:32 +0000 Subject: [PATCH 1/3] [BOLT] Add optional flag to Relocations --- bolt/include/bolt/Core/BinaryContext.h | 4 ++-- bolt/include/bolt/Core/BinaryFunction.h | 2 +- bolt/include/bolt/Core/BinarySection.h | 15 +++++++----- bolt/include/bolt/Core/Relocation.h | 3 +++ bolt/lib/Core/BinaryContext.cpp | 12 +++++----- bolt/lib/Core/BinaryFunction.cpp | 6 ++--- bolt/lib/Core/BinarySection.cpp | 7 +++--- bolt/lib/Core/JumpTable.cpp | 3 ++- bolt/lib/Rewrite/LinuxKernelRewriter.cpp | 21 +++++++++-------- bolt/lib/Rewrite/RewriteInstance.cpp | 23 +++++++++++-------- .../Target/AArch64/AArch64MCPlusBuilder.cpp | 3 ++- bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 3 ++- bolt/unittests/Core/BinaryContext.cpp | 16 ++++++------- 13 files changed, 67 insertions(+), 51 deletions(-) diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h index 9a0485989201f..0da3815306a60 100644 --- a/bolt/include/bolt/Core/BinaryContext.h +++ b/bolt/include/bolt/Core/BinaryContext.h @@ -1296,7 +1296,7 @@ class BinaryContext { /// Add a Section relocation at a given \p Address. void addRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t Type, - uint64_t Addend = 0, uint64_t Value = 0); + bool Optional, uint64_t Addend = 0, uint64_t Value = 0); /// Return a relocation registered at a given \p Address, or nullptr if there /// is no relocation at such address. @@ -1309,7 +1309,7 @@ class BinaryContext { /// Register dynamic relocation at \p Address. void addDynamicRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t Type, - uint64_t Addend, uint64_t Value = 0); + bool Optional, uint64_t Addend, uint64_t Value = 0); /// Return a dynamic relocation registered at a given \p Address, or nullptr /// if there is no dynamic relocation at such address. diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index a92cb466c5992..09e19a33986de 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1261,7 +1261,7 @@ class BinaryFunction { /// against \p Symbol. /// Assert if the \p Address is not inside this function. void addRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t RelType, - uint64_t Addend, uint64_t Value); + bool Optional, uint64_t Addend, uint64_t Value); /// Return the name of the section this function originated from. std::optional getOriginSectionName() const { diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index ad2fed2cf27eb..10f633fda6640 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -359,15 +359,18 @@ class BinarySection { /// Add a new relocation at the given /p Offset. void addRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, - uint64_t Addend, uint64_t Value = 0) { + bool Optional, uint64_t Addend, uint64_t Value = 0) { assert(Offset < getSize() && "offset not within section bounds"); - Relocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value}); + Relocations.emplace( + Relocation{Offset, Symbol, Type, Optional, Addend, Value}); } /// Add a dynamic relocation at the given /p Offset. void addDynamicRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, - uint64_t Addend, uint64_t Value = 0) { - addDynamicRelocation(Relocation{Offset, Symbol, Type, Addend, Value}); + bool Optional, uint64_t Addend, + uint64_t Value = 0) { + addDynamicRelocation( + Relocation{Offset, Symbol, Type, Optional, Addend, Value}); } void addDynamicRelocation(const Relocation &Reloc) { @@ -401,13 +404,13 @@ class BinarySection { /// Lookup the relocation (if any) at the given /p Offset. const Relocation *getDynamicRelocationAt(uint64_t Offset) const { - Relocation Key{Offset, 0, 0, 0, 0}; + Relocation Key{Offset, 0, 0, 0, 0, 0}; auto Itr = DynamicRelocations.find(Key); return Itr != DynamicRelocations.end() ? &*Itr : nullptr; } std::optional takeDynamicRelocationAt(uint64_t Offset) { - Relocation Key{Offset, 0, 0, 0, 0}; + Relocation Key{Offset, 0, 0, 0, 0, 0}; auto Itr = DynamicRelocations.find(Key); if (Itr == DynamicRelocations.end()) diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index 1f18dad008ca2..3a18249fa6bf9 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -47,6 +47,9 @@ struct Relocation { /// Relocation type. uint32_t Type; + /// Relocations added by optimizations can be optional. + bool Optional; + /// The offset from the \p Symbol base used to compute the final /// value of this relocation. uint64_t Addend; diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp index 7466d2f8c91eb..f6d191eef5b41 100644 --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -2284,21 +2284,21 @@ ErrorOr BinaryContext::getSignedValueAtAddress(uint64_t Address, } void BinaryContext::addRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t Type, uint64_t Addend, + uint32_t Type, bool Optional, uint64_t Addend, uint64_t Value) { ErrorOr Section = getSectionForAddress(Address); assert(Section && "cannot find section for address"); - Section->addRelocation(Address - Section->getAddress(), Symbol, Type, Addend, - Value); + Section->addRelocation(Address - Section->getAddress(), Symbol, Type, + Optional, Addend, Value); } void BinaryContext::addDynamicRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t Type, uint64_t Addend, - uint64_t Value) { + uint32_t Type, bool Optional, + uint64_t Addend, uint64_t Value) { ErrorOr Section = getSectionForAddress(Address); assert(Section && "cannot find section for address"); Section->addDynamicRelocation(Address - Section->getAddress(), Symbol, Type, - Addend, Value); + Optional, Addend, Value); } bool BinaryContext::removeRelocationAt(uint64_t Address) { diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index acf04cee769c1..4b97aebe37c3c 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -4623,8 +4623,8 @@ bool BinaryFunction::isAArch64Veneer() const { } void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t RelType, uint64_t Addend, - uint64_t Value) { + uint32_t RelType, bool Optional, + uint64_t Addend, uint64_t Value) { assert(Address >= getAddress() && Address < getAddress() + getMaxSize() && "address is outside of the function"); uint64_t Offset = Address - getAddress(); @@ -4635,7 +4635,7 @@ void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, std::map &Rels = IsCI ? Islands->Relocations : Relocations; if (BC.MIB->shouldRecordCodeRelocation(RelType)) - Rels[Offset] = Relocation{Offset, Symbol, RelType, Addend, Value}; + Rels[Offset] = Relocation{Offset, Symbol, RelType, Optional, Addend, Value}; } } // namespace bolt diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp index b16e0a4333aa2..a4968c27f3a25 100644 --- a/bolt/lib/Core/BinarySection.cpp +++ b/bolt/lib/Core/BinarySection.cpp @@ -49,8 +49,8 @@ BinarySection::hash(const BinaryData &BD, uint64_t Offset = BD.getAddress() - getAddress(); const uint64_t EndOffset = BD.getEndAddress() - getAddress(); - auto Begin = Relocations.lower_bound(Relocation{Offset, 0, 0, 0, 0}); - auto End = Relocations.upper_bound(Relocation{EndOffset, 0, 0, 0, 0}); + auto Begin = Relocations.lower_bound(Relocation{Offset, 0, 0, 0, 0, 0}); + auto End = Relocations.upper_bound(Relocation{EndOffset, 0, 0, 0, 0, 0}); const StringRef Contents = getContents(); while (Begin != End) { @@ -275,7 +275,8 @@ void BinarySection::reorderContents(const std::vector &Order, // of the reordered segment to force LLVM to recognize and map this // section. MCSymbol *ZeroSym = BC.registerNameAtAddress("Zero", 0, 0, 0); - addRelocation(OS.tell(), ZeroSym, Relocation::getAbs64(), 0xdeadbeef); + addRelocation(OS.tell(), ZeroSym, Relocation::getAbs64(), + /*Optional*/ false, 0xdeadbeef); uint64_t Zero = 0; OS.write(reinterpret_cast(&Zero), sizeof(Zero)); diff --git a/bolt/lib/Core/JumpTable.cpp b/bolt/lib/Core/JumpTable.cpp index 6f588d2b95fd6..b8124fc9dd129 100644 --- a/bolt/lib/Core/JumpTable.cpp +++ b/bolt/lib/Core/JumpTable.cpp @@ -92,7 +92,8 @@ void bolt::JumpTable::updateOriginal() { // to the original jump table. if (BC.HasRelocations) getOutputSection().removeRelocationAt(EntryOffset); - getOutputSection().addRelocation(EntryOffset, Entry, RelType, RelAddend); + getOutputSection().addRelocation(EntryOffset, Entry, RelType, + /*Optional*/ false, RelAddend); EntryOffset += EntrySize; } } diff --git a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp index 5a5e044184d0b..d142e002128e2 100644 --- a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp +++ b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp @@ -487,8 +487,8 @@ void LinuxKernelRewriter::processLKKSymtab(bool IsGPL) { if (!BF) continue; - BC.addRelocation(EntryAddress, BF->getSymbol(), Relocation::getPC32(), 0, - *Offset); + BC.addRelocation(EntryAddress, BF->getSymbol(), Relocation::getPC32(), + /*Optional*/ false, 0, *Offset); } } @@ -559,7 +559,7 @@ void LinuxKernelRewriter::processInstructionFixups() { Fixup.Section.addRelocation(Fixup.Offset, &Fixup.Label, Fixup.IsPCRelative ? ELF::R_X86_64_PC32 : ELF::R_X86_64_64, - /*Addend*/ 0); + /*Optional*/ false, /*Addend*/ 0); } } @@ -828,7 +828,8 @@ Error LinuxKernelRewriter::rewriteORCTables() { if (Label) ORCUnwindIPSection->addRelocation(UnwindIPWriter.getOffset(), Label, - Relocation::getPC32(), /*Addend*/ 0); + Relocation::getPC32(), + /*Optional*/ false, /*Addend*/ 0); const int32_t IPValue = IP - ORCUnwindIPSection->getAddress() - UnwindIPWriter.getOffset(); @@ -1074,7 +1075,8 @@ Error LinuxKernelRewriter::rewriteStaticCalls() { StaticCallSection->getAddress() + (Entry.ID - 1) * STATIC_CALL_ENTRY_SIZE; StaticCallSection->addRelocation(EntryOffset, Entry.Label, - ELF::R_X86_64_PC32, /*Addend*/ 0); + ELF::R_X86_64_PC32, /*Optional*/ false, + /*Addend*/ 0); } return Error::success(); @@ -1378,7 +1380,7 @@ Error LinuxKernelRewriter::rewriteBugTable() { BC.MIB->getOrCreateInstLabel(Inst, "__BUG_", BC.Ctx.get()); const uint64_t EntryOffset = (ID - 1) * BUG_TABLE_ENTRY_SIZE; BugTableSection->addRelocation(EntryOffset, Label, ELF::R_X86_64_PC32, - /*Addend*/ 0); + /*Optional*/ false, /*Addend*/ 0); } } @@ -1388,7 +1390,7 @@ Error LinuxKernelRewriter::rewriteBugTable() { if (!EmittedIDs.count(ID)) { const uint64_t EntryOffset = (ID - 1) * BUG_TABLE_ENTRY_SIZE; BugTableSection->addRelocation(EntryOffset, nullptr, ELF::R_X86_64_PC32, - /*Addend*/ 0); + /*Optional*/ false, /*Addend*/ 0); } } } @@ -1904,9 +1906,10 @@ Error LinuxKernelRewriter::rewriteStaticKeysJumpTable() { (EntryID - 1) * 16; StaticKeysJumpSection->addRelocation(EntryOffset, Label, ELF::R_X86_64_PC32, - /*Addend*/ 0); + /*Optional*/ false, /*Addend*/ 0); StaticKeysJumpSection->addRelocation(EntryOffset + 4, Target, - ELF::R_X86_64_PC32, /*Addend*/ 0); + ELF::R_X86_64_PC32, + /*Optional*/ false, /*Addend*/ 0); } } } diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index 560b30c6c676c..5c9fa0edf0f55 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1420,7 +1420,7 @@ void RewriteInstance::updateRtFiniReloc() { // desired value. FiniArraySection->addPendingRelocation(Relocation{ /*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(), - /*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0}); + /*Optional*/ false, /*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0}); } void RewriteInstance::registerFragments() { @@ -1919,7 +1919,8 @@ void RewriteInstance::relocateEHFrameSection() { // Create a relocation against an absolute value since the goal is to // preserve the contents of the section independent of the new values // of referenced symbols. - RelocatedEHFrameSection->addRelocation(Offset, nullptr, RelType, Value); + RelocatedEHFrameSection->addRelocation(Offset, nullptr, RelType, + /*Optional*/ false, Value); }; Error E = EHFrameParser::parse(DE, EHFrameSection->getAddress(), createReloc); @@ -2455,7 +2456,8 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section, if (Symbol) SymbolIndex[Symbol] = getRelocationSymbol(InputFile, Rel); - BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, Addend); + BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, /*Optional*/ false, + Addend); } } @@ -2486,7 +2488,8 @@ void RewriteInstance::readDynamicRelrRelocations(BinarySection &Section) { LLVM_DEBUG(dbgs() << "BOLT-DEBUG: R_*_RELATIVE relocation at 0x" << Twine::utohexstr(Address) << " to 0x" << Twine::utohexstr(Addend) << '\n';); - BC->addDynamicRelocation(Address, nullptr, RType, Addend); + BC->addDynamicRelocation(Address, nullptr, RType, /*Optional*/ false, + Addend); }; DataExtractor DE = DataExtractor(Section.getContents(), @@ -2686,8 +2689,8 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, // This might be a relocation for an ABS symbols like __global_pointer$ on // RISC-V ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, - Relocation::getType(Rel), 0, - cantFail(Symbol.getValue())); + Relocation::getType(Rel), /*Optional*/ false, + 0, cantFail(Symbol.getValue())); return; } } @@ -2719,7 +2722,7 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, // the code. It's required to properly handle cases where // "symbol + addend" references an object different from "symbol". ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, - Addend, ExtractedValue); + /*Optional*/ false, Addend, ExtractedValue); } else { LLVM_DEBUG({ dbgs() << "BOLT-DEBUG: not creating PC-relative relocation at" @@ -2953,10 +2956,10 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, if (IsFromCode) ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, - Addend, ExtractedValue); + /*Optional*/ false, Addend, ExtractedValue); else if (IsToCode || ForceRelocation) - BC->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, Addend, - ExtractedValue); + BC->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, + /*Optional*/ false, Addend, ExtractedValue); else LLVM_DEBUG(dbgs() << "BOLT-DEBUG: ignoring relocation from data to data\n"); } diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 613b24c4553e2..54422eb6103b7 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2291,7 +2291,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); - return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0}); + return Relocation( + {RelOffset, RelSymbol, RelType, /*Optional*/ false, RelAddend, 0}); } uint16_t getMinFunctionAlignment() const override { return 4; } diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index 51550857ae5ea..cb754af3943cb 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -2467,7 +2467,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); - return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0}); + return Relocation( + {RelOffset, RelSymbol, RelType, /*Optional*/ false, RelAddend, 0}); } bool replaceImmWithSymbolRef(MCInst &Inst, const MCSymbol *Symbol, diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp index 09d16966334da..de041becedf4e 100644 --- a/bolt/unittests/Core/BinaryContext.cpp +++ b/bolt/unittests/Core/BinaryContext.cpp @@ -96,12 +96,12 @@ TEST_P(BinaryContextTester, FlushPendingRelocCALL26) { DataSize, 4); MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1"); ASSERT_TRUE(RelSymbol1); - BS.addPendingRelocation( - Relocation{8, RelSymbol1, ELF::R_AARCH64_CALL26, 0, 0}); + BS.addPendingRelocation(Relocation{8, RelSymbol1, ELF::R_AARCH64_CALL26, + /*Optional*/ false, 0, 0}); MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2"); ASSERT_TRUE(RelSymbol2); - BS.addPendingRelocation( - Relocation{12, RelSymbol2, ELF::R_AARCH64_CALL26, 0, 0}); + BS.addPendingRelocation(Relocation{12, RelSymbol2, ELF::R_AARCH64_CALL26, + /*Optional*/ false, 0, 0}); SmallVector Vect(DataSize); raw_svector_ostream OS(Vect); @@ -138,12 +138,12 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) { (uint8_t *)Data, Size, 4); MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1"); ASSERT_TRUE(RelSymbol1); - BS.addPendingRelocation( - Relocation{8, RelSymbol1, ELF::R_AARCH64_JUMP26, 0, 0}); + BS.addPendingRelocation(Relocation{8, RelSymbol1, ELF::R_AARCH64_JUMP26, + /*Optional*/ false, 0, 0}); MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2"); ASSERT_TRUE(RelSymbol2); - BS.addPendingRelocation( - Relocation{12, RelSymbol2, ELF::R_AARCH64_JUMP26, 0, 0}); + BS.addPendingRelocation(Relocation{12, RelSymbol2, ELF::R_AARCH64_JUMP26, + /*Optional*/ false, 0, 0}); SmallVector Vect(Size); raw_svector_ostream OS(Vect); From 4b4bde5c6337bc3a7afa08a7f6073b9d14a540cd Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Tue, 18 Mar 2025 10:06:01 +0000 Subject: [PATCH 2/3] Default the Optional field to false. --- bolt/include/bolt/Core/BinaryContext.h | 4 ++-- bolt/include/bolt/Core/BinaryFunction.h | 2 +- bolt/include/bolt/Core/BinarySection.h | 9 ++++----- bolt/include/bolt/Core/Relocation.h | 6 +++++- bolt/lib/Core/BinaryContext.cpp | 12 ++++++------ bolt/lib/Core/BinaryFunction.cpp | 7 ++++--- bolt/lib/Core/BinarySection.cpp | 3 +-- bolt/lib/Core/JumpTable.cpp | 3 +-- bolt/lib/Rewrite/LinuxKernelRewriter.cpp | 21 +++++++++------------ bolt/lib/Rewrite/RewriteInstance.cpp | 21 +++++++++------------ 10 files changed, 42 insertions(+), 46 deletions(-) diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h index 0da3815306a60..9a0485989201f 100644 --- a/bolt/include/bolt/Core/BinaryContext.h +++ b/bolt/include/bolt/Core/BinaryContext.h @@ -1296,7 +1296,7 @@ class BinaryContext { /// Add a Section relocation at a given \p Address. void addRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t Type, - bool Optional, uint64_t Addend = 0, uint64_t Value = 0); + uint64_t Addend = 0, uint64_t Value = 0); /// Return a relocation registered at a given \p Address, or nullptr if there /// is no relocation at such address. @@ -1309,7 +1309,7 @@ class BinaryContext { /// Register dynamic relocation at \p Address. void addDynamicRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t Type, - bool Optional, uint64_t Addend, uint64_t Value = 0); + uint64_t Addend, uint64_t Value = 0); /// Return a dynamic relocation registered at a given \p Address, or nullptr /// if there is no dynamic relocation at such address. diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 09e19a33986de..a92cb466c5992 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1261,7 +1261,7 @@ class BinaryFunction { /// against \p Symbol. /// Assert if the \p Address is not inside this function. void addRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t RelType, - bool Optional, uint64_t Addend, uint64_t Value); + uint64_t Addend, uint64_t Value); /// Return the name of the section this function originated from. std::optional getOriginSectionName() const { diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index 10f633fda6640..b8fdd56520406 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -359,18 +359,17 @@ class BinarySection { /// Add a new relocation at the given /p Offset. void addRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, - bool Optional, uint64_t Addend, uint64_t Value = 0) { + uint64_t Addend, uint64_t Value = 0) { assert(Offset < getSize() && "offset not within section bounds"); Relocations.emplace( - Relocation{Offset, Symbol, Type, Optional, Addend, Value}); + Relocation{Offset, Symbol, Type, /* Optional */ false, Addend, Value}); } /// Add a dynamic relocation at the given /p Offset. void addDynamicRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, - bool Optional, uint64_t Addend, - uint64_t Value = 0) { + uint64_t Addend, uint64_t Value = 0) { addDynamicRelocation( - Relocation{Offset, Symbol, Type, Optional, Addend, Value}); + Relocation{Offset, Symbol, Type, /*Optional*/ false, Addend, Value}); } void addDynamicRelocation(const Relocation &Reloc) { diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index 3a18249fa6bf9..5c62b5ef58f0f 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -48,7 +48,7 @@ struct Relocation { uint32_t Type; /// Relocations added by optimizations can be optional. - bool Optional; + bool Optional = false; /// The offset from the \p Symbol base used to compute the final /// value of this relocation. @@ -61,6 +61,10 @@ struct Relocation { /// Return size in bytes of the given relocation \p Type. static size_t getSizeForType(uint32_t Type); + /// Some relocations added by optimizations are optional, meaning they can be + /// omitted under certain circumstances. + void setOptional() { Optional = true; } + /// Return size of this relocation. size_t getSize() const { return getSizeForType(Type); } diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp index f6d191eef5b41..7466d2f8c91eb 100644 --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -2284,21 +2284,21 @@ ErrorOr BinaryContext::getSignedValueAtAddress(uint64_t Address, } void BinaryContext::addRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t Type, bool Optional, uint64_t Addend, + uint32_t Type, uint64_t Addend, uint64_t Value) { ErrorOr Section = getSectionForAddress(Address); assert(Section && "cannot find section for address"); - Section->addRelocation(Address - Section->getAddress(), Symbol, Type, - Optional, Addend, Value); + Section->addRelocation(Address - Section->getAddress(), Symbol, Type, Addend, + Value); } void BinaryContext::addDynamicRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t Type, bool Optional, - uint64_t Addend, uint64_t Value) { + uint32_t Type, uint64_t Addend, + uint64_t Value) { ErrorOr Section = getSectionForAddress(Address); assert(Section && "cannot find section for address"); Section->addDynamicRelocation(Address - Section->getAddress(), Symbol, Type, - Optional, Addend, Value); + Addend, Value); } bool BinaryContext::removeRelocationAt(uint64_t Address) { diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 4b97aebe37c3c..07d0da7daaa48 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -4623,8 +4623,8 @@ bool BinaryFunction::isAArch64Veneer() const { } void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, - uint32_t RelType, bool Optional, - uint64_t Addend, uint64_t Value) { + uint32_t RelType, uint64_t Addend, + uint64_t Value) { assert(Address >= getAddress() && Address < getAddress() + getMaxSize() && "address is outside of the function"); uint64_t Offset = Address - getAddress(); @@ -4635,7 +4635,8 @@ void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, std::map &Rels = IsCI ? Islands->Relocations : Relocations; if (BC.MIB->shouldRecordCodeRelocation(RelType)) - Rels[Offset] = Relocation{Offset, Symbol, RelType, Optional, Addend, Value}; + Rels[Offset] = + Relocation{Offset, Symbol, RelType, /*Optional*/ false, Addend, Value}; } } // namespace bolt diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp index a4968c27f3a25..1cff7e7078977 100644 --- a/bolt/lib/Core/BinarySection.cpp +++ b/bolt/lib/Core/BinarySection.cpp @@ -275,8 +275,7 @@ void BinarySection::reorderContents(const std::vector &Order, // of the reordered segment to force LLVM to recognize and map this // section. MCSymbol *ZeroSym = BC.registerNameAtAddress("Zero", 0, 0, 0); - addRelocation(OS.tell(), ZeroSym, Relocation::getAbs64(), - /*Optional*/ false, 0xdeadbeef); + addRelocation(OS.tell(), ZeroSym, Relocation::getAbs64(), 0xdeadbeef); uint64_t Zero = 0; OS.write(reinterpret_cast(&Zero), sizeof(Zero)); diff --git a/bolt/lib/Core/JumpTable.cpp b/bolt/lib/Core/JumpTable.cpp index b8124fc9dd129..6f588d2b95fd6 100644 --- a/bolt/lib/Core/JumpTable.cpp +++ b/bolt/lib/Core/JumpTable.cpp @@ -92,8 +92,7 @@ void bolt::JumpTable::updateOriginal() { // to the original jump table. if (BC.HasRelocations) getOutputSection().removeRelocationAt(EntryOffset); - getOutputSection().addRelocation(EntryOffset, Entry, RelType, - /*Optional*/ false, RelAddend); + getOutputSection().addRelocation(EntryOffset, Entry, RelType, RelAddend); EntryOffset += EntrySize; } } diff --git a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp index d142e002128e2..5a5e044184d0b 100644 --- a/bolt/lib/Rewrite/LinuxKernelRewriter.cpp +++ b/bolt/lib/Rewrite/LinuxKernelRewriter.cpp @@ -487,8 +487,8 @@ void LinuxKernelRewriter::processLKKSymtab(bool IsGPL) { if (!BF) continue; - BC.addRelocation(EntryAddress, BF->getSymbol(), Relocation::getPC32(), - /*Optional*/ false, 0, *Offset); + BC.addRelocation(EntryAddress, BF->getSymbol(), Relocation::getPC32(), 0, + *Offset); } } @@ -559,7 +559,7 @@ void LinuxKernelRewriter::processInstructionFixups() { Fixup.Section.addRelocation(Fixup.Offset, &Fixup.Label, Fixup.IsPCRelative ? ELF::R_X86_64_PC32 : ELF::R_X86_64_64, - /*Optional*/ false, /*Addend*/ 0); + /*Addend*/ 0); } } @@ -828,8 +828,7 @@ Error LinuxKernelRewriter::rewriteORCTables() { if (Label) ORCUnwindIPSection->addRelocation(UnwindIPWriter.getOffset(), Label, - Relocation::getPC32(), - /*Optional*/ false, /*Addend*/ 0); + Relocation::getPC32(), /*Addend*/ 0); const int32_t IPValue = IP - ORCUnwindIPSection->getAddress() - UnwindIPWriter.getOffset(); @@ -1075,8 +1074,7 @@ Error LinuxKernelRewriter::rewriteStaticCalls() { StaticCallSection->getAddress() + (Entry.ID - 1) * STATIC_CALL_ENTRY_SIZE; StaticCallSection->addRelocation(EntryOffset, Entry.Label, - ELF::R_X86_64_PC32, /*Optional*/ false, - /*Addend*/ 0); + ELF::R_X86_64_PC32, /*Addend*/ 0); } return Error::success(); @@ -1380,7 +1378,7 @@ Error LinuxKernelRewriter::rewriteBugTable() { BC.MIB->getOrCreateInstLabel(Inst, "__BUG_", BC.Ctx.get()); const uint64_t EntryOffset = (ID - 1) * BUG_TABLE_ENTRY_SIZE; BugTableSection->addRelocation(EntryOffset, Label, ELF::R_X86_64_PC32, - /*Optional*/ false, /*Addend*/ 0); + /*Addend*/ 0); } } @@ -1390,7 +1388,7 @@ Error LinuxKernelRewriter::rewriteBugTable() { if (!EmittedIDs.count(ID)) { const uint64_t EntryOffset = (ID - 1) * BUG_TABLE_ENTRY_SIZE; BugTableSection->addRelocation(EntryOffset, nullptr, ELF::R_X86_64_PC32, - /*Optional*/ false, /*Addend*/ 0); + /*Addend*/ 0); } } } @@ -1906,10 +1904,9 @@ Error LinuxKernelRewriter::rewriteStaticKeysJumpTable() { (EntryID - 1) * 16; StaticKeysJumpSection->addRelocation(EntryOffset, Label, ELF::R_X86_64_PC32, - /*Optional*/ false, /*Addend*/ 0); + /*Addend*/ 0); StaticKeysJumpSection->addRelocation(EntryOffset + 4, Target, - ELF::R_X86_64_PC32, - /*Optional*/ false, /*Addend*/ 0); + ELF::R_X86_64_PC32, /*Addend*/ 0); } } } diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index 5c9fa0edf0f55..4741ae21b5e9f 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1919,8 +1919,7 @@ void RewriteInstance::relocateEHFrameSection() { // Create a relocation against an absolute value since the goal is to // preserve the contents of the section independent of the new values // of referenced symbols. - RelocatedEHFrameSection->addRelocation(Offset, nullptr, RelType, - /*Optional*/ false, Value); + RelocatedEHFrameSection->addRelocation(Offset, nullptr, RelType, Value); }; Error E = EHFrameParser::parse(DE, EHFrameSection->getAddress(), createReloc); @@ -2456,8 +2455,7 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section, if (Symbol) SymbolIndex[Symbol] = getRelocationSymbol(InputFile, Rel); - BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, /*Optional*/ false, - Addend); + BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, Addend); } } @@ -2488,8 +2486,7 @@ void RewriteInstance::readDynamicRelrRelocations(BinarySection &Section) { LLVM_DEBUG(dbgs() << "BOLT-DEBUG: R_*_RELATIVE relocation at 0x" << Twine::utohexstr(Address) << " to 0x" << Twine::utohexstr(Addend) << '\n';); - BC->addDynamicRelocation(Address, nullptr, RType, /*Optional*/ false, - Addend); + BC->addDynamicRelocation(Address, nullptr, RType, Addend); }; DataExtractor DE = DataExtractor(Section.getContents(), @@ -2689,8 +2686,8 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, // This might be a relocation for an ABS symbols like __global_pointer$ on // RISC-V ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, - Relocation::getType(Rel), /*Optional*/ false, - 0, cantFail(Symbol.getValue())); + Relocation::getType(Rel), 0, + cantFail(Symbol.getValue())); return; } } @@ -2722,7 +2719,7 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, // the code. It's required to properly handle cases where // "symbol + addend" references an object different from "symbol". ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, - /*Optional*/ false, Addend, ExtractedValue); + Addend, ExtractedValue); } else { LLVM_DEBUG({ dbgs() << "BOLT-DEBUG: not creating PC-relative relocation at" @@ -2956,10 +2953,10 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection, if (IsFromCode) ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, - /*Optional*/ false, Addend, ExtractedValue); + Addend, ExtractedValue); else if (IsToCode || ForceRelocation) - BC->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, - /*Optional*/ false, Addend, ExtractedValue); + BC->addRelocation(Rel.getOffset(), ReferencedSymbol, RType, Addend, + ExtractedValue); else LLVM_DEBUG(dbgs() << "BOLT-DEBUG: ignoring relocation from data to data\n"); } From a24c9fbdce0367ebfd6c1fc9d5fa3c52e01fe21e Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 19 Mar 2025 09:13:48 +0000 Subject: [PATCH 3/3] Relocation is now a class. --- bolt/include/bolt/Core/BinarySection.h | 10 ++++------ bolt/include/bolt/Core/Relocation.h | 13 ++++++++++++- bolt/lib/Core/BinaryFunction.cpp | 3 +-- bolt/lib/Core/BinarySection.cpp | 4 ++-- bolt/lib/Rewrite/RewriteInstance.cpp | 2 +- bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 3 +-- bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 3 +-- bolt/unittests/Core/BinaryContext.cpp | 16 ++++++++-------- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index b8fdd56520406..ad2fed2cf27eb 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -361,15 +361,13 @@ class BinarySection { void addRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, uint64_t Addend, uint64_t Value = 0) { assert(Offset < getSize() && "offset not within section bounds"); - Relocations.emplace( - Relocation{Offset, Symbol, Type, /* Optional */ false, Addend, Value}); + Relocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value}); } /// Add a dynamic relocation at the given /p Offset. void addDynamicRelocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, uint64_t Addend, uint64_t Value = 0) { - addDynamicRelocation( - Relocation{Offset, Symbol, Type, /*Optional*/ false, Addend, Value}); + addDynamicRelocation(Relocation{Offset, Symbol, Type, Addend, Value}); } void addDynamicRelocation(const Relocation &Reloc) { @@ -403,13 +401,13 @@ class BinarySection { /// Lookup the relocation (if any) at the given /p Offset. const Relocation *getDynamicRelocationAt(uint64_t Offset) const { - Relocation Key{Offset, 0, 0, 0, 0, 0}; + Relocation Key{Offset, 0, 0, 0, 0}; auto Itr = DynamicRelocations.find(Key); return Itr != DynamicRelocations.end() ? &*Itr : nullptr; } std::optional takeDynamicRelocationAt(uint64_t Offset) { - Relocation Key{Offset, 0, 0, 0, 0, 0}; + Relocation Key{Offset, 0, 0, 0, 0}; auto Itr = DynamicRelocations.find(Key); if (Itr == DynamicRelocations.end()) diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index 5c62b5ef58f0f..9292d0496d658 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -35,7 +35,16 @@ enum { R_X86_64_converted_reloc_bit = 0x80 }; namespace bolt { /// Relocation class. -struct Relocation { +class Relocation { +public: + Relocation(uint64_t Offset, MCSymbol *Symbol, uint32_t Type, uint64_t Addend, + uint64_t Value) + : Offset(Offset), Symbol(Symbol), Type(Type), Optional(false), + Addend(Addend), Value(Value) {} + + Relocation() + : Offset(0), Symbol(0), Type(0), Optional(0), Addend(0), Value(0) {} + static Triple::ArchType Arch; /// set by BinaryContext ctor. /// The offset of this relocation in the object it is contained in. @@ -47,9 +56,11 @@ struct Relocation { /// Relocation type. uint32_t Type; +private: /// Relocations added by optimizations can be optional. bool Optional = false; +public: /// The offset from the \p Symbol base used to compute the final /// value of this relocation. uint64_t Addend; diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 07d0da7daaa48..acf04cee769c1 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -4635,8 +4635,7 @@ void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, std::map &Rels = IsCI ? Islands->Relocations : Relocations; if (BC.MIB->shouldRecordCodeRelocation(RelType)) - Rels[Offset] = - Relocation{Offset, Symbol, RelType, /*Optional*/ false, Addend, Value}; + Rels[Offset] = Relocation{Offset, Symbol, RelType, Addend, Value}; } } // namespace bolt diff --git a/bolt/lib/Core/BinarySection.cpp b/bolt/lib/Core/BinarySection.cpp index 1cff7e7078977..b16e0a4333aa2 100644 --- a/bolt/lib/Core/BinarySection.cpp +++ b/bolt/lib/Core/BinarySection.cpp @@ -49,8 +49,8 @@ BinarySection::hash(const BinaryData &BD, uint64_t Offset = BD.getAddress() - getAddress(); const uint64_t EndOffset = BD.getEndAddress() - getAddress(); - auto Begin = Relocations.lower_bound(Relocation{Offset, 0, 0, 0, 0, 0}); - auto End = Relocations.upper_bound(Relocation{EndOffset, 0, 0, 0, 0, 0}); + auto Begin = Relocations.lower_bound(Relocation{Offset, 0, 0, 0, 0}); + auto End = Relocations.upper_bound(Relocation{EndOffset, 0, 0, 0, 0}); const StringRef Contents = getContents(); while (Begin != End) { diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index 4741ae21b5e9f..560b30c6c676c 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1420,7 +1420,7 @@ void RewriteInstance::updateRtFiniReloc() { // desired value. FiniArraySection->addPendingRelocation(Relocation{ /*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(), - /*Optional*/ false, /*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0}); + /*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0}); } void RewriteInstance::registerFragments() { diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 54422eb6103b7..613b24c4553e2 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2291,8 +2291,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); - return Relocation( - {RelOffset, RelSymbol, RelType, /*Optional*/ false, RelAddend, 0}); + return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0}); } uint16_t getMinFunctionAlignment() const override { return 4; } diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index cb754af3943cb..51550857ae5ea 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -2467,8 +2467,7 @@ class X86MCPlusBuilder : public MCPlusBuilder { auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); - return Relocation( - {RelOffset, RelSymbol, RelType, /*Optional*/ false, RelAddend, 0}); + return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0}); } bool replaceImmWithSymbolRef(MCInst &Inst, const MCSymbol *Symbol, diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp index de041becedf4e..09d16966334da 100644 --- a/bolt/unittests/Core/BinaryContext.cpp +++ b/bolt/unittests/Core/BinaryContext.cpp @@ -96,12 +96,12 @@ TEST_P(BinaryContextTester, FlushPendingRelocCALL26) { DataSize, 4); MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1"); ASSERT_TRUE(RelSymbol1); - BS.addPendingRelocation(Relocation{8, RelSymbol1, ELF::R_AARCH64_CALL26, - /*Optional*/ false, 0, 0}); + BS.addPendingRelocation( + Relocation{8, RelSymbol1, ELF::R_AARCH64_CALL26, 0, 0}); MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2"); ASSERT_TRUE(RelSymbol2); - BS.addPendingRelocation(Relocation{12, RelSymbol2, ELF::R_AARCH64_CALL26, - /*Optional*/ false, 0, 0}); + BS.addPendingRelocation( + Relocation{12, RelSymbol2, ELF::R_AARCH64_CALL26, 0, 0}); SmallVector Vect(DataSize); raw_svector_ostream OS(Vect); @@ -138,12 +138,12 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) { (uint8_t *)Data, Size, 4); MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1"); ASSERT_TRUE(RelSymbol1); - BS.addPendingRelocation(Relocation{8, RelSymbol1, ELF::R_AARCH64_JUMP26, - /*Optional*/ false, 0, 0}); + BS.addPendingRelocation( + Relocation{8, RelSymbol1, ELF::R_AARCH64_JUMP26, 0, 0}); MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2"); ASSERT_TRUE(RelSymbol2); - BS.addPendingRelocation(Relocation{12, RelSymbol2, ELF::R_AARCH64_JUMP26, - /*Optional*/ false, 0, 0}); + BS.addPendingRelocation( + Relocation{12, RelSymbol2, ELF::R_AARCH64_JUMP26, 0, 0}); SmallVector Vect(Size); raw_svector_ostream OS(Vect);