DWARF: Don't compute address ranges for type units

Summary:
Type units don't describe any code, so they should never be the result
of any address lookup queries.

Previously, we would compute the address ranges for the type units for
via the line tables they reference because the type units looked a lot
like line-tables-only compile units. However, this is not correct, as
the line tables are only referenced from type units so that other
declarations can use the file names contained in them.

In this patch I make the BuildAddressRangeTable function virtual, and
implement it only for compile units.

Testing this was a bit tricky, because the behavior depends on the order
in which we add things to the address range map. This rarely caused a
problem with DWARF v4 type units, as they are always added after all
CUs. It happened more frequently with DWARF v5, as there clang emits the
type units first. However, this is still not something that it is
required to do, so for testing I've created an assembly file where I've
deliberately sandwiched a compile unit between two type units, which
should isolate us from both changes in how the compiler emits the units
and changes in the order we process them.

Reviewers: clayborg, aprantl, JDevlieghere

Subscribers: jdoerfert, lldb-commits

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

llvm-svn: 361465

Author: labath

lldb/lit/SymbolFile/DWARF/debug-types-address-ranges.s

@@ -7,7 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "DWARFCompileUnit.h"
+#include "DWARFDebugAranges.h"
+#include "SymbolFileDWARFDebugMap.h"
 
+#include "lldb/Symbol/CompileUnit.h"
+#include "lldb/Symbol/LineTable.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
@@ -20,3 +24,92 @@ void DWARFCompileUnit::Dump(Stream *s) const {
             GetOffset(), GetLength(), GetVersion(), GetAbbrevOffset(),
             GetAddressByteSize(), GetNextUnitOffset());
 }
+
+void DWARFCompileUnit::BuildAddressRangeTable(
+    DWARFDebugAranges *debug_aranges) {
+  // This function is usually called if there in no .debug_aranges section in
+  // order to produce a compile unit level set of address ranges that is
+  // accurate.
+
+  size_t num_debug_aranges = debug_aranges->GetNumRanges();
+
+  // First get the compile unit DIE only and check if it has a DW_AT_ranges
+  const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
+
+  const dw_offset_t cu_offset = GetOffset();
+  if (die) {
+    DWARFRangeList ranges;
+    const size_t num_ranges =
+        die->GetAttributeAddressRanges(this, ranges, false);
+    if (num_ranges > 0) {
+      // This compile unit has DW_AT_ranges, assume this is correct if it is
+      // present since clang no longer makes .debug_aranges by default and it
+      // emits DW_AT_ranges for DW_TAG_compile_units. GCC also does this with
+      // recent GCC builds.
+      for (size_t i = 0; i < num_ranges; ++i) {
+        const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
+        debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
+                                   range.GetRangeEnd());
+      }
+
+      return; // We got all of our ranges from the DW_AT_ranges attribute
+    }
+  }
+  // We don't have a DW_AT_ranges attribute, so we need to parse the DWARF
+
+  // If the DIEs weren't parsed, then we don't want all dies for all compile
+  // units to stay loaded when they weren't needed. So we can end up parsing
+  // the DWARF and then throwing them all away to keep memory usage down.
+  ScopedExtractDIEs clear_dies(ExtractDIEsScoped());
+
+  die = DIEPtr();
+  if (die)
+    die->BuildAddressRangeTable(this, debug_aranges);
+
+  if (debug_aranges->GetNumRanges() == num_debug_aranges) {
+    // We got nothing from the functions, maybe we have a line tables only
+    // situation. Check the line tables and build the arange table from this.
+    SymbolContext sc;
+    sc.comp_unit = m_dwarf->GetCompUnitForDWARFCompUnit(this);
+    if (sc.comp_unit) {
+      SymbolFileDWARFDebugMap *debug_map_sym_file =
+          m_dwarf->GetDebugMapSymfile();
+      if (debug_map_sym_file == nullptr) {
+        if (LineTable *line_table = sc.comp_unit->GetLineTable()) {
+          LineTable::FileAddressRanges file_ranges;
+          const bool append = true;
+          const size_t num_ranges =
+              line_table->GetContiguousFileAddressRanges(file_ranges, append);
+          for (uint32_t idx = 0; idx < num_ranges; ++idx) {
+            const LineTable::FileAddressRanges::Entry &range =
+                file_ranges.GetEntryRef(idx);
+            debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
+                                       range.GetRangeEnd());
+          }
+        }
+      } else
+        debug_map_sym_file->AddOSOARanges(m_dwarf, debug_aranges);
+    }
+  }
+
+  if (debug_aranges->GetNumRanges() == num_debug_aranges) {
+    // We got nothing from the functions, maybe we have a line tables only
+    // situation. Check the line tables and build the arange table from this.
+    SymbolContext sc;
+    sc.comp_unit = m_dwarf->GetCompUnitForDWARFCompUnit(this);
+    if (sc.comp_unit) {
+      if (LineTable *line_table = sc.comp_unit->GetLineTable()) {
+        LineTable::FileAddressRanges file_ranges;
+        const bool append = true;
+        const size_t num_ranges =
+            line_table->GetContiguousFileAddressRanges(file_ranges, append);
+        for (uint32_t idx = 0; idx < num_ranges; ++idx) {
+          const LineTable::FileAddressRanges::Entry &range =
+              file_ranges.GetEntryRef(idx);
+          debug_aranges->AppendRange(GetOffset(), range.GetRangeBase(),
+                                     range.GetRangeEnd());
+        }
+      }
+    }
+  }
+}

lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp

@@ -14,6 +14,8 @@
 
 class DWARFCompileUnit : public DWARFUnit {
 public:
+  void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges) override;
+
   void Dump(lldb_private::Stream *s) const override;
 
 private:

lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h

@@ -14,6 +14,8 @@
 
 class DWARFTypeUnit : public DWARFUnit {
 public:
+  void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges) override {}
+
   void Dump(lldb_private::Stream *s) const override;
 
 private:

lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h

@@ -10,8 +10,6 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Host/StringConvert.h"
-#include "lldb/Symbol/CompileUnit.h"
-#include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/StreamString.h"
@@ -23,7 +21,6 @@
 #include "DWARFDebugInfo.h"
 #include "DWARFTypeUnit.h"
 #include "LogChannelDWARF.h"
-#include "SymbolFileDWARFDebugMap.h"
 #include "SymbolFileDWARFDwo.h"
 
 using namespace lldb;
@@ -407,98 +404,6 @@ void DWARFUnit::ClearDIEsRWLocked() {
     m_dwo_symbol_file->GetCompileUnit()->ClearDIEsRWLocked();
 }
 
-void DWARFUnit::BuildAddressRangeTable(DWARFDebugAranges *debug_aranges) {
-  // This function is usually called if there in no .debug_aranges section in
-  // order to produce a compile unit level set of address ranges that is
-  // accurate.
-
-  size_t num_debug_aranges = debug_aranges->GetNumRanges();
-
-  // First get the compile unit DIE only and check if it has a DW_AT_ranges
-  const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
-
-  const dw_offset_t cu_offset = GetOffset();
-  if (die) {
-    DWARFRangeList ranges;
-    const size_t num_ranges =
-        die->GetAttributeAddressRanges(this, ranges, false);
-    if (num_ranges > 0) {
-      // This compile unit has DW_AT_ranges, assume this is correct if it is
-      // present since clang no longer makes .debug_aranges by default and it
-      // emits DW_AT_ranges for DW_TAG_compile_units. GCC also does this with
-      // recent GCC builds.
-      for (size_t i = 0; i < num_ranges; ++i) {
-        const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
-        debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
-                                   range.GetRangeEnd());
-      }
-
-      return; // We got all of our ranges from the DW_AT_ranges attribute
-    }
-  }
-  // We don't have a DW_AT_ranges attribute, so we need to parse the DWARF
-
-  // If the DIEs weren't parsed, then we don't want all dies for all compile
-  // units to stay loaded when they weren't needed. So we can end up parsing
-  // the DWARF and then throwing them all away to keep memory usage down.
-  ScopedExtractDIEs clear_dies(ExtractDIEsScoped());
-
-  die = DIEPtr();
-  if (die)
-    die->BuildAddressRangeTable(this, debug_aranges);
-
-  if (debug_aranges->GetNumRanges() == num_debug_aranges) {
-    // We got nothing from the functions, maybe we have a line tables only
-    // situation. Check the line tables and build the arange table from this.
-    SymbolContext sc;
-    sc.comp_unit = m_dwarf->GetCompUnitForDWARFCompUnit(this);
-    if (sc.comp_unit) {
-      SymbolFileDWARFDebugMap *debug_map_sym_file =
-          m_dwarf->GetDebugMapSymfile();
-      if (debug_map_sym_file == NULL) {
-        LineTable *line_table = sc.comp_unit->GetLineTable();
-
-        if (line_table) {
-          LineTable::FileAddressRanges file_ranges;
-          const bool append = true;
-          const size_t num_ranges =
-              line_table->GetContiguousFileAddressRanges(file_ranges, append);
-          for (uint32_t idx = 0; idx < num_ranges; ++idx) {
-            const LineTable::FileAddressRanges::Entry &range =
-                file_ranges.GetEntryRef(idx);
-            debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
-                                       range.GetRangeEnd());
-          }
-        }
-      } else
-        debug_map_sym_file->AddOSOARanges(m_dwarf, debug_aranges);
-    }
-  }
-
-  if (debug_aranges->GetNumRanges() == num_debug_aranges) {
-    // We got nothing from the functions, maybe we have a line tables only
-    // situation. Check the line tables and build the arange table from this.
-    SymbolContext sc;
-    sc.comp_unit = m_dwarf->GetCompUnitForDWARFCompUnit(this);
-    if (sc.comp_unit) {
-      LineTable *line_table = sc.comp_unit->GetLineTable();
-
-      if (line_table) {
-        LineTable::FileAddressRanges file_ranges;
-        const bool append = true;
-        const size_t num_ranges =
-            line_table->GetContiguousFileAddressRanges(file_ranges, append);
-        for (uint32_t idx = 0; idx < num_ranges; ++idx) {
-          const LineTable::FileAddressRanges::Entry &range =
-              file_ranges.GetEntryRef(idx);
-          debug_aranges->AppendRange(GetOffset(), range.GetRangeBase(),
-                                     range.GetRangeEnd());
-        }
-      }
-    }
-  }
-}
-
 lldb::ByteOrder DWARFUnit::GetByteOrder() const {
   return m_dwarf->GetObjectFile()->GetByteOrder();
 }

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

@@ -143,7 +143,7 @@ class DWARFUnit : public lldb_private::UserID {
   void SetRangesBase(dw_addr_t ranges_base);
   void SetBaseObjOffset(dw_offset_t base_obj_offset);
   void SetStrOffsetsBase(dw_offset_t str_offsets_base);
-  void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges);
+  virtual void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges) = 0;
 
   lldb::ByteOrder GetByteOrder() const;
 
@@ -215,6 +215,24 @@ class DWARFUnit : public lldb_private::UserID {
                             const lldb_private::DWARFDataExtractor &data,
                             lldb::offset_t *offset_ptr);
 
+  // Get the DWARF unit DWARF debug information entry. Parse the single DIE
+  // if needed.
+  const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
+    ExtractUnitDIEIfNeeded();
+    // m_first_die_mutex is not required as m_first_die is never cleared.
+    if (!m_first_die)
+      return NULL;
+    return &m_first_die;
+  }
+
+  // Get all DWARF debug informration entries. Parse all DIEs if needed.
+  const DWARFDebugInfoEntry *DIEPtr() {
+    ExtractDIEsIfNeeded();
+    if (m_die_array.empty())
+      return NULL;
+    return &m_die_array[0];
+  }
+
   SymbolFileDWARF *m_dwarf = nullptr;
   std::unique_ptr<SymbolFileDWARFDwo> m_dwo_symbol_file;
   DWARFUnitHeader m_header;
@@ -257,24 +275,6 @@ class DWARFUnit : public lldb_private::UserID {
   void ExtractDIEsRWLocked();
   void ClearDIEsRWLocked();
 
-  // Get the DWARF unit DWARF debug informration entry. Parse the single DIE
-  // if needed.
-  const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
-    ExtractUnitDIEIfNeeded();
-    // m_first_die_mutex is not required as m_first_die is never cleared.
-    if (!m_first_die)
-      return NULL;
-    return &m_first_die;
-  }
-
-  // Get all DWARF debug informration entries. Parse all DIEs if needed.
-  const DWARFDebugInfoEntry *DIEPtr() {
-    ExtractDIEsIfNeeded();
-    if (m_die_array.empty())
-      return NULL;
-    return &m_die_array[0];
-  }
-
   void AddUnitDIE(const DWARFDebugInfoEntry &cu_die);
 
   void ComputeCompDirAndGuessPathStyle();

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

@@ -62,7 +62,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
   friend class SymbolFileDWARFDwo;
   friend class DebugMapModule;
   friend struct DIERef;
-  friend class DWARFUnit;
+  friend class DWARFCompileUnit;
   friend class DWARFDIE;
   friend class DWARFASTParserClang;
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

@@ -135,7 +135,7 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile {
   friend class DebugMapModule;
   friend struct DIERef;
   friend class DWARFASTParserClang;
-  friend class DWARFUnit;
+  friend class DWARFCompileUnit;
   friend class SymbolFileDWARF;
   struct OSOInfo {
     lldb::ModuleSP module_sp;