[llvm] [BOLT] Support instrumentation hook via DT_FINI_ARRAY (PR #67348)
Job Noorman via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 4 01:40:19 PDT 2023
https://github.com/mtvec updated https://github.com/llvm/llvm-project/pull/67348
>From dbe2993e1359eaa707f8d5942a254b646812bf56 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Mon, 25 Sep 2023 17:54:53 +0200
Subject: [PATCH 1/7] [BOLT] Support instrumentation hook via DT_FINI_ARRAY
BOLT currently hooks its its instrumentation finalization function via
`DT_FINI`. However, this method of calling finalization routines is not
supported anymore on newer ABIs like RISC-V. `DT_FINI_ARRAY` is
preferred there.
This patch adds support for hooking into `DT_FINI_ARRAY` instead if the
binary does not have a `DT_FINI` entry. If it does, `DT_FINI` takes
precedence so this patch should not change how the currently supported
instrumentation targets behave.
`DT_FINI_ARRAY` points to an array in memory of `DT_FINI_ARRAYSZ` bytes.
It consists of pointer-length entries that contain the addresses of
finalization functions. However, the addresses are only filled-in by the
dynamic linker at load time using relative relocations. This makes
hooking via `DT_FINI_ARRAY` a bit more complicated than via `DT_FINI`.
The implementation works as follows:
- While scanning the binary: find the section where `DT_FINI_ARRAY`
points to, read its first dynamic relocation and use its addend to
find the address of the fini function we will use to hook;
- While writing the output file: overwrite the addend of the dynamic
relocation with the address of the runtime library's fini function.
Updating the dynamic relocation required a bit of boiler plate: since
dynamic relocations are stored in a `std::multiset` which doesn't
support getting mutable references to its items, functions were added to
`BinarySection` to take an existing relocation and insert a new one.
Note on testing: I can currently only test this on RISC-V, but its
instrumentation support hasn't been upstreamed yet as it depends on this
patch which I would like to review separately. I cannot get this patch
to work on x86, even when forcing the linker to omit `DT_FINI`. The
reason is that, even though `DT_FINI_ARRAY` exists and has valid
entries, the functions do not show up in the symbols table which
triggers some asserts later in BOLT. I would propose to not add tests
right now but wait until RISC-V instrumentation is upstreamed which will
automatically test this patch.
---
bolt/include/bolt/Core/BinaryContext.h | 9 +++
bolt/include/bolt/Core/BinarySection.h | 20 +++++-
bolt/include/bolt/Rewrite/RewriteInstance.h | 9 +++
bolt/lib/Rewrite/RewriteInstance.cpp | 67 ++++++++++++++++++-
.../InstrumentationRuntimeLibrary.cpp | 6 --
5 files changed, 102 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index ef57ff3541dc8c9..caba1e7a4ab3915 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -680,6 +680,15 @@ class BinaryContext {
/// the execution of the binary is completed.
std::optional<uint64_t> FiniFunctionAddress;
+ /// DT_FINI_ARRAY. Only used when DT_FINI is not set.
+ std::optional<uint64_t> FiniArrayAddress;
+
+ /// DT_FINI_ARRAYSZ. Only used when DT_FINI is not set.
+ std::optional<uint64_t> FiniArraySize;
+
+ /// Section where DT_FINI_ARRAY points to. Only used when DT_FINI is not set.
+ BinarySection *FiniArraySection{nullptr};
+
/// Page alignment used for code layout.
uint64_t PageAlign{HugePageSize};
diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h
index 326d088d1f0465d..92ab6ea0d38e14e 100644
--- a/bolt/include/bolt/Core/BinarySection.h
+++ b/bolt/include/bolt/Core/BinarySection.h
@@ -375,8 +375,12 @@ class BinarySection {
/// Add a dynamic relocation at the given /p Offset.
void addDynamicRelocation(uint64_t Offset, MCSymbol *Symbol, uint64_t Type,
uint64_t Addend, uint64_t Value = 0) {
- assert(Offset < getSize() && "offset not within section bounds");
- DynamicRelocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value});
+ addDynamicRelocation(Relocation{Offset, Symbol, Type, Addend, Value});
+ }
+
+ void addDynamicRelocation(const Relocation &Reloc) {
+ assert(Reloc.Offset < getSize() && "offset not within section bounds");
+ DynamicRelocations.emplace(Reloc);
}
/// Add relocation against the original contents of this section.
@@ -410,6 +414,18 @@ class BinarySection {
return Itr != DynamicRelocations.end() ? &*Itr : nullptr;
}
+ std::optional<Relocation> takeDynamicRelocationAt(uint64_t Offset) {
+ Relocation Key{Offset, 0, 0, 0, 0};
+ auto Itr = DynamicRelocations.find(Key);
+
+ if (Itr == DynamicRelocations.end())
+ return std::nullopt;
+
+ Relocation Reloc = *Itr;
+ DynamicRelocations.erase(Itr);
+ return Reloc;
+ }
+
uint64_t hash(const BinaryData &BD) const {
std::map<const BinaryData *, uint64_t> Cache;
return hash(BD, Cache);
diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h
index 1dacdc944f9775b..a002d9d88098bc6 100644
--- a/bolt/include/bolt/Rewrite/RewriteInstance.h
+++ b/bolt/include/bolt/Rewrite/RewriteInstance.h
@@ -95,6 +95,15 @@ class RewriteInstance {
/// from meta data in the file.
void discoverFileObjects();
+ /// Check whether we should use DT_FINI or DT_FINI_ARRAY for instrumentation.
+ /// DT_FINI is preferred; DT_FINI_ARRAY is only used when no DT_FINI entry was
+ /// found.
+ Error discoverFiniAddress();
+
+ /// If DT_FINI_ARRAY is used for instrumentation, update the relocation of its
+ /// first entry to point to the instrumentation library's fini address.
+ void updateFiniReloc();
+
/// Create and initialize metadata rewriters for this instance.
void initializeMetadataManager();
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 85c0092b505f73b..fe9e0981f6c9443 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -700,6 +700,10 @@ Error RewriteInstance::run() {
adjustCommandLineOptions();
discoverFileObjects();
+ if (opts::Instrument)
+ if (Error E = discoverFiniAddress())
+ return E;
+
preprocessProfileData();
// Skip disassembling if we have a translation table and we are running an
@@ -1276,6 +1280,60 @@ void RewriteInstance::discoverFileObjects() {
registerFragments();
}
+Error RewriteInstance::discoverFiniAddress() {
+ // If FiniFunctionAddress is already set, we got if from DT_FINI. We use
+ // DT_FINI instead of DT_FINI_ARRAY if it's available.
+ if (BC->FiniFunctionAddress)
+ return Error::success();
+
+ if (BC->IsStaticExecutable)
+ return Error::success();
+
+ if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
+ return createStringError(
+ std::errc::not_supported,
+ "Instrumentation needs either DT_FINI or DT_FINI_ARRAY");
+ }
+
+ if (*BC->FiniArraySize < BC->AsmInfo->getCodePointerSize()) {
+ return createStringError(std::errc::not_supported,
+ "Need at least 1 DT_FINI_ARRAY slot");
+ }
+
+ ErrorOr<BinarySection &> FiniArraySection =
+ BC->getSectionForAddress(*BC->FiniArrayAddress);
+ if (auto EC = FiniArraySection.getError())
+ return errorCodeToError(EC);
+
+ BC->FiniArraySection = &*FiniArraySection;
+ const Relocation *FiniFunctionReloc =
+ FiniArraySection->getDynamicRelocationAt(0);
+
+ if (!FiniFunctionReloc) {
+ return createStringError(std::errc::not_supported,
+ "No relocation for first DT_FINI_ARRAY slot");
+ }
+
+ BC->FiniFunctionAddress = FiniFunctionReloc->Addend;
+ return Error::success();
+}
+
+void RewriteInstance::updateFiniReloc() {
+ if (!BC->FiniArraySection)
+ return;
+
+ const RuntimeLibrary *RT = BC->getRuntimeLibrary();
+ if (!RT)
+ return;
+
+ std::optional<Relocation> FiniFunctionReloc =
+ BC->FiniArraySection->takeDynamicRelocationAt(0);
+ assert(FiniFunctionReloc && "First DT_FINI_ARRAY relocation removed");
+
+ FiniFunctionReloc->Addend = RT->getRuntimeFiniAddress();
+ BC->FiniArraySection->addDynamicRelocation(*FiniFunctionReloc);
+}
+
void RewriteInstance::registerFragments() {
if (!BC->HasSplitFunctions)
return;
@@ -5008,7 +5066,7 @@ void RewriteInstance::patchELFDynamic(ELFObjectFile<ELFT> *File) {
}
}
RuntimeLibrary *RtLibrary = BC->getRuntimeLibrary();
- if (RtLibrary && Dyn.getTag() == ELF::DT_FINI) {
+ if (RtLibrary && Dyn.getTag() == ELF::DT_FINI && !BC->FiniArraySection) {
if (uint64_t Addr = RtLibrary->getRuntimeFiniAddress())
NewDE.d_un.d_ptr = Addr;
}
@@ -5091,6 +5149,12 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile<ELFT> *File) {
case ELF::DT_FINI:
BC->FiniFunctionAddress = Dyn.getPtr();
break;
+ case ELF::DT_FINI_ARRAY:
+ BC->FiniArrayAddress = Dyn.getPtr();
+ break;
+ case ELF::DT_FINI_ARRAYSZ:
+ BC->FiniArraySize = Dyn.getPtr();
+ break;
case ELF::DT_RELA:
DynamicRelocationsAddress = Dyn.getPtr();
break;
@@ -5339,6 +5403,7 @@ void RewriteInstance::rewriteFile() {
// Copy non-allocatable sections once allocatable part is finished.
rewriteNoteSections();
+ updateFiniReloc();
if (BC->HasRelocations) {
patchELFAllocatableRelaSections();
diff --git a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
index cc36406543f3995..3d0830eb4f6b1e9 100644
--- a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
@@ -57,12 +57,6 @@ void InstrumentationRuntimeLibrary::adjustCommandLineOptions(
"the input binary\n";
exit(1);
}
- if (!BC.FiniFunctionAddress && !BC.IsStaticExecutable) {
- errs() << "BOLT-ERROR: input binary lacks DT_FINI entry in the dynamic "
- "section but instrumentation currently relies on patching "
- "DT_FINI to write the profile\n";
- exit(1);
- }
if ((opts::InstrumentationWaitForks || opts::InstrumentationSleepTime) &&
opts::InstrumentationFileAppendPID) {
>From 85a737f5773306fa68ba27a6cd5dc44d1a17fa09 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 26 Sep 2023 10:03:30 +0200
Subject: [PATCH 2/7] Address reviewer comments
---
bolt/lib/Rewrite/RewriteInstance.cpp | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index fe9e0981f6c9443..55b619e7c6ca037 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -700,7 +700,7 @@ Error RewriteInstance::run() {
adjustCommandLineOptions();
discoverFileObjects();
- if (opts::Instrument)
+ if (opts::Instrument && !BC->IsStaticExecutable)
if (Error E = discoverFiniAddress())
return E;
@@ -1286,9 +1286,6 @@ Error RewriteInstance::discoverFiniAddress() {
if (BC->FiniFunctionAddress)
return Error::success();
- if (BC->IsStaticExecutable)
- return Error::success();
-
if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
return createStringError(
std::errc::not_supported,
@@ -1323,7 +1320,7 @@ void RewriteInstance::updateFiniReloc() {
return;
const RuntimeLibrary *RT = BC->getRuntimeLibrary();
- if (!RT)
+ if (!RT || !RT->getRuntimeFiniAddress())
return;
std::optional<Relocation> FiniFunctionReloc =
@@ -5066,7 +5063,7 @@ void RewriteInstance::patchELFDynamic(ELFObjectFile<ELFT> *File) {
}
}
RuntimeLibrary *RtLibrary = BC->getRuntimeLibrary();
- if (RtLibrary && Dyn.getTag() == ELF::DT_FINI && !BC->FiniArraySection) {
+ if (RtLibrary && Dyn.getTag() == ELF::DT_FINI) {
if (uint64_t Addr = RtLibrary->getRuntimeFiniAddress())
NewDE.d_un.d_ptr = Addr;
}
>From 6a2349537c7a4433e9bc557cf36e1c7ffc5a39d8 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 26 Sep 2023 16:19:59 +0200
Subject: [PATCH 3/7] Support non-PIE binaries via static relocation patching
---
bolt/lib/Rewrite/RewriteInstance.cpp | 39 ++++++++++++++++++----------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 55b619e7c6ca037..f04851893b224b3 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -740,6 +740,8 @@ Error RewriteInstance::run() {
updateMetadata();
+ updateFiniReloc();
+
if (opts::LinuxKernelMode) {
errs() << "BOLT-WARNING: not writing the output file for Linux Kernel\n";
return Error::success();
@@ -1303,16 +1305,19 @@ Error RewriteInstance::discoverFiniAddress() {
return errorCodeToError(EC);
BC->FiniArraySection = &*FiniArraySection;
- const Relocation *FiniFunctionReloc =
- FiniArraySection->getDynamicRelocationAt(0);
- if (!FiniFunctionReloc) {
- return createStringError(std::errc::not_supported,
- "No relocation for first DT_FINI_ARRAY slot");
+ if (const Relocation *Reloc = FiniArraySection->getDynamicRelocationAt(0)) {
+ BC->FiniFunctionAddress = Reloc->Addend;
+ return Error::success();
}
- BC->FiniFunctionAddress = FiniFunctionReloc->Addend;
- return Error::success();
+ if (const Relocation *Reloc = FiniArraySection->getRelocationAt(0)) {
+ BC->FiniFunctionAddress = Reloc->Value;
+ return Error::success();
+ }
+
+ return createStringError(std::errc::not_supported,
+ "No relocation for first DT_FINI_ARRAY slot");
}
void RewriteInstance::updateFiniReloc() {
@@ -1323,12 +1328,21 @@ void RewriteInstance::updateFiniReloc() {
if (!RT || !RT->getRuntimeFiniAddress())
return;
- std::optional<Relocation> FiniFunctionReloc =
- BC->FiniArraySection->takeDynamicRelocationAt(0);
- assert(FiniFunctionReloc && "First DT_FINI_ARRAY relocation removed");
+ if (std::optional<Relocation> Reloc =
+ BC->FiniArraySection->takeDynamicRelocationAt(0)) {
+ Reloc->Addend = RT->getRuntimeFiniAddress();
+ BC->FiniArraySection->addDynamicRelocation(*Reloc);
+ return;
+ }
- FiniFunctionReloc->Addend = RT->getRuntimeFiniAddress();
- BC->FiniArraySection->addDynamicRelocation(*FiniFunctionReloc);
+ // There is no dynamic relocation so we have to patch the static one. Add a
+ // pending relocation which will get patched when flushPendingRelocations is
+ // called in rewriteFile. Note that flushPendingRelocations will calculate the
+ // value to patch as "Symbol + Addend". Since we don't have a symbol, just set
+ // the addend to the desired value.
+ BC->FiniArraySection->addPendingRelocation(Relocation{
+ /*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(),
+ /*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0});
}
void RewriteInstance::registerFragments() {
@@ -5400,7 +5414,6 @@ void RewriteInstance::rewriteFile() {
// Copy non-allocatable sections once allocatable part is finished.
rewriteNoteSections();
- updateFiniReloc();
if (BC->HasRelocations) {
patchELFAllocatableRelaSections();
>From 37c3cd5154943d8dde6339b30e84a26046f6e817 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Mon, 2 Oct 2023 13:29:53 +0200
Subject: [PATCH 4/7] Rename discover/update functions to include Rt in their
name
---
bolt/include/bolt/Rewrite/RewriteInstance.h | 4 ++--
bolt/lib/Rewrite/RewriteInstance.cpp | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h
index a002d9d88098bc6..294550bd8db3351 100644
--- a/bolt/include/bolt/Rewrite/RewriteInstance.h
+++ b/bolt/include/bolt/Rewrite/RewriteInstance.h
@@ -98,11 +98,11 @@ class RewriteInstance {
/// Check whether we should use DT_FINI or DT_FINI_ARRAY for instrumentation.
/// DT_FINI is preferred; DT_FINI_ARRAY is only used when no DT_FINI entry was
/// found.
- Error discoverFiniAddress();
+ Error discoverRtFiniAddress();
/// If DT_FINI_ARRAY is used for instrumentation, update the relocation of its
/// first entry to point to the instrumentation library's fini address.
- void updateFiniReloc();
+ void updateRtFiniReloc();
/// Create and initialize metadata rewriters for this instance.
void initializeMetadataManager();
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index f04851893b224b3..2e53776268d21a3 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -701,7 +701,7 @@ Error RewriteInstance::run() {
discoverFileObjects();
if (opts::Instrument && !BC->IsStaticExecutable)
- if (Error E = discoverFiniAddress())
+ if (Error E = discoverRtFiniAddress())
return E;
preprocessProfileData();
@@ -740,7 +740,7 @@ Error RewriteInstance::run() {
updateMetadata();
- updateFiniReloc();
+ updateRtFiniReloc();
if (opts::LinuxKernelMode) {
errs() << "BOLT-WARNING: not writing the output file for Linux Kernel\n";
@@ -1282,7 +1282,7 @@ void RewriteInstance::discoverFileObjects() {
registerFragments();
}
-Error RewriteInstance::discoverFiniAddress() {
+Error RewriteInstance::discoverRtFiniAddress() {
// If FiniFunctionAddress is already set, we got if from DT_FINI. We use
// DT_FINI instead of DT_FINI_ARRAY if it's available.
if (BC->FiniFunctionAddress)
@@ -1320,7 +1320,7 @@ Error RewriteInstance::discoverFiniAddress() {
"No relocation for first DT_FINI_ARRAY slot");
}
-void RewriteInstance::updateFiniReloc() {
+void RewriteInstance::updateRtFiniReloc() {
if (!BC->FiniArraySection)
return;
>From 279696e1e9a938cc803b050c10a49a0a99e5c273 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 3 Oct 2023 16:10:40 +0200
Subject: [PATCH 5/7] Always patch static relocations
---
bolt/lib/Rewrite/RewriteInstance.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 2e53776268d21a3..448cf75c693319a 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1332,14 +1332,13 @@ void RewriteInstance::updateRtFiniReloc() {
BC->FiniArraySection->takeDynamicRelocationAt(0)) {
Reloc->Addend = RT->getRuntimeFiniAddress();
BC->FiniArraySection->addDynamicRelocation(*Reloc);
- return;
}
- // There is no dynamic relocation so we have to patch the static one. Add a
- // pending relocation which will get patched when flushPendingRelocations is
- // called in rewriteFile. Note that flushPendingRelocations will calculate the
- // value to patch as "Symbol + Addend". Since we don't have a symbol, just set
- // the addend to the desired value.
+ // Update the static relocation by adding a pending relocation which will get
+ // patched when flushPendingRelocations is called in rewriteFile. Note that
+ // flushPendingRelocations will calculate the value to patch as
+ // "Symbol + Addend". Since we don't have a symbol, just set the addend to the
+ // desired value.
BC->FiniArraySection->addPendingRelocation(Relocation{
/*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(),
/*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0});
>From b39017fd84eb0c0b74d860a96d5e43c4d12d14c8 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Wed, 4 Oct 2023 08:51:44 +0200
Subject: [PATCH 6/7] Remove BinaryContext::FiniArraySection
---
bolt/include/bolt/Core/BinaryContext.h | 3 ---
bolt/lib/Rewrite/RewriteInstance.cpp | 14 ++++++++------
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index caba1e7a4ab3915..db0d94f43c84207 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -686,9 +686,6 @@ class BinaryContext {
/// DT_FINI_ARRAYSZ. Only used when DT_FINI is not set.
std::optional<uint64_t> FiniArraySize;
- /// Section where DT_FINI_ARRAY points to. Only used when DT_FINI is not set.
- BinarySection *FiniArraySection{nullptr};
-
/// Page alignment used for code layout.
uint64_t PageAlign{HugePageSize};
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 448cf75c693319a..fdcd3b79fe3e32c 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1304,8 +1304,6 @@ Error RewriteInstance::discoverRtFiniAddress() {
if (auto EC = FiniArraySection.getError())
return errorCodeToError(EC);
- BC->FiniArraySection = &*FiniArraySection;
-
if (const Relocation *Reloc = FiniArraySection->getDynamicRelocationAt(0)) {
BC->FiniFunctionAddress = Reloc->Addend;
return Error::success();
@@ -1321,17 +1319,21 @@ Error RewriteInstance::discoverRtFiniAddress() {
}
void RewriteInstance::updateRtFiniReloc() {
- if (!BC->FiniArraySection)
+ if (!BC->FiniArrayAddress)
return;
const RuntimeLibrary *RT = BC->getRuntimeLibrary();
if (!RT || !RT->getRuntimeFiniAddress())
return;
+ ErrorOr<BinarySection &> FiniArraySection =
+ BC->getSectionForAddress(*BC->FiniArrayAddress);
+ assert(FiniArraySection && ".fini_array removed");
+
if (std::optional<Relocation> Reloc =
- BC->FiniArraySection->takeDynamicRelocationAt(0)) {
+ FiniArraySection->takeDynamicRelocationAt(0)) {
Reloc->Addend = RT->getRuntimeFiniAddress();
- BC->FiniArraySection->addDynamicRelocation(*Reloc);
+ FiniArraySection->addDynamicRelocation(*Reloc);
}
// Update the static relocation by adding a pending relocation which will get
@@ -1339,7 +1341,7 @@ void RewriteInstance::updateRtFiniReloc() {
// flushPendingRelocations will calculate the value to patch as
// "Symbol + Addend". Since we don't have a symbol, just set the addend to the
// desired value.
- BC->FiniArraySection->addPendingRelocation(Relocation{
+ FiniArraySection->addPendingRelocation(Relocation{
/*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(),
/*Addend*/ RT->getRuntimeFiniAddress(), /*Value*/ 0});
}
>From 4e7069f7b9e521429b7cd3278a5dfeec1a4d6d3c Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Wed, 4 Oct 2023 10:37:30 +0200
Subject: [PATCH 7/7] Make sure we can distinguish using DT_FINI and
DT_FINI_ARRAY
---
bolt/include/bolt/Core/BinaryContext.h | 3 +++
bolt/lib/Rewrite/RewriteInstance.cpp | 17 ++++++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index db0d94f43c84207..39613228e908142 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -680,6 +680,9 @@ class BinaryContext {
/// the execution of the binary is completed.
std::optional<uint64_t> FiniFunctionAddress;
+ /// DT_FINI.
+ std::optional<uint64_t> FiniAddress;
+
/// DT_FINI_ARRAY. Only used when DT_FINI is not set.
std::optional<uint64_t> FiniArrayAddress;
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index fdcd3b79fe3e32c..339bc0730dc47f0 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1283,10 +1283,11 @@ void RewriteInstance::discoverFileObjects() {
}
Error RewriteInstance::discoverRtFiniAddress() {
- // If FiniFunctionAddress is already set, we got if from DT_FINI. We use
- // DT_FINI instead of DT_FINI_ARRAY if it's available.
- if (BC->FiniFunctionAddress)
+ // Use DT_FINI if it's available.
+ if (BC->FiniAddress) {
+ BC->FiniFunctionAddress = BC->FiniAddress;
return Error::success();
+ }
if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
return createStringError(
@@ -1319,19 +1320,25 @@ Error RewriteInstance::discoverRtFiniAddress() {
}
void RewriteInstance::updateRtFiniReloc() {
- if (!BC->FiniArrayAddress)
+ // Updating DT_FINI is handled by patchELFDynamic.
+ if (BC->FiniAddress)
return;
const RuntimeLibrary *RT = BC->getRuntimeLibrary();
if (!RT || !RT->getRuntimeFiniAddress())
return;
+ assert(BC->FiniArrayAddress && BC->FiniArraySize &&
+ "inconsistent .fini_array state");
+
ErrorOr<BinarySection &> FiniArraySection =
BC->getSectionForAddress(*BC->FiniArrayAddress);
assert(FiniArraySection && ".fini_array removed");
if (std::optional<Relocation> Reloc =
FiniArraySection->takeDynamicRelocationAt(0)) {
+ assert(Reloc->Addend == BC->FiniFunctionAddress &&
+ "inconsistent .fini_array dynamic relocation");
Reloc->Addend = RT->getRuntimeFiniAddress();
FiniArraySection->addDynamicRelocation(*Reloc);
}
@@ -5159,7 +5166,7 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile<ELFT> *File) {
}
break;
case ELF::DT_FINI:
- BC->FiniFunctionAddress = Dyn.getPtr();
+ BC->FiniAddress = Dyn.getPtr();
break;
case ELF::DT_FINI_ARRAY:
BC->FiniArrayAddress = Dyn.getPtr();
More information about the llvm-commits
mailing list