[BOLT][DWARF] Remove caching of ranges/abbrevs

[ayermolo]

Summary

Removing caching of ranges/abbrevs to simplify the code.
Before we were doing it to get around a gdb limitation.
FBD34015613

[Amir]

Reviewed internally

[maksfb]

LGTM

[ayermolo]

Fixed test.

bolt/include/bolt/Rewrite/DWARFRewriter.h

class DWARFRewriter {
   void updateGdbIndexSection(CUOffsetMap &CUMap);
 
   /// Output .dwo files.
   void writeDWOFiles(std::unordered_map<uint64_t, std::string> &DWOIdToName);
 
   /// Output .dwp files.
   void writeDWP(std::unordered_map<uint64_t, std::string> &DWOIdToName);
 
-  /// Abbreviations that were converted to use DW_AT_ranges.
-  std::set<const DWARFAbbreviationDeclaration *> ConvertedRangesAbbrevs;
-
   /// DWARFDie contains a pointer to a DIE and hence gets invalidated once the
   /// embedded DIE is destroyed. This wrapper class stores a DIE internally and
   /// could be cast to a DWARFDie that is valid even after the initial DIE is
   /// destroyed.
   struct DWARFDieWrapper {
     DWARFUnit *Unit;
     DWARFDebugInfoEntry DIE;
 
     DWARFDieWrapper(DWARFUnit *Unit, DWARFDebugInfoEntry DIE)
         : Unit(Unit), DIE(DIE) {}
 
     DWARFDieWrapper(DWARFDie &Die)
         : Unit(Die.getDwarfUnit()), DIE(*Die.getDebugInfoEntry()) {}
 
     operator DWARFDie() { return DWARFDie(Unit, &DIE); }
   };
 
   /// DIEs with abbrevs that were not converted to DW_AT_ranges.
   /// We only update those when all DIEs have been processed to guarantee that
   /// the abbrev (which is shared) is intact.
   using PendingRangesType = std::unordered_map<
       const DWARFAbbreviationDeclaration *,
       std::vector<std::pair<DWARFDieWrapper, DebugAddressRange>>>;
 
-  PendingRangesType PendingRanges;
-
   /// Convert \p Abbrev from using a simple DW_AT_(low|high)_pc range to
   /// DW_AT_ranges with optional \p RangesBase.
   void convertToRangesPatchAbbrev(const DWARFUnit &Unit,
                                   const DWARFAbbreviationDeclaration *Abbrev,
                                   DebugAbbrevWriter &AbbrevWriter,
                                   Optional<uint64_t> RangesBase = None);
 
   /// Update \p DIE that was using DW_AT_(low|high)_pc with DW_AT_ranges offset.
   /// Updates to the DIE should be synced with abbreviation updates using the
   /// function above.
   void convertToRangesPatchDebugInfo(DWARFDie DIE, uint64_t RangesSectionOffset,
                                      SimpleBinaryPatcher &DebugInfoPatcher,
                                      Optional<uint64_t> RangesBase = None);
 
-  /// Same as above, but takes a vector of \p Ranges as a parameter.
-  void convertToRanges(DWARFDie DIE, const DebugAddressRangesVector &Ranges,
-                       SimpleBinaryPatcher &DebugInfoPatcher);
-
   /// Patch DW_AT_(low|high)_pc values for the \p DIE based on \p Range.
   void patchLowHigh(DWARFDie DIE, DebugAddressRange Range,
-                    SimpleBinaryPatcher &DebugInfoPatcher);
-
-  /// Convert pending ranges associated with the given \p Abbrev.
-  void convertPending(const DWARFUnit &Unit,
-                      const DWARFAbbreviationDeclaration *Abbrev,
-                      SimpleBinaryPatcher &DebugInfoPatcher,
-                      DebugAbbrevWriter &AbbrevWriter);
-
-  /// Adds to Pending Ranges.
-  /// For Debug Fission also adding to .debug_addr to take care of a case where
-  /// some entries are not converted to ranges and left as
-  /// DW_AT_low_pc/DW_AT_high_pc.
-  void addToPendingRanges(const DWARFAbbreviationDeclaration *Abbrev,
-                          DWARFDie DIE, DebugAddressRangesVector &Ranges,
-                          Optional<uint64_t> DWOId);
 
-  /// Once all DIEs were seen, update DW_AT_(low|high)_pc values.
-  void flushPendingRanges(SimpleBinaryPatcher &DebugInfoPatcher);
-
   /// Helper function for creating and returning per-DWO patchers/writers.
   template <class T, class Patcher>
   Patcher *getBinaryDWOPatcherHelper(T &BinaryPatchers, uint64_t DwoId) {
     std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
     auto Iter = BinaryPatchers.find(DwoId);
     if (Iter == BinaryPatchers.end()) {
       // Using make_pair instead of {} to work around bug in older version of
       // the library. https://timsong-cpp.github.io/lwg-issues/2354

bolt/lib/Rewrite/DWARFRewriter.cpp

if (opts::NoThreads || opts::DeterministicDebugInfo) {
     for (std::unique_ptr<DWARFUnit> &CU : BC.DwCtx->compile_units()) {
       ThreadPool.async(processUnitDIE, CUIndex, CU.get());
       CUIndex++;
     }
     ThreadPool.wait();
   }
 
   DebugInfoPatcher->clearDestinationLabels();
-  flushPendingRanges(*DebugInfoPatcher);
-
   CUOffsetMap OffsetMap = finalizeDebugSections(*DebugInfoPatcher);
 
   if (opts::WriteDWP)
     writeDWP(DWOIdToName);
   else
     writeDWOFiles(DWOIdToName);
 
   updateGdbIndexSection(OffsetMap);
case dwarf::DW_TAG_compile_unit: {
         ARangesSectionWriter->addCURanges(Unit.getOffset(),
                                           std::move(OutputRanges));
       updateDWARFObjectAddressRanges(DIE, RangesSectionOffset, DebugInfoPatcher,
                                      AbbrevWriter, RangesBase);
       break;
     }
     case dwarf::DW_TAG_subprogram: {
       // Get function address either from ranges or [LowPC, HighPC) pair.
-      bool UsesRanges = false;
       uint64_t Address;
       uint64_t SectionIndex, HighPC;
       if (!DIE.getLowAndHighPC(Address, HighPC, SectionIndex)) {
         Expected<DWARFAddressRangesVector> RangesOrError =
             DIE.getAddressRanges();
         if (!RangesOrError) {
           consumeError(RangesOrError.takeError());
           break;
         }
         DWARFAddressRangesVector Ranges = *RangesOrError;
         // Not a function definition.
         if (Ranges.empty())
           break;
 
         Address = Ranges.front().LowPC;
-        UsesRanges = true;
       }
 
       // Clear cached ranges as the new function will have its own set.
       CachedRanges.clear();
 
       DebugAddressRangesVector FunctionRanges;
       if (const BinaryFunction *Function =
               BC.getBinaryFunctionAtAddress(Address))
         FunctionRanges = Function->getOutputAddressRanges();
-      // Update ranges.
-      if (UsesRanges) {
-        updateDWARFObjectAddressRanges(
-            DIE, RangesSectionWriter->addRanges(FunctionRanges),
-            DebugInfoPatcher, AbbrevWriter);
-      } else {
-        // Delay conversion of [LowPC, HighPC) into DW_AT_ranges if possible.
-        const DWARFAbbreviationDeclaration *Abbrev =
-            DIE.getAbbreviationDeclarationPtr();
-        assert(Abbrev && "abbrev expected");
 
-        // Create a critical section.
-        static std::shared_timed_mutex CriticalSectionMutex;
-        std::unique_lock<std::shared_timed_mutex> Lock(CriticalSectionMutex);
-
-        if (FunctionRanges.size() > 1) {
-          convertPending(Unit, Abbrev, DebugInfoPatcher, AbbrevWriter);
-          // Exit critical section early.
-          Lock.unlock();
-          convertToRanges(DIE, FunctionRanges, DebugInfoPatcher);
-        } else if (ConvertedRangesAbbrevs.find(Abbrev) !=
-                   ConvertedRangesAbbrevs.end()) {
-          // Exit critical section early.
-          Lock.unlock();
-          convertToRanges(DIE, FunctionRanges, DebugInfoPatcher);
-        } else {
-          if (FunctionRanges.empty())
-            FunctionRanges.emplace_back(DebugAddressRange());
-          addToPendingRanges(Abbrev, DIE, FunctionRanges, Unit.getDWOId());
-        }
-      }
+        FunctionRanges.push_back({0, 0});
+
+      updateDWARFObjectAddressRanges(
+          DIE, RangesSectionWriter->addRanges(FunctionRanges), DebugInfoPatcher,
+          AbbrevWriter);
+
       break;
     }
     case dwarf::DW_TAG_lexical_block:
     case dwarf::DW_TAG_inlined_subroutine:
     case dwarf::DW_TAG_try_block:
     case dwarf::DW_TAG_catch_block: {
       uint64_t RangesSectionOffset =
           RangesSectionWriter->getEmptyRangesOffset();
void DWARFRewriter::updateGdbIndexSection(CUOffsetMap &CUMap) {
   // Copy over the rest of the original data.
   memcpy(Buffer, Data, TrailingSize);
 
   // Register the new section.
   BC.registerOrUpdateNoteSection(".gdb_index", NewGdbIndexContents,
                                  NewGdbIndexSize);
 }
 
-void DWARFRewriter::convertToRanges(DWARFDie DIE,
-                                    const DebugAddressRangesVector &Ranges,
-                                    SimpleBinaryPatcher &DebugInfoPatcher) {
-  uint64_t RangesSectionOffset;
-  if (Ranges.empty())
-    RangesSectionOffset = RangesSectionWriter->getEmptyRangesOffset();
-  else
-    RangesSectionOffset = RangesSectionWriter->addRanges(Ranges);
-
-  convertToRangesPatchDebugInfo(DIE, RangesSectionOffset, DebugInfoPatcher);
-}
-
-void DWARFRewriter::convertPending(const DWARFUnit &Unit,
-                                   const DWARFAbbreviationDeclaration *Abbrev,
-                                   SimpleBinaryPatcher &DebugInfoPatcher,
-                                   DebugAbbrevWriter &AbbrevWriter) {
-  if (ConvertedRangesAbbrevs.count(Abbrev))
-    return;
-
-  convertToRangesPatchAbbrev(Unit, Abbrev, AbbrevWriter);
-
-  auto I = PendingRanges.find(Abbrev);
-  if (I != PendingRanges.end()) {
-    for (std::pair<DWARFDieWrapper, DebugAddressRange> &Pair : I->second)
-      convertToRanges(Pair.first, {Pair.second}, DebugInfoPatcher);
-    PendingRanges.erase(I);
-  }
-
-  ConvertedRangesAbbrevs.emplace(Abbrev);
-}
-
-void DWARFRewriter::addToPendingRanges(
-    const DWARFAbbreviationDeclaration *Abbrev, DWARFDie DIE,
-    DebugAddressRangesVector &FunctionRanges, Optional<uint64_t> DWOId) {
-  Optional<DWARFFormValue> LowPcValue = DIE.find(dwarf::DW_AT_low_pc);
-  Optional<DWARFFormValue> HighPcValue = DIE.find(dwarf::DW_AT_high_pc);
-  if (LowPcValue &&
-      LowPcValue->getForm() == dwarf::Form::DW_FORM_GNU_addr_index) {
-    assert(DWOId && "Invalid DWO ID.");
-    (void)DWOId;
-    assert(HighPcValue && "Low PC exists, but not High PC.");
-    (void)HighPcValue;
-    uint64_t IndexL = LowPcValue->getRawUValue();
-    uint64_t IndexH = HighPcValue->getRawUValue();
-    for (auto Address : FunctionRanges) {
-      AddrWriter->addIndexAddress(Address.LowPC, IndexL, *DWOId);
-      // 2.17.2
-      // If the value of the DW_AT_high_pc is of class address, it is the
-      // relocated address of the first location past the last instruction
-      // associated with the entity; if it is of class constant, the value is
-      // an unsigned integer offset which when added to the low PC gives the
-      // address of the first location past the last instruction associated
-      // with the entity.
-      if (!HighPcValue->isFormClass(DWARFFormValue::FC_Constant))
-        AddrWriter->addIndexAddress(Address.HighPC, IndexH, *DWOId);
-    }
-  }
-  PendingRanges[Abbrev].emplace_back(
-      std::make_pair(DWARFDieWrapper(DIE), FunctionRanges.front()));
-}
-
 std::unique_ptr<DebugBufferVector>
 DWARFRewriter::makeFinalLocListsSection(SimpleBinaryPatcher &DebugInfoPatcher) {
   auto LocBuffer = std::make_unique<DebugBufferVector>();
   auto LocStream = std::make_unique<raw_svector_ostream>(*LocBuffer);
   auto Writer =
       std::unique_ptr<MCObjectWriter>(BC.createObjectWriter(*LocStream));
 
   uint64_t SectionOffset = 0;
std::unique_ptr<DebugBufferVector> CurrCULocationLists =
         LocWriter->getBuffer();
     *LocStream << *CurrCULocationLists;
     SectionOffset += CurrCULocationLists->size();
   }
 
   return LocBuffer;
 }
 
-void DWARFRewriter::flushPendingRanges(SimpleBinaryPatcher &DebugInfoPatcher) {
-  for (std::pair<const DWARFAbbreviationDeclaration *const,
-                 std::vector<std::pair<DWARFDieWrapper, DebugAddressRange>>>
-           &I : PendingRanges)
-    for (std::pair<DWARFDieWrapper, DebugAddressRange> &RangePair : I.second)
-      patchLowHigh(RangePair.first, RangePair.second, DebugInfoPatcher);
-  clearList(PendingRanges);
-}
-
 namespace {
 
 void getRangeAttrData(DWARFDie DIE, Optional<AttrInfo> &LowPCVal,
                       Optional<AttrInfo> &HighPCVal) {
   LowPCVal = findAttributeInfo(DIE, dwarf::DW_AT_low_pc);
   HighPCVal = findAttributeInfo(DIE, dwarf::DW_AT_high_pc);
   uint64_t LowPCOffset = LowPCVal->Offset;
   uint64_t HighPCOffset = HighPCVal->Offset;
errs() << "BOLT-WARNING: high_pc expected immediately after low_pc. "
            << Twine::utohexstr(DIE.getOffset()) << '\n';
     return;
   }
 }
 
 } // namespace
 
 void DWARFRewriter::patchLowHigh(DWARFDie DIE, DebugAddressRange Range,
-                                 SimpleBinaryPatcher &DebugInfoPatcher) {
+                                 SimpleBinaryPatcher &DebugInfoPatcher,
+                                 Optional<uint64_t> DWOId) {
   Optional<AttrInfo> LowPCVal = None;
   Optional<AttrInfo> HighPCVal = None;
   getRangeAttrData(DIE, LowPCVal, HighPCVal);
   uint64_t LowPCOffset = LowPCVal->Offset;
   uint64_t HighPCOffset = HighPCVal->Offset;
   auto *TempDebugPatcher = &DebugInfoPatcher;
   if (LowPCVal->V.getForm() == dwarf::DW_FORM_GNU_addr_index) {
-    DWARFUnit *Unit = DIE.getDwarfUnit();
-    assert(Unit->isDWOUnit() && "DW_FORM_GNU_addr_index not part of DWO.");
     uint32_t AddressIndex =
-        AddrWriter->getIndexFromAddress(Range.LowPC, *Unit->getDWOId());
-    TempDebugPatcher = getBinaryDWODebugInfoPatcher(*Unit->getDWOId());
-    TempDebugPatcher->addUDataPatch(LowPCOffset, AddressIndex,
-                                    std::abs(int(HighPCOffset - LowPCOffset)));
-    // TODO: In DWARF5 support ULEB128 for high_pc
+        AddrWriter->getIndexFromAddress(Range.LowPC, *DWOId);
+    TempDebugPatcher = getBinaryDWODebugInfoPatcher(*DWOId);
+    TempDebugPatcher->addUDataPatch(LowPCOffset, AddressIndex, LowPCVal->Size);
+    // 2.17.2
+    // If the value of the DW_AT_high_pc is of class address, it is the
+    // relocated address of the first location past the last instruction
+    // associated with the entity; if it is of class constant, the value is
+    // an unsigned integer offset which when added to the low PC gives the
+    // address of the first location past the last instruction associated
+    // with the entity.
+    if (!HighPCVal->V.isFormClass(DWARFFormValue::FC_Constant)) {
+      AddressIndex = AddrWriter->getIndexFromAddress(Range.HighPC, *DWOId);
+      TempDebugPatcher->addUDataPatch(HighPCOffset, AddressIndex,
+                                      HighPCVal->Size);
+    }
   } else {
     TempDebugPatcher->addLE64Patch(LowPCOffset, Range.LowPC);
   }
 
   uint64_t HighPC = Range.HighPC;
   // The DW_FORM_data* is delta between high and low pc
   if (HighPCVal->V.getForm() != dwarf::Form::DW_FORM_addr)
     HighPC -= Range.LowPC;

bolt/test/X86/debug-fission-single.s

 # RUN:   -nostartfiles \
 # RUN:   -Wl,--script=%p/Inputs/debug-fission-script.txt \
 # RUN:   %t.o -o %t.exe
 # RUN: llvm-bolt %t.exe \
 # RUN:   --reorder-blocks=reverse \
 # RUN:   --update-debug-sections \
 # RUN:   --dwarf-output-path=%T \
 # RUN:   -o %t.bolt.1.exe 2>&1 | FileCheck %s
-# RUN: llvm-dwarfdump --show-form --verbose \
-# RUN:   --debug-info \
-# RUN:   %T/debug-fission-simple.dwo0.dwo \
-# RUN:   | grep DW_FORM_GNU_addr_index \
-# RUN:   | FileCheck %s --check-prefix=CHECK-ADDR-INDEX
-# RUN: llvm-dwarfdump --show-form --verbose \
-# RUN:   --debug-addr \
-# RUN:   %t.bolt.1.exe \
-# RUN:   | FileCheck %s --check-prefix=CHECK-ADDR-SEC
+# RUN: not llvm-dwarfdump --show-form --verbose --debug-info %T/debug-fission-simple.dwo0.dwo -o %tAddrIndexTest 2> /dev/null
+# RUN: cat %tAddrIndexTest | grep DW_FORM_GNU_addr_index | FileCheck %s --check-prefix=CHECK-ADDR-INDEX
+# RUN: llvm-dwarfdump --show-form --verbose   --debug-addr  %t.bolt.1.exe | FileCheck %s --check-prefix=CHECK-ADDR-SEC
 
 # CHECK-NOT: warning: DWARF unit from offset {{.*}} incl. to offset {{.*}} excl. tries to read DIEs at offset {{.*}}
 
-# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000001)
-# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000002)
-# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000003)
+# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000000)
+# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000000)
+# CHECK-ADDR-INDEX: DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000000)
 
 # CHECK-ADDR-SEC: .debug_addr contents:
 # CHECK-ADDR-SEC: 0x00000000: Addrs: [
 # CHECK-ADDR-SEC: 0x0000000000601000
-# CHECK-ADDR-SEC: 0x0000000000a00000
-# CHECK-ADDR-SEC: 0x0000000000000000
-# CHECK-ADDR-SEC: 0x0000000000a00040
-
-# RUN: llvm-bolt %t.exe --reorder-blocks=reverse -update-debug-sections \
-# RUN:   -dwarf-output-path=%T -o %t.bolt.2.exe --write-dwp=true
-# RUN: llvm-dwarfdump --show-form --verbose --debug-info %t.bolt.2.exe.dwp \
-# RUN:   | FileCheck %s --check-prefix=CHECK-DWP-DEBUG
+
+# RUN: llvm-bolt %t.exe --reorder-blocks=reverse -update-debug-sections -dwarf-output-path=%T -o %t.bolt.2.exe --write-dwp=true
+# RUN: not llvm-dwarfdump --show-form --verbose --debug-info %t.bolt.2.exe.dwp -o %tAddrIndexTestDwp 2> /dev/null
+# RUN: cat %tAddrIndexTestDwp | FileCheck %s --check-prefix=CHECK-DWP-DEBUG
 
 # CHECK-DWP-DEBUG: DW_TAG_compile_unit [1] *
 # CHECK-DWP-DEBUG:  DW_AT_producer [DW_FORM_GNU_str_index]  (indexed (0000000a) string = "clang version 13.0.0")
 # CHECK-DWP-DEBUG:  DW_AT_language [DW_FORM_data2]  (DW_LANG_C_plus_plus)
 # CHECK-DWP-DEBUG:  DW_AT_name [DW_FORM_GNU_str_index]  (indexed (0000000b) string = "foo")
 # CHECK-DWP-DEBUG:  DW_AT_GNU_dwo_name [DW_FORM_GNU_str_index]  (indexed (0000000c) string = "foo")
 # CHECK-DWP-DEBUG:  DW_AT_GNU_dwo_id [DW_FORM_data8]  (0x06105e732fad3796)
 

bolt/test/X86/gdbindex.test

 ; gold --gdb-index test.o test2.o -o dwarfdump-gdbindex-v7.elf-x86-64
 ; gcc version 5.3.1 20160413, GNU gold (GNU Binutils for Ubuntu 2.26) 1.11
 ; Info about gdb-index: https://sourceware.org/gdb/onlinedocs/gdb/Index-Section-Format.html
 
 ; CHECK-LABEL: .gdb_index contents:
 ; CHECK: Version = 7
 
 ; CHECK:      CU list offset = 0x18, has 2 entries:
-; CHECK-NEXT:   0: Offset = 0x0, Length = 0x4f
-; CHECK-NEXT:   1: Offset = 0x4f, Length = 0x53
+; CHECK-NEXT:   0: Offset = 0x0, Length = 0x4b
+; CHECK-NEXT:   1: Offset = 0x4b, Length = 0x4f
 
 ; CHECK:      Types CU list offset = 0x38, has 0 entries:
 
 ; CHECK:      Address area offset = 0x38, has 2 entries:
 ; CHECK-NEXT:   Low/High address = [0x20117c, 0x201187) (Size: 0xb), CU id = 0
 ; CHECK-NEXT:   Low/High address = [0x201188, 0x201193) (Size: 0xb), CU id = 1
 
 ; CHECK:      Symbol table offset = 0x60, size = 1024, filled slots:
 
 ; CHECK:      Constant pool offset = 0x2060, has 0 CU vectors: