[llvm] [BOLT] Support instrumentation hook via DT_FINI_ARRAY (PR #67348)

Job Noorman via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 11 05:16:46 PDT 2023


https://github.com/mtvec updated https://github.com/llvm/llvm-project/pull/67348

>From 2bcd71540fba82f899fcbd600db5f992362e03f1 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 01/14] [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 59460105f231371..eadf73e03db4758 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 329fe356603d0e5..f38a209563564c3 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 ddcc21878abb818..171e52f28a76c82 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -704,6 +704,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
@@ -1280,6 +1284,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;
@@ -5037,7 +5095,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;
       }
@@ -5120,6 +5178,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;
@@ -5368,6 +5432,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 ba4385e7edadd3790b585572dbc1efe082a88b8a 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 02/14] 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 171e52f28a76c82..e996ee7f067c936 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -704,7 +704,7 @@ Error RewriteInstance::run() {
   adjustCommandLineOptions();
   discoverFileObjects();
 
-  if (opts::Instrument)
+  if (opts::Instrument && !BC->IsStaticExecutable)
     if (Error E = discoverFiniAddress())
       return E;
 
@@ -1290,9 +1290,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,
@@ -1327,7 +1324,7 @@ void RewriteInstance::updateFiniReloc() {
     return;
 
   const RuntimeLibrary *RT = BC->getRuntimeLibrary();
-  if (!RT)
+  if (!RT || !RT->getRuntimeFiniAddress())
     return;
 
   std::optional<Relocation> FiniFunctionReloc =
@@ -5095,7 +5092,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 5b1dcf2b04a41b4ec7453d250f656a2f3cf54da7 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 03/14] 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 e996ee7f067c936..887126b70a14e9f 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -744,6 +744,8 @@ Error RewriteInstance::run() {
 
   updateMetadata();
 
+  updateFiniReloc();
+
   if (opts::LinuxKernelMode) {
     errs() << "BOLT-WARNING: not writing the output file for Linux Kernel\n";
     return Error::success();
@@ -1307,16 +1309,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() {
@@ -1327,12 +1332,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() {
@@ -5429,7 +5443,6 @@ void RewriteInstance::rewriteFile() {
 
   // Copy non-allocatable sections once allocatable part is finished.
   rewriteNoteSections();
-  updateFiniReloc();
 
   if (BC->HasRelocations) {
     patchELFAllocatableRelaSections();

>From e815c66025d97af6114013c40faf231aee09ce66 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 04/14] 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 f38a209563564c3..b4c90a283227c04 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 887126b70a14e9f..fcc4578249f35ea 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -705,7 +705,7 @@ Error RewriteInstance::run() {
   discoverFileObjects();
 
   if (opts::Instrument && !BC->IsStaticExecutable)
-    if (Error E = discoverFiniAddress())
+    if (Error E = discoverRtFiniAddress())
       return E;
 
   preprocessProfileData();
@@ -744,7 +744,7 @@ Error RewriteInstance::run() {
 
   updateMetadata();
 
-  updateFiniReloc();
+  updateRtFiniReloc();
 
   if (opts::LinuxKernelMode) {
     errs() << "BOLT-WARNING: not writing the output file for Linux Kernel\n";
@@ -1286,7 +1286,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)
@@ -1324,7 +1324,7 @@ Error RewriteInstance::discoverFiniAddress() {
                            "No relocation for first DT_FINI_ARRAY slot");
 }
 
-void RewriteInstance::updateFiniReloc() {
+void RewriteInstance::updateRtFiniReloc() {
   if (!BC->FiniArraySection)
     return;
 

>From 50e8eef57e17fcfe5e2fb245da1bebca622a1316 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 05/14] 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 fcc4578249f35ea..89a80987fd0c8fd 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1336,14 +1336,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 a54a937b77b192f044c4d25a3f2a136fe9730046 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 06/14] 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 eadf73e03db4758..77ed859f4180a3d 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 89a80987fd0c8fd..6f223c7eb75dada 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1308,8 +1308,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();
@@ -1325,17 +1323,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
@@ -1343,7 +1345,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 2e7312e75f15feac1ba098f8553d4363c261295d 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 07/14] 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 77ed859f4180a3d..c2ff2ce3df1751f 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 6f223c7eb75dada..73dfa898e85f7d0 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1287,10 +1287,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(
@@ -1323,19 +1324,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);
   }
@@ -5188,7 +5195,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();

>From 8cf7d68d9629fdf1d76006edcee06805a51f3d26 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Wed, 4 Oct 2023 11:28:15 +0200
Subject: [PATCH 08/14] fix test: only update relocs when insrumentation
 enabled and not -static

---
 bolt/lib/Rewrite/RewriteInstance.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 73dfa898e85f7d0..a696e2447a198d7 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -744,7 +744,8 @@ Error RewriteInstance::run() {
 
   updateMetadata();
 
-  updateRtFiniReloc();
+  if (opts::Instrument && !BC->IsStaticExecutable)
+    updateRtFiniReloc();
 
   if (opts::LinuxKernelMode) {
     errs() << "BOLT-WARNING: not writing the output file for Linux Kernel\n";

>From 94eba2f83a109d7eb387ed753a15e5e8e00737f7 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Thu, 5 Oct 2023 17:50:11 +0200
Subject: [PATCH 09/14] Add tests

---
 bolt/lib/Core/Relocation.cpp             |  1 +
 bolt/test/runtime/AArch64/hook-fini.test | 61 ++++++++++++++++++++++++
 2 files changed, 62 insertions(+)
 create mode 100644 bolt/test/runtime/AArch64/hook-fini.test

diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp
index e4d0f26c305f4e8..4c62a3db9288780 100644
--- a/bolt/lib/Core/Relocation.cpp
+++ b/bolt/lib/Core/Relocation.cpp
@@ -366,6 +366,7 @@ static uint64_t encodeValueAArch64(uint64_t Type, uint64_t Value, uint64_t PC) {
   default:
     llvm_unreachable("unsupported relocation");
   case ELF::R_AARCH64_ABS32:
+  case ELF::R_AARCH64_ABS64:
     break;
   case ELF::R_AARCH64_PREL16:
   case ELF::R_AARCH64_PREL32:
diff --git a/bolt/test/runtime/AArch64/hook-fini.test b/bolt/test/runtime/AArch64/hook-fini.test
new file mode 100644
index 000000000000000..56cc471db8b1d0f
--- /dev/null
+++ b/bolt/test/runtime/AArch64/hook-fini.test
@@ -0,0 +1,61 @@
+# Test the different ways of hooking the fini function for instrumentation (via
+# DT_FINI and via DT_FINI_ARRAY). We test the latter for both PIE and non-PIE
+# binaries because of the different ways of handling relocations (static or
+# dynamic).
+# 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:
+#   - DT_FINI or DT_FINI_ARRAY in dynamic section
+#   - No relative relocations for non-PIE
+# - Instrument
+# - Run instrumented binary
+# - Verify generated profile
+REQUIRES: system-linux,bolt-runtime
+
+RUN: %clang %p/Inputs/basic-instrumentation.s -Wl,-q -o %t.exe
+RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-FINI %s
+RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
+RUN: llvm-bolt %t.exe -o %t --instrument \
+RUN:     --instrumentation-file=%t \
+RUN:     --instrumentation-file-append-pid
+RUN: rm -f %t.*.fdata
+RUN: %t
+RUN: cat %t.*.fdata | FileCheck %s
+
+RUN: %clang %p/Inputs/basic-instrumentation.s -Wl,-q,-fini=0 -o %t-no-fini.exe
+RUN: llvm-readelf -d %t-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
+RUN: llvm-readelf -r %t-no-fini.exe | FileCheck --check-prefix=RELOC-PIE %s
+RUN: llvm-bolt %t-no-fini.exe -o %t-no-fini --instrument \
+RUN:     --instrumentation-file=%t-no-fini \
+RUN:     --instrumentation-file-append-pid
+RUN: rm -f %t-no-fini.*.fdata
+RUN: %t-no-fini
+RUN: cat %t-no-fini.*.fdata | FileCheck %s
+
+RUN: %clang %p/Inputs/basic-instrumentation.s -no-pie -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
+RUN: llvm-readelf -d %t-no-pie-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
+RUN: llvm-readelf -r %t-no-pie-no-fini.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
+RUN: llvm-bolt %t-no-pie-no-fini.exe -o %t-no-pie-no-fini --instrument \
+RUN:     --instrumentation-file=%t-no-pie-no-fini \
+RUN:     --instrumentation-file-append-pid
+RUN: rm -f %t-no-pie-no-fini.*.fdata
+RUN: %t-no-pie-no-fini
+RUN: cat %t-no-pie-no-fini.*.fdata | FileCheck %s
+
+# With fini: dynamic section should contain DT_FINI
+DYN-FINI: (FINI)
+
+# Without fini: dynamic section should only contain DT_FINI_ARRAY
+DYN-NO-FINI-NOT: (FINI)
+DYN-NO-FINI:     (FINI_ARRAY)
+DYN-NO-FINI:     (FINI_ARRAYSZ)
+
+# With PIE: binary should have relative relocations
+RELOC-PIE: R_AARCH64_RELATIVE
+
+# Without PIE: binary should not have relative relocations
+RELOC-NO-PIE-NOT: R_AARCH64_RELATIVE
+
+# The instrumented profile should at least say main was called once
+CHECK: main 0 0 1{{$}}

>From 8ca7060fdd6714af0c168576e6d96b6a979681d7 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 10 Oct 2023 11:13:07 +0200
Subject: [PATCH 10/14] Add non-runtime tests

---
 bolt/test/AArch64/hook-fini.s | 80 +++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 bolt/test/AArch64/hook-fini.s

diff --git a/bolt/test/AArch64/hook-fini.s b/bolt/test/AArch64/hook-fini.s
new file mode 100644
index 000000000000000..d2901858ad25856
--- /dev/null
+++ b/bolt/test/AArch64/hook-fini.s
@@ -0,0 +1,80 @@
+## Test the different ways of hooking the fini function for instrumentation (via
+## DT_FINI and via DT_FINI_ARRAY). We test the latter for both PIE and non-PIE
+## binaries because of the different ways of handling relocations (static or
+## dynamic).
+## 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:
+##   - DT_FINI or DT_FINI_ARRAY in dynamic section
+##   - No relative relocations for non-PIE
+## - Instrument
+## - Verify generated binary
+# REQUIRES: system-linux,bolt-runtime
+
+# RUN: %clang  %s -Wl,-q -o %t.exe
+# RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-FINI %s
+# RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
+# RUN: llvm-bolt %t.exe -o %t --instrument
+# RUN: llvm-readelf -ds %t | FileCheck --check-prefix=CHECK-FINI %s
+
+# RUN: %clang  %s -Wl,-q,-fini=0 -o %t-no-fini.exe
+# RUN: llvm-readelf -d %t-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
+# RUN: llvm-readelf -r %t-no-fini.exe | FileCheck --check-prefix=RELOC-PIE %s
+# RUN: llvm-bolt %t-no-fini.exe -o %t-no-fini --instrument
+# RUN: llvm-readelf -drs %t-no-fini | FileCheck --check-prefix=CHECK-NO-FINI %s
+
+# RUN: %clang  %s -no-pie -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
+# RUN: llvm-readelf -d %t-no-pie-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
+# RUN: llvm-readelf -r %t-no-pie-no-fini.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
+# RUN: llvm-bolt %t-no-pie-no-fini.exe -o %t-no-pie-no-fini --instrument
+# RUN: llvm-readelf -ds -x .fini_array %t-no-pie-no-fini | FileCheck --check-prefix=CHECK-NO-PIE-NO-FINI %s
+
+## With fini: dynamic section should contain DT_FINI
+# DYN-FINI: (FINI)
+
+## Without fini: dynamic section should only contain DT_FINI_ARRAY
+# DYN-NO-FINI-NOT: (FINI)
+# DYN-NO-FINI:     (FINI_ARRAY)
+# DYN-NO-FINI:     (FINI_ARRAYSZ)
+
+## With PIE: binary should have relative relocations
+# RELOC-PIE: R_AARCH64_RELATIVE
+
+## Without PIE: binary should not have relative relocations
+# RELOC-NO-PIE-NOT: R_AARCH64_RELATIVE
+
+## Check that DT_FINI is set to __bolt_runtime_fini
+# CHECK-FINI: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-FINI: (FINI) 0x[[FINI:[[:xdigit:]]+]]
+# CHECK-FINI: Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-FINI: {{0+}}[[FINI]] {{.*}} __bolt_runtime_fini
+
+## Check that DT_FINI_ARRAY has a dynamic relocation for __bolt_runtime_fini
+# CHECK-NO-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-NO-FINI-NOT: (FINI)
+# CHECK-NO-FINI:     (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
+# CHECK-NO-FINI:     Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-NO-FINI:     {{0+}}[[FINI_ARRAY]] {{.*}} R_AARCH64_RELATIVE [[FINI_ADDR:[[:xdigit:]]+]]
+# CHECK-NO-FINI:     Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-NO-FINI:     {{0+}}[[FINI_ADDR]] {{.*}} __bolt_runtime_fini
+
+## Check that DT_FINI_ARRAY has static relocation applied for __bolt_runtime_fini
+# CHECK-NO-PIE-NO-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-NO-PIE-NO-FINI-NOT: (FINI)
+# CHECK-NO-PIE-NO-FINI:     (FINI_ARRAY) 0x[[FINI_ARRAY:[a-f0-9]+]]
+# CHECK-NO-PIE-NO-FINI:     Symbol table '.symtab' contains {{.*}} entries:
+## Read address bytes separately so we can reverse them later
+# CHECK-NO-PIE-NO-FINI:     {{0+}}[[FINI_ADDR_B0:[[:xdigit:]]{2}]][[FINI_ADDR_B1:[[:xdigit:]]{2}]][[FINI_ADDR_B2:[[:xdigit:]]{2}]][[FINI_ADDR_B3:[[:xdigit:]]{2}]] {{.*}} __bolt_runtime_fini
+# CHECK-NO-PIE-NO-FINI:     Hex dump of section '.fini_array':
+# CHECK-NO-PIE-NO-FINI:     0x{{0+}}[[FINI_ARRAY]] [[FINI_ADDR_B3]][[FINI_ADDR_B2]][[FINI_ADDR_B1]][[FINI_ADDR_B0]] 00000000
+
+  .globl main
+  .type main, %function
+main:
+	sub		sp, sp, #16
+	mov		w0, wzr
+	str		wzr, [sp, #12]
+	add		sp, sp, #16
+	ret
+.size main, .-main

>From 679543eaaf94988d5cb9d81c5aa6c85ccc05430c Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 10 Oct 2023 13:53:31 +0200
Subject: [PATCH 11/14] Add R_AARCH64_ABS16

---
 bolt/lib/Core/Relocation.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp
index 4c62a3db9288780..c9165d973c71add 100644
--- a/bolt/lib/Core/Relocation.cpp
+++ b/bolt/lib/Core/Relocation.cpp
@@ -365,6 +365,7 @@ static uint64_t encodeValueAArch64(uint64_t Type, uint64_t Value, uint64_t PC) {
   switch (Type) {
   default:
     llvm_unreachable("unsupported relocation");
+  case ELF::R_AARCH64_ABS16:
   case ELF::R_AARCH64_ABS32:
   case ELF::R_AARCH64_ABS64:
     break;

>From 389a6fecc02838e537003ce0d425ca4b4e8a1874 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 10 Oct 2023 14:48:42 +0200
Subject: [PATCH 12/14] Add @yota9's suggestions for the tests

---
 bolt/test/AArch64/hook-fini.s | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/bolt/test/AArch64/hook-fini.s b/bolt/test/AArch64/hook-fini.s
index d2901858ad25856..bc650092c36f7aa 100644
--- a/bolt/test/AArch64/hook-fini.s
+++ b/bolt/test/AArch64/hook-fini.s
@@ -16,13 +16,14 @@
 # RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-FINI %s
 # RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
 # RUN: llvm-bolt %t.exe -o %t --instrument
-# RUN: llvm-readelf -ds %t | FileCheck --check-prefix=CHECK-FINI %s
+# RUN: llvm-readelf -drs %t | FileCheck --check-prefix=CHECK-FINI %s
 
 # RUN: %clang  %s -Wl,-q,-fini=0 -o %t-no-fini.exe
 # RUN: llvm-readelf -d %t-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
 # RUN: llvm-readelf -r %t-no-fini.exe | FileCheck --check-prefix=RELOC-PIE %s
 # RUN: llvm-bolt %t-no-fini.exe -o %t-no-fini --instrument
 # RUN: llvm-readelf -drs %t-no-fini | FileCheck --check-prefix=CHECK-NO-FINI %s
+# RUN: llvm-readelf -ds -x .fini_array %t-no-fini | FileCheck --check-prefix=CHECK-NO-FINI-RELOC %s
 
 # RUN: %clang  %s -no-pie -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
 # RUN: llvm-readelf -d %t-no-pie-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
@@ -47,6 +48,10 @@
 ## Check that DT_FINI is set to __bolt_runtime_fini
 # CHECK-FINI: Dynamic section at offset {{.*}} contains {{.*}} entries:
 # CHECK-FINI: (FINI) 0x[[FINI:[[:xdigit:]]+]]
+# CHECK-FINI: (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
+## Check that the dynamic relocation at .fini_array was not patched
+# CHECK-FINI: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-FINI-NOT: {{0+}}[[FINI_ARRAY]] {{.*}} R_AARCH64_RELATIVE [[FINI]]
 # CHECK-FINI: Symbol table '.symtab' contains {{.*}} entries:
 # CHECK-FINI: {{0+}}[[FINI]] {{.*}} __bolt_runtime_fini
 
@@ -59,6 +64,15 @@
 # CHECK-NO-FINI:     Symbol table '.symtab' contains {{.*}} entries:
 # CHECK-NO-FINI:     {{0+}}[[FINI_ADDR]] {{.*}} __bolt_runtime_fini
 
+## Check that the static relocation in .fini_array is patched even for PIE
+# CHECK-NO-FINI-RELOC: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-NO-FINI-RELOC: (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
+# CHECK-NO-FINI-RELOC: Symbol table '.symtab' contains {{.*}} entries:
+## Read  bytes separately so we can reverse them later
+# CHECK-NO-FINI-RELOC: {{0+}}[[FINI_ADDR_B0:[[:xdigit:]]{2}]][[FINI_ADDR_B1:[[:xdigit:]]{2}]][[FINI_ADDR_B2:[[:xdigit:]]{2}]][[FINI_ADDR_B3:[[:xdigit:]]{2}]] {{.*}} __bolt_runtime_fini
+# CHECK-NO-FINI-RELOC: Hex dump of section '.fini_array':
+# CHECK-NO-FINI-RELOC: 0x{{0+}}[[FINI_ARRAY]] [[FINI_ADDR_B3]][[FINI_ADDR_B2]][[FINI_ADDR_B1]][[FINI_ADDR_B0]] 00000000
+
 ## Check that DT_FINI_ARRAY has static relocation applied for __bolt_runtime_fini
 # CHECK-NO-PIE-NO-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:
 # CHECK-NO-PIE-NO-FINI-NOT: (FINI)

>From cbcb365c282d0dfe48afbfeea9b84befa5758e1d Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 10 Oct 2023 14:50:12 +0200
Subject: [PATCH 13/14] Realign

---
 bolt/test/AArch64/hook-fini.s | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/bolt/test/AArch64/hook-fini.s b/bolt/test/AArch64/hook-fini.s
index bc650092c36f7aa..6fdbfe2486b07d7 100644
--- a/bolt/test/AArch64/hook-fini.s
+++ b/bolt/test/AArch64/hook-fini.s
@@ -45,15 +45,15 @@
 ## Without PIE: binary should not have relative relocations
 # RELOC-NO-PIE-NOT: R_AARCH64_RELATIVE
 
-## Check that DT_FINI is set to __bolt_runtime_fini
-# CHECK-FINI: Dynamic section at offset {{.*}} contains {{.*}} entries:
-# CHECK-FINI: (FINI) 0x[[FINI:[[:xdigit:]]+]]
-# CHECK-FINI: (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
+## Check that     DT_FINI is set to __bolt_runtime_fini
+# CHECK-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-FINI:     (FINI) 0x[[FINI:[[:xdigit:]]+]]
+# CHECK-FINI:     (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
 ## Check that the dynamic relocation at .fini_array was not patched
-# CHECK-FINI: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+# CHECK-FINI:     Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
 # CHECK-FINI-NOT: {{0+}}[[FINI_ARRAY]] {{.*}} R_AARCH64_RELATIVE [[FINI]]
-# CHECK-FINI: Symbol table '.symtab' contains {{.*}} entries:
-# CHECK-FINI: {{0+}}[[FINI]] {{.*}} __bolt_runtime_fini
+# CHECK-FINI:     Symbol table '.symtab' contains {{.*}} entries:
+# CHECK-FINI:     {{0+}}[[FINI]] {{.*}} __bolt_runtime_fini
 
 ## Check that DT_FINI_ARRAY has a dynamic relocation for __bolt_runtime_fini
 # CHECK-NO-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:

>From ab11fbc8b468146883b5562593fe8949777bc383 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Wed, 11 Oct 2023 13:57:20 +0200
Subject: [PATCH 14/14] Add %cflags to tests

---
 bolt/test/AArch64/hook-fini.s            | 41 +++++++++++++++---------
 bolt/test/runtime/AArch64/hook-fini.test |  6 ++--
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/bolt/test/AArch64/hook-fini.s b/bolt/test/AArch64/hook-fini.s
index 6fdbfe2486b07d7..9e828851bf99a8c 100644
--- a/bolt/test/AArch64/hook-fini.s
+++ b/bolt/test/AArch64/hook-fini.s
@@ -12,21 +12,22 @@
 ## - Verify generated binary
 # REQUIRES: system-linux,bolt-runtime
 
-# RUN: %clang  %s -Wl,-q -o %t.exe
+# RUN: %clang %cflags -pie %s -Wl,-q -o %t.exe
 # RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-FINI %s
 # RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
 # RUN: llvm-bolt %t.exe -o %t --instrument
 # RUN: llvm-readelf -drs %t | FileCheck --check-prefix=CHECK-FINI %s
 
-# RUN: %clang  %s -Wl,-q,-fini=0 -o %t-no-fini.exe
+# RUN: %clang %cflags -pie %s -Wl,-q,-fini=0 -o %t-no-fini.exe
 # RUN: llvm-readelf -d %t-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
 # RUN: llvm-readelf -r %t-no-fini.exe | FileCheck --check-prefix=RELOC-PIE %s
 # RUN: llvm-bolt %t-no-fini.exe -o %t-no-fini --instrument
 # RUN: llvm-readelf -drs %t-no-fini | FileCheck --check-prefix=CHECK-NO-FINI %s
 # RUN: llvm-readelf -ds -x .fini_array %t-no-fini | FileCheck --check-prefix=CHECK-NO-FINI-RELOC %s
 
-# RUN: %clang  %s -no-pie -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
-# RUN: llvm-readelf -d %t-no-pie-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %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-stubs.so
+# RUN: %clang %cflags %s -no-pie -Wl,-q,-fini=0 %t-stub.so -o %t-no-pie-no-fini.exe
 # RUN: llvm-readelf -r %t-no-pie-no-fini.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
 # RUN: llvm-bolt %t-no-pie-no-fini.exe -o %t-no-pie-no-fini --instrument
 # RUN: llvm-readelf -ds -x .fini_array %t-no-pie-no-fini | FileCheck --check-prefix=CHECK-NO-PIE-NO-FINI %s
@@ -45,10 +46,10 @@
 ## Without PIE: binary should not have relative relocations
 # RELOC-NO-PIE-NOT: R_AARCH64_RELATIVE
 
-## Check that     DT_FINI is set to __bolt_runtime_fini
+## Check that DT_FINI is set to __bolt_runtime_fini
 # CHECK-FINI:     Dynamic section at offset {{.*}} contains {{.*}} entries:
-# CHECK-FINI:     (FINI) 0x[[FINI:[[:xdigit:]]+]]
-# CHECK-FINI:     (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
+# CHECK-FINI-DAG: (FINI) 0x[[FINI:[[:xdigit:]]+]]
+# CHECK-FINI-DAG: (FINI_ARRAY) 0x[[FINI_ARRAY:[[:xdigit:]]+]]
 ## Check that the dynamic relocation at .fini_array was not patched
 # CHECK-FINI:     Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
 # CHECK-FINI-NOT: {{0+}}[[FINI_ARRAY]] {{.*}} R_AARCH64_RELATIVE [[FINI]]
@@ -83,12 +84,20 @@
 # CHECK-NO-PIE-NO-FINI:     Hex dump of section '.fini_array':
 # CHECK-NO-PIE-NO-FINI:     0x{{0+}}[[FINI_ARRAY]] [[FINI_ADDR_B3]][[FINI_ADDR_B2]][[FINI_ADDR_B1]][[FINI_ADDR_B0]] 00000000
 
-  .globl main
-  .type main, %function
-main:
-	sub		sp, sp, #16
-	mov		w0, wzr
-	str		wzr, [sp, #12]
-	add		sp, sp, #16
-	ret
-.size main, .-main
+  .globl _start
+  .type _start, %function
+_start:
+  # Dummy relocation to force relocation mode.
+  .reloc 0, R_AARCH64_NONE
+  ret
+.size _start, .-_start
+
+  .globl _fini
+  .type _fini, %function
+_fini:
+  ret
+  .size _fini, .-_fini
+
+  .section .fini_array,"aw"
+  .align 3
+  .dword _fini
diff --git a/bolt/test/runtime/AArch64/hook-fini.test b/bolt/test/runtime/AArch64/hook-fini.test
index 56cc471db8b1d0f..8d23b21b6d612f5 100644
--- a/bolt/test/runtime/AArch64/hook-fini.test
+++ b/bolt/test/runtime/AArch64/hook-fini.test
@@ -13,7 +13,7 @@
 # - Verify generated profile
 REQUIRES: system-linux,bolt-runtime
 
-RUN: %clang %p/Inputs/basic-instrumentation.s -Wl,-q -o %t.exe
+RUN: %clang %cflags -pie %p/Inputs/basic-instrumentation.s -Wl,-q -o %t.exe
 RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=DYN-FINI %s
 RUN: llvm-readelf -r %t.exe | FileCheck --check-prefix=RELOC-PIE %s
 RUN: llvm-bolt %t.exe -o %t --instrument \
@@ -23,7 +23,7 @@ RUN: rm -f %t.*.fdata
 RUN: %t
 RUN: cat %t.*.fdata | FileCheck %s
 
-RUN: %clang %p/Inputs/basic-instrumentation.s -Wl,-q,-fini=0 -o %t-no-fini.exe
+RUN: %clang %cflags -pie %p/Inputs/basic-instrumentation.s -Wl,-q,-fini=0 -o %t-no-fini.exe
 RUN: llvm-readelf -d %t-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
 RUN: llvm-readelf -r %t-no-fini.exe | FileCheck --check-prefix=RELOC-PIE %s
 RUN: llvm-bolt %t-no-fini.exe -o %t-no-fini --instrument \
@@ -33,7 +33,7 @@ RUN: rm -f %t-no-fini.*.fdata
 RUN: %t-no-fini
 RUN: cat %t-no-fini.*.fdata | FileCheck %s
 
-RUN: %clang %p/Inputs/basic-instrumentation.s -no-pie -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
+RUN: %clang %cflags -no-pie %p/Inputs/basic-instrumentation.s -Wl,-q,-fini=0 -o %t-no-pie-no-fini.exe
 RUN: llvm-readelf -d %t-no-pie-no-fini.exe | FileCheck --check-prefix=DYN-NO-FINI %s
 RUN: llvm-readelf -r %t-no-pie-no-fini.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
 RUN: llvm-bolt %t-no-pie-no-fini.exe -o %t-no-pie-no-fini --instrument \



More information about the llvm-commits mailing list