[lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit

[labath]

Summary

This is the second dwp preparatory patch. When a SymbolFileDWARFDwo will
hold more than one split unit, it will not be able to be uniquely owned
by a single DWARFUnit. I achieve this by changing the
unique_ptr<SymbolFileDWARFDwo> member of DWARFUnit to
shared_ptr<DWARFUnit>. The shared_ptr points to a DWARFUnit, but it is
in fact holding the entire SymbolFileDWARFDwo alive. This is the same
method used by llvm DWARFUnit (except that is uses the DWARFContext
class).

[clayborg]

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

56

How often does this function get called? Should we cache the DW_AT_GNU_dwo_id in the DWARFCompileUnit to avoid extracting the DW_AT_GNU_dwo_id attribute maybe multiple times?

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

67

Curious: what is a single compile unit? A bit of a comment in header doc here might be nice.

[labath]

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

56

In a dwo file, this function will be called exactly once. With a DWP file (the next patch) it will get called once for each compile unit, but it will return a different compile unit each time, so for a single compile unit, the dwo_id attribute will still be accessed only once.

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

67

I've added a simple comment. The idea is that a dwo file will normally(*) contain exactly one compile unit. This function will find it and return it.

(*) This may not be 100% as I believe llvm can produce multiple CUs in a dwo file under some circumstances, but I'm not sure if this is fully conforming, and it is definitely not something that works in lldb right now.

[labath]

• clang-format
• add some doxygen

[labath]

Do you have any more comments on this patch, Greg?

[clayborg]

LGTM, sorry for the delay!

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

protected:
   const DWARFDebugInfoEntry *DIEPtr() {
     ExtractDIEsIfNeeded();
     if (m_die_array.empty())
       return NULL;
     return &m_die_array[0];
   }
 
   SymbolFileDWARF &m_dwarf;
-  std::unique_ptr<SymbolFileDWARFDwo> m_dwo_symbol_file;
+  std::shared_ptr<DWARFUnit> m_dwo;
   DWARFUnitHeader m_header;
   const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;
   mutable llvm::sys::RWMutex m_die_array_mutex;
   // It is used for tracking of ScopedExtractDIEs instances.
   mutable llvm::sys::RWMutex m_die_array_scoped_mutex;

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

if (depth == 0) {
       if (!m_first_die)
         AddUnitDIE(m_die_array.front());
 
       // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton compile
       // units. We are not able to access these DIE *and* the dwo file
       // simultaneously. We also don't need to do that as the dwo file will
       // contain a superset of information. So, we don't even attempt to parse
       // any remaining DIEs.
-      if (m_dwo_symbol_file) {
+      if (m_dwo) {
         m_die_array.front().SetHasChildren(false);
         break;
       }
 
     } else {
       if (null_die) {
         if (prev_die_had_children) {
           // This will only happen if a DIE says is has children but all it
if (m_first_die) {
       m_first_die.SetHasChildren(m_die_array.front().HasChildren());
       lldbassert(m_first_die == m_die_array.front());
     }
     m_first_die = m_die_array.front();
   }
 
   m_die_array.shrink_to_fit();
 
-  if (m_dwo_symbol_file) {
-    DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
-    dwo_cu->ExtractDIEsIfNeeded();
-  }
+  if (m_dwo)
+    m_dwo->ExtractDIEsIfNeeded();
 }
 
 // This is used when a split dwarf is enabled.
 // A skeleton compilation unit may contain the DW_AT_str_offsets_base attribute
 // that points to the first string offset of the CU contribution to the
 // .debug_str_offsets. At the same time, the corresponding split debug unit also
 // may use DW_FORM_strx* forms pointing to its own .debug_str_offsets.dwo and
 // for that case, we should find the offset (skip the section header).
case DW_AT_GNU_ranges_base:
       gnu_ranges_base = form_value.Unsigned();
       break;
     }
   }
 
   if (m_is_dwo)
     return;
 
-  std::unique_ptr<SymbolFileDWARFDwo> dwo_symbol_file =
+  std::shared_ptr<SymbolFileDWARFDwo> dwo_symbol_file =
       m_dwarf.GetDwoSymbolFileForCompileUnit(*this, cu_die);
   if (!dwo_symbol_file)
     return;
 
-  DWARFUnit *dwo_cu = dwo_symbol_file->GetCompileUnit();
+  uint64_t main_dwo_id =
+      cu_die.GetAttributeValueAsUnsigned(this, DW_AT_GNU_dwo_id, 0);
+  DWARFUnit *dwo_cu = dwo_symbol_file->GetDWOCompileUnitForHash(main_dwo_id);
   if (!dwo_cu)
     return; // Can't fetch the compile unit from the dwo file.
   dwo_cu->SetUserData(this);
 
   DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
   if (!dwo_cu_die.IsValid())
     return; // Can't fetch the compile unit DIE from the dwo file.
 
-  uint64_t main_dwo_id =
-      cu_die.GetAttributeValueAsUnsigned(this, DW_AT_GNU_dwo_id, 0);
-  uint64_t sub_dwo_id =
-      dwo_cu_die.GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0);
-  if (main_dwo_id != sub_dwo_id)
-    return; // The 2 dwo ID isn't match. Don't use the dwo file as it belongs to
-  // a differectn compilation.
-
-  m_dwo_symbol_file = std::move(dwo_symbol_file);
-
   // Here for DWO CU we want to use the address base set in the skeleton unit
   // (DW_AT_addr_base) if it is available and use the DW_AT_GNU_addr_base
   // otherwise. We do that because pre-DWARF v5 could use the DW_AT_GNU_*
   // attributes which were applicable to the DWO units. The corresponding
   // DW_AT_* attributes standardized in DWARF v5 are also applicable to the main
   // unit in contrast.
   if (addr_base)
     dwo_cu->SetAddrBase(*addr_base);
   else if (gnu_addr_base)
     dwo_cu->SetAddrBase(*gnu_addr_base);
 
   if (GetVersion() <= 4 && gnu_ranges_base)
     dwo_cu->SetRangesBase(*gnu_ranges_base);
-  else if (m_dwo_symbol_file->GetDWARFContext()
+  else if (dwo_symbol_file->GetDWARFContext()
                .getOrLoadRngListsData()
                .GetByteSize() > 0)
     dwo_cu->SetRangesBase(llvm::DWARFListTableHeader::getHeaderSize(DWARF32));
 
-  if (GetVersion() >= 5 && m_dwo_symbol_file->GetDWARFContext()
-                                   .getOrLoadLocListsData()
-                                   .GetByteSize() > 0)
+  if (GetVersion() >= 5 &&
+      dwo_symbol_file->GetDWARFContext().getOrLoadLocListsData().GetByteSize() >
+          0)
     dwo_cu->SetLoclistsBase(llvm::DWARFListTableHeader::getHeaderSize(DWARF32));
   dwo_cu->SetBaseAddress(GetBaseAddress());
 
-  for (size_t i = 0; i < m_dwo_symbol_file->DebugInfo()->GetNumUnits(); ++i) {
-    DWARFUnit *unit = m_dwo_symbol_file->DebugInfo()->GetUnitAtIndex(i);
+  for (size_t i = 0; i < dwo_symbol_file->DebugInfo()->GetNumUnits(); ++i) {
+    DWARFUnit *unit = dwo_symbol_file->DebugInfo()->GetUnitAtIndex(i);
     SetDwoStrOffsetsBase(unit);
   }
+  m_dwo = std::shared_ptr<DWARFUnit>(std::move(dwo_symbol_file), dwo_cu);
 }
 
 DWARFDIE DWARFUnit::LookupAddress(const dw_addr_t address) {
   if (DIE()) {
     const DWARFDebugAranges &func_aranges = GetFunctionAranges();
 
     // Re-check the aranges auto pointer contents in case it was created above
     if (!func_aranges.IsEmpty())
void DWARFUnit::SetStrOffsetsBase(dw_offset_t str_offsets_base) {
   m_str_offsets_base = str_offsets_base;
 }
 
 // It may be called only with m_die_array_mutex held R/W.
 void DWARFUnit::ClearDIEsRWLocked() {
   m_die_array.clear();
   m_die_array.shrink_to_fit();
 
-  if (m_dwo_symbol_file)
-    m_dwo_symbol_file->GetCompileUnit()->ClearDIEsRWLocked();
+  if (m_dwo)
+    m_dwo->ClearDIEsRWLocked();
 }
 
 lldb::ByteOrder DWARFUnit::GetByteOrder() const {
   return m_dwarf.GetObjectFile()->GetByteOrder();
 }
 
 void DWARFUnit::SetBaseAddress(dw_addr_t base_addr) { m_base_addr = base_addr; }
 
if (ContainsDIEOffset(die_offset)) {
       GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
           "GetDIE for DIE 0x%" PRIx32 " is outside of its CU 0x%" PRIx32,
           die_offset, GetOffset());
   }
   return DWARFDIE(); // Not found
 }
 
 DWARFUnit &DWARFUnit::GetNonSkeletonUnit() {
-  if (SymbolFileDWARFDwo *dwo = GetDwoSymbolFile())
-    return *dwo->GetCompileUnit();
+  ExtractUnitDIEIfNeeded();
+  if (m_dwo)
+    return *m_dwo;
   return *this;
 }
 
 uint8_t DWARFUnit::GetAddressByteSize(const DWARFUnit *cu) {
   if (cu)
     return cu->GetAddressByteSize();
   return DWARFUnit::GetDefaultAddressSize();
 }
m_file_spec =
                GetPathStyle());
 
   if (m_file_spec->IsRelative())
     m_file_spec->MakeAbsolute(GetCompilationDirectory());
 }
 
 SymbolFileDWARFDwo *DWARFUnit::GetDwoSymbolFile() {
   ExtractUnitDIEIfNeeded();
-  return m_dwo_symbol_file.get();
+  if (m_dwo)
+    return &llvm::cast<SymbolFileDWARFDwo>(m_dwo->GetSymbolFileDWARF());
+  return nullptr;
 }
 
 const DWARFDebugAranges &DWARFUnit::GetFunctionAranges() {
   if (m_func_aranges_up == nullptr) {
     m_func_aranges_up.reset(new DWARFDebugAranges());
     const DWARFDebugInfoEntry *die = DIEPtr();
     if (die)
       die->BuildFunctionAddressRangeTable(this, m_func_aranges_up.get());
 
-    if (m_dwo_symbol_file) {
-      DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
-      const DWARFDebugInfoEntry *dwo_die = dwo_cu->DIEPtr();
+    if (m_dwo) {
+      const DWARFDebugInfoEntry *dwo_die = m_dwo->DIEPtr();
       if (dwo_die)
-        dwo_die->BuildFunctionAddressRangeTable(dwo_cu,
+        dwo_die->BuildFunctionAddressRangeTable(m_dwo.get(),
                                                 m_func_aranges_up.get());
     }
 
     const bool minimize = false;
     m_func_aranges_up->Sort(minimize);
   }
   return *m_func_aranges_up;
 }

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

public:
   }
 
   lldb::user_id_t GetUID(const llvm::Optional<DIERef> &ref) {
     return ref ? GetUID(*ref) : LLDB_INVALID_UID;
   }
 
   lldb::user_id_t GetUID(DIERef ref);
 
-  std::unique_ptr<SymbolFileDWARFDwo>
+  std::shared_ptr<SymbolFileDWARFDwo>
   GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
                                  const DWARFDebugInfoEntry &cu_die);
 
   virtual llvm::Optional<uint32_t> GetDwoNum() { return llvm::None; }
 
   /// If this is a DWARF object with a single CU, return its DW_AT_dwo_id.
   llvm::Optional<uint64_t> GetDWOId();
 

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

if (auto comp_unit = GetCompileUnitAtIndex(0))
               GetDWARFCompileUnit(comp_unit.get())))
         if (DWARFDebugInfoEntry *cu_die = cu->DIE().GetDIE())
           if (uint64_t dwo_id = ::GetDWOId(*cu, *cu_die))
             return dwo_id;
   }
   return {};
 }
 
-std::unique_ptr<SymbolFileDWARFDwo>
+std::shared_ptr<SymbolFileDWARFDwo>
 SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
     DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
   // If this is a Darwin-style debug map (non-.dSYM) symbol file,
   // never attempt to load ELF-style DWO files since the -gmodules
   // support uses the same DWO machanism to specify full debug info
   // files for modules. This is handled in
   // UpdateExternalModuleListIfNeeded().
   if (GetDebugMapSymfile())
SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   lldb::offset_t dwo_file_data_offset = 0;
   ObjectFileSP dwo_obj_file = ObjectFile::FindPlugin(
       GetObjectFile()->GetModule(), &dwo_file, file_offset,
       FileSystem::Instance().GetByteSize(dwo_file), dwo_file_data_sp,
       dwo_file_data_offset);
   if (dwo_obj_file == nullptr)
     return nullptr;
 
-  return std::make_unique<SymbolFileDWARFDwo>(*this, dwo_obj_file,
+  return std::make_shared<SymbolFileDWARFDwo>(*this, dwo_obj_file,
                                               dwarf_cu->GetID());
 }
 
 void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
   if (m_fetched_external_modules)
     return;
   m_fetched_external_modules = true;
   DWARFDebugInfo *debug_info = DebugInfo();

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

public:
   static bool classof(const SymbolFile *obj) { return obj->isA(&ID); }
   /// \}
 
   SymbolFileDWARFDwo(SymbolFileDWARF &m_base_symbol_file,
                      lldb::ObjectFileSP objfile, uint32_t id);
 
   ~SymbolFileDWARFDwo() override = default;
 
-  DWARFCompileUnit *GetCompileUnit();
-
-  DWARFUnit *
-  GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
+  DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
   size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
                                  DIEArray &method_die_offsets) override;
 
   llvm::Expected<lldb_private::TypeSystem &>
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
   DWARFDIE
protected:
   lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(
       const DWARFDeclContext &die_decl_ctx) override;
 
   lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
       const DWARFDIE &die, lldb_private::ConstString type_name,
       bool must_be_implementation) override;
 
   SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
 
-  DWARFCompileUnit *ComputeCompileUnit();
+  /// If this file contains exactly one compile unit, this function will return
+  /// it. Otherwise it returns nullptr.
+  DWARFCompileUnit *FindSingleCompileUnit();
 
   SymbolFileDWARF &m_base_symbol_file;
-  DWARFCompileUnit *m_cu = nullptr;
 };
 
 #endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_

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

if (section_sp) {
 
       data.Clear();
     }
   }
 
   SymbolFileDWARF::LoadSectionData(sect_type, data);
 }
 
-DWARFCompileUnit *SymbolFileDWARFDwo::GetCompileUnit() {
-  if (!m_cu)
-    m_cu = ComputeCompileUnit();
-  return m_cu;
+DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
+  DWARFCompileUnit *cu = FindSingleCompileUnit();
+  if (!cu)
+    return nullptr;
+  if (hash !=
+      cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+    return nullptr;
+  return cu;
 }
 
-DWARFCompileUnit *SymbolFileDWARFDwo::ComputeCompileUnit() {
+DWARFCompileUnit *SymbolFileDWARFDwo::FindSingleCompileUnit() {
   DWARFDebugInfo *debug_info = DebugInfo();
   if (!debug_info)
     return nullptr;
 
   // Right now we only support dwo files with one compile unit. If we don't have
   // type units, we can just check for the unit count.
   if (!debug_info->ContainsTypeUnits() && debug_info->GetNumUnits() == 1)
     return llvm::cast<DWARFCompileUnit>(debug_info->GetUnitAtIndex(0));
 
   // Otherwise, we have to run through all units, and find the compile unit that
   // way.
   DWARFCompileUnit *cu = nullptr;
   for (size_t i = 0; i < debug_info->GetNumUnits(); ++i) {
     if (auto *candidate =
             llvm::dyn_cast<DWARFCompileUnit>(debug_info->GetUnitAtIndex(i))) {
       if (cu)
         return nullptr; // More that one CU found.
       cu = candidate;
     }
   }
   return cu;
 }
 
-DWARFUnit *
-SymbolFileDWARFDwo::GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) {
-  return GetCompileUnit();
-}
-
 SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() {
   return GetBaseSymbolFile().GetDIEToType();
 }
 
 SymbolFileDWARF::DIEToVariableSP &SymbolFileDWARFDwo::GetDIEToVariable() {
   return GetBaseSymbolFile().GetDIEToVariable();
 }