From a09659fd54183adf1065369fb2c2ae291290c7c3 Mon Sep 17 00:00:00 2001 From: Maksim Panchenko Date: Mon, 18 Nov 2019 14:08:17 -0800 Subject: [PATCH] [BOLT] Refactor markAmbiguousRelocations() Summary: Refactor markAmbiguousRelocations() code and move it to BinaryContext. Also remove a redundant check. (cherry picked from FBD18623815) --- bolt/src/BinaryContext.cpp | 31 ++++++++++++++++++ bolt/src/BinaryContext.h | 5 +++ bolt/src/RewriteInstance.cpp | 63 ++---------------------------------- 3 files changed, 39 insertions(+), 60 deletions(-) diff --git a/bolt/src/BinaryContext.cpp b/bolt/src/BinaryContext.cpp index 012f4e7978ba..7b5e9524e5df 100644 --- a/bolt/src/BinaryContext.cpp +++ b/bolt/src/BinaryContext.cpp @@ -1730,6 +1730,37 @@ const Relocation *BinaryContext::getRelocationAt(uint64_t Address) { return Section->getRelocationAt(Address - Section->getAddress()); } +void BinaryContext::markAmbiguousRelocations(BinaryData &BD, + const uint64_t Address) { + auto setImmovable = [&](BinaryData &BD) { + auto *Root = BD.getAtomicRoot(); + DEBUG(if (Root->isMoveable()) { + dbgs() << "BOLT-DEBUG: setting " << *Root << " as immovable " + << "due to ambiguous relocation referencing 0x" + << Twine::utohexstr(Address) << '\n'; + }); + Root->setIsMoveable(false); + }; + + if (Address == BD.getAddress()) { + setImmovable(BD); + + // Set previous symbol as immovable + auto *Prev = getBinaryDataContainingAddress(Address-1); + if (Prev && Prev->getEndAddress() == BD.getAddress()) + setImmovable(*Prev); + } + + if (Address == BD.getEndAddress()) { + setImmovable(BD); + + // Set next symbol as immovable + auto *Next = getBinaryDataContainingAddress(BD.getEndAddress()); + if (Next && Next->getAddress() == BD.getEndAddress()) + setImmovable(*Next); + } +} + void BinaryContext::exitWithBugReport(StringRef Message, const BinaryFunction &Function) const { errs() << "=======================================\n"; diff --git a/bolt/src/BinaryContext.h b/bolt/src/BinaryContext.h index 8fd8717342b1..95e40ea1e1b9 100644 --- a/bolt/src/BinaryContext.h +++ b/bolt/src/BinaryContext.h @@ -912,6 +912,11 @@ public: /// is no relocation at such address. const Relocation *getRelocationAt(uint64_t Address); + /// This function makes sure that symbols referenced by ambiguous relocations + /// are marked as immovable. For now, if a section relocation points at the + /// boundary between two symbols then those symbols are marked as immovable. + void markAmbiguousRelocations(BinaryData &BD, const uint64_t Address); + /// This function is thread safe. const BinaryFunction *getFunctionForSymbol(const MCSymbol *Symbol) const { std::shared_lock Lock(SymbolToFunctionMapMutex); diff --git a/bolt/src/RewriteInstance.cpp b/bolt/src/RewriteInstance.cpp index afe7e73031fe..829eb2fca471 100644 --- a/bolt/src/RewriteInstance.cpp +++ b/bolt/src/RewriteInstance.cpp @@ -2346,7 +2346,6 @@ void RewriteInstance::readRelocations(const SectionRef &Section, } } - uint64_t RefFunctionOffset = 0; MCSymbol *ReferencedSymbol = nullptr; if (ForceRelocation) { auto Name = Relocation::isGOT(RType) ? "Zero" : SymbolName; @@ -2358,6 +2357,7 @@ void RewriteInstance::readRelocations(const SectionRef &Section, << SymbolName << " with addend " << Addend << '\n'); } else if (ReferencedBF) { ReferencedSymbol = ReferencedBF->getSymbol(); + uint64_t RefFunctionOffset = 0; // Adjust the point of reference to a code location inside a function. if (ReferencedBF->containsAddress(Address, /*UseMaxSize = */true)) { @@ -2400,60 +2400,6 @@ void RewriteInstance::readRelocations(const SectionRef &Section, Addend = 0; } - // This function makes sure that symbols referenced by ambiguous - // relocations are marked as unmoveable. For now, if a section - // relocation points at the boundary between two symbols then - // those symbols are marked as unmoveable. - auto markAmbiguousRelocations = [&](BinaryData *BD) { - if (Address == BD->getAddress()) { - BD = BD->getAtomicRoot(); - DEBUG(if (BD->isMoveable()) { - dbgs() << "BOLT-DEBUG: setting " << *BD << " as unmoveable " - << "due to ambiguous relocation (0x" - << Twine::utohexstr(Address) << ") @ 0x" - << Twine::utohexstr(Rel.getOffset()) << "\n"; - }); - BD->setIsMoveable(false); - - // set previous symbol as unmoveable - auto *Prev = BC->getBinaryDataContainingAddress(Address-1); - if (Prev && Prev->getEndAddress() == BD->getAddress()) { - Prev = Prev->getAtomicRoot(); - DEBUG(if (Prev->isMoveable()) { - dbgs() << "BOLT-DEBUG: setting " << *Prev << " as unmoveable " - << "due to ambiguous relocation (0x" - << Twine::utohexstr(Address) << ") @ 0x" - << Twine::utohexstr(Rel.getOffset()) << "\n"; - }); - Prev->setIsMoveable(false); - } - } - - if (Address == BD->getEndAddress()) { - BD = BD->getAtomicRoot(); - DEBUG(if (BD->isMoveable()) { - dbgs() << "BOLT-DEBUG: setting " << *BD << " as unmoveable " - << "due to ambiguous relocation (0x" - << Twine::utohexstr(Address) << ") @ 0x" - << Twine::utohexstr(Rel.getOffset()) << "\n"; - }); - BD->setIsMoveable(false); - - // set next symbol as unmoveable - auto *Next = BC->getBinaryDataContainingAddress(BD->getEndAddress()); - if (Next && Next->getAddress() == BD->getEndAddress()) { - Next = Next->getAtomicRoot(); - DEBUG(if (Next->isMoveable()) { - dbgs() << "BOLT-DEBUG: setting " << *Next << " as unmoveable " - << "due to ambiguous relocation (0x" - << Twine::utohexstr(Address) << ") @ 0x" - << Twine::utohexstr(Rel.getOffset()) << "\n"; - }); - Next->setIsMoveable(false); - } - } - }; - // If we are allowing section relocations, we assign relocations // that are pointing to the end of a symbol to that symbol rather // than the following symbol. @@ -2486,7 +2432,7 @@ void RewriteInstance::readRelocations(const SectionRef &Section, "up with symbol names referenced in the relocation"); if (!opts::AllowSectionRelocations && IsSectionRelocation) { - markAmbiguousRelocations(BD); + BC->markAmbiguousRelocations(*BD, Address); } ReferencedSymbol = BD->getSymbol(); @@ -2531,7 +2477,7 @@ void RewriteInstance::readRelocations(const SectionRef &Section, if (!opts::AllowSectionRelocations && IsSectionRelocation) { auto *BD = BC->getBinaryDataByName(ReferencedSymbol->getName()); - markAmbiguousRelocations(BD); + BC->markAmbiguousRelocations(*BD, Address); } } } @@ -2553,9 +2499,6 @@ void RewriteInstance::readRelocations(const SectionRef &Section, NumDataRelocations < opts::MaxDataRelocations); }; - if (IsFromCode && IsAArch64) - ForceRelocation = true; - if ((RefSection && refersToReorderedSection(RefSection)) || (opts::ForceToDataRelocations && checkMaxDataRelocations())) ForceRelocation = true;