[BOLT] Refactor PatchEntries pass

Summary:
Use injected functions with fixed addresses to patch original function
entries.

(cherry picked from FBD24133890)

bolt/src/BinaryContext.cpp

@@ -1341,6 +1341,20 @@ std::vector<BinaryFunction *> BinaryContext::getSortedFunctions() {
   return SortedFunctions;
 }
 
+std::vector<BinaryFunction *> BinaryContext::getAllBinaryFunctions() {
+  std::vector<BinaryFunction *> AllFunctions;
+  AllFunctions.reserve(BinaryFunctions.size() + InjectedBinaryFunctions.size());
+  std::transform(BinaryFunctions.begin(), BinaryFunctions.end(),
+                 std::back_inserter(AllFunctions),
+                 [](std::pair<const uint64_t, BinaryFunction> &BFI) {
+                   return &BFI.second;
+                 });
+  std::copy(InjectedBinaryFunctions.begin(), InjectedBinaryFunctions.end(),
+            std::back_inserter(AllFunctions));
+
+  return AllFunctions;
+}
+
 void BinaryContext::preprocessDebugInfo() {
   // Populate MCContext with DWARF files.
   for (const auto &CU : DwCtx->compile_units()) {

bolt/src/BinaryContext.h

@@ -409,6 +409,10 @@ public:
     return InjectedBinaryFunctions;
   }
 
+  /// Return vector with all functions, i.e. include functions from the input
+  /// binary and functions created by BOLT.
+  std::vector<BinaryFunction *> getAllBinaryFunctions();
+
   /// Construct a jump table for \p Function at \p Address or return an existing
   /// one at that location.
   ///

bolt/src/BinaryEmitter.cpp

@@ -269,7 +269,7 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function, bool EmitColdPart) {
   if (Function.getState() == BinaryFunction::State::Empty)
     return false;
 
-  auto *Section =
+  MCSection *Section =
       BC.getCodeSection(EmitColdPart ? Function.getColdCodeSectionName()
                                      : Function.getCodeSectionName());
   Streamer.SwitchSection(Section);
@@ -479,12 +479,12 @@ void BinaryEmitter::emitConstantIslands(BinaryFunction &BF, bool EmitColdPart,
   assert(!BF.isInjected() &&
          "injected functions should not have constant islands");
   // Raw contents of the function.
-  StringRef SectionContents = BF.getSection().getContents();
+  StringRef SectionContents = BF.getOriginSection()->getContents();
 
   // Raw contents of the function.
   StringRef FunctionContents =
       SectionContents.substr(
-          BF.getAddress() - BF.getSection().getAddress(),
+          BF.getAddress() - BF.getOriginSection()->getAddress(),
           BF.getMaxSize());
 
   if (opts::Verbosity && !OnBehalfOf)
@@ -964,11 +964,11 @@ void BinaryEmitter::emitFunctionBodyRaw(BinaryFunction &BF) {
   assert(!BF.isInjected() && "cannot emit raw body of injected function");
 
   // Raw contents of the function.
-  StringRef SectionContents = BF.getSection().getContents();
+  StringRef SectionContents = BF.getOriginSection()->getContents();
 
   // Raw contents of the function.
   StringRef FunctionContents = SectionContents.substr(
-      BF.getAddress() - BF.getSection().getAddress(), BF.getSize());
+      BF.getAddress() - BF.getOriginSection()->getAddress(), BF.getSize());
 
   if (opts::Verbosity)
     outs() << "BOLT-INFO: emitting function " << BF << " in raw ("

bolt/src/BinaryFunction.cpp

@@ -14,6 +14,7 @@
 #include "BinaryFunction.h"
 #include "DynoStats.h"
 #include "MCPlusBuilder.h"
+#include "NameResolver.h"
 #include "NameShortener.h"
 #include "llvm/ADT/edit_distance.h"
 #include "llvm/ADT/SmallSet.h"
@@ -246,8 +247,7 @@ BinaryFunction::hasNameRegex(const StringRef Name) const {
 }
 
 std::string BinaryFunction::getDemangledName() const {
-  StringRef MangledName = getOneName();
-  MangledName = MangledName.substr(0, MangledName.find_first_of('/'));
+  StringRef MangledName = NameResolver::restore(getOneName());
   int Status = 0;
   char *const Name =
       abi::__cxa_demangle(MangledName.str().c_str(), 0, 0, &Status);
@@ -396,7 +396,7 @@ void BinaryFunction::print(raw_ostream &OS, std::string Annotation,
     return;
 
   StringRef SectionName =
-      IsInjected ? "<no input section>" : InputSection->getName();
+      OriginSection ? OriginSection->getName() : "<no origin section>";
   OS << "Binary Function \"" << *this << "\" " << Annotation << " {";
   auto AllNames = getNames();
   if (AllNames.size() > 1) {
@@ -911,7 +911,7 @@ MCSymbol *BinaryFunction::getOrCreateLocalLabel(uint64_t Address,
 }
 
 ErrorOr<ArrayRef<uint8_t>> BinaryFunction::getData() const {
-  auto &Section = getSection();
+  BinarySection &Section = *getOriginSection();
   assert(Section.containsRange(getAddress(), getMaxSize()) &&
          "wrong section for function");
 
@@ -1552,7 +1552,7 @@ bool BinaryFunction::scanExternalRefs() {
         Success = false;
         continue;
       }
-      Rel->Offset +=  getAddress() - getSection().getAddress() + Offset;
+      Rel->Offset +=  getAddress() - getOriginSection()->getAddress() + Offset;
       FunctionRelocations.push_back(*Rel);
     }
 
@@ -1563,7 +1563,7 @@ bool BinaryFunction::scanExternalRefs() {
   // Add relocations unless disassembly failed for this function.
   if (!DisassemblyFailed) {
     for (auto &Rel : FunctionRelocations) {
-      getSection().addPendingRelocation(Rel);
+      getOriginSection()->addPendingRelocation(Rel);
     }
   }
 
@@ -3924,7 +3924,7 @@ bool BinaryFunction::isSymbolValidInScope(const SymbolRef &Symbol,
                                           uint64_t SymbolSize) const {
   // If this symbol is in a different section from the one where the
   // function symbol is, don't consider it as valid.
-  if (!getSection().containsAddress(
+  if (!getOriginSection()->containsAddress(
           cantFail(Symbol.getAddress(), "cannot get symbol address")))
     return false;
 

bolt/src/BinaryFunction.h

@@ -197,8 +197,8 @@ private:
   /// the function to its name in a profile, command line, etc.
   std::vector<std::string> Aliases;
 
-  /// Containing section
-  BinarySection *InputSection = nullptr;
+  /// Containing section in the input file.
+  BinarySection *OriginSection = nullptr;
 
   /// Address of the function in memory. Also could be an offset from
   /// base address for position independent binaries.
@@ -316,6 +316,8 @@ private:
   bool IsPatched{false};
 
   /// The address for the code for this function in codegen memory.
+  /// Used for functions that are emitted in a dedicated section with a fixed
+  /// address. E.g. for functions that are overwritten in-place.
   uint64_t ImageAddress{0};
 
   /// The size of the code in memory.
@@ -665,7 +667,7 @@ private:
   /// Creation should be handled by RewriteInstance or BinaryContext
   BinaryFunction(const std::string &Name, BinarySection &Section,
                  uint64_t Address, uint64_t Size, BinaryContext &BC)
-      : InputSection(&Section), Address(Address), Size(Size), BC(BC),
+      : OriginSection(&Section), Address(Address), Size(Size), BC(BC),
         CodeSectionName(buildCodeSectionName(Name, BC)),
         ColdCodeSectionName(buildColdCodeSectionName(Name, BC)),
         FunctionNumber(++Count) {
@@ -1068,11 +1070,17 @@ public:
            getState() == State::Emitted;
   }
 
-  BinarySection &getSection() const {
-    assert(InputSection);
-    return *InputSection;
+  /// Return the section in the input binary this function originated from or
+  /// nullptr if the function did not originate from the file.
+  BinarySection *getOriginSection() const {
+    return OriginSection;
   }
 
+  void setOriginSection(BinarySection *Section) {
+    OriginSection = Section;
+  }
+
+  /// Return true if the function did not originate from the primary input file.
   bool isInjected() const {
     return IsInjected;
   }
@@ -1330,9 +1338,11 @@ public:
     //      Relocation{Offset, Symbol, RelType, Addend, Value};
   }
 
-  /// Return then name of the section this function originated from.
-  StringRef getOriginSectionName() const {
-    return getSection().getName();
+  /// Return the name of the section this function originated from.
+  Optional<StringRef> getOriginSectionName() const {
+    if (!OriginSection)
+      return NoneType();
+    return OriginSection->getName();
   }
 
   /// Return internal section name for this function.

bolt/src/NameResolver.h

@@ -24,11 +24,30 @@ class NameResolver {
   /// Track the number of duplicate names.
   StringMap<uint64_t> Counters;
 
+  /// Character guaranteed not to be used by any "native" name passed to
+  /// uniquify() function.
+  static constexpr char Sep = '/';
+
 public:
-  /// Return unique version of a symbol name in the form "<name>/<number>".
+  /// Return unique version of the \p Name in the form "Name<Sep><Number>".
   std::string uniquify(StringRef Name) {
     const auto ID = ++Counters[Name];
-    return (Name + "/" + Twine(ID)).str();
+    return (Name + Twine(Sep) + Twine(ID)).str();
+  }
+
+  /// For uniquified \p Name, return the original form (that may no longer be
+  /// unique).
+  static StringRef restore(StringRef Name) {
+    return Name.substr(0, Name.find_first_of(Sep));
+  }
+
+  /// Append \p Suffix to the original string in \p UniqueName  preserving the
+  /// deduplication form. E.g. append("Name<Sep>42", "Suffix") will return
+  /// "NameSuffix<Sep>42".
+  static std::string append(StringRef UniqueName, StringRef Suffix) {
+    StringRef LHS, RHS;
+    std::tie(LHS, RHS) = UniqueName.split(Sep);
+    return (LHS + Suffix + Twine(Sep) + RHS).str();
   }
 };
 

bolt/src/Passes/PatchEntries.cpp

@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "PatchEntries.h"
+#include "NameResolver.h"
 #include "llvm/Support/Options.h"
 
 namespace opts {
@@ -57,25 +58,19 @@ void PatchEntries::runOnFunctions(BinaryContext &BC) {
     outs() << "BOLT-INFO: patching entries in original code\n";
   }
 
-  static auto Emitter = BC.createIndependentMCCodeEmitter();
-
   for (auto &BFI : BC.getBinaryFunctions()) {
     auto &Function = BFI.second;
 
-    // Patch original code only for functions that are emitted.
+    // Patch original code only for functions that will be emitted.
     if (!BC.shouldEmit(Function))
       continue;
 
     // Check if we can skip patching the function.
-    if (!Function.hasEHRanges() && Function.getSize() < PatchThreshold) {
+    if (!opts::ForcePatch && !Function.hasEHRanges() &&
+        Function.getSize() < PatchThreshold) {
       continue;
     }
 
-    // List of patches for function entries. We either successfully patch
-    // all entries or, if we cannot patch any, do no patch the rest and
-    // mark the function as ignorable.
-    std::vector<InstructionPatch> PendingPatches;
-
     uint64_t NextValidByte = 0; // offset of the byte past the last patch
     bool Success = Function.forEachEntryPoint([&](uint64_t Offset,
                                                   const MCSymbol *Symbol) {
@@ -87,20 +82,21 @@ void PatchEntries::runOnFunctions(BinaryContext &BC) {
         return false;
       }
 
+      BinaryFunction *PatchFunction =
+          BC.createInjectedBinaryFunction(
+              NameResolver::append(Symbol->getName(), ".org.0"));
+      PatchFunction->setOutputAddress(Function.getAddress() + Offset);
+      PatchFunction->setFileOffset(Function.getFileOffset() + Offset);
+      PatchFunction->setOriginSection(Function.getOriginSection());
+
       MCInst TailCallInst;
       BC.MIB->createTailCall(TailCallInst, Symbol, BC.Ctx.get());
+      PatchFunction->addBasicBlock(0)->addInstruction(TailCallInst);
 
-      PendingPatches.push_back(InstructionPatch());
-      InstructionPatch &InstPatch = PendingPatches.back();
-      InstPatch.Offset =
-        Function.getAddress() - Function.getSection().getAddress() + Offset;
-
-      SmallVector<MCFixup, 4> Fixups;
-      raw_svector_ostream VecOS(InstPatch.Code);
-
-      Emitter.MCE->encodeInstruction(TailCallInst, VecOS, Fixups, *BC.STI);
-
-      NextValidByte = Offset + InstPatch.Code.size();
+      uint64_t HotSize, ColdSize;
+      std::tie(HotSize, ColdSize) = BC.calculateEmittedSize(*PatchFunction);
+      assert(!ColdSize && "unexpected cold code");
+      NextValidByte = Offset + HotSize;
       if (NextValidByte > Function.getMaxSize()) {
         if (opts::Verbosity >= 1) {
           outs() << "BOLT-INFO: function " << Function << " too small to patch "
@@ -109,13 +105,6 @@ void PatchEntries::runOnFunctions(BinaryContext &BC) {
         return false;
       }
 
-      assert(Fixups.size() == 1 && "unexpected fixup size");
-      Optional<Relocation> Rel = BC.MIB->createRelocation(Fixups[0], *BC.MAB);
-      assert(Rel && "unable to convert fixup to relocation");
-
-      InstPatch.Rel = *Rel;
-      InstPatch.Rel.Offset += InstPatch.Offset;
-
       return true;
     });
 
@@ -128,11 +117,6 @@ void PatchEntries::runOnFunctions(BinaryContext &BC) {
       continue;
     }
 
-    // Apply all recorded patches.
-    for (auto &Patch : PendingPatches) {
-      Function.getSection().addPatch(Patch.Offset, Patch.Code);
-      Function.getSection().addPendingRelocation(Patch.Rel);
-    }
     Function.setIsPatched(true);
   }
 }

bolt/src/RewriteInstance.cpp

@@ -396,7 +396,8 @@ WriteBoltInfoSection("bolt-info",
 
 bool isHotTextMover(const BinaryFunction &Function) {
   for (auto &SectionName : opts::HotTextMoveSections) {
-    if (Function.getOriginSectionName() == SectionName)
+    if (Function.getOriginSectionName() &&
+        *Function.getOriginSectionName() == SectionName)
       return true;
   }
 
@@ -1420,7 +1421,7 @@ void RewriteInstance::adjustFunctionBoundaries() {
     }
 
     // Function runs at most till the end of the containing section.
-    uint64_t NextObjectAddress = Function.getSection().getEndAddress();
+    uint64_t NextObjectAddress = Function.getOriginSection()->getEndAddress();
     // Or till the next object marked by a symbol.
     if (NextSymRefI != FileSymRefs.end()) {
       NextObjectAddress = std::min(NextSymRefI->first, NextObjectAddress);
@@ -2344,7 +2345,7 @@ void RewriteInstance::readRelocations(const SectionRef &Section) {
       }
     } else if (ReferencedBF) {
       assert(RefSection && "section expected for section relocation");
-      if (ReferencedBF->getSection() != *RefSection) {
+      if (*ReferencedBF->getOriginSection() != *RefSection) {
         DEBUG(dbgs() << "BOLT-DEBUG: ignoring false function reference\n");
         ReferencedBF = nullptr;
       }
@@ -3006,17 +3007,23 @@ void RewriteInstance::emitAndLink() {
 
   // Once the code is emitted, we can rename function sections to actual
   // output sections and de-register sections used for emission.
-  if (!BC->HasRelocations) {
-    for (auto &BFI : BC->getBinaryFunctions()) {
-      auto &Function = BFI.second;
-      if (auto Section = Function.getCodeSection())
-        BC->deregisterSection(*Section);
-      Function.CodeSectionName = Function.getOriginSectionName();
-      if (Function.isSplit()) {
-        if (auto ColdSection = Function.getColdCodeSection())
-          BC->deregisterSection(*ColdSection);
-        Function.ColdCodeSectionName = ".bolt.text";
-      }
+  for (BinaryFunction *Function : BC->getAllBinaryFunctions()) {
+    ErrorOr<BinarySection &> Section = Function->getCodeSection();
+    if (Section && (Function->getImageAddress() == 0 ||
+                    Function->getImageSize() == 0)) {
+      continue;
+    }
+
+    // Restore origin section for functions that were emitted or supposed to
+    // be emitted to patch sections.
+    if (Section)
+      BC->deregisterSection(*Section);
+    assert(Function->getOriginSectionName() && "expected origin section");
+    Function->CodeSectionName = *Function->getOriginSectionName();
+    if (Function->isSplit()) {
+      if (auto ColdSection = Function->getColdCodeSection())
+        BC->deregisterSection(*ColdSection);
+      Function->ColdCodeSectionName = ".bolt.text";
     }
   }
 
@@ -3159,8 +3166,32 @@ void RewriteInstance::mapCodeSections(orc::VModuleKey Key) {
   if (BC->HasRelocations) {
     assert(TextSection->hasValidSectionID() && ".text section should be valid");
 
+    // Map sections for functions with pre-assigned addresses.
+    for (auto *InjectedFunction : BC->getInjectedBinaryFunctions()) {
+      const uint64_t OutputAddress = InjectedFunction->getOutputAddress();
+      if (!OutputAddress)
+        continue;
+
+      ErrorOr<BinarySection &> FunctionSection =
+          InjectedFunction->getCodeSection();
+      assert(FunctionSection && "function should have section");
+      FunctionSection->setOutputAddress(OutputAddress);
+      OLT->mapSectionAddress(Key, FunctionSection->getSectionID(),
+                             OutputAddress);
+      InjectedFunction->setImageAddress(FunctionSection->getAllocAddress());
+      InjectedFunction->setImageSize(FunctionSection->getOutputSize());
+    }
+
     // Populate the list of sections to be allocated.
-    auto CodeSections = getCodeSections();
+    std::vector<BinarySection *> CodeSections = getCodeSections();
+
+    // Remove sections that were pre-allocated (patch sections).
+    CodeSections.erase(
+        std::remove_if(CodeSections.begin(), CodeSections.end(),
+                       [](BinarySection *Section) {
+                         return Section->getOutputAddress();
+                       }),
+        CodeSections.end());
     DEBUG(dbgs() << "Code sections in the order of output:\n";
       for (const auto *Section : CodeSections) {
         dbgs() << Section->getName() << '\n';
@@ -3435,13 +3466,8 @@ void RewriteInstance::mapExtraSections(orc::VModuleKey Key) {
 }
 
 void RewriteInstance::updateOutputValues(const MCAsmLayout &Layout) {
-  for (auto &BFI : BC->getBinaryFunctions()) {
-    auto &Function = BFI.second;
-    Function.updateOutputValues(Layout);
-  }
-
-  for (auto *InjectedFunction : BC->getInjectedBinaryFunctions()) {
-    InjectedFunction->updateOutputValues(Layout);
+  for (BinaryFunction *Function : BC->getAllBinaryFunctions()) {
+    Function->updateOutputValues(Layout);
   }
 }
 
@@ -4038,34 +4064,6 @@ void RewriteInstance::updateELFSymbolTable(
   // point.
   auto addExtraSymbols = [&](const BinaryFunction &Function,
                              const ELFSymTy &FunctionSymbol) {
-    if (Function.isPatched()) {
-      Function.forEachEntryPoint([&](uint64_t Offset, const MCSymbol *Symbol) {
-        ELFSymTy OrgSymbol = FunctionSymbol;
-        SmallVector<char, 256> Buf;
-        if (!Offset) {
-          // Use the original function symbol name. This guarantees that the
-          // name will be unique.
-          OrgSymbol.st_name = AddToStrTab(
-              Twine(cantFail(FunctionSymbol.getName(StringSection)))
-                .concat(".org.0").
-                toStringRef(Buf));
-          OrgSymbol.st_size = Function.getSize();
-        } else {
-          // It's unlikely that multiple functions with secondary entries will
-          // get folded/merged. However, in case this happens, we force local
-          // symbol visibility for secondary entries.
-          OrgSymbol.st_name = AddToStrTab(
-              Twine(Symbol->getName()).concat(".org.0").toStringRef(Buf));
-          OrgSymbol.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FUNC);
-          OrgSymbol.st_size = 0;
-        }
-        OrgSymbol.st_value = Function.getAddress() + Offset;
-        OrgSymbol.st_shndx =
-          NewSectionIndex[Function.getSection().getSectionRef().getIndex()];
-        Symbols.emplace_back(OrgSymbol);
-        return true;
-      });
-    }
     if (Function.isFolded()) {
       auto *ICFParent = Function.getFoldedIntoFunction();
       while (ICFParent->isFolded())
@@ -4173,7 +4171,8 @@ void RewriteInstance::updateELFSymbolTable(
     // and its value is ignored by the runtime if it's different from
     // SHN_UNDEF and SHN_ABS.
     if (!PatchExisting && Function &&
-        Symbol.st_shndx != Function->getSection().getSectionRef().getIndex())
+        Symbol.st_shndx !=
+                      Function->getOriginSection()->getSectionRef().getIndex())
       Function = nullptr;
 
     // Create a new symbol based on the existing symbol.
@@ -4299,7 +4298,10 @@ void RewriteInstance::updateELFSymbolTable(
   // Add symbols of injected functions
   for (BinaryFunction *Function : BC->getInjectedBinaryFunctions()) {
     ELFSymTy NewSymbol;
-    NewSymbol.st_shndx = Function->getCodeSection()->getIndex();
+    BinarySection *OriginSection = Function->getOriginSection();
+    NewSymbol.st_shndx = OriginSection ?
+          NewSectionIndex[OriginSection->getSectionRef().getIndex()] :
+          Function->getCodeSection()->getIndex();
     NewSymbol.st_value = Function->getOutputAddress();
     NewSymbol.st_name = AddToStrTab(Function->getOneName());
     NewSymbol.st_size = Function->getOutputSize();
@@ -4441,7 +4443,7 @@ void RewriteInstance::patchELFSymTabs(ELFObjectFile<ELFT> *File) {
       },
       [&](StringRef Str) {
         size_t Idx = NewStrTab.size();
-        NewStrTab.append(Str.data(), Str.size());
+        NewStrTab.append(NameResolver::restore(Str).str());
         NewStrTab.append(1, '\0');
         return Idx;
       });
@@ -4699,92 +4701,91 @@ void RewriteInstance::rewriteFile() {
   assert(Offset == getFileOffsetForAddress(NextAvailableAddress) &&
          "error resizing output file");
 
-  if (!BC->HasRelocations) {
-    // Overwrite functions in the output file.
-    uint64_t CountOverwrittenFunctions = 0;
-    uint64_t OverwrittenScore = 0;
-    for (auto &BFI : BC->getBinaryFunctions()) {
-      auto &Function = BFI.second;
+  // Overwrite functions with fixed output address.
+  uint64_t CountOverwrittenFunctions = 0;
+  uint64_t OverwrittenScore = 0;
+  for (BinaryFunction *Function : BC->getAllBinaryFunctions()) {
 
-      if (Function.getImageAddress() == 0 || Function.getImageSize() == 0)
-        continue;
+    if (Function->getImageAddress() == 0 || Function->getImageSize() == 0)
+      continue;
 
-      if (Function.getImageSize() > Function.getMaxSize()) {
-        if (opts::Verbosity >= 1) {
-          errs() << "BOLT-WARNING: new function size (0x"
-                 << Twine::utohexstr(Function.getImageSize())
-                 << ") is larger than maximum allowed size (0x"
-                 << Twine::utohexstr(Function.getMaxSize())
-                 << ") for function " << Function << '\n';
-        }
-        FailedAddresses.emplace_back(Function.getAddress());
-        continue;
+    if (Function->getImageSize() > Function->getMaxSize()) {
+      if (opts::Verbosity >= 1) {
+        errs() << "BOLT-WARNING: new function size (0x"
+               << Twine::utohexstr(Function->getImageSize())
+               << ") is larger than maximum allowed size (0x"
+               << Twine::utohexstr(Function->getMaxSize())
+               << ") for function " << *Function << '\n';
       }
+      FailedAddresses.emplace_back(Function->getAddress());
+      continue;
+    }
 
-      if (Function.isSplit() && (Function.cold().getImageAddress() == 0 ||
-                                 Function.cold().getImageSize() == 0))
-        continue;
+    if (Function->isSplit() && (Function->cold().getImageAddress() == 0 ||
+                                Function->cold().getImageSize() == 0))
+      continue;
 
-      OverwrittenScore += Function.getFunctionScore();
-      // Overwrite function in the output file.
-      if (opts::Verbosity >= 2) {
-        outs() << "BOLT: rewriting function \"" << Function << "\"\n";
-      }
-      OS.pwrite(reinterpret_cast<char *>(Function.getImageAddress()),
-                Function.getImageSize(),
-                Function.getFileOffset());
+    OverwrittenScore += Function->getFunctionScore();
+    // Overwrite function in the output file.
+    if (opts::Verbosity >= 2) {
+      outs() << "BOLT: rewriting function \"" << *Function << "\"\n";
+    }
+    OS.pwrite(reinterpret_cast<char *>(Function->getImageAddress()),
+              Function->getImageSize(),
+              Function->getFileOffset());
 
-      // Write nops at the end of the function.
+    // Write nops at the end of the function.
+    if (Function->getMaxSize() != std::numeric_limits<uint64_t>::max()) {
       auto Pos = OS.tell();
-      OS.seek(Function.getFileOffset() + Function.getImageSize());
-      MAB->writeNopData(Function.getMaxSize() - Function.getImageSize(),
+      OS.seek(Function->getFileOffset() + Function->getImageSize());
+      MAB->writeNopData(Function->getMaxSize() - Function->getImageSize(),
                         &Writer);
       OS.seek(Pos);
+    }
 
-      // Write jump tables if updating in-place.
-      if (opts::JumpTables == JTS_BASIC) {
-        for (auto &JTI : Function.JumpTables) {
-          auto *JT = JTI.second;
-          auto &Section = JT->getOutputSection();
-          Section.setOutputFileOffset(
-              getFileOffsetForAddress(JT->getAddress()));
-          assert(Section.getOutputFileOffset() && "no matching offset in file");
-          OS.pwrite(reinterpret_cast<const char*>(Section.getOutputData()),
-                    Section.getOutputSize(),
-                    Section.getOutputFileOffset());
-        }
+    // Write jump tables if updating in-place.
+    if (opts::JumpTables == JTS_BASIC) {
+      for (auto &JTI : Function->JumpTables) {
+        auto *JT = JTI.second;
+        auto &Section = JT->getOutputSection();
+        Section.setOutputFileOffset(
+            getFileOffsetForAddress(JT->getAddress()));
+        assert(Section.getOutputFileOffset() && "no matching offset in file");
+        OS.pwrite(reinterpret_cast<const char*>(Section.getOutputData()),
+                  Section.getOutputSize(),
+                  Section.getOutputFileOffset());
       }
+    }
 
-      if (!Function.isSplit()) {
-        ++CountOverwrittenFunctions;
-        if (opts::MaxFunctions &&
-            CountOverwrittenFunctions == opts::MaxFunctions) {
-          outs() << "BOLT: maximum number of functions reached\n";
-          break;
-        }
-        continue;
-      }
-
-      // Write cold part
-      if (opts::Verbosity >= 2) {
-        outs() << "BOLT: rewriting function \"" << Function
-               << "\" (cold part)\n";
-      }
-      OS.pwrite(reinterpret_cast<char*>(Function.cold().getImageAddress()),
-                Function.cold().getImageSize(),
-                Function.cold().getFileOffset());
-
-      // FIXME: write nops after cold part too.
-
+    if (!Function->isSplit()) {
       ++CountOverwrittenFunctions;
       if (opts::MaxFunctions &&
           CountOverwrittenFunctions == opts::MaxFunctions) {
         outs() << "BOLT: maximum number of functions reached\n";
         break;
       }
+      continue;
     }
 
-    // Print function statistics.
+    // Write cold part
+    if (opts::Verbosity >= 2) {
+      outs() << "BOLT: rewriting function \"" << *Function
+             << "\" (cold part)\n";
+    }
+    OS.pwrite(reinterpret_cast<char*>(Function->cold().getImageAddress()),
+              Function->cold().getImageSize(),
+              Function->cold().getFileOffset());
+
+    ++CountOverwrittenFunctions;
+    if (opts::MaxFunctions &&
+        CountOverwrittenFunctions == opts::MaxFunctions) {
+      outs() << "BOLT: maximum number of functions reached\n";
+      break;
+    }
+  }
+
+  // Print function statistics for non-relocation mode.
+  if (!BC->HasRelocations) {
     outs() << "BOLT: " << CountOverwrittenFunctions
            << " out of " << BC->getBinaryFunctions().size()
            << " functions were overwritten.\n";