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

[labath]

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.

[JDevlieghere]

Seems straightforward.

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

65–66

This seems inverted?

[labath]

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

65–66

Actually, it is correct, although somewhat confusing -- the member is set to nullptr once we finished indexing. I think I can rephrase that comment to be a bit clearer..

[JDevlieghere]

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

65–66

I see, thanks. I think that would help.

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

 //===-- DebugNamesDWARFIndex.h ---------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DEBUGNAMESDWARFINDEX_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DEBUGNAMESDWARFINDEX_H
 
 #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"
 
 namespace lldb_private {
 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(); }
 
   void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
                           DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override {}
public:
   void ReportInvalidDIERef(const DIERef &ref, llvm::StringRef name) override {}
   void Dump(Stream &s) override;
 
 private:
   DebugNamesDWARFIndex(Module &module,
                        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;
 
   // LLVM DWARFDebugNames will hold a non-owning reference to this data, so keep
   // track of the ownership here.
   DWARFDataExtractor m_debug_names_data;
   DWARFDataExtractor m_debug_str_data;
 

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

 #include "lldb/Utility/Stream.h"
 
 using namespace lldb_private;
 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>
 DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   llvm::DenseSet<dw_offset_t> result;
   for (const DebugNames::NameIndex &ni : debug_names) {
     for (uint32_t cu = 0; cu < ni.getCUCount(); ++cu)
       result.insert(ni.getCUOffset(cu));

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

 //===-- ManualDWARFIndex.h --------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_MANUALDWARFINDEX_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_MANUALDWARFINDEX_H
 
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/NameToDIE.h"
 #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(); }
 
   void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
                           DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &unit, DIEArray &offsets) override;
struct IndexSet {
     NameToDIE function_methods;
     NameToDIE function_selectors;
     NameToDIE objc_class_selectors;
     NameToDIE globals;
     NameToDIE types;
     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;
 
   IndexSet m_set;
 };
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_MANUALDWARFINDEX_H

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

 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
 
 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;
 
   std::vector<IndexSet> sets(units_to_index.size());
 
   // Keep memory down by clearing DIEs for any units if indexing
   // caused us to load the unit's DIEs.
   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) {
     clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
   };
 
   // Create a task runner that extracts dies for each DWARF unit in a
   // separate thread
TaskPool::RunTasks([&]() { finalize_fn(&IndexSet::function_basenames); },
                      [&]() { finalize_fn(&IndexSet::function_methods); },
                      [&]() { finalize_fn(&IndexSet::function_selectors); },
                      [&]() { finalize_fn(&IndexSet::objc_class_selectors); },
                      [&]() { finalize_fn(&IndexSet::globals); },
                      [&]() { finalize_fn(&IndexSet::types); },
                      [&]() { 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) {
     m_module.LogMessage(
         log, "ManualDWARFIndex::IndexUnit for unit at .debug_info[0x%8.8x]",
         unit.GetOffset());
   }
 
   const LanguageType cu_language = SymbolFileDWARF::GetLanguage(unit);
 
   IndexUnitImpl(unit, cu_language, set);
 
   if (SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile()) {
+    // 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);
     }
   }
+}
 
 void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
                                      const LanguageType cu_language,
                                      IndexSet &set) {
   for (const DWARFDebugInfoEntry &die : unit.dies()) {
     const dw_tag_t tag = die.Tag();
 
     switch (tag) {

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

public:
   ParseCallEdgesInFunction(UserID func_id) override;
 
   void Dump(lldb_private::Stream &s) override;
 
   void DumpClangAST(lldb_private::Stream &s) override;
 
   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 &>
   GetTypeSystem(DWARFUnit &unit);
 
   static DWARFASTParser *GetDWARFParser(DWARFUnit &unit);
 
   // CompilerDecl related functions

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

if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
 
     if (m_index)
       return;
 
     DWARFDataExtractor debug_names;
     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;
       }
       LLDB_LOG_ERROR(log, index_or.takeError(),
                      "Unable to read .debug_names data: {0}");
     }
   }
 
-  m_index = std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(),
-                                                DebugInfo());
+  m_index =
+      std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(), *this);
 }
 
 bool SymbolFileDWARF::SupportedVersion(uint16_t version) {
   return version >= 2 && version <= 5;
 }
 
 uint32_t SymbolFileDWARF::CalculateAbilities() {
   uint32_t abilities = 0;
SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   // Only compile units can be split into two parts.
   if (!dwarf_cu)
     return nullptr;
 
   const char *dwo_name = GetDWOName(*dwarf_cu, cu_die);
   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);
   if (dwo_file.IsRelative()) {
     const char *comp_dir =
         cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_comp_dir, nullptr);
     if (!comp_dir)
       return nullptr;
if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
     if (module_sp) {
       m_debug_map_symfile =
           (SymbolFileDWARFDebugMap *)module_sp->GetSymbolFile();
     }
   }
   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();
     module_spec.GetSymbolFileSpec() =
         FileSpec(m_objfile_sp->GetFileSpec().GetPath() + ".dwp");
 
     FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
     FileSpec dwp_filespec =
         Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
     if (FileSystem::Instance().Exists(dwp_filespec)) {
       DataBufferSP dwp_file_data_sp;
       lldb::offset_t dwp_file_data_offset = 0;
       ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
           GetObjectFile()->GetModule(), &dwp_filespec, 0,
           FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
           dwp_file_data_offset);
       if (!dwp_obj_file)
         return;
       m_dwp_symfile =
           std::make_shared<SymbolFileDWARFDwo>(*this, dwp_obj_file, 0x3fffffff);
     }
   });
+  return m_dwp_symfile;
 }
 
 llvm::Expected<TypeSystem &> SymbolFileDWARF::GetTypeSystem(DWARFUnit &unit) {
   return unit.GetSymbolFileDWARF().GetTypeSystemForLanguage(GetLanguage(unit));
 }
 
 DWARFASTParser *SymbolFileDWARF::GetDWARFParser(DWARFUnit &unit) {
   auto type_system_or_err = GetTypeSystem(unit);

lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s

 # REQUIRES: x86
 
 # 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 {
 # CHECK-NEXT:   case0
 # CHECK-NEXT: }
 
 # CHECK-LABEL: target variable A
 # 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,"",@progbits
         .byte   1                       # Abbreviation Code
         .byte   17                      # DW_TAG_compile_unit
         .byte   0                       # DW_CHILDREN_no
         .ascii  "\260B"                 # DW_AT_GNU_dwo_name
         .byte   8                       # DW_FORM_string
         .ascii  "\261B"                 # DW_AT_GNU_dwo_id