[BOLT] Fix ICF non-determinism in non-relocation mode

Summary:
ICF may fold functions in arbitrary order when running multi-threaded.
This is fine in relocation mode as we end up with just one function
holding all function symbols.

However, in non-relocation mode we keep all function bodies, and if we
keep merging profiles in non-deterministic order, we end up with
functions with non deterministic profiles. The fix for non-relocation
mode is to not merge profiles as the factual new profile could be
different from the merged one since both function instances are
potentially callable.

Additionally, emit extra symbols for ICF functions in non-relocation
mode to make it possible to track the folding.

(cherry picked from FBD20889866)

bolt/src/BinaryContext.cpp

@@ -1113,10 +1113,12 @@ void BinaryContext::foldFunction(BinaryFunction &ChildBF,
             std::back_inserter(ParentBF.Aliases));
   ChildBF.Aliases.clear();
 
-  // Merge execution counts of ChildBF into those of ParentBF.
-  ChildBF.mergeProfileDataInto(ParentBF);
-
   if (HasRelocations) {
+    // Merge execution counts of ChildBF into those of ParentBF.
+    // Without relocations, we cannot reliably merge profiles as both functions
+    // continue to exist and either one can be executed.
+    ChildBF.mergeProfileDataInto(ParentBF);
+
     std::shared_lock<std::shared_timed_mutex> ReadBfsLock(BinaryFunctionsMutex,
                                                           std::defer_lock);
     std::unique_lock<std::shared_timed_mutex> WriteBfsLock(BinaryFunctionsMutex,
@@ -1141,7 +1143,7 @@ void BinaryContext::foldFunction(BinaryFunction &ChildBF,
     ChildBF.getSymbols().push_back(Ctx->getOrCreateSymbol(NewName));
     WriteCtxLock.unlock();
 
-    ChildBF.setFolded();
+    ChildBF.setFolded(&ParentBF);
   }
 }
 
@@ -2006,6 +2008,22 @@ BinaryContext::getBinaryFunctionContainingAddress(uint64_t Address,
 
 BinaryFunction *
 BinaryContext::getBinaryFunctionAtAddress(uint64_t Address, bool Shallow) {
+  // First, try to find a function starting at the given address. If the
+  // function was folded, this will get us the original folded function if it
+  // wasn't removed from the list, e.g. in non-relocation mode.
+  auto BFI = BinaryFunctions.find(Address);
+  if (BFI != BinaryFunctions.end()) {
+    auto *BF = &BFI->second;
+    while (!Shallow && BF->getParentFunction() && !Shallow) {
+      BF = BF->getParentFunction();
+    }
+    return BF;
+  }
+
+  // We might have folded the function matching the object at the given
+  // address. In such case, we look for a function matching the symbol
+  // registered at the original address. The new function (the one that the
+  // original was folded into) will hold the symbol.
   if (const auto *BD = getBinaryDataAtAddress(Address)) {
     uint64_t EntryID{0};
     auto *BF = getFunctionForSymbol(BD->getSymbol(), &EntryID);

bolt/src/BinaryFunction.h

@@ -274,10 +274,6 @@ private:
   /// conditions.
   bool HasProfileAvailable{false};
 
-  /// Indicate if the function body was folded into another function. Used
-  /// for ICF optimization without relocations.
-  bool IsFolded{false};
-
   /// Execution halts whenever this function is entered.
   bool TrapsOnEntry{false};
 
@@ -309,6 +305,10 @@ private:
   /// Parent function for split function fragments.
   BinaryFunction *ParentFunction{nullptr};
 
+  /// Indicate if the function body was folded into another function.
+  /// Used by ICF optimization.
+  BinaryFunction *FoldedIntoFunction{nullptr};
+
   /// All fragments for a parent function.
   std::unordered_set<BinaryFunction *> Fragments;
 
@@ -1333,8 +1333,14 @@ public:
     return HasProfileAvailable;
   }
 
+  /// Return true if the body of the function was merged into another function.
   bool isFolded() const {
-    return IsFolded;
+    return FoldedIntoFunction != nullptr;
+  }
+
+  /// If this function was folded, return the function it was folded into.
+  BinaryFunction *getFoldedIntoFunction() const {
+    return FoldedIntoFunction;
   }
 
   /// Return true if the function uses jump tables.
@@ -1736,9 +1742,8 @@ public:
     return *this;
   }
 
-  BinaryFunction &setFolded(bool Folded = true) {
-    IsFolded = Folded;
-    return *this;
+  void setFolded(BinaryFunction *BF) {
+    FoldedIntoFunction = BF;
   }
 
   BinaryFunction &setPersonalityFunction(uint64_t Addr) {

bolt/src/RewriteInstance.cpp

@@ -3695,13 +3695,27 @@ void RewriteInstance::updateELFSymbolTable(
   // Add extra symbols for the emitted function.
   auto addExtraSymbols = [&](const BinaryFunction &Function,
                              const ELFSymTy &FunctionSymbol) {
-    if (Function.isSplit()) {
+    if (Function.isFolded()) {
+      auto *ICFParent = Function.getFoldedIntoFunction();
+      while (ICFParent->isFolded())
+        ICFParent = ICFParent->getFoldedIntoFunction();
+      auto ICFSymbol = FunctionSymbol;
+      SmallVector<char, 256> Buf;
+      ICFSymbol.st_name =
+        AddToStrTab(Twine(cantFail(FunctionSymbol.getName(StringSection)))
+                        .concat(".icf.0")
+                        .toStringRef(Buf));
+      ICFSymbol.st_value = ICFParent->getOutputAddress();
+      ICFSymbol.st_size = ICFParent->getOutputSize();
+      Symbols.emplace_back(ICFSymbol);
+    }
+    if (Function.isSplit() && Function.cold().getAddress()) {
       auto NewColdSym = FunctionSymbol;
       SmallVector<char, 256> Buf;
       NewColdSym.st_name =
-          AddToStrTab(Twine(cantFail(FunctionSymbol.getName(StringSection)))
-                          .concat(".cold.0")
-                          .toStringRef(Buf));
+        AddToStrTab(Twine(cantFail(FunctionSymbol.getName(StringSection)))
+                        .concat(".cold.0")
+                        .toStringRef(Buf));
       NewColdSym.st_shndx = Function.getColdCodeSection()->getIndex();
       NewColdSym.st_value = Function.cold().getAddress();
       NewColdSym.st_size = Function.cold().getImageSize();