[BOLT] Refactor ELF parts of instrumentation code

Summary:
This is a prerequisite for larger emitter refactoring.

Since .dynamic is read unconditionally, add an error message if the
section is missing, or the size of the section is zero.

(cherry picked from FBD20331735)

bolt/src/BinaryContext.h

@@ -469,6 +469,14 @@ public:
   uint64_t OldTextSectionOffset{0};
   uint64_t OldTextSectionSize{0};
 
+  /// Address of the code/function that is executed before any other code in
+  /// the binary.
+  Optional<uint64_t> StartFunctionAddress;
+
+  /// Address of the code/function that is going to be executed right before
+  /// the execution of the binary is completed.
+  Optional<uint64_t> FiniFunctionAddress;
+
   /// Page alignment used for code layout.
   uint64_t PageAlign{HugePageSize};
 

bolt/src/Passes/Instrumentation.cpp

@@ -655,9 +655,20 @@ void Instrumentation::emitTablesAsELFNote(BinaryContext &BC) {
                                  /*IsReadOnly=*/true, ELF::SHT_NOTE);
 }
 
-void Instrumentation::emit(BinaryContext &BC, MCStreamer &Streamer,
+void Instrumentation::emit(BinaryContext &BC, MCStreamer &Streamer) {
-                           const BinaryFunction &InitFunction,
+  const auto *StartFunction =
-                           const BinaryFunction &FiniFunction) {
+    BC.getBinaryFunctionAtAddress(*BC.StartFunctionAddress);
+  if (!StartFunction) {
+    errs() << "BOLT-ERROR: failed to locate function at binary start address\n";
+    exit(1);
+  }
+  const auto *FiniFunction =
+    BC.getBinaryFunctionAtAddress(*BC.FiniFunctionAddress);
+  if (!FiniFunction) {
+    errs() << "BOLT-ERROR: failed to locate function at binary fini address\n";
+    exit(1);
+  }
+
   const auto Flags = BinarySection::getFlags(/*IsReadOnly=*/false,
                                              /*IsText=*/false,
                                              /*IsAllocatable=*/true);
@@ -739,12 +750,12 @@ void Instrumentation::emit(BinaryContext &BC, MCStreamer &Streamer,
   Streamer.EmitLabel(InitPtr);
   Streamer.EmitSymbolAttribute(InitPtr,
                                MCSymbolAttr::MCSA_Global);
-  Streamer.EmitValue(MCSymbolRefExpr::create(InitFunction.getSymbol(), *BC.Ctx),
+  Streamer.EmitValue(
-                     /*Size=*/8);
+      MCSymbolRefExpr::create(StartFunction->getSymbol(), *BC.Ctx), /*Size=*/8);
   Streamer.EmitLabel(FiniPtr);
   Streamer.EmitSymbolAttribute(FiniPtr, MCSymbolAttr::MCSA_Global);
-  Streamer.EmitValue(MCSymbolRefExpr::create(FiniFunction.getSymbol(), *BC.Ctx),
+  Streamer.EmitValue(
-                     /*Size=*/8);
+      MCSymbolRefExpr::create(FiniFunction->getSymbol(), *BC.Ctx), /*Size=*/8);
 
   uint32_t FuncDescSize = getFDSize();
   outs() << "BOLT-INSTRUMENTER: Number of indirect call site descriptors: "

bolt/src/Passes/Instrumentation.h

@@ -33,9 +33,7 @@ public:
   void runOnFunctions(BinaryContext &BC);
 
   /// Emit data structures that will be necessary during runtime (second step)
-  void emit(BinaryContext &BC, MCStreamer &Streamer,
+  void emit(BinaryContext &BC, MCStreamer &Streamer);
-            const BinaryFunction &InitFunction,
-            const BinaryFunction &FiniFunction);
 
   /// Create a non-allocatable ELF section with read-only tables necessary for
   /// writing the instrumented data profile during program finish. The runtime

bolt/src/RewriteInstance.cpp

@@ -603,7 +603,7 @@ void RewriteInstance::discoverStorage() {
     BC->HasFixedLoadAddress = false;
   }
 
-  EntryPoint = Obj->getHeader()->e_entry;
+  BC->StartFunctionAddress = Obj->getHeader()->e_entry;
 
   NextAvailableAddress = 0;
   uint64_t NextAvailableOffset = 0;
@@ -1694,6 +1694,9 @@ void RewriteInstance::readSpecialSections() {
   }
 
   parseSDTNotes();
+
+  // Read .dynamic/PT_DYNAMIC.
+  readELFDynamic();
 }
 
 void RewriteInstance::adjustCommandLineOptions() {
@@ -1702,9 +1705,23 @@ void RewriteInstance::adjustCommandLineOptions() {
               "supported\n";
   }
 
-  if (opts::Instrument && !BC->HasRelocations) {
+  if (opts::Instrument) {
-    errs() << "BOLT-ERROR: instrumentation requires relocations\n";
+    if (!BC->HasRelocations) {
-    exit(1);
+      errs() << "BOLT-ERROR: instrumentation requires relocations\n";
+      exit(1);
+    }
+    if (!BC->StartFunctionAddress) {
+      errs() << "BOLT-ERROR: instrumentation requires a known entry point of "
+                "the input binary\n";
+      exit(1);
+    }
+    if (!BC->FiniFunctionAddress) {
+      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::AlignMacroOpFusion != MFT_NONE && !BC->isX86()) {
@@ -2731,10 +2748,7 @@ void RewriteInstance::emitAndLink() {
 
   emitFunctions(Streamer.get());
   if (opts::Instrument) {
-    readELFDynamic();
+    Instrumenter->emit(*BC, *Streamer.get());
-    assert(StartFunction && FiniFunction &&
-           "_start and DT_FINI functions must be set");
-    Instrumenter->emit(*BC, *Streamer.get(), *StartFunction, *FiniFunction);
   }
 
   if (!BC->HasRelocations && opts::UpdateDebugSections)
@@ -4400,55 +4414,36 @@ void RewriteInstance::readELFDynamic(ELFObjectFile<ELFT> *File) {
   using Elf_Phdr = typename ELFFile<ELFT>::Elf_Phdr;
   using Elf_Dyn  = typename ELFFile<ELFT>::Elf_Dyn;
 
-  if (!opts::Instrument || !BC->HasRelocations)
-    return;
-
   // Locate DYNAMIC by looking through program headers.
   const Elf_Phdr *DynamicPhdr = 0;
   for (auto &Phdr : cantFail(Obj->program_headers())) {
     if (Phdr.p_type == ELF::PT_DYNAMIC) {
       DynamicPhdr = &Phdr;
-      assert(Phdr.p_memsz == Phdr.p_filesz && "dynamic sizes should match");
       break;
     }
   }
-  assert(DynamicPhdr && "missing dynamic in ELF binary");
 
-  // Go through all dynamic entries and patch functions addresses with
+  // Tools such as objcopy can strip the section contents but leave the header
-  // new ones.
+  // entry with zero file size.
+  if (!DynamicPhdr || !DynamicPhdr->p_filesz) {
+    errs() << "BOLT-ERROR: input binary is not a valid ELF executable as it "
+              "lacks a dynamic section or the section is empty\n";
+    exit(1);
+  }
+
+  assert(DynamicPhdr->p_memsz == DynamicPhdr->p_filesz &&
+        "dynamic section sizes should match");
+
+  // Go through all dynamic entries to locate entries of interest.
   const Elf_Dyn *DTB = cantFail(Obj->dynamic_table_begin(DynamicPhdr),
                                 "error accessing dynamic table");
   const Elf_Dyn *DTE = cantFail(Obj->dynamic_table_end(DynamicPhdr),
                                 "error accessing dynamic table");
-  bool FiniFound = false;
   for (auto *DE = DTB; DE != DTE; ++DE) {
     if (DE->getTag() != ELF::DT_FINI)
       continue;
-    const auto *Function = BC->getBinaryFunctionAtAddress(DE->getPtr());
+    BC->FiniFunctionAddress = DE->getPtr();
-    if (!Function) {
-      errs() << "BOLT-ERROR: failed to locate fini function.\n";
-      exit(1);
-    }
-    FiniFunction = Function;
-    FiniFound = true;
   }
-
-  if (!FiniFound) {
-    errs()
-        << "BOLT-ERROR: input binary lacks DT_INIT/FINI entry in the dynamic "
-           "section but instrumentation currently relies on patching "
-           "DT_FINI to write the profile.\n";
-    exit(1);
-  }
-
-  // Read start function
-  auto Ehdr = *Obj->getHeader();
-  const auto *Function = BC->getBinaryFunctionAtAddress(Ehdr.e_entry);
-  if (!Function) {
-    errs() << "BOLT-ERROR: failed to locate _start function.\n";
-    exit(1);
-  }
-  StartFunction = Function;
 }
 
 

bolt/src/RewriteInstance.h

@@ -235,8 +235,7 @@ private:
   /// Create the regular symbol table and patch dyn symbol tables.
   ELF_FUNCTION(patchELFSymTabs);
 
-  /// Read dynamic section/segment of ELF to allow us to link a runtime lib
+  /// Read dynamic section/segment of ELF.
-  /// later.
   ELF_FUNCTION(readELFDynamic);
 
   /// Patch dynamic section/segment of ELF.
@@ -395,9 +394,6 @@ private:
   /// Track next available address for new allocatable sections.
   uint64_t NextAvailableAddress{0};
 
-  /// Entry point in the file (first instructions to be executed).
-  uint64_t EntryPoint{0};
-
   /// Store all non-zero symbols in this map for a quick address lookup.
   std::map<uint64_t, llvm::object::SymbolRef> FileSymRefs;