[dsymutil][DWARFLinker][NFC] Make interface of AddressMap more general.

Current interface of AddressMap assumes that relocations exist.
That is correct for not-linked object file but is not correct
for linked executable. This patch changes interface in such way
that AddressMap could be used not only with not-linked object files:

hasValidRelocationAt()

replaced with:

hasLiveMemoryLocation()
hasLiveAddressRange()

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

4 files changed, 78 insertions, 58 deletions

diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index a96f403..edf7416 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -64,14 +64,17 @@ public:
   /// section. Reset current relocation pointer if neccessary.
   virtual bool hasValidRelocs(bool ResetRelocsPtr = true) = 0;
 
-  /// Checks that there is a relocation against .debug_info
-  /// table between \p StartOffset and \p NextOffset.
-  ///
-  /// This function must be called with offsets in strictly ascending
-  /// order because it never looks back at relocations it already 'went past'.
-  /// \returns true and sets Info.InDebugMap if it is the case.
-  virtual bool hasValidRelocationAt(uint64_t StartOffset, uint64_t EndOffset,
-                                    CompileUnit::DIEInfo &Info) = 0;
+  /// Checks that the specified DIE has a DW_AT_Location attribute
+  /// that references into a live code section. This function
+  /// must be called with DIE offsets in strictly ascending order.
+  virtual bool hasLiveMemoryLocation(const DWARFDie &DIE,
+                                     CompileUnit::DIEInfo &Info) = 0;
+
+  /// Checks that the specified DIE has a DW_AT_Low_pc attribute
+  /// that references into a live code section. This function
+  /// must be called with DIE offsets in strictly ascending order.
+  virtual bool hasLiveAddressRange(const DWARFDie &DIE,
+                                   CompileUnit::DIEInfo &Info) = 0;
 
   /// Apply the valid relocations to the buffer \p Data, taking into
   /// account that Data is at \p BaseOffset in the debug_info section.
@@ -497,7 +500,6 @@ private:
   /// Check if a variable describing DIE should be kept.
   /// \returns updated TraversalFlags.
   unsigned shouldKeepVariableDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
-                                 CompileUnit &Unit,
                                  CompileUnit::DIEInfo &MyInfo, unsigned Flags);
 
   unsigned shouldKeepSubprogramDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 3ccbe12..b48f6ea 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -421,32 +421,11 @@ void DWARFLinker::cleanupAuxiliarryData(LinkContext &Context) {
   DIEAlloc.Reset();
 }
 
-/// Get the starting and ending (exclusive) offset for the
-/// attribute with index \p Idx descibed by \p Abbrev. \p Offset is
-/// supposed to point to the position of the first attribute described
-/// by \p Abbrev.
-/// \return [StartOffset, EndOffset) as a pair.
-static std::pair<uint64_t, uint64_t>
-getAttributeOffsets(const DWARFAbbreviationDeclaration *Abbrev, unsigned Idx,
-                    uint64_t Offset, const DWARFUnit &Unit) {
-  DataExtractor Data = Unit.getDebugInfoExtractor();
-
-  for (unsigned I = 0; I < Idx; ++I)
-    DWARFFormValue::skipValue(Abbrev->getFormByIndex(I), Data, &Offset,
-                              Unit.getFormParams());
-
-  uint64_t End = Offset;
-  DWARFFormValue::skipValue(Abbrev->getFormByIndex(Idx), Data, &End,
-                            Unit.getFormParams());
-
-  return std::make_pair(Offset, End);
-}
 
 /// Check if a variable describing DIE should be kept.
 /// \returns updated TraversalFlags.
 unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
                                             const DWARFDie &DIE,
-                                            CompileUnit &Unit,
                                             CompileUnit::DIEInfo &MyInfo,
                                             unsigned Flags) {
   const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
@@ -458,24 +437,12 @@ unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
     return Flags | TF_Keep;
   }
 
-  Optional<uint32_t> LocationIdx =
-      Abbrev->findAttributeIndex(dwarf::DW_AT_location);
-  if (!LocationIdx)
-    return Flags;
-
-  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
-  const DWARFUnit &OrigUnit = Unit.getOrigUnit();
-  uint64_t LocationOffset, LocationEndOffset;
-  std::tie(LocationOffset, LocationEndOffset) =
-      getAttributeOffsets(Abbrev, *LocationIdx, Offset, OrigUnit);
-
   // See if there is a relocation to a valid debug map entry inside
   // this variable's location. The order is important here. We want to
   // always check if the variable has a valid relocation, so that the
   // DIEInfo is filled. However, we don't want a static variable in a
   // function to force us to keep the enclosing function.
-  if (!RelocMgr.hasValidRelocationAt(LocationOffset, LocationEndOffset,
-                                     MyInfo) ||
+  if (!RelocMgr.hasLiveMemoryLocation(DIE, MyInfo) ||
       (Flags & TF_InFunctionScope))
     return Flags;
 
@@ -496,24 +463,14 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE(
     AddressesMap &RelocMgr, RangesTy &Ranges, const DWARFDie &DIE,
     const DWARFFile &File, CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo,
     unsigned Flags) {
-  const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
-
   Flags |= TF_InFunctionScope;
 
-  Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
-  if (!LowPcIdx)
+  auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
+  if (!LowPc)
     return Flags;
 
-  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
-  DWARFUnit &OrigUnit = Unit.getOrigUnit();
-  uint64_t LowPcOffset, LowPcEndOffset;
-  std::tie(LowPcOffset, LowPcEndOffset) =
-      getAttributeOffsets(Abbrev, *LowPcIdx, Offset, OrigUnit);
-
-  auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
   assert(LowPc.hasValue() && "low_pc attribute is not an address.");
-  if (!LowPc ||
-      !RelocMgr.hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo))
+  if (!RelocMgr.hasLiveAddressRange(DIE, MyInfo))
     return Flags;
 
   if (Options.Verbose) {
@@ -527,6 +484,8 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE(
   if (DIE.getTag() == dwarf::DW_TAG_label) {
     if (Unit.hasLabelAt(*LowPc))
       return Flags;
+
+    DWARFUnit &OrigUnit = Unit.getOrigUnit();
     // FIXME: dsymutil-classic compat. dsymutil-classic doesn't consider labels
     // that don't fall into the CU's aranges. This is wrong IMO. Debug info
     // generation bugs aside, this is really wrong in the case of labels, where
@@ -563,7 +522,7 @@ unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
   switch (DIE.getTag()) {
   case dwarf::DW_TAG_constant:
   case dwarf::DW_TAG_variable:
-    return shouldKeepVariableDIE(RelocMgr, DIE, Unit, MyInfo, Flags);
+    return shouldKeepVariableDIE(RelocMgr, DIE, MyInfo, Flags);
   case dwarf::DW_TAG_subprogram:
   case dwarf::DW_TAG_label:
     return shouldKeepSubprogramDIE(RelocMgr, Ranges, DIE, File, Unit, MyInfo,
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index 3c71567..891c38c 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -649,6 +649,60 @@ bool DwarfLinkerForBinary::AddressManager::hasValidRelocationAt(
   return true;
 }
 
+/// Get the starting and ending (exclusive) offset for the
+/// attribute with index \p Idx descibed by \p Abbrev. \p Offset is
+/// supposed to point to the position of the first attribute described
+/// by \p Abbrev.
+/// \return [StartOffset, EndOffset) as a pair.
+static std::pair<uint64_t, uint64_t>
+getAttributeOffsets(const DWARFAbbreviationDeclaration *Abbrev, unsigned Idx,
+                    uint64_t Offset, const DWARFUnit &Unit) {
+  DataExtractor Data = Unit.getDebugInfoExtractor();
+
+  for (unsigned I = 0; I < Idx; ++I)
+    DWARFFormValue::skipValue(Abbrev->getFormByIndex(I), Data, &Offset,
+                              Unit.getFormParams());
+
+  uint64_t End = Offset;
+  DWARFFormValue::skipValue(Abbrev->getFormByIndex(Idx), Data, &End,
+                            Unit.getFormParams());
+
+  return std::make_pair(Offset, End);
+}
+
+bool DwarfLinkerForBinary::AddressManager::hasLiveMemoryLocation(
+    const DWARFDie &DIE, CompileUnit::DIEInfo &MyInfo) {
+
+  const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
+
+  Optional<uint32_t> LocationIdx =
+      Abbrev->findAttributeIndex(dwarf::DW_AT_location);
+  if (!LocationIdx)
+    return false;
+
+  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
+  uint64_t LocationOffset, LocationEndOffset;
+  std::tie(LocationOffset, LocationEndOffset) =
+      getAttributeOffsets(Abbrev, *LocationIdx, Offset, *DIE.getDwarfUnit());
+
+  return hasValidRelocationAt(LocationOffset, LocationEndOffset, MyInfo);
+}
+
+bool DwarfLinkerForBinary::AddressManager::hasLiveAddressRange(
+    const DWARFDie &DIE, CompileUnit::DIEInfo &MyInfo) {
+  const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
+
+  Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
+  if (!LowPcIdx)
+    return false;
+
+  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
+  uint64_t LowPcOffset, LowPcEndOffset;
+  std::tie(LowPcOffset, LowPcEndOffset) =
+      getAttributeOffsets(Abbrev, *LowPcIdx, Offset, *DIE.getDwarfUnit());
+
+  return hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo);
+}
 /// Apply the valid relocations found by findValidRelocs() to
 /// the buffer \p Data, taking into account that Data is at \p BaseOffset
 /// in the debug_info section.
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
index 842b27c..ec157e3 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
@@ -144,7 +144,12 @@ private:
     /// @}
 
     bool hasValidRelocationAt(uint64_t StartOffset, uint64_t EndOffset,
-                              CompileUnit::DIEInfo &Info) override;
+                              CompileUnit::DIEInfo &Info);
+
+    bool hasLiveMemoryLocation(const DWARFDie &DIE,
+                               CompileUnit::DIEInfo &Info) override;
+    bool hasLiveAddressRange(const DWARFDie &DIE,
+                             CompileUnit::DIEInfo &Info) override;
 
     bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
                           bool IsLittleEndian) override;