[BOLT] Allow missing DT_FINI{,_ARRAY} if instrumentation-sleep-time is used (llvm#170086)

This PR allows instrument binaries without the .fini and .fini_array
entries if the user passes the `instrumentation-sleep-time` option.
The `.fini` or `.fini_array` entries are used to hook into the process
finalization process and write a profile during finalization. However,
with the `instrumentation-sleep-time` option, the profile should be
written periodically, without the need for it to be written at
finalization.

Co-authored-by: Vasily Leonenko <[email protected]>

Author: 2 people

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -80,6 +80,7 @@ namespace opts {
 extern cl::list<std::string> HotTextMoveSections;
 extern cl::opt<bool> Hugify;
 extern cl::opt<bool> Instrument;
+extern cl::opt<uint32_t> InstrumentationSleepTime;
 extern cl::opt<bool> KeepNops;
 extern cl::opt<bool> Lite;
 extern cl::list<std::string> PrintOnly;
@@ -1507,6 +1508,9 @@ Error RewriteInstance::discoverRtFiniAddress() {
   }
 
   if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
+    // Missing fini hooks are allowed when instrumentation-sleep-time is in use.
+    if (opts::InstrumentationSleepTime > 0)
+      return Error::success();
     return createStringError(
         std::errc::not_supported,
         "Instrumentation needs either DT_FINI or DT_FINI_ARRAY");
@@ -1616,9 +1620,13 @@ Error RewriteInstance::updateRtFiniReloc() {
   if (!RT || !RT->getRuntimeFiniAddress())
     return Error::success();
 
-  if (!BC->FiniArrayAddress || !BC->FiniArraySize)
+  if (!BC->FiniArrayAddress || !BC->FiniArraySize) {
+    // Missing fini hooks are allowed when instrumentation-sleep-time is in use.
+    if (opts::InstrumentationSleepTime > 0)
+      return Error::success();
     return createStringError(std::errc::not_supported,
                              "inconsistent .fini_array state");
+  }
 
   ErrorOr<BinarySection &> FiniArraySection =
       BC->getSectionForAddress(*BC->FiniArrayAddress);

bolt/test/AArch64/instrument-no-fini.s

@@ -0,0 +1,34 @@
+# Test that BOLT will produce error by default and pass with instrumentation-sleep-time option
+
+# REQUIRES: system-linux,bolt-runtime,target=aarch64{{.*}}
+
+# RUN: llvm-mc -triple aarch64 -filetype=obj %s -o %t.o
+# RUN: ld.lld -q -pie -o %t.exe %t.o
+# RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=CHECK-NO-FINI %s
+# RUN: not llvm-bolt --instrument -o %t.out %t.exe 2>&1 | FileCheck %s --check-prefix=CHECK-BOLT-FAIL
+# RUN: llvm-bolt --instrument --instrumentation-sleep-time=1 -o %t.out %t.exe 2>&1 | FileCheck %s --check-prefix=CHECK-BOLT-PASS
+
+# CHECK-NO-FINI: INIT
+# CHECK-NO-FINI-NOT: FINI
+# CHECK-NO-FINI-NOT: FINI_ARRAY
+
+# CHECK-BOLT-FAIL: Instrumentation needs either DT_FINI or DT_FINI_ARRAY
+
+# CHECK-BOLT-PASS-NOT: Instrumentation needs either DT_FINI or DT_FINI_ARRAY
+# CHECK-BOLT-PASS: runtime library initialization was hooked via DT_INIT
+
+    .text
+    .globl _start
+    .type _start, %function
+_start:
+    # BOLT errs when instrumenting without relocations; create a dummy one.
+    .reloc 0, R_AARCH64_NONE
+    ret
+    .size _start, .-_start
+
+    .globl _init
+    .type _init, %function
+    # Force DT_INIT to be created (needed for instrumentation).
+_init:
+    ret
+    .size _init, .-_init

bolt/test/X86/instrument-no-fini.s

@@ -0,0 +1,34 @@
+# Test that BOLT will produce error by default and pass with instrumentation-sleep-time option
+
+# REQUIRES: system-linux,bolt-runtime,target=x86_64-{{.*}}
+
+# RUN: llvm-mc -triple x86_64 -filetype=obj %s -o %t.o
+# RUN: ld.lld -q -pie -o %t.exe %t.o
+# RUN: llvm-readelf -d %t.exe | FileCheck --check-prefix=CHECK-NO-FINI %s
+# RUN: not llvm-bolt --instrument -o %t.out %t.exe 2>&1 | FileCheck %s --check-prefix=CHECK-BOLT-FAIL
+# RUN: llvm-bolt --instrument --instrumentation-sleep-time=1 -o %t.out %t.exe 2>&1 | FileCheck %s --check-prefix=CHECK-BOLT-PASS
+
+# CHECK-NO-FINI: INIT
+# CHECK-NO-FINI-NOT: FINI
+# CHECK-NO-FINI-NOT: FINI_ARRAY
+
+# CHECK-BOLT-FAIL: Instrumentation needs either DT_FINI or DT_FINI_ARRAY
+
+# CHECK-BOLT-PASS-NOT: Instrumentation needs either DT_FINI or DT_FINI_ARRAY
+# CHECK-BOLT-PASS: runtime library initialization was hooked via DT_INIT
+
+    .text
+    .globl _start
+    .type _start, %function
+_start:
+    # BOLT errs when instrumenting without relocations; create a dummy one.
+    .reloc 0, R_X86_64_NONE
+    retq
+    .size _start, .-_start
+
+    .globl _init
+    .type _init, %function
+    # Force DT_INIT to be created (needed for instrumentation).
+_init:
+    retq
+    .size _init, .-_init