[Lldb-commits] [lldb] Improve type and namespace lookup using parent chain (PR #108907)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 16 18:16:01 PDT 2024
https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/108907
## Context
We have a customer reporting scenario that expanding "this" pointer for a non-trivial class method context takes more than **70~90 seconds**. This is a linux target with .debug_names index table enabled and split dwarf dwo files.
Profiling shows the majority of the bottleneck is spent in `SymbolFileDWARF::FindTypes()` and `SymbolFileDWARF:: FindNamespace()` two functions searching types/namespaces in .debug_names index table with base name, then parsing/loading the underlying dwo files which can be very slow.
## Context
This PR improves `SymbolFileDWARF::FindTypes()` and `SymbolFileDWARF:: FindNamespace()` by using the newly added parent chain in .debug_names.
>From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 19 Aug 2024 10:57:35 -0700
Subject: [PATCH 1/4] 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 <jeffreytan at fb.com>
Date: Mon, 16 Sep 2024 12:02:48 -0700
Subject: [PATCH 2/4] 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 <jeffreytan at fb.com>
Date: Mon, 16 Sep 2024 18:03:01 -0700
Subject: [PATCH 3/4] 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 <jeffreytan at fb.com>
Date: Mon, 16 Sep 2024 18:08:25 -0700
Subject: [PATCH 4/4] 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);
}
More information about the lldb-commits
mailing list