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

Vladislav Khmelevsky via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 3 05:23:22 PDT 2023


================
@@ -1276,6 +1282,69 @@ void RewriteInstance::discoverFileObjects() {
   registerFragments();
 }
 
+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)
+    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;
+
+  if (const Relocation *Reloc = FiniArraySection->getDynamicRelocationAt(0)) {
+    BC->FiniFunctionAddress = Reloc->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::updateRtFiniReloc() {
+  if (!BC->FiniArraySection)
+    return;
+
+  const RuntimeLibrary *RT = BC->getRuntimeLibrary();
+  if (!RT || !RT->getRuntimeFiniAddress())
+    return;
+
+  if (std::optional<Relocation> Reloc =
+          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
----------------
yota9 wrote:

> To be honest, I just assumed there wouldn't be static relocations in that case.

Having the dynamic relocation doesn't necessary mean we don't have static relocation on this place. Basically with Wl,q it would be on its place as usual.

> I might be wrong here (not very familiar with RELR) but isn't this case already handled by...

It is. But if there would be static relocation (and normally it would) the patching of value in binary would be skipped and the old symbol value would be written to data section.

> So you propose to patch them for testing purposes only? 

It is not only about testing, it is about consistency. Why should we skip patching the value and emit old/wrong even if it is not used at runtime? At least it complicates debugging and making the binary non consistent. I just don't see any reason not to move it upper and patch it to the right value.

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


More information about the llvm-commits mailing list