llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) <details> <summary>Changes</summary> DebugNamesDWARFIndex was jumping through hoops to construct a DIERef from an index entry only to jump through them back a short while later to construct a DWARFDIE. This used to be necessary as the index lookup was a two stage process, where we first enumerated all matches, and then examined them (so it was important that the enumeration was cheap -- does not trigger unnecessary parsing). However, now that the processing is callback based, we are always immediately examening the DWARFDIE right after finding the entry, and the DIERef just gets in the way. --- Full diff: https://github.com/llvm/llvm-project/pull/93296.diff 5 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp (+6-2) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (+1-6) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (+4-5) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp (+25-33) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h (+1-1) ``````````diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp index 33537df4f5076..1703597a7cd2f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp @@ -284,8 +284,12 @@ void AppleDWARFIndex::GetFunctions( for (const auto &entry : m_apple_names_up->equal_range(name)) { DIERef die_ref(std::nullopt, DIERef::Section::DebugInfo, *entry.getDIESectionOffset()); - if (!ProcessFunctionDIE(lookup_info, die_ref, dwarf, parent_decl_ctx, - callback)) + DWARFDIE die = dwarf.GetDIE(die_ref); + if (!die) { + ReportInvalidDIERef(die_ref, name); + continue; + } + if (!ProcessFunctionDIE(lookup_info, die, parent_decl_ctx, callback)) return; } } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp index 20c07a94b5076..30fb5d5ebdb0d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp @@ -24,16 +24,11 @@ using namespace lldb_private::plugin::dwarf; DWARFIndex::~DWARFIndex() = default; bool DWARFIndex::ProcessFunctionDIE( - const Module::LookupInfo &lookup_info, DIERef ref, SymbolFileDWARF &dwarf, + const Module::LookupInfo &lookup_info, DWARFDIE die, const CompilerDeclContext &parent_decl_ctx, llvm::function_ref<bool(DWARFDIE die)> callback) { llvm::StringRef name = lookup_info.GetLookupName().GetStringRef(); FunctionNameType name_type_mask = lookup_info.GetNameTypeMask(); - DWARFDIE die = dwarf.GetDIE(ref); - if (!die) { - ReportInvalidDIERef(ref, name); - return true; - } if (!(name_type_mask & eFunctionNameTypeFull)) { ConstString name_to_match_against; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h index 0551b07100a96..cb3ae8a06d788 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h @@ -81,11 +81,10 @@ class DWARFIndex { StatsDuration m_index_time; /// Helper function implementing common logic for processing function dies. If - /// the function given by "ref" matches search criteria given by - /// "parent_decl_ctx" and "name_type_mask", it is inserted into the "dies" - /// vector. - bool ProcessFunctionDIE(const Module::LookupInfo &lookup_info, DIERef ref, - SymbolFileDWARF &dwarf, + /// the function given by "die" matches search criteria given by + /// "parent_decl_ctx" and "name_type_mask", it calls the callback with the + /// given die. + bool ProcessFunctionDIE(const Module::LookupInfo &lookup_info, DWARFDIE die, const CompilerDeclContext &parent_decl_ctx, llvm::function_ref<bool(DWARFDIE die)> callback); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 79400e36e04f3..90e42be7202d8 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -64,27 +64,25 @@ DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const { return cu ? &cu->GetNonSkeletonUnit() : nullptr; } -std::optional<DIERef> -DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const { +DWARFDIE DebugNamesDWARFIndex::GetDIE(const DebugNames::Entry &entry) const { DWARFUnit *unit = GetNonSkeletonUnit(entry); - if (!unit) - return std::nullopt; - if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset()) - return DIERef(unit->GetSymbolFileDWARF().GetFileIndex(), - DIERef::Section::DebugInfo, unit->GetOffset() + *die_offset); - - return std::nullopt; + std::optional<uint64_t> die_offset = entry.getDIEUnitOffset(); + if (!unit || !die_offset) + return DWARFDIE(); + if (DWARFDIE die = unit->GetDIE(unit->GetOffset() + *die_offset)) + return die; + + m_module.ReportErrorIfModifyDetected( + "the DWARF debug information has been modified (bad offset {0:x} in " + "debug_names section)\n", + *die_offset); + return DWARFDIE(); } bool DebugNamesDWARFIndex::ProcessEntry( const DebugNames::Entry &entry, llvm::function_ref<bool(DWARFDIE die)> callback) { - std::optional<DIERef> ref = ToDIERef(entry); - if (!ref) - return true; - SymbolFileDWARF &dwarf = *llvm::cast<SymbolFileDWARF>( - m_module.GetSymbolFile()->GetBackingSymbolFile()); - DWARFDIE die = dwarf.GetDIE(*ref); + DWARFDIE die = GetDIE(entry); if (!die) return true; return callback(die); @@ -183,7 +181,7 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass( llvm::function_ref<bool(DWARFDIE die)> callback) { // Keep a list of incomplete types as fallback for when we don't find the // complete type. - DIEArray incomplete_types; + std::vector<DWARFDIE> incomplete_types; for (const DebugNames::Entry &entry : m_debug_names_up->equal_range(class_name.GetStringRef())) { @@ -191,19 +189,14 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass( entry.tag() != DW_TAG_class_type) continue; - std::optional<DIERef> ref = ToDIERef(entry); - if (!ref) - continue; - - DWARFUnit *cu = m_debug_info.GetUnit(*ref); - if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) { - incomplete_types.push_back(*ref); + DWARFDIE die = GetDIE(entry); + if (!die) { + // Report invalid continue; } - - DWARFDIE die = m_debug_info.GetDIE(*ref); - if (!die) { - ReportInvalidDIERef(*ref, class_name.GetStringRef()); + DWARFUnit *cu = die.GetCU(); + if (!cu->Supports_DW_AT_APPLE_objc_complete_type()) { + incomplete_types.push_back(die); continue; } @@ -212,12 +205,11 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass( callback(die); return; } - incomplete_types.push_back(*ref); + incomplete_types.push_back(die); } - auto dierefcallback = DIERefCallback(callback, class_name.GetStringRef()); - for (DIERef ref : incomplete_types) - if (!dierefcallback(ref)) + for (DWARFDIE die : incomplete_types) + if (!callback(die)) return; m_fallback.GetCompleteObjCClass(class_name, must_be_implementation, callback); @@ -379,8 +371,8 @@ void DebugNamesDWARFIndex::GetFunctions( if (tag != DW_TAG_subprogram && tag != DW_TAG_inlined_subroutine) continue; - if (std::optional<DIERef> ref = ToDIERef(entry)) { - if (!ProcessFunctionDIE(lookup_info, *ref, dwarf, parent_decl_ctx, + if (DWARFDIE die = GetDIE(entry)) { + if (!ProcessFunctionDIE(lookup_info, die, parent_decl_ctx, [&](DWARFDIE die) { if (!seen.insert(die.GetDIE()).second) return true; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h index 81fb8f88b805a..a27a414ecdd19 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -84,7 +84,7 @@ class DebugNamesDWARFIndex : public DWARFIndex { ManualDWARFIndex m_fallback; DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const; - std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const; + DWARFDIE GetDIE(const DebugNames::Entry &entry) const; bool ProcessEntry(const DebugNames::Entry &entry, llvm::function_ref<bool(DWARFDIE die)> callback); `````````` </details> https://github.com/llvm/llvm-project/pull/93296 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
