[BOLT] Manage section naming/cloning in RewriteInstance

[treapster]

Summary

This is a first step towards -rewrite option from D144560.

EFMM after https://reviews.llvm.org/D135494 managed section names in the
following manner: if a section was emitted with orgSecPrefix, it updated
original BinarySection, and if it was emitted with original name, the
section was duplicated with newSecPrefix. The problem is that we can't
control what name is used to emit .text or .eh_frame, and the old code
made it impossible to rewrite these sections in-place(correct me if i'm wrong). Here we remove
all the management from EFMM and explicitly rename sections in
RewriteInstance if we want them duplicated, or don't touch them when we
rewrite, thus allowing EFMM to update them. We also have to rename some
sections like eh_frame back in case they were not emitted, which is a
bit combersome, but overall we get more flexibility over what sections
we want and under which names. In case of -reorder-data pass, i think
the simplest solution is to just create new section with .hot suffix,
and emit it under this name.

[treapster]

bolt/lib/Core/BinaryEmitter.cpp

1162

I'm somewhat confused about this part because intuitively we should not have relocations for .bolt.org sections, but there are such cases(e.g instrumentation-dup-jts test) and i'm not sure sure it's ok to not emit them, but it's working.. I'll dig more into it.
As of .text, we should probably fail on assert here if it has relocations?

bolt/include/bolt/Rewrite/ExecutableFileMemoryManager.h

   void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr,
                         size_t Size) override {}
   void deregisterEHFrames() override {}
-
-  /// Section name management.
-  void setNewSecPrefix(StringRef Prefix) { NewSecPrefix = Prefix; }
-  void setOrgSecPrefix(StringRef Prefix) { OrgSecPrefix = Prefix; }
 };
 
 } // namespace bolt

bolt/include/bolt/Rewrite/RewriteInstance.h

 
   void postProcessFunctions();
 
-  void preregisterSections();
+  void renameAndPreregisterSections();
 
   /// Run optimizations that operate at the binary, or post-linker, level.
   void runOptimizationPasses();

   /// Common section names.
   static StringRef getEHFrameSectionName() { return ".eh_frame"; }
 
+  static StringRef getEHFrameHeaderSectionName() { return ".eh_frame_hdr"; }
   /// An instance of the input binary we are processing, externally owned.
   llvm::object::ELFObjectFileBase *InputFile;
 

bolt/lib/Core/BinaryEmitter.cpp

 
 void BinaryEmitter::emitDataSections(StringRef OrgSecPrefix) {
   for (BinarySection &Section : BC.sections()) {
-    if (!Section.hasRelocations())
+    if (!Section.hasRelocations() ||
+        Section.getOutputName() == BC.getMainCodeSectionName() ||
+        Section.getOutputName().startswith(OrgSecPrefix))
       continue;
 
-    StringRef Prefix = Section.hasSectionRef() ? OrgSecPrefix : "";
-    Section.emitAsData(Streamer, Prefix + Section.getName());
+    Section.emitAsData(Streamer, Section.getOutputName());
     Section.clearRelocations();
   }
 }

bolt/lib/Passes/ReorderData.cpp

       // Rename sections.
       BinarySection &Hot =
           BC.registerSection(Section->getName() + ".hot", *Section);
-      Hot.setOutputName(Section->getName());
-      Section->setOutputName(".bolt.org" + Section->getName());
-
       // Reorder contents of original section.
       setSectionOrder(BC, Hot, Order.begin(), SplitPoint);
 

bolt/lib/Rewrite/ExecutableFileMemoryManager.cpp

     }
   }
 
-  BinarySection *Section = nullptr;
-  if (!OrgSecPrefix.empty() && SectionName.startswith(OrgSecPrefix)) {
-    // Update the original section contents.
-    ErrorOr<BinarySection &> OrgSection =
-        BC.getUniqueSectionByName(SectionName.substr(OrgSecPrefix.length()));
-    assert(OrgSection && OrgSection->isAllocatable() &&
-           "Original section must exist and be allocatable.");
-
-    Section = &OrgSection.get();
-    Section->updateContents(Ret, Size);
-  } else {
-    // If the input contains a section with the section name, rename it in the
-    // output file to avoid the section name conflict and emit the new section
-    // under a unique internal name.
-    ErrorOr<BinarySection &> OrgSection =
-        BC.getUniqueSectionByName(SectionName);
-    bool UsePrefix = false;
-    if (OrgSection && OrgSection->hasSectionRef()) {
-      OrgSection->setOutputName(OrgSecPrefix + SectionName);
-      UsePrefix = true;
-    }
-
-    // Register the new section under a unique name to avoid name collision with
-    // sections in the input file.
-    BinarySection &NewSection = BC.registerOrUpdateSection(
-        UsePrefix ? NewSecPrefix + SectionName : SectionName, ELF::SHT_PROGBITS,
-        BinarySection::getFlags(IsReadOnly, IsCode, true), Ret, Size,
-        Alignment);
-    if (UsePrefix)
-      NewSection.setOutputName(SectionName);
-    Section = &NewSection;
-  }
+  BinarySection *Section = &BC.registerOrUpdateSection(
+      SectionName, ELF::SHT_PROGBITS,
+      BinarySection::getFlags(IsReadOnly, IsCode, true), Ret, Size, Alignment);
+  assert(Section->isAllocatable() &&
+         "verify that allocatable is marked as allocatable");
 
   LLVM_DEBUG({
     dbgs() << "BOLT: allocating "

bolt/lib/Rewrite/MachORewriteInstance.cpp

       static_cast<MCObjectStreamer *>(Streamer.get())->getAssembler());
 
   BC->EFMM.reset(new ExecutableFileMemoryManager(*BC, /*AllowStubs*/ false));
-  BC->EFMM->setOrgSecPrefix(getOrgSecPrefix());
-  BC->EFMM->setNewSecPrefix(getNewSecPrefix());
 
   RTDyld.reset(new decltype(RTDyld)::element_type(*BC->EFMM, Resolver));
   RTDyld->setProcessAllSections(true);

bolt/lib/Rewrite/RewriteInstance.cpp

   if (opts::DiffOnly)
     return Error::success();
 
-  preregisterSections();
+  renameAndPreregisterSections();
 
   runOptimizationPasses();
 

 
 } // anonymous namespace
 
-void RewriteInstance::preregisterSections() {
-  // Preregister sections before emission to set their order in the output.
-  const unsigned ROFlags = BinarySection::getFlags(/*IsReadOnly*/ true,
-                                                   /*IsText*/ false,
-                                                   /*IsAllocatable*/ true);
-  if (BinarySection *EHFrameSection = getSection(getEHFrameSectionName())) {
-    // New .eh_frame.
-    BC->registerOrUpdateSection(getNewSecPrefix() + getEHFrameSectionName(),
-                                ELF::SHT_PROGBITS, ROFlags);
-    // Fully register a relocatable copy of the original .eh_frame.
+void RewriteInstance::renameAndPreregisterSections() {
+
+  auto Rename = [this](BinarySection *Section) {
+    if (!Section)
+      return;
+    std::string OldName = Section->getName().str();
+    const Twine NewName = getOrgSecPrefix() + Section->getName();
+    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: renaming input section "
+                      << Section->getName() << " to " << NewName << "\n";);
+    BC->renameSection(*Section, NewName);
+    BC->registerOrUpdateSection(OldName, ELF::SHT_PROGBITS,
+                                Section->getELFFlags());
+  };
+
+  if (BC->HasRelocations)
+    Rename(getSection(BC->getMainCodeSectionName()));
+  Rename(getSection(getEHFrameSectionName()));
+  if (EHFrameSection)
     BC->registerSection(".relocated.eh_frame", *EHFrameSection);
-  }
-  BC->registerOrUpdateSection(getNewSecPrefix() + ".gcc_except_table",
-                              ELF::SHT_PROGBITS, ROFlags);
-  BC->registerOrUpdateSection(getNewSecPrefix() + ".rodata", ELF::SHT_PROGBITS,
-                              ROFlags);
-  BC->registerOrUpdateSection(getNewSecPrefix() + ".rodata.cold",
-                              ELF::SHT_PROGBITS, ROFlags);
+  Rename(getSection(".eh_frame_hdr"));
+  Rename(getSection(".gcc_except_table"));
+
+  if (opts::JumpTables > JTS_BASIC)
+    Rename(getSection(".rodata"));
 }
 
 void RewriteInstance::emitAndLink() {
   NamedRegionTimer T("emitAndLink", "emit and link", TimerGroupName,
                      TimerGroupDesc, opts::TimeRewrite);
   std::error_code EC;
-
   // This is an object file, which we keep for debugging purposes.
   // Once we decide it's useless, we should create it in memory.
   SmallString<128> OutObjectPath;

     exit(1);
   }
 
-  ErrorOr<BinarySection &> TextSection =
-      BC->getUniqueSectionByName(BC->getMainCodeSectionName());
-  if (BC->HasRelocations && TextSection)
-    BC->renameSection(*TextSection, getOrgSecPrefix() + ".text");
-
   //////////////////////////////////////////////////////////////////////////////
   // Assign addresses to new sections.
   //////////////////////////////////////////////////////////////////////////////

   // same size seen in the input binary, in case this section is a copy
   // of the original one seen in the binary.
   BC->EFMM.reset(new ExecutableFileMemoryManager(*BC, /*AllowStubs=*/false));
-  BC->EFMM->setNewSecPrefix(getNewSecPrefix());
-  BC->EFMM->setOrgSecPrefix(getOrgSecPrefix());
 
   RTDyld.reset(new decltype(RTDyld)::element_type(*BC->EFMM, Resolver));
   RTDyld->setProcessAllSections(false);

 void RewriteInstance::mapFileSections(RuntimeDyld &RTDyld) {
   BC->deregisterUnusedSections();
 
+  auto RestoreName = [this](BinarySection *Section, StringRef Name) {
+    if (!Section)
+      return;
+    LLVM_DEBUG(dbgs() << formatv("BOLT-DEBUG: original section {0} was "
+                                 "not duplicated, restoring name\n",
+                                 Name));
+    assert(!getSection(Name) && "Unexpected section emitted");
+    BC->renameSection(*Section, Name);
+  };
   // If no new .eh_frame was written, remove relocated original .eh_frame.
   BinarySection *RelocatedEHFrameSection =
       getSection(".relocated" + getEHFrameSectionName());
   if (RelocatedEHFrameSection && RelocatedEHFrameSection->hasValidSectionID()) {
-    BinarySection *NewEHFrameSection =
-        getSection(getNewSecPrefix() + getEHFrameSectionName());
+    BinarySection *NewEHFrameSection = getSection(getEHFrameSectionName());
     if (!NewEHFrameSection || !NewEHFrameSection->isFinalized()) {
       // RTDyld will still have to process relocations for the section, hence
       // we need to assign it the address that wouldn't result in relocation

       RTDyld.reassignSectionAddress(RelocatedEHFrameSection->getSectionID(),
                                     NextAvailableAddress);
       BC->deregisterSection(*RelocatedEHFrameSection);
+
+      // Remove org prefix from sections since they weren't duplicated
+      RestoreName(&*EHFrameSection, getEHFrameSectionName());
+      RestoreName(getSection(getOrgSecPrefix() + getEHFrameHeaderSectionName()),
+                  getEHFrameHeaderSectionName());
+      RestoreName(getSection(getOrgSecPrefix() + ".gcc_except_table"),
+                  ".gcc_except_table");
     }
   }
 
+  if (!getSection(".rodata"))
+    RestoreName(getSection(getOrgSecPrefix() + ".rodata"), ".rodata");
+
   mapCodeSections(RTDyld);
 
   // Map the rest of the sections.

 }
 
 void RewriteInstance::writeEHFrameHeader() {
-  BinarySection *NewEHFrameSection =
-      getSection(getNewSecPrefix() + getEHFrameSectionName());
+  BinarySection *NewEHFrameSection = getSection(getEHFrameSectionName());
 
   // No need to update the header if no new .eh_frame was created.
-  if (!NewEHFrameSection)
+  if (!NewEHFrameSection || !NewEHFrameSection->hasValidSectionID()) {
     return;
+  }
 
   DWARFDebugFrame NewEHFrame(BC->TheTriple->getArch(), true,
                              NewEHFrameSection->getOutputAddress());
+
   Error E = NewEHFrame.parse(DWARFDataExtractor(
       NewEHFrameSection->getOutputContents(), BC->AsmInfo->isLittleEndian(),
       BC->AsmInfo->getCodePointerSize()));
   check_error(std::move(E), "failed to parse EH frame");
+  LLVM_DEBUG(dbgs() << "BOLT: writing a new .eh_frame_hdr\n");
 
   uint64_t RelocatedEHFrameAddress = 0;
   StringRef RelocatedEHFrameContents;

   const unsigned Flags = BinarySection::getFlags(/*IsReadOnly=*/true,
                                                  /*IsText=*/false,
                                                  /*IsAllocatable=*/true);
-  BinarySection *OldEHFrameHdrSection = getSection(".eh_frame_hdr");
-  if (OldEHFrameHdrSection)
-    OldEHFrameHdrSection->setOutputName(getOrgSecPrefix() + ".eh_frame_hdr");
 
   BinarySection &EHFrameHdrSec = BC->registerOrUpdateSection(
       getNewSecPrefix() + ".eh_frame_hdr", ELF::SHT_PROGBITS, Flags, nullptr,

bolt/test/reorder-data-writable-ptload.c

 // RUN:   -data %S/Inputs/reorder-data-writable-ptload.fdata
 // RUN: llvm-readelf -SlW %t.bolt | FileCheck %s
 
-// CHECK: .bolt.org.data
-// CHECK: {{.*}} .data PROGBITS [[#%x,ADDR:]] [[#%x,OFF:]]
+// CHECK: .data
+// CHECK: {{.*}} .data.hot PROGBITS [[#%x,ADDR:]] [[#%x,OFF:]]
 // CHECK: LOAD 0x{{.*}}[[#OFF]] 0x{{.*}}[[#ADDR]] {{.*}} RW
 
 volatile int cold1 = 42;