[BOLT] Fix non-determinism while reading debug info

Summary:
When reading debug info in parallel, CUs for functions were populated in
parallel and the order was non-deterministic. We used the first CU from
the non-deterministically-ordered list to set the line number resulting
in different outputs.

The fix is to sort the list after it's been created and before assigning
the line table unit.

(cherry picked from FBD17946889)

bolt/src/BinaryContext.cpp

@@ -1185,8 +1185,7 @@ void BinaryContext::assignMemData() {
 namespace {
 
 /// Recursively finds DWARF DW_TAG_subprogram DIEs and match them with
-/// BinaryFunctions. Record DIEs for unknown subprograms (mostly functions that
-/// are never called and removed from the binary) in Unknown.
+/// BinaryFunctions.
 void findSubprograms(const DWARFDie DIE,
                      std::map<uint64_t, BinaryFunction> &BinaryFunctions) {
   if (DIE.isSubprogramDIE()) {
@@ -1326,41 +1325,62 @@ void BinaryContext::preprocessDebugInfo() {
     ThreadPool.wait();
   }
 
-  // Some functions may not have a corresponding subprogram DIE
-  // yet they will be included in some CU and will have line number information.
-  // Hence we need to associate them with the CU and include in CU ranges.
-  for (auto &AddrFunctionPair : BinaryFunctions) {
-    auto FunctionAddress = AddrFunctionPair.first;
-    auto &Function = AddrFunctionPair.second;
-    if (!Function.getSubprogramDIEs().empty())
-      continue;
-    if (auto DebugAranges = DwCtx->getDebugAranges()) {
-      auto CUOffset = DebugAranges->findAddress(FunctionAddress);
-      if (CUOffset != -1U) {
-        Function.addSubprogramDIE(
-            DWARFDie(DwCtx->getCompileUnitForOffset(CUOffset), nullptr));
-        continue;
+  for (auto &KV : BinaryFunctions) {
+    const auto FunctionAddress = KV.first;
+    auto &Function = KV.second;
+
+    // Sort associated CUs for deterministic update.
+    std::sort(Function.getSubprogramDIEs().begin(),
+              Function.getSubprogramDIEs().end(),
+              [](const DWARFDie &A, const DWARFDie &B) {
+                return A.getDwarfUnit()->getOffset() <
+                       B.getDwarfUnit()->getOffset();
+              });
+
+    // Some functions may not have a corresponding subprogram DIE
+    // yet they will be included in some CU and will have line number
+    // information. Hence we need to associate them with the CU and include
+    // in CU ranges.
+    if (Function.getSubprogramDIEs().empty()) {
+      if (auto DebugAranges = DwCtx->getDebugAranges()) {
+        auto CUOffset = DebugAranges->findAddress(FunctionAddress);
+        if (CUOffset != -1U) {
+          Function.addSubprogramDIE(
+              DWARFDie(DwCtx->getCompileUnitForOffset(CUOffset), nullptr));
+        }
       }
     }
 
 #ifdef DWARF_LOOKUP_ALL_RANGES
-    // Last resort - iterate over all compile units. This should not happen
-    // very often. If it does, we need to create a separate lookup table
-    // similar to .debug_aranges internally. This slows down processing
-    // considerably.
-    for (const auto &CU : DwCtx->compile_units()) {
-      const auto *CUDie = CU->getUnitDIE();
-      for (const auto &Range : CUDie->getAddressRanges(CU.get())) {
-        if (FunctionAddress >= Range.first &&
-            FunctionAddress < Range.second) {
-          Function.addSubprogramDIE(DWARFDie(CU.get(), nullptr));
-          break;
+    if (Function.getSubprogramDIEs().empty()) {
+      // Last resort - iterate over all compile units. This should not happen
+      // very often. If it does, we need to create a separate lookup table
+      // similar to .debug_aranges internally. This slows down processing
+      // considerably.
+      for (const auto &CU : DwCtx->compile_units()) {
+        const auto *CUDie = CU->getUnitDIE();
+        for (const auto &Range : CUDie->getAddressRanges(CU.get())) {
+          if (FunctionAddress >= Range.first &&
+              FunctionAddress < Range.second) {
+            Function.addSubprogramDIE(DWARFDie(CU.get(), nullptr));
+            break;
+          }
         }
       }
     }
 #endif
+
+    // Set line table for function to the first CU with such table.
+    for (const auto &DIE : Function.getSubprogramDIEs()) {
+      if (const auto *LineTable =
+              DwCtx->getLineTableForUnit(DIE.getDwarfUnit())) {
+        Function.setDWARFUnitLineTable(DIE.getDwarfUnit(), LineTable);
+        break;
+      }
+    }
+
   }
- }
+}
 
 void BinaryContext::printCFI(raw_ostream &OS, const MCCFIInstruction &Inst) {
   uint32_t Operation = Inst.getOperation();

bolt/src/BinaryFunction.h

@@ -37,11 +37,11 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
 #include <limits>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
-#include <algorithm>
 
 using namespace llvm::object;
 
@@ -2172,22 +2172,29 @@ public:
     static std::mutex CriticalSectionMutex;
     std::lock_guard<std::mutex> Lock(CriticalSectionMutex);
     SubprogramDIEs.emplace_back(DIE);
-    if (!UnitLineTable.first) {
-      if (const auto *LineTable =
-              BC.DwCtx->getLineTableForUnit(DIE.getDwarfUnit())) {
-        UnitLineTable = std::make_pair(DIE.getDwarfUnit(), LineTable);
-      }
-    }
+  }
+
+  void setDWARFUnitLineTable(DWARFUnit *Unit,
+                             const DWARFDebugLine::LineTable *Table) {
+    UnitLineTable = std::make_pair(Unit, Table);
   }
 
   /// Return all compilation units with entry for this function.
   /// Because of identical code folding there could be multiple of these.
+  decltype(SubprogramDIEs) &getSubprogramDIEs() {
+    return SubprogramDIEs;
+  }
+
   const decltype(SubprogramDIEs) &getSubprogramDIEs() const {
     return SubprogramDIEs;
   }
 
   /// Return DWARF compile unit with line info for this function.
-  DWARFUnitLineTable getDWARFUnitLineTable() const {
+  DWARFUnitLineTable &getDWARFUnitLineTable() {
+    return UnitLineTable;
+  }
+
+  const DWARFUnitLineTable &getDWARFUnitLineTable() const {
     return UnitLineTable;
   }