jankratochvil updated this revision to Diff 148801.
jankratochvil added a comment.

Split out `ExtractUnitDIEIfNeeded()` out of `ExtractDIEsIfNeeded()`.


https://reviews.llvm.org/D46810

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -50,9 +50,9 @@
   auto extract_fn = [&debug_info, &clear_cu_dies](size_t cu_idx) {
     DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
     if (dwarf_cu) {
-      // dwarf_cu->ExtractDIEsIfNeeded(false) will return zero if the DIEs
+      // dwarf_cu->ExtractDIEsIfNeeded() will return false if the DIEs
       // for a compile unit have already been parsed.
-      if (dwarf_cu->ExtractDIEsIfNeeded(false) > 1)
+      if (dwarf_cu->ExtractDIEsIfNeeded())
         clear_cu_dies[cu_idx] = true;
     }
   };
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -39,7 +39,8 @@
 public:
   virtual ~DWARFUnit();
 
-  size_t ExtractDIEsIfNeeded(bool cu_die_only);
+  void ExtractUnitDIEIfNeeded();
+  bool ExtractDIEsIfNeeded();
   DWARFDIE LookupAddress(const dw_addr_t address);
   size_t AppendDIEsWithTag(const dw_tag_t tag,
                            DWARFDIECollection &matching_dies,
@@ -160,7 +161,7 @@
   dw_offset_t GetBaseObjOffset() const;
 
   die_iterator_range dies() {
-    ExtractDIEsIfNeeded(false);
+    ExtractDIEsIfNeeded();
     return die_iterator_range(m_die_array.begin(), m_die_array.end());
   }
 
@@ -173,6 +174,10 @@
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;
+  // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
+  // But the first element of m_die_array after ExtractUnitDIEIfNeeded()
+  // would possibly move in memory after later ExtractDIEsIfNeeded().
+  DWARFDebugInfoEntry m_first_die;
   // A table similar to the .debug_aranges table, but this one points to the
   // exact DW_TAG_subprogram DIEs
   std::unique_ptr<DWARFDebugAranges> m_func_aranges_ap;
@@ -202,21 +207,22 @@
   // Get the DWARF unit DWARF debug informration entry. Parse the single DIE
   // if needed.
   const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
-    ExtractDIEsIfNeeded(true);
-    if (m_die_array.empty())
+    ExtractUnitDIEIfNeeded();
+    if (!m_first_die)
       return NULL;
-    return &m_die_array[0];
+    return &m_first_die;
   }
 
   // Get all DWARF debug informration entries. Parse all DIEs if needed.
   const DWARFDebugInfoEntry *DIEPtr() {
-    ExtractDIEsIfNeeded(false);
+    ExtractDIEsIfNeeded();
     if (m_die_array.empty())
       return NULL;
     return &m_die_array[0];
   }
 
-  void AddUnitDIE(DWARFDebugInfoEntry &die);
+  void AddUnitDIE(const DWARFDebugInfoEntry &cu_die);
+  void ExtractDIEsEndCheck(lldb::offset_t offset) const;
 
   DISALLOW_COPY_AND_ASSIGN(DWARFUnit);
 };
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -14,6 +14,7 @@
 #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"
 #include "lldb/Utility/Timer.h"
 
@@ -35,28 +36,54 @@
 DWARFUnit::~DWARFUnit() {}
 
 //----------------------------------------------------------------------
-// ParseCompileUnitDIEsIfNeeded
-//
+// Parses first DIE of a compile unit.
+//----------------------------------------------------------------------
+void DWARFUnit::ExtractUnitDIEIfNeeded() {
+  if (m_first_die)
+    return; // Already parsed
+
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(
+      func_cat, "%8.8x: DWARFUnit::ExtractUnitDIEIfNeeded()", m_offset);
+
+  // Set the offset to that of the first DIE and calculate the start of the
+  // next compilation unit header.
+  lldb::offset_t offset = GetFirstDIEOffset();
+
+  // We are in our compile unit, parse starting at the offset we were told to
+  // parse
+  const DWARFDataExtractor &data = GetData();
+  DWARFFormValue::FixedFormSizes fixed_form_sizes =
+      DWARFFormValue::GetFixedFormSizesForAddressSize(GetAddressByteSize(),
+                                                      IsDWARF64());
+  if (offset < GetNextCompileUnitOffset() &&
+      m_first_die.FastExtract(data, this, fixed_form_sizes, &offset)) {
+    AddUnitDIE(m_first_die);
+    return;
+  }
+
+  ExtractDIEsEndCheck(offset);
+}
+
+//----------------------------------------------------------------------
 // Parses a compile unit and indexes its DIEs if it hasn't already been done.
 //----------------------------------------------------------------------
-size_t DWARFUnit::ExtractDIEsIfNeeded(bool cu_die_only) {
-  const size_t initial_die_array_size = m_die_array.size();
-  if ((cu_die_only && initial_die_array_size > 0) || initial_die_array_size > 1)
+bool DWARFUnit::ExtractDIEsIfNeeded() {
+  if (!m_die_array.empty())
     return 0; // Already parsed
 
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(
-      func_cat, "%8.8x: DWARFUnit::ExtractDIEsIfNeeded( cu_die_only = %i )",
-      m_offset, cu_die_only);
+      func_cat, "%8.8x: DWARFUnit::ExtractDIEsIfNeeded()", m_offset);
 
   // Set the offset to that of the first DIE and calculate the start of the
   // next compilation unit header.
   lldb::offset_t offset = GetFirstDIEOffset();
   lldb::offset_t next_cu_offset = GetNextCompileUnitOffset();
 
   DWARFDebugInfoEntry die;
   // Keep a flat array of the DIE for binary lookup by DIE offset
-  if (!cu_die_only) {
+  {
     Log *log(
         LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO | DWARF_LOG_LOOKUPS));
     if (log) {
@@ -90,16 +117,21 @@
 
     const bool null_die = die.IsNULL();
     if (depth == 0) {
-      if (initial_die_array_size == 0)
-        AddUnitDIE(die);
-      uint64_t base_addr = die.GetAttributeValueAsAddress(
-          m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
-      if (base_addr == LLDB_INVALID_ADDRESS)
-        base_addr =
-            die.GetAttributeValueAsAddress(m_dwarf, this, DW_AT_entry_pc, 0);
-      SetBaseAddress(base_addr);
-      if (cu_die_only)
-        return 1;
+      assert(m_die_array.empty() && "Compile unit DIE already added");
+
+      // The average bytes per DIE entry has been seen to be around 14-20 so
+      // lets pre-reserve half of that since we are now stripping the NULL
+      // tags.
+
+      // Only reserve the memory if we are adding children of the main
+      // compile unit DIE. The compile unit DIE is always the first entry, so
+      // if our size is 1, then we are adding the first compile unit child
+      // DIE and should reserve the memory.
+      m_die_array.reserve(GetDebugInfoSize() / 24);
+      m_die_array.push_back(die);
+
+      if (!m_first_die)
+        AddUnitDIE(m_die_array.front());
     } else {
       if (null_die) {
         if (prev_die_had_children) {
@@ -145,14 +177,9 @@
     }
   }
 
-  // Give a little bit of info if we encounter corrupt DWARF (our offset should
-  // always terminate at or before the start of the next compilation unit
-  // header).
-  if (offset > next_cu_offset) {
-    m_dwarf->GetObjectFile()->GetModule()->ReportWarning(
-        "DWARF compile unit extends beyond its bounds cu 0x%8.8x at "
-        "0x%8.8" PRIx64 "\n",
-        GetOffset(), offset);
+  if (!m_die_array.empty()) {
+    lldbassert(!m_first_die || m_first_die == m_die_array.front());
+    m_first_die = m_die_array.front();
   }
 
   // Since std::vector objects will double their size, we really need to make a
@@ -164,6 +191,34 @@
                                                          m_die_array.end());
     exact_size_die_array.swap(m_die_array);
   }
+
+  ExtractDIEsEndCheck(offset);
+
+  if (!m_dwo_symbol_file)
+    return m_die_array.size();
+
+  DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
+  size_t dwo_die_count = dwo_cu->ExtractDIEsIfNeeded();
+  return m_die_array.size() + dwo_die_count -
+         1; // We have 2 CU die, but we want to count it only as one
+}
+
+//--------------------------------------------------------------------------
+// Final checks for both ExtractUnitDIEIfNeeded() and ExtractDIEsIfNeeded().
+//--------------------------------------------------------------------------
+void DWARFUnit::ExtractDIEsEndCheck(lldb::offset_t offset) const {
+  lldb::offset_t next_cu_offset = GetNextCompileUnitOffset();
+
+  // Give a little bit of info if we encounter corrupt DWARF (our offset should
+  // always terminate at or before the start of the next compilation unit
+  // header).
+  if (offset > GetNextCompileUnitOffset()) {
+    m_dwarf->GetObjectFile()->GetModule()->ReportWarning(
+        "DWARF compile unit extends beyond its bounds cu 0x%8.8x at "
+        "0x%8.8" PRIx64 "\n",
+        GetOffset(), offset);
+  }
+
   Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO));
   if (log && log->GetVerbose()) {
     StreamString strm;
@@ -174,30 +229,16 @@
       m_die_array[0].Dump(m_dwarf, this, strm, UINT32_MAX);
     log->PutString(strm.GetString());
   }
-
-  if (!m_dwo_symbol_file)
-    return m_die_array.size();
-
-  DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
-  size_t dwo_die_count = dwo_cu->ExtractDIEsIfNeeded(cu_die_only);
-  return m_die_array.size() + dwo_die_count -
-         1; // We have 2 CU die, but we want to count it only as one
 }
 
-void DWARFUnit::AddUnitDIE(DWARFDebugInfoEntry &die) {
-  assert(m_die_array.empty() && "Compile unit DIE already added");
-
-  // The average bytes per DIE entry has been seen to be around 14-20 so lets
-  // pre-reserve half of that since we are now stripping the NULL tags.
-  
-  // Only reserve the memory if we are adding children of the main compile unit
-  // DIE. The compile unit DIE is always the first entry, so if our size is 1,
-  // then we are adding the first compile unit child DIE and should reserve the
-  // memory.
-  m_die_array.reserve(GetDebugInfoSize() / 24);
-  m_die_array.push_back(die);
+void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry &cu_die) {
+  uint64_t base_addr = cu_die.GetAttributeValueAsAddress(
+      m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  if (base_addr == LLDB_INVALID_ADDRESS)
+    base_addr = cu_die.GetAttributeValueAsAddress(
+        m_dwarf, this, DW_AT_entry_pc, 0);
+  SetBaseAddress(base_addr);
 
-  const DWARFDebugInfoEntry &cu_die = m_die_array.front();
   std::unique_ptr<SymbolFileDWARFDwo> dwo_symbol_file =
       m_dwarf->GetDwoSymbolFileForCompileUnit(*this, cu_die);
   if (!dwo_symbol_file)
@@ -289,19 +330,8 @@
 }
 
 void DWARFUnit::ClearDIEs() {
-  if (m_die_array.size() > 1) {
-    // std::vectors never get any smaller when resized to a smaller size, or
-    // when clear() or erase() are called, the size will report that it is
-    // smaller, but the memory allocated remains intact (call capacity() to see
-    // this). So we need to create a temporary vector and swap the contents
-    // which will cause just the internal pointers to be swapped so that when
-    // "tmp_array" goes out of scope, it will destroy the contents.
-
-    // Save at least the compile unit DIE
-    DWARFDebugInfoEntry::collection tmp_array;
-    m_die_array.swap(tmp_array);
-    m_die_array.push_back(tmp_array.front());
-  }
+  m_die_array.clear();
+  m_die_array.shrink_to_fit();
 
   if (m_dwo_symbol_file)
     m_dwo_symbol_file->GetCompileUnit()->ClearDIEs();
@@ -342,7 +372,7 @@
   // 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.
-  const bool clear_dies = ExtractDIEsIfNeeded(false) > 1;
+  const bool clear_dies = ExtractDIEsIfNeeded();
 
   die = DIEPtr();
   if (die)
@@ -447,7 +477,7 @@
       return GetDwoSymbolFile()->GetCompileUnit()->GetDIE(die_offset);
 
     if (ContainsDIEOffset(die_offset)) {
-      ExtractDIEsIfNeeded(false);
+      ExtractDIEsIfNeeded();
       DWARFDebugInfoEntry::const_iterator end = m_die_array.cend();
       DWARFDebugInfoEntry::const_iterator pos =
           lower_bound(m_die_array.cbegin(), end, die_offset, CompareDIEOffset);
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -60,6 +60,10 @@
         m_empty_children(false), m_abbr_idx(0), m_has_children(false),
         m_tag(0) {}
 
+  explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
+  bool operator==(const DWARFDebugInfoEntry &rhs) const;
+  bool operator!=(const DWARFDebugInfoEntry &rhs) const;
+
   void BuildAddressRangeTable(SymbolFileDWARF *dwarf2Data,
                               const DWARFUnit *cu,
                               DWARFDebugAranges *debug_aranges) const;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1825,3 +1825,15 @@
                 die_ref.HasChildren() ? " *" : "");
   }
 }
+
+bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
+  return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
+         m_sibling_idx == rhs.m_sibling_idx &&
+         m_empty_children == rhs.m_empty_children &&
+         m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children &&
+         m_tag == rhs.m_tag;
+}
+
+bool DWARFDebugInfoEntry::operator!=(const DWARFDebugInfoEntry &rhs) const {
+  return !(*this == rhs);
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to