https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/108907
>From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 19 Aug 2024 10:57:35 -0700 Subject: [PATCH 1/5] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap --- lldb/tools/lldb-dap/DAP.h | 2 -- lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 57562a14983519..7828272aa15a7d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -192,8 +192,6 @@ struct DAP { std::mutex call_mutex; std::map<int /* request_seq */, ResponseCallback /* reply handler */> inflight_reverse_requests; - StartDebuggingRequestHandler start_debugging_request_handler; - ReplModeRequestHandler repl_mode_request_handler; ReplMode repl_mode; std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index ea84f31aec3a6c..f50a6c17310739 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object &request) { "lldb-dap", "Commands for managing lldb-dap."); if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) { cmd.AddCommand( - "startDebugging", &g_dap.start_debugging_request_handler, + "startDebugging", new StartDebuggingRequestHandler(), "Sends a startDebugging request from the debug adapter to the client " "to start a child debug session of the same type as the caller."); } cmd.AddCommand( - "repl-mode", &g_dap.repl_mode_request_handler, + "repl-mode", new ReplModeRequestHandler(), "Get or set the repl behavior of lldb-dap evaluation requests."); g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction); >From 30e8b1870004ae6ca9319680f6ef16e173415026 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 16 Sep 2024 12:02:48 -0700 Subject: [PATCH 2/5] Improve type/namespace lookup using parent chain --- .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 59 ++++++ .../Plugins/SymbolFile/DWARF/DWARFIndex.h | 22 ++ .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 168 +++++++++++++-- .../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 29 +++ .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 192 +++++++++++------- .../SymbolFile/DWARF/SymbolFileDWARF.h | 2 + 6 files changed, 385 insertions(+), 87 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp index eafddbad03f57b..7bdea4eae4529b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp @@ -126,3 +126,62 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl( return callback(die); return true; } + +void DWARFIndex::GetNamespacesWithParents( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) { + GetNamespaces(name, [&](DWARFDIE die) { + return ProcessDieMatchParentNames(name, parent_names, die, callback); + }); +} + +void DWARFIndex::GetTypesWithParents( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) { + GetTypes(name, [&](DWARFDIE die) { + return ProcessDieMatchParentNames(name, parent_names, die, callback); + }); +} + +bool DWARFIndex::ProcessDieMatchParentNames( + ConstString name, llvm::ArrayRef<llvm::StringRef> query_parent_names, + DWARFDIE die, llvm::function_ref<bool(DWARFDIE die)> callback) { + std::vector<lldb_private::CompilerContext> type_context = + die.GetTypeLookupContext(); + if (type_context.empty()) { + // If both type_context and query_parent_names and empty we have a match. + // Otherwise, this one does not match and we keep on searching. + if (query_parent_names.empty()) + return callback(die); + return true; + } + + // Type lookup context includes the current DIE as the last element. + // so revert it for easy matching. + std::reverse(type_context.begin(), type_context.end()); + + // type_context includes the name of the current DIE while query_parent_names + // doesn't. So start check from index 1 for dwarf_decl_ctx. + uint32_t i = 1, j = 0; + while (i < type_context.size() && j < query_parent_names.size()) { + // If type_context[i] has no name, skip it. + // e.g. this can happen for anonymous namespaces. + if (type_context[i].name.IsNull() || type_context[i].name.IsEmpty()) { + ++i; + continue; + } + // If the name doesn't match, skip it. + // e.g. this can happen for inline namespaces. + if (query_parent_names[j] != type_context[i].name) { + ++i; + continue; + } + ++i; + ++j; + } + // If not all query_parent_names were found in type_context. + // This die does not meet the criteria, try next one. + if (j != query_parent_names.size()) + return true; + return callback(die); +} diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h index cb3ae8a06d7885..1f457fda0282f5 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h @@ -64,6 +64,23 @@ class DWARFIndex { virtual void GetNamespaces(ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) = 0; + + /// Get type DIEs whose base name match \param name with \param parent_names + /// in its decl parent chain as subset. A base implementation is provided, + /// Specializations should override this if they are able to provide a faster + /// implementation. + virtual void + GetTypesWithParents(ConstString name, + llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback); + /// Get namespace DIEs whose base name match \param name with \param + /// parent_names in its decl parent chain as subset. A base implementation is + /// provided. Specializations should override this if they are able to provide + /// a faster implementation. + virtual void + GetNamespacesWithParents(ConstString name, + llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback); virtual void GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf, const CompilerDeclContext &parent_decl_ctx, @@ -115,6 +132,11 @@ class DWARFIndex { bool GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die, llvm::function_ref<bool(DWARFDIE die)> callback); + + /// Check if the \a die has \a parent_names in its decl parent chain. + bool ProcessDieMatchParentNames( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + DWARFDIE die, llvm::function_ref<bool(DWARFDIE die)> callback); }; } // namespace dwarf } // namespace lldb_private::plugin diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 32d8a92305aafa..c84fc0b5c60717 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -16,6 +16,8 @@ #include "lldb/Utility/Stream.h" #include "llvm/ADT/Sequence.h" #include <optional> +#include "lldb/Core/Progress.h" +#include "lldb/Utility/LLDBLog.h" using namespace lldb_private; using namespace lldb; @@ -301,7 +303,8 @@ using Entry = llvm::DWARFDebugNames::Entry; /// If any parent does not have an `IDX_parent`, or the Entry data is corrupted, /// nullopt is returned. std::optional<llvm::SmallVector<Entry, 4>> -getParentChain(Entry entry, uint32_t max_parents) { +getParentChain(Entry entry, + uint32_t max_parents = std::numeric_limits<uint32_t>::max()) { llvm::SmallVector<Entry, 4> parent_entries; do { @@ -374,25 +377,40 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( m_fallback.GetFullyQualifiedType(context, callback); } +bool DebugNamesDWARFIndex::SameAsEntryATName( + llvm::StringRef query_name, const DebugNames::Entry &entry) const { + auto maybe_dieoffset = entry.getDIEUnitOffset(); + if (!maybe_dieoffset) + return false; + + // [Optimization] instead of parsing the entry from dwo file, we simply + // check if the query_name can point to an entry of the same DIE offset. + // This greatly reduced number of dwo file parsed and thus improved the + // performance. + for (const DebugNames::Entry &query_entry : + entry.getNameIndex()->equal_range(query_name)) { + auto query_dieoffset = query_entry.getDIEUnitOffset(); + if (!query_dieoffset) + continue; + + if (*query_dieoffset == *maybe_dieoffset) { + return true; + } else if (*query_dieoffset > *maybe_dieoffset) { + // The pool entries of the same name are sequentially cluttered together + // so if the query name from `query_name` is after the target entry, this + // is definitely not the correct one, we can stop searching. + return false; + } + } + return false; +} + bool DebugNamesDWARFIndex::SameParentChain( llvm::ArrayRef<llvm::StringRef> parent_names, llvm::ArrayRef<DebugNames::Entry> parent_entries) const { - if (parent_entries.size() != parent_names.size()) return false; - auto SameAsEntryATName = [this](llvm::StringRef name, - const DebugNames::Entry &entry) { - // Peek at the AT_name of `entry` and test equality to `name`. - auto maybe_dieoffset = entry.getDIEUnitOffset(); - if (!maybe_dieoffset) - return false; - DWARFUnit *unit = GetNonSkeletonUnit(entry); - if (!unit) - return false; - return name == unit->PeekDIEName(unit->GetOffset() + *maybe_dieoffset); - }; - // If the AT_name of any parent fails to match the expected name, we don't // have a match. for (auto [parent_name, parent_entry] : @@ -402,6 +420,36 @@ bool DebugNamesDWARFIndex::SameParentChain( return true; } +bool DebugNamesDWARFIndex::WithinParentChain( + llvm::ArrayRef<llvm::StringRef> query_parent_names, + llvm::ArrayRef<DebugNames::Entry> parent_chain) const { + if (parent_chain.size() < query_parent_names.size()) + return false; + + size_t query_idx = 0, chain_idx = 0; + while (query_idx < query_parent_names.size() && + chain_idx < parent_chain.size()) { + if (SameAsEntryATName(query_parent_names[query_idx], + parent_chain[chain_idx])) { + ++query_idx; + ++chain_idx; + } else { + // Name does not match, try next parent_chain entry if the current entry + // is namespace because the current one can be an inline namespace. + if (parent_chain[chain_idx].tag() != DW_TAG_namespace) + return false; + + ++chain_idx; + if (query_parent_names.size() - query_idx > + parent_chain.size() - chain_idx) { + // Parent chain has not enough entries, we can't possibly have a match. + return false; + } + } + } + return query_idx == query_parent_names.size(); +} + void DebugNamesDWARFIndex::GetTypes( ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) { for (const DebugNames::Entry &entry : @@ -444,6 +492,98 @@ void DebugNamesDWARFIndex::GetNamespaces( m_fallback.GetNamespaces(name, callback); } +void DebugNamesDWARFIndex::GetNamespacesWithParents( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) { + Progress progress("Get namespace from index for %s", name.GetCString()); + for (const DebugNames::Entry &entry : + m_debug_names_up->equal_range(name.GetStringRef())) { + lldb_private::dwarf::Tag entry_tag = entry.tag(); + if (entry_tag == DW_TAG_namespace || + entry_tag == DW_TAG_imported_declaration) { + std::optional<llvm::SmallVector<Entry, 4>> parent_chain = + getParentChain(entry); + if (!parent_chain) { + // Fallback: use the base class implementation. + if (!ProcessEntry(entry, [&](DWARFDIE die) { + return ProcessDieMatchParentNames(name, parent_names, die, callback); + })) + return; + continue; + } + + if (parent_chain->size() < parent_names.size()) + continue; + else if (parent_chain->size() == parent_names.size()) { + if (SameParentChain(parent_names, *parent_chain) && + !ProcessEntry(entry, callback)) + return; + } else { + // parent_chain->size() > parent_names.size() + if (WithinParentChain(parent_names, *parent_chain) && + !ProcessEntry(entry, callback)) + return; + } + } + } + + m_fallback.GetNamespaces(name, callback); +} + +void DebugNamesDWARFIndex::GetTypesWithParents( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) { + if (parent_names.empty()) + return GetTypes(name, callback); + + Log *log = GetLog(LLDBLog::Temporary); + int count = 0; + Progress progress("GetTypesWithParents from index for %s", name.GetCString()); + // For each entry, grab its parent chain and check if we have a match. + for (const DebugNames::Entry &entry : m_debug_names_up->equal_range(name)) { + if (!isType(entry.tag())) + continue; + + // If we get a NULL foreign_tu back, the entry doesn't match the type unit + // in the .dwp file, or we were not able to load the .dwo file or the DWO ID + // didn't match. + std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry); + if (foreign_tu && foreign_tu.value() == nullptr) + continue; + + // Grab at most one extra parent, subsequent parents are not necessary to + // test equality. + std::optional<llvm::SmallVector<Entry, 4>> parent_chain = + getParentChain(entry); + ++count; + if (!parent_chain) { + // Fallback: use the base class implementation. + if (!ProcessEntry(entry, [&](DWARFDIE die) { + return ProcessDieMatchParentNames(name, parent_names, die, callback); + })) + return; + continue; + } + + if (parent_chain->size() < parent_names.size()) + continue; + else if (parent_chain->size() == parent_names.size()) { + if (SameParentChain(parent_names, *parent_chain) && + !ProcessEntry(entry, callback)) + return; + } else { + // parent_chain->size() > parent_names.size() + if (WithinParentChain(parent_names, *parent_chain) && + !ProcessEntry(entry, callback)) + return; + } + } + LLDB_LOG(log, + "GetTypesWithParents searched {0} entries for {1} but found none", + count, name.GetCString()); + m_fallback.GetTypesWithParents(name, parent_names, callback); +} + void DebugNamesDWARFIndex::GetFunctions( const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf, const CompilerDeclContext &parent_decl_ctx, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h index cb15c1d4f994b3..9d76ddaa043e2b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -52,6 +52,14 @@ class DebugNamesDWARFIndex : public DWARFIndex { llvm::function_ref<bool(DWARFDIE die)> callback) override; void GetNamespaces(ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) override; + void + GetTypesWithParents(ConstString name, + llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) override; + void GetNamespacesWithParents( + ConstString name, llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::function_ref<bool(DWARFDIE die)> callback) override; + void GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf, const CompilerDeclContext &parent_decl_ctx, @@ -118,6 +126,27 @@ class DebugNamesDWARFIndex : public DWARFIndex { bool SameParentChain(llvm::ArrayRef<llvm::StringRef> parent_names, llvm::ArrayRef<DebugNames::Entry> parent_entries) const; + /// Returns true if \a parent_names entries are within \a parent_chain. + /// This is diffferent from SameParentChain() which checks for exact match. + /// This function is required because \a parent_chain can contain inline + /// namespace entries which may not be specified in parent_names by client. + /// + /// \param[in] parent_names + /// The list of parent names to check for. + /// + /// \param[in] parent_chain + /// The fully qualified parent chain entries from .debug_names index table + /// to check against. + /// + /// \returns + /// True if all \a parent_names entries are can be sequentially found inside + /// \a parent_chain, otherwise False. + bool WithinParentChain(llvm::ArrayRef<llvm::StringRef> parent_names, + llvm::ArrayRef<DebugNames::Entry> parent_chain) const; + + /// Returns true if .debug_names pool entry \p entry has name \p query_name. + bool SameAsEntryATName(llvm::StringRef query_name, const DebugNames::Entry &entry) const; + static void MaybeLogLookupError(llvm::Error error, const DebugNames::NameIndex &ni, llvm::StringRef name); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index f721ca00fd3559..833b1ae87dc873 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2731,6 +2731,22 @@ uint64_t SymbolFileDWARF::GetDebugInfoSize(bool load_all_debug_info) { return debug_info_size; } +llvm::SmallVector<llvm::StringRef> +SymbolFileDWARF::GetTypeQueryParentNames(TypeQuery query) { + std::vector<lldb_private::CompilerContext> &query_decl_context = + query.GetContextRef(); + llvm::SmallVector<llvm::StringRef> parent_names; + if (!query_decl_context.empty()) { + auto rbegin = query_decl_context.rbegin(), rend = query_decl_context.rend(); + for (auto rit = rbegin + 1; rit != rend; ++rit) + if ((rit->kind & CompilerContextKind::AnyType) != + CompilerContextKind::Invalid && + !rit->name.IsEmpty()) + parent_names.push_back(rit->name.GetStringRef()); + } + return parent_names; +} + void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { // Make sure we haven't already searched this SymbolFile before. @@ -2748,45 +2764,50 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); + TypeQuery query_full(query); + llvm::SmallVector<llvm::StringRef> parent_names = + GetTypeQueryParentNames(query_full); bool have_index_match = false; - m_index->GetTypes(type_basename, [&](DWARFDIE die) { - // Check the language, but only if we have a language filter. - if (query.HasLanguage()) { - if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) - return true; // Keep iterating over index types, language mismatch. - } + m_index->GetTypesWithParents( + query.GetTypeBasename(), parent_names, [&](DWARFDIE die) { + // Check the language, but only if we have a language filter. + if (query.HasLanguage()) { + if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) + return true; // Keep iterating over index types, language mismatch. + } - // Check the context matches - std::vector<lldb_private::CompilerContext> die_context; - if (query.GetModuleSearch()) - die_context = die.GetDeclContext(); - else - die_context = die.GetTypeLookupContext(); - assert(!die_context.empty()); - if (!query.ContextMatches(die_context)) - return true; // Keep iterating over index types, context mismatch. - - // Try to resolve the type. - if (Type *matching_type = ResolveType(die, true, true)) { - if (matching_type->IsTemplateType()) { - // We have to watch out for case where we lookup a type by basename and - // it matches a template with simple template names. Like looking up - // "Foo" and if we have simple template names then we will match - // "Foo<int>" and "Foo<double>" because all the DWARF has is "Foo" in - // the accelerator tables. The main case we see this in is when the - // expression parser is trying to parse "Foo<int>" and it will first do - // a lookup on just "Foo". We verify the type basename matches before - // inserting the type in the results. - auto CompilerTypeBasename = - matching_type->GetForwardCompilerType().GetTypeName(true); - if (CompilerTypeBasename != query.GetTypeBasename()) - return true; // Keep iterating over index types, basename mismatch. - } - have_index_match = true; - results.InsertUnique(matching_type->shared_from_this()); - } - return !results.Done(query); // Keep iterating if we aren't done. - }); + // Check the context matches + std::vector<lldb_private::CompilerContext> die_context; + if (query.GetModuleSearch()) + die_context = die.GetDeclContext(); + else + die_context = die.GetTypeLookupContext(); + assert(!die_context.empty()); + if (!query.ContextMatches(die_context)) + return true; // Keep iterating over index types, context mismatch. + + // Try to resolve the type. + if (Type *matching_type = ResolveType(die, true, true)) { + if (matching_type->IsTemplateType()) { + // We have to watch out for case where we lookup a type by basename + // and it matches a template with simple template names. Like + // looking up "Foo" and if we have simple template names then we + // will match "Foo<int>" and "Foo<double>" because all the DWARF has + // is "Foo" in the accelerator tables. The main case we see this in + // is when the expression parser is trying to parse "Foo<int>" and + // it will first do a lookup on just "Foo". We verify the type + // basename matches before inserting the type in the results. + auto CompilerTypeBasename = + matching_type->GetForwardCompilerType().GetTypeName(true); + if (CompilerTypeBasename != query.GetTypeBasename()) + return true; // Keep iterating over index types, basename + // mismatch. + } + have_index_match = true; + results.InsertUnique(matching_type->shared_from_this()); + } + return !results.Done(query); // Keep iterating if we aren't done. + }); if (results.Done(query)) { if (log) { @@ -2811,44 +2832,50 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { TypeQuery query_simple(query); if (UpdateCompilerContextForSimpleTemplateNames(query_simple)) { auto type_basename_simple = query_simple.GetTypeBasename(); + llvm::SmallVector<llvm::StringRef> parent_names = + GetTypeQueryParentNames(query_simple); // Copy our match's context and update the basename we are looking for // so we can use this only to compare the context correctly. - m_index->GetTypes(type_basename_simple, [&](DWARFDIE die) { - // Check the language, but only if we have a language filter. - if (query.HasLanguage()) { - if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) - return true; // Keep iterating over index types, language mismatch. - } - - // Check the context matches - std::vector<lldb_private::CompilerContext> die_context; - if (query.GetModuleSearch()) - die_context = die.GetDeclContext(); - else - die_context = die.GetTypeLookupContext(); - assert(!die_context.empty()); - if (!query_simple.ContextMatches(die_context)) - return true; // Keep iterating over index types, context mismatch. - - // Try to resolve the type. - if (Type *matching_type = ResolveType(die, true, true)) { - ConstString name = matching_type->GetQualifiedName(); - // We have found a type that still might not match due to template - // parameters. If we create a new TypeQuery that uses the new type's - // fully qualified name, we can find out if this type matches at all - // context levels. We can't use just the "match_simple" context - // because all template parameters were stripped off. The fully - // qualified name of the type will have the template parameters and - // will allow us to make sure it matches correctly. - TypeQuery die_query(name.GetStringRef(), - TypeQueryOptions::e_exact_match); - if (!query.ContextMatches(die_query.GetContextRef())) - return true; // Keep iterating over index types, context mismatch. + m_index->GetTypesWithParents( + query_simple.GetTypeBasename(), parent_names, [&](DWARFDIE die) { + // Check the language, but only if we have a language filter. + if (query.HasLanguage()) { + if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) + return true; // Keep iterating over index types, language + // mismatch. + } - results.InsertUnique(matching_type->shared_from_this()); - } - return !results.Done(query); // Keep iterating if we aren't done. - }); + // Check the context matches + std::vector<lldb_private::CompilerContext> die_context; + if (query.GetModuleSearch()) + die_context = die.GetDeclContext(); + else + die_context = die.GetTypeLookupContext(); + assert(!die_context.empty()); + if (!query_simple.ContextMatches(die_context)) + return true; // Keep iterating over index types, context mismatch. + + // Try to resolve the type. + if (Type *matching_type = ResolveType(die, true, true)) { + ConstString name = matching_type->GetQualifiedName(); + // We have found a type that still might not match due to template + // parameters. If we create a new TypeQuery that uses the new + // type's fully qualified name, we can find out if this type + // matches at all context levels. We can't use just the + // "match_simple" context because all template parameters were + // stripped off. The fully qualified name of the type will have + // the template parameters and will allow us to make sure it + // matches correctly. + TypeQuery die_query(name.GetStringRef(), + TypeQueryOptions::e_exact_match); + if (!query.ContextMatches(die_query.GetContextRef())) + return true; // Keep iterating over index types, context + // mismatch. + + results.InsertUnique(matching_type->shared_from_this()); + } + return !results.Done(query); // Keep iterating if we aren't done. + }); if (results.Done(query)) { if (log) { GetObjectFile()->GetModule()->LogMessage( @@ -2898,7 +2925,24 @@ SymbolFileDWARF::FindNamespace(ConstString name, if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx)) return namespace_decl_ctx; - m_index->GetNamespaces(name, [&](DWARFDIE die) { + llvm::SmallVector<llvm::StringRef> parent_names; + auto parent_ctx = parent_decl_ctx.GetCompilerContext(); + auto rbegin = parent_ctx.rbegin(), rend = parent_ctx.rend(); + for (auto rit = rbegin;rit != rend; ++rit) { + if (!rit->name.IsEmpty()) + parent_names.push_back(rit->name); + } + + std::string parent_name_strings; + for (auto &parent_name : parent_names) { + parent_name_strings += "::"; + parent_name_strings += parent_name; + } + Log *log1 = GetLog(LLDBLog::Temporary); + LLDB_LOGF(log1, "GetNamespacesWithParents() for %s with parent_names %s -- start", + name.GetCString(), parent_name_strings.c_str()); + + m_index->GetNamespacesWithParents(name, parent_names, [&](DWARFDIE die) { if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces)) return true; // The containing decl contexts don't match @@ -2909,6 +2953,8 @@ SymbolFileDWARF::FindNamespace(ConstString name, namespace_decl_ctx = dwarf_ast->GetDeclContextForUIDFromDWARF(die); return !namespace_decl_ctx.IsValid(); }); + LLDB_LOGF(log1, "GetNamespacesWithParents() for %s with parent_names %s -- end", + name.GetCString(), parent_name_strings.c_str()); if (log && namespace_decl_ctx) { GetObjectFile()->GetModule()->LogMessage( diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 4967b37d753a09..c5cd7fd1bf61ec 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -506,6 +506,8 @@ class SymbolFileDWARF : public SymbolFileCommon { void GetCompileOptions(std::unordered_map<lldb::CompUnitSP, Args> &args) override; + llvm::SmallVector<llvm::StringRef> GetTypeQueryParentNames(TypeQuery query); + lldb::ModuleWP m_debug_map_module_wp; SymbolFileDWARFDebugMap *m_debug_map_symfile; >From 49f869286ebe7791f47cc4fef8d8195aef32b76d Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 16 Sep 2024 18:03:01 -0700 Subject: [PATCH 3/5] Remove unwanted logging --- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 833b1ae87dc873..cc2497b7e484ba 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2928,20 +2928,10 @@ SymbolFileDWARF::FindNamespace(ConstString name, llvm::SmallVector<llvm::StringRef> parent_names; auto parent_ctx = parent_decl_ctx.GetCompilerContext(); auto rbegin = parent_ctx.rbegin(), rend = parent_ctx.rend(); - for (auto rit = rbegin;rit != rend; ++rit) { + for (auto rit = rbegin; rit != rend; ++rit) { if (!rit->name.IsEmpty()) parent_names.push_back(rit->name); } - - std::string parent_name_strings; - for (auto &parent_name : parent_names) { - parent_name_strings += "::"; - parent_name_strings += parent_name; - } - Log *log1 = GetLog(LLDBLog::Temporary); - LLDB_LOGF(log1, "GetNamespacesWithParents() for %s with parent_names %s -- start", - name.GetCString(), parent_name_strings.c_str()); - m_index->GetNamespacesWithParents(name, parent_names, [&](DWARFDIE die) { if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces)) return true; // The containing decl contexts don't match @@ -2953,8 +2943,6 @@ SymbolFileDWARF::FindNamespace(ConstString name, namespace_decl_ctx = dwarf_ast->GetDeclContextForUIDFromDWARF(die); return !namespace_decl_ctx.IsValid(); }); - LLDB_LOGF(log1, "GetNamespacesWithParents() for %s with parent_names %s -- end", - name.GetCString(), parent_name_strings.c_str()); if (log && namespace_decl_ctx) { GetObjectFile()->GetModule()->LogMessage( >From 6ce666cdd26dd13c568d4e4208c5a24e23ed8316 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 16 Sep 2024 18:08:25 -0700 Subject: [PATCH 4/5] Remove more unwanted logging --- .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index c84fc0b5c60717..c97c9248a4478b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -506,12 +506,14 @@ void DebugNamesDWARFIndex::GetNamespacesWithParents( if (!parent_chain) { // Fallback: use the base class implementation. if (!ProcessEntry(entry, [&](DWARFDIE die) { - return ProcessDieMatchParentNames(name, parent_names, die, callback); + return ProcessDieMatchParentNames(name, parent_names, die, + callback); })) return; continue; } + // If parent_chain is shorter than parent_names, we can't possibly match. if (parent_chain->size() < parent_names.size()) continue; else if (parent_chain->size() == parent_names.size()) { @@ -519,7 +521,8 @@ void DebugNamesDWARFIndex::GetNamespacesWithParents( !ProcessEntry(entry, callback)) return; } else { - // parent_chain->size() > parent_names.size() + // When parent_chain has more entries than parent_names we still + // possibly match if parent_chain contains inline namespaces. if (WithinParentChain(parent_names, *parent_chain) && !ProcessEntry(entry, callback)) return; @@ -535,10 +538,7 @@ void DebugNamesDWARFIndex::GetTypesWithParents( llvm::function_ref<bool(DWARFDIE die)> callback) { if (parent_names.empty()) return GetTypes(name, callback); - - Log *log = GetLog(LLDBLog::Temporary); - int count = 0; - Progress progress("GetTypesWithParents from index for %s", name.GetCString()); + // For each entry, grab its parent chain and check if we have a match. for (const DebugNames::Entry &entry : m_debug_names_up->equal_range(name)) { if (!isType(entry.tag())) @@ -551,20 +551,19 @@ void DebugNamesDWARFIndex::GetTypesWithParents( if (foreign_tu && foreign_tu.value() == nullptr) continue; - // Grab at most one extra parent, subsequent parents are not necessary to - // test equality. std::optional<llvm::SmallVector<Entry, 4>> parent_chain = getParentChain(entry); - ++count; if (!parent_chain) { // Fallback: use the base class implementation. if (!ProcessEntry(entry, [&](DWARFDIE die) { - return ProcessDieMatchParentNames(name, parent_names, die, callback); + return ProcessDieMatchParentNames(name, parent_names, die, + callback); })) return; continue; } + // If parent_chain is shorter than parent_names, we can't possibly match. if (parent_chain->size() < parent_names.size()) continue; else if (parent_chain->size() == parent_names.size()) { @@ -572,15 +571,13 @@ void DebugNamesDWARFIndex::GetTypesWithParents( !ProcessEntry(entry, callback)) return; } else { - // parent_chain->size() > parent_names.size() + // When parent_chain has more entries than parent_names we still possibly + // match if parent_chain contains inline namespaces. if (WithinParentChain(parent_names, *parent_chain) && !ProcessEntry(entry, callback)) return; } } - LLDB_LOG(log, - "GetTypesWithParents searched {0} entries for {1} but found none", - count, name.GetCString()); m_fallback.GetTypesWithParents(name, parent_names, callback); } >From 1398459ce3aeb31e948ab3d7670bbe601d4d8740 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 16 Sep 2024 20:01:32 -0700 Subject: [PATCH 5/5] Fix clang-format --- lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 2 +- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 4 ++-- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp index 7bdea4eae4529b..3101b1e137fbf3 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp @@ -150,7 +150,7 @@ bool DWARFIndex::ProcessDieMatchParentNames( die.GetTypeLookupContext(); if (type_context.empty()) { // If both type_context and query_parent_names and empty we have a match. - // Otherwise, this one does not match and we keep on searching. + // Otherwise, this one does not match and we keep on searching. if (query_parent_names.empty()) return callback(die); return true; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index c97c9248a4478b..76c80011791de6 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -12,12 +12,12 @@ #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h" #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h" #include "lldb/Core/Module.h" +#include "lldb/Core/Progress.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/Stream.h" #include "llvm/ADT/Sequence.h" #include <optional> -#include "lldb/Core/Progress.h" -#include "lldb/Utility/LLDBLog.h" using namespace lldb_private; using namespace lldb; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h index 9d76ddaa043e2b..1f9c87c76a99f8 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -145,7 +145,8 @@ class DebugNamesDWARFIndex : public DWARFIndex { llvm::ArrayRef<DebugNames::Entry> parent_chain) const; /// Returns true if .debug_names pool entry \p entry has name \p query_name. - bool SameAsEntryATName(llvm::StringRef query_name, const DebugNames::Entry &entry) const; + bool SameAsEntryATName(llvm::StringRef query_name, + const DebugNames::Entry &entry) const; static void MaybeLogLookupError(llvm::Error error, const DebugNames::NameIndex &ni, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits