[BOLT] Support runtime library hook via DT_INIT_ARRAY #167467
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vleonen
requested review from
aaupov,
ayermolo,
maksfb,
paschalis-mpeis,
rafaelauler,
yota9 and
yozhu
as code owners
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Member
|
@llvm/pr-subscribers-bolt Author: Vasily Leonenko (vleonen) ChangesMajor part of this PR is commit implementing support for DT_INIT_ARRAY for instrumentation initialization. Also, it adds related hook-init test & fixes couple of X86 instrumentation tests. This commit follows implementation of instrumentation hook via Initialization has has differences compared to finalization:
Patch is 22.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167467.diff 7 Files Affected:
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index 2af1d330b7545..8a90febcea3cc 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -807,6 +807,15 @@ class BinaryContext {
/// the execution of the binary is completed.
std::optional<uint64_t> FiniFunctionAddress;
+ /// DT_INIT.
+ std::optional<uint64_t> InitAddress;
+
+ /// DT_INIT_ARRAY. Only used when DT_INIT is not set.
+ std::optional<uint64_t> InitArrayAddress;
+
+ /// DT_INIT_ARRAYSZ. Only used when DT_INIT is not set.
+ std::optional<uint64_t> InitArraySize;
+
/// DT_FINI.
std::optional<uint64_t> FiniAddress;
diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h
index 0fe2e32b61933..fe3f92bcaa657 100644
--- a/bolt/include/bolt/Rewrite/RewriteInstance.h
+++ b/bolt/include/bolt/Rewrite/RewriteInstance.h
@@ -93,11 +93,20 @@ class RewriteInstance {
/// section allocations if found.
void discoverBOLTReserved();
+ /// Check whether we should use DT_INIT or DT_INIT_ARRAY for instrumentation.
+ /// DT_INIT is preferred; DT_INIT_ARRAY is only used when no DT_INIT entry was
+ /// found.
+ Error discoverRtInitAddress();
+
/// 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 discoverRtFiniAddress();
+ /// If DT_INIT_ARRAY is used for instrumentation, update the relocation of its
+ /// first entry to point to the instrumentation library's init address.
+ void updateRtInitReloc();
+
/// 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 updateRtFiniReloc();
diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp
index 150461b020f06..f6af0f85b8d1a 100644
--- a/bolt/lib/Passes/Instrumentation.cpp
+++ b/bolt/lib/Passes/Instrumentation.cpp
@@ -34,6 +34,12 @@ cl::opt<std::string> InstrumentationFilename(
"/tmp/prof.fdata)"),
cl::init("/tmp/prof.fdata"), cl::Optional, cl::cat(BoltInstrCategory));
+cl::opt<bool> InstrumentNoUseEntryPoint(
+ "instrumentation-no-use-entry-point",
+ cl::desc("Do not use ELF entry point for instrumentation "
+ "initialization (e.g. for libc) (default: false)"),
+ cl::init(false), cl::Optional, cl::cat(BoltInstrCategory));
+
cl::opt<std::string> InstrumentationBinpath(
"instrumentation-binpath",
cl::desc("path to instrumented binary in case if /proc/self/map_files "
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 5769577aa3f74..48ca013c00e3e 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -80,6 +80,8 @@ namespace opts {
extern cl::list<std::string> HotTextMoveSections;
extern cl::opt<bool> Hugify;
extern cl::opt<bool> Instrument;
+extern cl::opt<bool> InstrumentNoUseEntryPoint;
+extern cl::opt<uint32_t> InstrumentationSleepTime;
extern cl::opt<bool> KeepNops;
extern cl::opt<bool> Lite;
extern cl::list<std::string> ReorderData;
@@ -737,9 +739,12 @@ Error RewriteInstance::run() {
adjustCommandLineOptions();
discoverFileObjects();
- if (opts::Instrument && !BC->IsStaticExecutable)
+ if (opts::Instrument && !BC->IsStaticExecutable) {
+ if (Error E = discoverRtInitAddress())
+ return E;
if (Error E = discoverRtFiniAddress())
return E;
+ }
preprocessProfileData();
@@ -781,8 +786,10 @@ Error RewriteInstance::run() {
updateMetadata();
- if (opts::Instrument && !BC->IsStaticExecutable)
+ if (opts::Instrument && !BC->IsStaticExecutable) {
+ updateRtInitReloc();
updateRtFiniReloc();
+ }
if (opts::OutputFilename == "/dev/null") {
BC->outs() << "BOLT-INFO: skipping writing final binary to disk\n";
@@ -1407,6 +1414,55 @@ void RewriteInstance::discoverBOLTReserved() {
NextAvailableAddress = BC->BOLTReserved.start();
}
+Error RewriteInstance::discoverRtInitAddress() {
+ if (BC->HasInterpHeader && !opts::InstrumentNoUseEntryPoint)
+ return Error::success();
+
+ // Use DT_INIT if it's available.
+ if (BC->InitAddress) {
+ BC->StartFunctionAddress = BC->InitAddress;
+ return Error::success();
+ }
+
+ if (!BC->InitArrayAddress || !BC->InitArraySize) {
+ return createStringError(std::errc::not_supported,
+ "Instrumentation of shared library needs either "
+ "DT_INIT or DT_INIT_ARRAY");
+ }
+
+ if (*BC->InitArraySize < BC->AsmInfo->getCodePointerSize()) {
+ return createStringError(std::errc::not_supported,
+ "Need at least 1 DT_FINI_ARRAY slot");
+ }
+
+ ErrorOr<BinarySection &> InitArraySection =
+ BC->getSectionForAddress(*BC->InitArrayAddress);
+ if (auto EC = InitArraySection.getError())
+ return errorCodeToError(EC);
+
+ if (const Relocation *Reloc = InitArraySection->getDynamicRelocationAt(0)) {
+ if (Reloc->isRelative()) {
+ BC->StartFunctionAddress = Reloc->Addend;
+ } else {
+ MCSymbol *Sym = Reloc->Symbol;
+ assert(Sym && "Failed to locate symbol for 0 entry of .init_array");
+ const BinaryFunction *BF = BC->getFunctionForSymbol(Sym);
+ assert(BF &&
+ "Failed to locate binary function for 0 entry of .init_array");
+ BC->StartFunctionAddress = BF->getAddress() + Reloc->Addend;
+ }
+ return Error::success();
+ }
+
+ if (const Relocation *Reloc = InitArraySection->getRelocationAt(0)) {
+ BC->StartFunctionAddress = Reloc->Value;
+ return Error::success();
+ }
+
+ return createStringError(std::errc::not_supported,
+ "No relocation for first DT_INIT_ARRAY slot");
+}
+
Error RewriteInstance::discoverRtFiniAddress() {
// Use DT_FINI if it's available.
if (BC->FiniAddress) {
@@ -1415,6 +1471,10 @@ Error RewriteInstance::discoverRtFiniAddress() {
}
if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
+ // It is still possible to generate profile without fini hook if
+ // InstrumentationSleepTime is set
+ if (opts::InstrumentationSleepTime > 0)
+ return Error::success();
return createStringError(
std::errc::not_supported,
"Instrumentation needs either DT_FINI or DT_FINI_ARRAY");
@@ -1444,6 +1504,58 @@ Error RewriteInstance::discoverRtFiniAddress() {
"No relocation for first DT_FINI_ARRAY slot");
}
+void RewriteInstance::updateRtInitReloc() {
+ if (BC->HasInterpHeader && !opts::InstrumentNoUseEntryPoint)
+ return;
+
+ // Updating DT_INIT is handled by patchELFDynamic.
+ if (BC->InitAddress)
+ return;
+
+ const RuntimeLibrary *RT = BC->getRuntimeLibrary();
+ if (!RT || !RT->getRuntimeStartAddress())
+ return;
+
+ if (!BC->InitArrayAddress)
+ return;
+
+ assert(BC->InitArrayAddress && BC->InitArraySize &&
+ "inconsistent .init_array state");
+
+ ErrorOr<BinarySection &> InitArraySection =
+ BC->getSectionForAddress(*BC->InitArrayAddress);
+ assert(InitArraySection && ".init_array removed");
+
+ if (std::optional<Relocation> Reloc =
+ InitArraySection->takeDynamicRelocationAt(0)) {
+ if (Reloc->isRelative()) {
+ assert(Reloc->Addend == BC->StartFunctionAddress &&
+ "inconsistent .init_array dynamic relocation");
+ Reloc->Addend = RT->getRuntimeStartAddress();
+ InitArraySection->addDynamicRelocation(*Reloc);
+ } else {
+ MCSymbol *Sym = Reloc->Symbol;
+ assert(Sym && "Failed to locate symbol for 0 entry of .init_array");
+ const BinaryFunction *BF = BC->getFunctionForSymbol(Sym);
+ assert(BF &&
+ "Failed to locate binary function for 0 entry of .init_array");
+ assert(BF->getAddress() + Reloc->Addend == BC->StartFunctionAddress &&
+ "inconsistent .init_array dynamic relocation");
+ InitArraySection->addDynamicRelocation(Relocation{
+ /*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(),
+ /*Addend*/ RT->getRuntimeStartAddress(), /*Value*/ 0});
+ }
+ }
+ // 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.
+ InitArraySection->addPendingRelocation(Relocation{
+ /*Offset*/ 0, /*Symbol*/ nullptr, /*Type*/ Relocation::getAbs64(),
+ /*Addend*/ RT->getRuntimeStartAddress(), /*Value*/ 0});
+}
+
void RewriteInstance::updateRtFiniReloc() {
// Updating DT_FINI is handled by patchELFDynamic.
if (BC->FiniAddress)
@@ -1453,6 +1565,13 @@ void RewriteInstance::updateRtFiniReloc() {
if (!RT || !RT->getRuntimeFiniAddress())
return;
+ // It is still possible to generate profile without fini hook if
+ // InstrumentationSleepTime is set
+ if ((!BC->FiniArrayAddress || !BC->FiniArraySize) &&
+ opts::InstrumentationSleepTime > 0) {
+ return;
+ }
+
assert(BC->FiniArrayAddress && BC->FiniArraySize &&
"inconsistent .fini_array state");
@@ -4838,7 +4957,8 @@ void RewriteInstance::patchELFSectionHeaderTable(ELFObjectFile<ELFT> *File) {
ELFEhdrTy NewEhdr = Obj.getHeader();
if (BC->HasRelocations) {
- if (RuntimeLibrary *RtLibrary = BC->getRuntimeLibrary())
+ RuntimeLibrary *RtLibrary = BC->getRuntimeLibrary();
+ if (RtLibrary && !opts::InstrumentNoUseEntryPoint)
NewEhdr.e_entry = RtLibrary->getRuntimeStartAddress();
else
NewEhdr.e_entry = getNewFunctionAddress(NewEhdr.e_entry);
@@ -5684,7 +5804,8 @@ void RewriteInstance::patchELFDynamic(ELFObjectFile<ELFT> *File) {
if (uint64_t Addr = RtLibrary->getRuntimeFiniAddress())
NewDE.d_un.d_ptr = Addr;
}
- if (RtLibrary && Dyn.getTag() == ELF::DT_INIT && !BC->HasInterpHeader) {
+ if (RtLibrary && Dyn.getTag() == ELF::DT_INIT &&
+ (!BC->HasInterpHeader || opts::InstrumentNoUseEntryPoint)) {
if (auto Addr = RtLibrary->getRuntimeStartAddress()) {
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Set DT_INIT to 0x"
<< Twine::utohexstr(Addr) << '\n');
@@ -5760,6 +5881,13 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile<ELFT> *File) {
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Set start function address\n");
BC->StartFunctionAddress = Dyn.getPtr();
}
+ BC->InitAddress = Dyn.getPtr();
+ break;
+ case ELF::DT_INIT_ARRAY:
+ BC->InitArrayAddress = Dyn.getPtr();
+ break;
+ case ELF::DT_INIT_ARRAYSZ:
+ BC->InitArraySize = Dyn.getPtr();
break;
case ELF::DT_FINI:
BC->FiniAddress = Dyn.getPtr();
diff --git a/bolt/test/AArch64/hook-init.s b/bolt/test/AArch64/hook-init.s
new file mode 100644
index 0000000000000..1bf4bbddea7d0
--- /dev/null
+++ b/bolt/test/AArch64/hook-init.s
@@ -0,0 +1,191 @@
+## Test the different ways of hooking the init function for instrumentation (via
+## entry point, DT_INIT and via DT_INIT_ARRAY). We test the latter for both PIE
+## and non-PIE binaries because of the different ways of handling relocations
+## (static or dynamic), executable and shared library.
+## All tests perform the following steps:
+## - Compile and link for the case to be tested
+## - Some sanity-checks on the dynamic section and relocations in the binary to
+## verify it has the shape we want for testing:
+## - INTERP in Program Headers
+## - DT_INIT or DT_INIT_ARRAY in dynamic section
+## - No relative relocations for non-PIE
+## - Instrument (with extra instrumentation-no-use-entry-point option in some cases)
+## - Verify generated binary
+# REQUIRES: system-linux,bolt-runtime,target=aarch64{{.*}}
+
+# RUN: %clang %cflags -pie %s -Wl,-q -o %t.exe
+# RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-INIT %s
+# RUN: llvm-readelf -l %t.exe | FileCheck --check-prefix=PH-INTERP %s
+# RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
+# RUN: llvm-bolt %t.exe -o %t --instrument
+# RUN: llvm-readelf -hdrs %t | FileCheck --check-prefix=CHECK-INIT-EP %s
+# RUN: llvm-bolt %t.exe -o %t-no-ep --instrument --instrumentation-no-use-entry-point
+# RUN: llvm-readelf -hdrs %t-no-ep | FileCheck --check-prefix=CHECK-INIT-NO-EP %s
+
+# RUN: %clang -shared %cflags -pie %s -Wl,-q -o %t-shared.exe
+# RUN: llvm-readelf -d %t-shared.exe | FileCheck --check-prefix=DYN-INIT %s
+# RUN: llvm-readelf -l %t-shared.exe | FileCheck --check-prefix=PH-INTERP-SHARED %s
+# RUN: llvm-readelf -r %t-shared.exe | FileCheck --check-prefix=RELOC-SHARED-PIE %s
+# RUN: llvm-bolt %t-shared.exe -o %t-shared --instrument
+# RUN: llvm-readelf -hdrs %t-shared | FileCheck --check-prefix=CHECK-SHARED-INIT %s
+
+# RUN: %clang %cflags -pie %s -Wl,-q,-init=0 -o %t-no-init.exe
+# RUN: llvm-readelf -d %t-no-init.exe | FileCheck --check-prefix=DYN-NO-INIT %s
+# RUN: llvm-readelf -l %t-no-init.exe | FileCheck --check-prefix=PH-INTERP %s
+# RUN: llvm-readelf -r %t-no-init.exe | FileCheck --check-prefix=RELOC-PIE %s
+# RUN: llvm-bolt %t-no-init.exe -o %t-no-init --instrument
+# RUN: llvm-readelf -hdrs %t-no-init | FileCheck --check-prefix=CHECK-NO-INIT-EP %s
+# RUN: llvm-bolt %t-no-init.exe -o %t-no-init-no-ep --instrument --instrumentation-no-use-entry-point
+# TODO: CHECK DT_INIT_ARRAY with instrumentation-no-use-entry-point
+# RUN: llvm-readelf -hdrs %t-no-init-no-ep | FileCheck --check-prefix=CHECK-NO-INIT-NO-EP %s
+
+# RUN: %clang -shared %cflags -pie %s -Wl,-q,-init=0 -o %t-shared-no-init.exe
+# RUN: llvm-readelf -d %t-shared-no-init.exe | FileCheck --check-prefix=DYN-NO-INIT %s
+# RUN: llvm-readelf -l %t-shared-no-init.exe | FileCheck --check-prefix=PH-INTERP-SHARED %s
+# RUN: llvm-readelf -r %t-shared-no-init.exe | FileCheck --check-prefix=RELOC-SHARED-PIE %s
+# RUN: llvm-bolt %t-shared-no-init.exe -o %t-shared-no-init --instrument
+# RUN: llvm-readelf -drs %t-shared-no-init | FileCheck --check-prefix=CHECK-SHARED-NO-INIT %s
+
+## Create a dummy shared library to link against to force creation of the dynamic section.
+# RUN: %clang %cflags %p/../Inputs/stub.c -fPIC -shared -o %t-stub.so
+# RUN: %clang %cflags %s -no-pie -Wl,-q,-init=0 %t-stub.so -o %t-no-pie-no-init.exe
+# RUN: llvm-readelf -r %t-no-pie-no-init.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
+# RUN: llvm-bolt %t-no-pie-no-init.exe -o %t-no-pie-no-init --instrument
+# RUN: llvm-readelf -hds %t-no-pie-no-init | FileCheck --check-prefix=CHECK-NO-PIE-NO-INIT-EP %s
+
+## With init: dynamic section should contain DT_INIT
+# DYN-INIT: (INIT)
+
+## Without init: dynamic section should only contain DT_INIT_ARRAY
+# DYN-NO-INIT-NOT: (INIT)
+# DYN-NO-INIT: (INIT_ARRAY)
+# DYN-NO-INIT: (INIT_ARRAYSZ)
+
+## With interp program header (executable)
+# PH-INTERP: Program Headers:
+# PH-INTERP: INTERP
+
+## Without interp program header (shared library)
+# PH-INTERP-SHARED: Program Headers:
+# PH-INTERP-SHARED-NOT: INTERP
+
+## With PIE: binary should have relative relocations
+# RELOC-PIE: R_AARCH64_RELATIVE
+
+## With PIE: binary should have relative relocations
+# RELOC-SHARED-PIE: R_AARCH64_ABS64
+
+## Without PIE: binary should not have relative relocations
+# RELOC-NO-PIE-NOT: R_AARCH64_RELATIVE
+
+## Check that entry point address is set to __bolt_runtime_start for PIE executable with DT_INIT
+# CHECK-INIT-EP: ELF Header:
+# CHECK-INIT-EP: Entry point address: 0x[[#%X,EP_ADDR:]]
+## Check that the dynamic relocation at .init and .init_array were not patched
+# CHECK-INIT-EP: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-INIT-EP-NOT: (INIT) 0x[[#%x, EP_ADDR]]
+# CHECK-INIT-EP-NOT: (INIT_ARRAY) 0x[[#%x, EP_ADDR]]
+## Check that the new entry point address points to __bolt_runtime_start
+# CHECK-INIT-EP: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-INIT-EP: {{0+}}[[#%x, EP_ADDR]] {{.*}} __bolt_runtime_start
+
+## Check that DT_INIT address is set to __bolt_runtime_start for PIE executable with DT_INIT
+# CHECK-INIT-NO-EP: ELF Header:
+# CHECK-INIT-NO-EP: Entry point address: 0x[[#%X,EP_ADDR:]]
+## Read Dynamic section DT_INIT and DT_INIT_ARRAY entries
+# CHECK-INIT-NO-EP: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-INIT-NO-EP-DAG: (INIT) 0x[[#%x,INIT:]]
+# CHECK-INIT-NO-EP-DAG: (INIT_ARRAY) 0x[[#%x,INIT_ARRAY:]]
+## Check if ELF entry point address points to _start symbol and new DT_INIT entry points to __bolt_runtime_start
+# CHECK-INIT-NO-EP: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-INIT-NO-EP-DAG: {{0+}}[[#%x, EP_ADDR]] {{.*}} _start
+# CHECK-INIT-NO-EP-DAG: {{0+}}[[#%x, INIT]] {{.*}} __bolt_runtime_start
+
+## Check that entry point address is set to __bolt_runtime_start for PIE executable without DT_INIT
+# CHECK-NO-INIT-EP: ELF Header:
+# CHECK-NO-INIT-EP: Entry point address: 0x[[#%X,EP_ADDR:]]
+## Check that the dynamic relocation at .init and .init_array were not patched
+# CHECK-NO-INIT-EP: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-NO-INIT-EP-NOT: (INIT) 0x[[#%x, EP_ADDR]]
+# CHECK-NO-INIT-EP-NOT: (INIT_ARRAY) 0x[[#%x, EP_ADDR]]
+## Check that the new entry point address points to __bolt_runtime_start
+# CHECK-NO-INIT-EP: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-NO-INIT-EP: {{0+}}[[#%x, EP_ADDR]] {{.*}} __bolt_runtime_start
+
+## Check that DT_INIT is set to __bolt_runtime_start for shared library with DT_INIT
+# CHECK-SHARED-INIT: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-SHARED-INIT-DAG: (INIT) 0x[[#%x, INIT:]]
+# CHECK-SHARED-INIT-DAG: (INIT_ARRAY) 0x[[#%x, INIT_ARRAY:]]
+## Check that the dynamic relocation at .init_array was not patched
+# CHECK-SHARED-INIT: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-SHARED-INIT-NOT: {{0+}}[[#%x, INIT_ARRAY]] {{.*}} R_AARCH64_ABS64 {{0+}}[[#%x, INIT]]
+## Check that dynamic section DT_INIT points to __bolt_runtime_start
+# CHECK-SHARED-INIT: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-SHARED-INIT: {{0+}}[[#%x, INIT]] {{.*}} __bolt_runtime_start
+
+## Check that 1st entry of DT_INIT_ARRAY is set to __bolt_runtime_start for shared library without DT_INIT and
+## ELF header entry point not changed
+
+## Check that entry point address is set to __bolt_runtime_start for PIE executable without DT_INIT
+# CHECK-NO-INIT-NO-EP: ELF Header:
+# CHECK-NO-INIT-NO-EP: Entry point address: 0x[[#%X,EP_ADDR:]]
+# CHECK-NO-INIT-NO-EP: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-NO-INIT-NO-EP-NOT: (INIT)
+# CHECK-NO-INIT-NO-EP: (INIT_ARRAY) 0x[[#%x,INIT_ARRAY:]]
+## Read the dynamic relocation from 1st entry of .init_array
+# CHECK-NO-INIT-NO-EP: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-NO-INIT-NO-EP: {{0+}}[[#%x,INIT_ARRAY]] {{.*}} R_AARCH64_RELATIVE [[#%x,INIT_ADDR:]]
+## Check that 1st entry of .init_array points to __bolt_runtime_start
+# CHECK-NO-INIT-NO-EP: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-NO-INIT-NO-EP-DAG: {{0+}}[[#%x, EP_ADDR]] {{.*}} _start
+# CHECK-NO-INIT-NO-EP-DAG: {{[0-9]]*}}: {{0+}}[[#%x, INIT_ADDR]] {{.*}} __bolt_runtime_start
+
+## Check that entry point address is set to __bolt_runtime_start for shared library without DT_INIT
+# CHECK-SHARED-NO-INIT: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-SHARED-NO-INIT-NOT: (INIT)
+# CHECK-SHARED-NO-INIT: (INIT_ARRAY) 0x[[#%x,INIT_ARRAY:]]
+## Read the dynamic relocation from 1st entry of .init_array
+# CHECK-SHARED-NO-INIT: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-SHARED-NO-INIT: {{0+}}[[#%x, INIT_ARRAY]] {{.*}} R_AARCH64_ABS64 [[#%x,INIT_ADDR:]]
+## Check that 1st entry of .init_array points to __bolt_runtime_start
+# CHECK-SHARED-NO-INIT: Symbol table '.symtab' contains ...
[truncated]
|
Member
Author
|
@paschalis-mpeis We had internal implementation of DT_INIT_ARRAY support (for our internal branch based older llvm baseline which had no DT_FINI_ARRAY). In fact this PR is an adaptation of that version to the current LLVM/BOLT baseline which includes DT_FINI_ARRAY support (following it's approach to make more uniform INIT_ARRAY / FINI_ARRAY hooking support). |
aaupov
reviewed
Nov 11, 2025
bolt/lib/Passes/Instrumentation.cpp
Outdated
Comment on lines
37
to
38
| cl::opt<bool> InstrumentNoUseEntryPoint( | ||
| "instrumentation-no-use-entry-point", |
Contributor
There was a problem hiding this comment.
I think it would be clearer to state what this option would use instead, ie. instrumentation-use-init-array.
Alternatively, provide a multiple choice option e.g. instrumentation-hook={init,init_array} defaulting to init.
Author
There was a problem hiding this comment.
I wanted to keep the original priority for using hooks: entry point -> init -> init_array, in the order in which they are available in the input binary. However, in the libc binary, we can also find a non-zero entry point, so it will be used first. For this reason, I have introduced the "instrumentation-no-use-entry-point" option as an exception.
I will try to rework the current implementation with the "instrumentation-hook" option. This option will have an "entry-point" default value, and it will fall back to init or init_array if there are no corresponding entries in the input binary.
While making these changes, I realized that these changes are actually related to the runtime library, not just instrumentation. It is also relevant for "hugify" code hooking. Therefore, we should probably consider another name, such as "runtime-lib-hook."
Contributor
There was a problem hiding this comment.
Agree with using a common option for all runtime libraries, and entry-point as the default value.
Author
There was a problem hiding this comment.
Introduced "runtime-lib-init-hook" option with such functionality in recent version
bolt/lib/Rewrite/RewriteInstance.cpp
Outdated
Comment on lines
1522
to
1523
| assert(BC->InitArrayAddress && BC->InitArraySize && | ||
| "inconsistent .init_array state"); |
Contributor
There was a problem hiding this comment.
LLVM's convention is to use assertions to express invariants, and errors to report bad inputs. So let's also switch to error here and below.
Member
|
Thanks for the details @vleonen, and again thanks for the patch. 🙏 I believe @kaadam was keen in finishing up his patch, however, as mentioned, this was not reflected on upstream. I believe the functionality is quite similar, @kaadam could you please review this? TMU, some bits Adam intended to cover are already implemented here. Adam, is there anything else in your patch that isn't covered here? |
Contributor
|
@vleonen Thanks for your contribution, indeed there was no any update on my PR for a while. At the first glance your change covers all things what I wanted to add. So I agree to proceed with this PR. |
Member
paschalis-mpeis
left a comment
paschalis-mpeis
left a comment
There was a problem hiding this comment.
Hey @vleonen,
I get some lit errors on the below regex (lines 90, 113, 163 in hook-init.s):
{{0+}}[[#%x, EP_ADDR]] {{.*}} __bolt_runtime_startProbably the row number is not matching?
23: 0000000000123abc 0 NOTYPE WEAK DEFAULT ABS __bolt_runtime_start
d5fbcff to
9bf29d4
Compare
Author
Thanks for checking it. For some reasons this issue doesn't fire on my current environment. I'll check it in next version. |
vleonen
force-pushed
the
instrumentation_init_array
branch
from
9bf29d4 to
47c5d2b
Compare
Author
|
@paschalis-mpeis |
added 5 commits
November 13, 2025 06:46
This commit follows implementation of instrumentation hook via DT_FINI_ARRAY (llvm#67348) and extends it for BOLT runtime libraries (including instrumentation library) initialization hooking. Initialization has has differences compared to finalization: - Executables always use ELF entry point address. Update code checks it and updates init_array entry if ELF is shared library (have no interp entry) and have no DT_INIT entry. Also this commit introduces "runtime-lib-init-hook" option to select primary initialization hook (entry_point, init, init_array) with fall back to next available hook in input binary. e.g. in case of libc we can explicitly set it to init_array. - Shared library init_array entries relocations usually has R_AARCH64_ABS64 type on AArch64 binaries. We check relocation type and adjust methods for reading init_array relocations in discovery and update methods.
…n't have fini & fini_array
vleonen
force-pushed
the
instrumentation_init_array
branch
from
47c5d2b to
cda23e8
Compare
vleonen
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Major part of this PR is commit implementing support for DT_INIT_ARRAY for BOLT runtime libraries initialization. Also, it adds related hook-init test & fixes couple of X86 instrumentation tests.
This commit follows implementation of instrumentation hook via DT_FINI_ARRAY (#67348) and extends it for BOLT runtime libraries (including instrumentation library) initialization hooking.
Initialization has has differences compared to finalization: