[Lldb-commits] [lldb] 0f7cfb2 - [lldb/DWARF] Don't index dwp file multiple times

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Sun Feb 23 23:51:29 PST 2020


Author: Pavel Labath
Date: 2020-02-24T08:50:51+01:00
New Revision: 0f7cfb25432e405aeb12251598a9798662979ced

URL: https://github.com/llvm/llvm-project/commit/0f7cfb25432e405aeb12251598a9798662979ced
DIFF: https://github.com/llvm/llvm-project/commit/0f7cfb25432e405aeb12251598a9798662979ced.diff

LOG: [lldb/DWARF] Don't index dwp file multiple times

Summary:
When we added support for type units in dwo files, we changed the
"manual" dwarf index to index _all_ dwarf units in the dwo file instead
of just the split unit belonging to our skeleton unit. This was fine for
dwo files, as they contain only a single compile units and type units do
not have a split type unit which would point to them.

However, this does not work for dwp files because, these files do
contain multiple split compile units, and the current approach means
that each unit gets indexed multiple times (once for each split unit =>
n^2 complexity).

This patch teaches the manual dwarf index to treat dwp files specially.
Any type units in the dwp file added to the main list of compile units
and indexed with them in a single batch. Split compile units in dwp
files are still indexed as a part of their skeleton unit -- this is done
because we need the DW_AT_language attribute from the skeleton unit to
index them properly.

Handling of dwo files remains unchanged -- all units (type and skeleton)
are indexed when we reach the dwo file through the split unit.

Reviewers: clayborg, JDevlieghere, aprantl

Subscribers: arphaman, lldb-commits

Tags: #lldb

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

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
    lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
    lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
    lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index c11df30696f0..5a2fa70138dd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -19,14 +19,14 @@ using namespace lldb;
 llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>>
 DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
                              DWARFDataExtractor debug_str,
-                             DWARFDebugInfo &debug_info) {
+                             SymbolFileDWARF &dwarf) {
   auto index_up = std::make_unique<DebugNames>(debug_names.GetAsLLVM(),
                                                 debug_str.GetAsLLVM());
   if (llvm::Error E = index_up->extract())
     return std::move(E);
 
   return std::unique_ptr<DebugNamesDWARFIndex>(new DebugNamesDWARFIndex(
-      module, std::move(index_up), debug_names, debug_str, debug_info));
+      module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
 llvm::DenseSet<dw_offset_t>

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 5ba8577d59f4..00f51e7e4128 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -12,6 +12,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Utility/ConstString.h"
 #include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 
@@ -20,7 +21,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
 public:
   static llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>>
   Create(Module &module, DWARFDataExtractor debug_names,
-         DWARFDataExtractor debug_str, DWARFDebugInfo &debug_info);
+         DWARFDataExtractor debug_str, SymbolFileDWARF &dwarf);
 
   void Preload() override { m_fallback.Preload(); }
 
@@ -49,11 +50,11 @@ class DebugNamesDWARFIndex : public DWARFIndex {
                        std::unique_ptr<llvm::DWARFDebugNames> debug_names_up,
                        DWARFDataExtractor debug_names_data,
                        DWARFDataExtractor debug_str_data,
-                       DWARFDebugInfo &debug_info)
-      : DWARFIndex(module), m_debug_info(debug_info),
+                       SymbolFileDWARF &dwarf)
+      : DWARFIndex(module), m_debug_info(dwarf.DebugInfo()),
         m_debug_names_data(debug_names_data), m_debug_str_data(debug_str_data),
         m_debug_names_up(std::move(debug_names_up)),
-        m_fallback(module, debug_info, GetUnits(*m_debug_names_up)) {}
+        m_fallback(module, dwarf, GetUnits(*m_debug_names_up)) {}
 
   DWARFDebugInfo &m_debug_info;
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 502bec3bf0f7..cdeb30964987 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -22,22 +22,38 @@ using namespace lldb_private;
 using namespace lldb;
 
 void ManualDWARFIndex::Index() {
-  if (!m_debug_info)
+  if (!m_dwarf)
     return;
 
-  DWARFDebugInfo &debug_info = *m_debug_info;
-  m_debug_info = nullptr;
+  SymbolFileDWARF &main_dwarf = *m_dwarf;
+  m_dwarf = nullptr;
 
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
-  Timer scoped_timer(func_cat, "%p", static_cast<void *>(&debug_info));
+  Timer scoped_timer(func_cat, "%p", static_cast<void *>(&main_dwarf));
+
+  DWARFDebugInfo &main_info = main_dwarf.DebugInfo();
+  SymbolFileDWARFDwo *dwp_dwarf = main_dwarf.GetDwpSymbolFile().get();
+  DWARFDebugInfo *dwp_info = dwp_dwarf ? &dwp_dwarf->DebugInfo() : nullptr;
 
   std::vector<DWARFUnit *> units_to_index;
-  units_to_index.reserve(debug_info.GetNumUnits());
-  for (size_t U = 0; U < debug_info.GetNumUnits(); ++U) {
-    DWARFUnit *unit = debug_info.GetUnitAtIndex(U);
+  units_to_index.reserve(main_info.GetNumUnits() +
+                         (dwp_info ? dwp_info->GetNumUnits() : 0));
+
+  // Process all units in the main file, as well as any type units in the dwp
+  // file. Type units in dwo files are handled when we reach the dwo file in
+  // IndexUnit.
+  for (size_t U = 0; U < main_info.GetNumUnits(); ++U) {
+    DWARFUnit *unit = main_info.GetUnitAtIndex(U);
     if (unit && m_units_to_avoid.count(unit->GetOffset()) == 0)
       units_to_index.push_back(unit);
   }
+  if (dwp_info && dwp_info->ContainsTypeUnits()) {
+    for (size_t U = 0; U < dwp_info->GetNumUnits(); ++U) {
+      if (auto *tu = llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U)))
+        units_to_index.push_back(tu);
+    }
+  }
+
   if (units_to_index.empty())
     return;
 
@@ -48,7 +64,7 @@ void ManualDWARFIndex::Index() {
   std::vector<llvm::Optional<DWARFUnit::ScopedExtractDIEs>> clear_cu_dies(
       units_to_index.size());
   auto parser_fn = [&](size_t cu_idx) {
-    IndexUnit(*units_to_index[cu_idx], sets[cu_idx]);
+    IndexUnit(*units_to_index[cu_idx], dwp_dwarf, sets[cu_idx]);
   };
 
   auto extract_fn = [&units_to_index, &clear_cu_dies](size_t cu_idx) {
@@ -87,11 +103,8 @@ void ManualDWARFIndex::Index() {
                      [&]() { finalize_fn(&IndexSet::namespaces); });
 }
 
-void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, IndexSet &set) {
-  assert(
-      !unit.IsDWOUnit() &&
-      "DWARFUnit associated with .dwo or .dwp should not be indexed directly");
-
+void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp,
+                                 IndexSet &set) {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_LOOKUPS);
 
   if (log) {
@@ -105,9 +118,16 @@ void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, IndexSet &set) {
   IndexUnitImpl(unit, cu_language, set);
 
   if (SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile()) {
-    DWARFDebugInfo &dwo_info = dwo_symbol_file->DebugInfo();
-    for (size_t i = 0; i < dwo_info.GetNumUnits(); ++i)
-      IndexUnitImpl(*dwo_info.GetUnitAtIndex(i), cu_language, set);
+    // Type units in a dwp file are indexed separately, so we just need to
+    // process the split unit here. However, if the split unit is in a dwo file,
+    // then we need to process type units here.
+    if (dwo_symbol_file == dwp) {
+      IndexUnitImpl(unit.GetNonSkeletonUnit(), cu_language, set);
+    } else {
+      DWARFDebugInfo &dwo_info = dwo_symbol_file->DebugInfo();
+      for (size_t i = 0; i < dwo_info.GetNumUnits(); ++i)
+        IndexUnitImpl(*dwo_info.GetUnitAtIndex(i), cu_language, set);
+    }
   }
 }
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index 0dd27068bbfb..68eda58221d8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -14,13 +14,14 @@
 #include "llvm/ADT/DenseSet.h"
 
 class DWARFDebugInfo;
+class SymbolFileDWARFDwo;
 
 namespace lldb_private {
 class ManualDWARFIndex : public DWARFIndex {
 public:
-  ManualDWARFIndex(Module &module, DWARFDebugInfo &debug_info,
+  ManualDWARFIndex(Module &module, SymbolFileDWARF &dwarf,
                    llvm::DenseSet<dw_offset_t> units_to_avoid = {})
-      : DWARFIndex(module), m_debug_info(&debug_info),
+      : DWARFIndex(module), m_dwarf(&dwarf),
         m_units_to_avoid(std::move(units_to_avoid)) {}
 
   void Preload() override { Index(); }
@@ -56,14 +57,15 @@ class ManualDWARFIndex : public DWARFIndex {
     NameToDIE namespaces;
   };
   void Index();
-  void IndexUnit(DWARFUnit &unit, IndexSet &set);
+  void IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp, IndexSet &set);
 
   static void IndexUnitImpl(DWARFUnit &unit,
                             const lldb::LanguageType cu_language,
                             IndexSet &set);
 
-  /// Non-null value means we haven't built the index yet.
-  DWARFDebugInfo *m_debug_info;
+  /// The DWARF file which we are indexing. Set to nullptr after the index is
+  /// built.
+  SymbolFileDWARF *m_dwarf;
   /// Which dwarf units should we skip while building the index.
   llvm::DenseSet<dw_offset_t> m_units_to_avoid;
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 44aeff92922d..df1290c91782 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -458,9 +458,9 @@ void SymbolFileDWARF::InitializeObject() {
     LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
     if (debug_names.GetByteSize() > 0) {
       llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>> index_or =
-          DebugNamesDWARFIndex::Create(
-              *GetObjectFile()->GetModule(), debug_names,
-              m_context.getOrLoadStrData(), DebugInfo());
+          DebugNamesDWARFIndex::Create(*GetObjectFile()->GetModule(),
+                                       debug_names,
+                                       m_context.getOrLoadStrData(), *this);
       if (index_or) {
         m_index = std::move(*index_or);
         return;
@@ -470,8 +470,8 @@ void SymbolFileDWARF::InitializeObject() {
     }
   }
 
-  m_index = std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(),
-                                                DebugInfo());
+  m_index =
+      std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(), *this);
 }
 
 bool SymbolFileDWARF::SupportedVersion(uint16_t version) {
@@ -1555,9 +1555,8 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   if (!dwo_name)
     return nullptr;
 
-  FindDwpSymbolFile();
-  if (m_dwp_symfile)
-    return m_dwp_symfile;
+  if (std::shared_ptr<SymbolFileDWARFDwo> dwp_sp = GetDwpSymbolFile())
+    return dwp_sp;
 
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
@@ -3876,7 +3875,7 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
   return m_debug_map_symfile;
 }
 
-void SymbolFileDWARF::FindDwpSymbolFile() {
+const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
   llvm::call_once(m_dwp_symfile_once_flag, [this]() {
     ModuleSpec module_spec;
     module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
@@ -3899,6 +3898,7 @@ void SymbolFileDWARF::FindDwpSymbolFile() {
           std::make_shared<SymbolFileDWARFDwo>(*this, dwp_obj_file, 0x3fffffff);
     }
   });
+  return m_dwp_symfile;
 }
 
 llvm::Expected<TypeSystem &> SymbolFileDWARF::GetTypeSystem(DWARFUnit &unit) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index f05d455eb8d9..a3928c8c3dd4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -292,6 +292,8 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
 
   lldb_private::DWARFContext &GetDWARFContext() { return m_context; }
 
+  const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();
+
   lldb_private::FileSpec GetFile(DWARFUnit &unit, size_t file_idx);
 
   static llvm::Expected<lldb_private::TypeSystem &>

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s b/lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s
index 8680d27a3b42..8c6cb5421b4e 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s
@@ -3,6 +3,7 @@
 # RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t --defsym MAIN=0
 # RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.dwp --defsym DWP=0
 # RUN: %lldb %t -o "type lookup ENUM0" -o "target variable A" -b | FileCheck %s
+# RUN: lldb-test symbols %t | FileCheck %s --check-prefix=SYMBOLS
 
 # CHECK-LABEL: type lookup ENUM0
 # CHECK-NEXT: enum ENUM0 {
@@ -13,6 +14,19 @@
 # CHECK: (ENUM0) A = case0
 # CHECK: (ENUM1) A = case0
 
+# Make sure each entity is present in the index only once.
+# SYMBOLS:      Globals and statics:
+# SYMBOLS-NEXT: 3fffffff/INFO/00000023 "A"
+# SYMBOLS-NEXT: 3fffffff/INFO/0000005a "A"
+# SYMBOLS-EMPTY:
+
+# SYMBOLS: Types:
+# SYMBOLS-NEXT: 3fffffff/TYPE/00000018 "ENUM0"
+# SYMBOLS-NEXT: 3fffffff/TYPE/0000002d "int"
+# SYMBOLS-NEXT: 3fffffff/TYPE/00000062 "int"
+# SYMBOLS-NEXT: 3fffffff/TYPE/0000004d "ENUM1"
+# SYMBOLS-EMPTY:
+
 .ifdef MAIN
         .section        .debug_abbrev,"", at progbits
         .byte   1                       # Abbreviation Code


        


More information about the lldb-commits mailing list