-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[BOLT] Prevent addRelocation from adding pending relocs #123635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BOLT] Prevent addRelocation from adding pending relocs #123635
Conversation
@llvm/pr-subscribers-bolt Author: Paschalis Mpeis (paschalis-mpeis) ChangesUse addPendingRelocation for that. Full diff: https://github.com/llvm/llvm-project/pull/123635.diff 2 Files Affected:
diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h
index d362961176b326..dedee361882497 100644
--- a/bolt/include/bolt/Core/BinarySection.h
+++ b/bolt/include/bolt/Core/BinarySection.h
@@ -358,15 +358,9 @@ class BinarySection {
/// Add a new relocation at the given /p Offset.
void addRelocation(uint64_t Offset, MCSymbol *Symbol, uint64_t Type,
- uint64_t Addend, uint64_t Value = 0,
- bool Pending = false) {
+ uint64_t Addend, uint64_t Value = 0) {
assert(Offset < getSize() && "offset not within section bounds");
- if (!Pending) {
- Relocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value});
- } else {
- PendingRelocations.emplace_back(
- Relocation{Offset, Symbol, Type, Addend, Value});
- }
+ Relocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value});
}
/// Add a dynamic relocation at the given /p Offset.
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 05b898d34af56c..6e8ad4f62baeff 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -92,12 +92,13 @@ TEST_P(BinaryContextTester, FlushPendingRelocCALL26) {
DataSize, 4);
MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1");
ASSERT_TRUE(RelSymbol1);
- BS.addRelocation(8, RelSymbol1, ELF::R_AARCH64_CALL26, 0, 0, true);
+ BS.addPendingRelocation(
+ Relocation{8, RelSymbol1, ELF::R_AARCH64_CALL26, 0, 0});
MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2");
ASSERT_TRUE(RelSymbol2);
- BS.addRelocation(12, RelSymbol2, ELF::R_AARCH64_CALL26, 0, 0, true);
+ BS.addPendingRelocation(
+ Relocation{12, RelSymbol2, ELF::R_AARCH64_CALL26, 0, 0});
- std::error_code EC;
SmallVector<char> Vect(DataSize);
raw_svector_ostream OS(Vect);
@@ -133,12 +134,13 @@ TEST_P(BinaryContextTester, FlushPendingRelocJUMP26) {
(uint8_t *)Data, Size, 4);
MCSymbol *RelSymbol1 = BC->getOrCreateGlobalSymbol(4, "Func1");
ASSERT_TRUE(RelSymbol1);
- BS.addRelocation(8, RelSymbol1, ELF::R_AARCH64_JUMP26, 0, 0, true);
+ BS.addPendingRelocation(
+ Relocation{8, RelSymbol1, ELF::R_AARCH64_JUMP26, 0, 0});
MCSymbol *RelSymbol2 = BC->getOrCreateGlobalSymbol(16, "Func2");
ASSERT_TRUE(RelSymbol2);
- BS.addRelocation(12, RelSymbol2, ELF::R_AARCH64_JUMP26, 0, 0, true);
+ BS.addPendingRelocation(
+ Relocation{12, RelSymbol2, ELF::R_AARCH64_JUMP26, 0, 0});
- std::error_code EC;
SmallVector<char> Vect(Size);
raw_svector_ostream OS(Vect);
@@ -216,4 +218,4 @@ TEST_P(BinaryContextTester, BaseAddressSegmentsSmallerThanAlignment) {
BC->getBaseAddressForMapping(0xaaaaaaab1000, 0x1000);
ASSERT_TRUE(BaseAddress.has_value());
ASSERT_EQ(*BaseAddress, 0xaaaaaaaa0000ULL);
-}
\ No newline at end of file
+}
|
`addPendingRelocation` now becomes the only way to add a pending relocation. Can no longer use `addRelocation` for this. Update the only user (`BinaryContextTester`).
f235cee
to
186a2ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but please convert the title into imperative form before commit. Thanks!
Done. Thanks Maks. |
`addPendingRelocation` is the only way to add a pending relocation. Can no longer use `addRelocation` for this. Update the only user (`BinaryContextTester`).
`addPendingRelocation` is the only way to add a pending relocation. Can no longer use `addRelocation` for this. Update the only user (`BinaryContextTester`).
`addPendingRelocation` is the only way to add a pending relocation. Can no longer use `addRelocation` for this. Update the only user (`BinaryContextTester`).
addPendingRelocation
now becomes the only way to add a pendingrelocation. Can no longer use
addRelocation
for this.Update the only user (
BinaryContextTester
).