[DWARFLinker][NFC] Remove RangesTy &getValidAddressRanges().

This patch simplifies line table generation. It removes global
array of all units ranges(RangesTy &getValidAddressRanges()).
The comment says that global array of all units ranges is necessary
to handle corner cases inside line table rows. Removing that
special handling shows that its current usage is handling of
"end of range case" which is already handled correctly
(without special handling). .debug_line tables for clang binary
built with and without this patch are equal.

Differential Revision: https://reviews.llvm.org/D154288

1 files changed, 28 insertions, 47 deletions

diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index a954a1a..ceeb26a 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -565,9 +565,8 @@ unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
 /// Check if a function describing DIE should be kept.
 /// \returns updated TraversalFlags.
 unsigned DWARFLinker::shouldKeepSubprogramDIE(
-    AddressesMap &RelocMgr, RangesTy &Ranges, const DWARFDie &DIE,
-    const DWARFFile &File, CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo,
-    unsigned Flags) {
+    AddressesMap &RelocMgr, const DWARFDie &DIE, const DWARFFile &File,
+    CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo, unsigned Flags) {
   Flags |= TF_InFunctionScope;
 
   auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
@@ -622,16 +621,14 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE(
   }
 
   // Replace the debug map range with a more accurate one.
-  Ranges.insert({*LowPc, *HighPc}, MyInfo.AddrAdjust);
   Unit.addFunctionRange(*LowPc, *HighPc, MyInfo.AddrAdjust);
   return Flags;
 }
 
 /// Check if a DIE should be kept.
 /// \returns updated TraversalFlags.
-unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
-                                    const DWARFDie &DIE, const DWARFFile &File,
-                                    CompileUnit &Unit,
+unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
+                                    const DWARFFile &File, CompileUnit &Unit,
                                     CompileUnit::DIEInfo &MyInfo,
                                     unsigned Flags) {
   switch (DIE.getTag()) {
@@ -640,8 +637,7 @@ unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
     return shouldKeepVariableDIE(RelocMgr, DIE, MyInfo, Flags);
   case dwarf::DW_TAG_subprogram:
   case dwarf::DW_TAG_label:
-    return shouldKeepSubprogramDIE(RelocMgr, Ranges, DIE, File, Unit, MyInfo,
-                                   Flags);
+    return shouldKeepSubprogramDIE(RelocMgr, DIE, File, Unit, MyInfo, Flags);
   case dwarf::DW_TAG_base_type:
     // DWARF Expressions may reference basic types, but scanning them
     // is expensive. Basic types are tiny, so just keep all of them.
@@ -861,7 +857,7 @@ void DWARFLinker::lookForParentDIEsToKeep(
 ///
 /// The return value indicates whether the DIE is incomplete.
 void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
-                                    RangesTy &Ranges, const UnitListTy &Units,
+                                    const UnitListTy &Units,
                                     const DWARFDie &Die, const DWARFFile &File,
                                     CompileUnit &Cu, unsigned Flags) {
   // LIFO work list.
@@ -916,8 +912,8 @@ void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
       continue;
 
     if (!(Current.Flags & TF_DependencyWalk))
-      Current.Flags = shouldKeepDIE(AddressesMap, Ranges, Current.Die, File,
-                                    Current.CU, MyInfo, Current.Flags);
+      Current.Flags = shouldKeepDIE(AddressesMap, Current.Die, File, Current.CU,
+                                    MyInfo, Current.Flags);
 
     // We need to mark context for the canonical die in the end of normal
     // traversing(not TF_DependencyWalk) or after normal traversing if die
@@ -2097,8 +2093,6 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
 
       LineTable.Sequences = LT->Sequences;
     } else {
-      RangesTy &Ranges = ObjFile.Addresses->getValidAddressRanges();
-
       // This vector is the output line table.
       std::vector<DWARFDebugLine::Row> NewRows;
       NewRows.reserve(LT->Rows.size());
@@ -2127,27 +2121,12 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
         // it is marked as end_sequence in the input (because in that
         // case, the relocation offset is accurate and that entry won't
         // serve as the start of another function).
-        if (!CurrRange || !CurrRange->Range.contains(Row.Address.Address) ||
-            (Row.Address.Address == CurrRange->Range.end() &&
-             !Row.EndSequence)) {
+        if (!CurrRange || !CurrRange->Range.contains(Row.Address.Address)) {
           // We just stepped out of a known range. Insert a end_sequence
           // corresponding to the end of the range.
           uint64_t StopAddress =
               CurrRange ? CurrRange->Range.end() + CurrRange->Value : -1ULL;
           CurrRange = FunctionRanges.getRangeThatContains(Row.Address.Address);
-          if (!CurrRange) {
-            if (StopAddress != -1ULL) {
-              // Try harder by looking in the Address ranges map.
-              // There are corner cases where this finds a
-              // valid entry. It's unclear if this is right or wrong, but
-              // for now do as dsymutil.
-              // FIXME: Understand exactly what cases this addresses and
-              // potentially remove it along with the Ranges map.
-              if (std::optional<AddressRangeValuePair> Range =
-                      Ranges.getRangeThatContains(Row.Address.Address))
-                StopAddress = Row.Address.Address + (*Range).Value;
-            }
-          }
           if (StopAddress != -1ULL && !Seq.empty()) {
             // Insert end sequence row with the computed end address, but
             // the same line as the previous one.
@@ -2236,14 +2215,20 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) {
 /// This is actually pretty easy as the data of the CIEs and FDEs can
 /// be considered as black boxes and moved as is. The only thing to do
 /// is to patch the addresses in the headers.
-void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
-                                          RangesTy &Ranges,
-                                          DWARFContext &OrigDwarf,
-                                          unsigned AddrSize) {
+void DWARFLinker::patchFrameInfoForObject(LinkContext &Context) {
+  DWARFContext &OrigDwarf = *Context.File.Dwarf;
+  unsigned SrcAddrSize = OrigDwarf.getDWARFObj().getAddressSize();
+
   StringRef FrameData = OrigDwarf.getDWARFObj().getFrameSection().Data;
   if (FrameData.empty())
     return;
 
+  RangesTy AllUnitsRanges;
+  for (std::unique_ptr<CompileUnit> &Unit : Context.CompileUnits) {
+    for (auto CurRange : Unit->getFunctionRanges())
+      AllUnitsRanges.insert(CurRange.Range, CurRange.Value);
+  }
+
   DataExtractor Data(FrameData, OrigDwarf.isLittleEndian(), 0);
   uint64_t InputOffset = 0;
 
@@ -2255,7 +2240,7 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
     uint64_t EntryOffset = InputOffset;
     uint32_t InitialLength = Data.getU32(&InputOffset);
     if (InitialLength == 0xFFFFFFFF)
-      return reportWarning("Dwarf64 bits no supported", File);
+      return reportWarning("Dwarf64 bits no supported", Context.File);
 
     uint32_t CIEId = Data.getU32(&InputOffset);
     if (CIEId == 0xFFFFFFFF) {
@@ -2267,14 +2252,14 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
       continue;
     }
 
-    uint64_t Loc = Data.getUnsigned(&InputOffset, AddrSize);
+    uint64_t Loc = Data.getUnsigned(&InputOffset, SrcAddrSize);
 
     // Some compilers seem to emit frame info that doesn't start at
     // the function entry point, thus we can't just lookup the address
     // in the debug map. Use the AddressInfo's range map to see if the FDE
     // describes something that we can relocate.
     std::optional<AddressRangeValuePair> Range =
-        Ranges.getRangeThatContains(Loc);
+        AllUnitsRanges.getRangeThatContains(Loc);
     if (!Range) {
       // The +4 is to account for the size of the InitialLength field itself.
       InputOffset = EntryOffset + InitialLength + 4;
@@ -2285,7 +2270,8 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
     // Have we already emitted a corresponding CIE?
     StringRef CIEData = LocalCIES[CIEId];
     if (CIEData.empty())
-      return reportWarning("Inconsistent debug_frame content. Dropping.", File);
+      return reportWarning("Inconsistent debug_frame content. Dropping.",
+                           Context.File);
 
     // Look if we already emitted a CIE that corresponds to the
     // referenced one (the CIE data is the key of that lookup).
@@ -2301,8 +2287,8 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
     // Emit the FDE with updated address and CIE pointer.
     // (4 + AddrSize) is the size of the CIEId + initial_location
     // fields that will get reconstructed by emitFDE().
-    unsigned FDERemainingBytes = InitialLength - (4 + AddrSize);
-    TheDwarfEmitter->emitFDE(IteratorInserted.first->getValue(), AddrSize,
+    unsigned FDERemainingBytes = InitialLength - (4 + SrcAddrSize);
+    TheDwarfEmitter->emitFDE(IteratorInserted.first->getValue(), SrcAddrSize,
                              Loc + Range->Value,
                              FrameData.substr(InputOffset, FDERemainingBytes));
     InputOffset += FDERemainingBytes;
@@ -2868,9 +2854,7 @@ Error DWARFLinker::link() {
       copyInvariantDebugSection(*OptContext.File.Dwarf);
     } else {
       for (auto &CurrentUnit : OptContext.CompileUnits) {
-        lookForDIEsToKeep(*OptContext.File.Addresses,
-                          OptContext.File.Addresses->getValidAddressRanges(),
-                          OptContext.CompileUnits,
+        lookForDIEsToKeep(*OptContext.File.Addresses, OptContext.CompileUnits,
                           CurrentUnit->getOrigUnit().getUnitDIE(),
                           OptContext.File, *CurrentUnit, 0);
 #ifndef NDEBUG
@@ -2895,10 +2879,7 @@ Error DWARFLinker::link() {
     }
     if ((TheDwarfEmitter != nullptr) && !OptContext.CompileUnits.empty() &&
         LLVM_LIKELY(!Options.Update))
-      patchFrameInfoForObject(
-          OptContext.File, OptContext.File.Addresses->getValidAddressRanges(),
-          *OptContext.File.Dwarf,
-          OptContext.CompileUnits[0]->getOrigUnit().getAddressByteSize());
+      patchFrameInfoForObject(OptContext);
 
     // Clean-up before starting working on the next object.
     cleanupAuxiliarryData(OptContext);