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/2] 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 bb42917c2bea4cde9f7b48685d85df89f0bc7eb8 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 23 Sep 2024 16:40:16 -0700 Subject: [PATCH 2/2] Improve type query using .debug_names parent chain --- .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 25 ++++ .../Plugins/SymbolFile/DWARF/DWARFIndex.h | 12 ++ .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 128 +++++++++++++++++- .../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 34 +++++ .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 5 +- 5 files changed, 201 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp index eafddbad03f57b..2b454d1e676098 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp @@ -126,3 +126,28 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl( return callback(die); return true; } + +void DWARFIndex::GetTypesWithQuery( + TypeQuery &query, llvm::function_ref<bool(DWARFDIE die)> callback) { + GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) { + return ProcessTypeDieMatchQuery(query, die, callback); + }); +} + +bool DWARFIndex::ProcessTypeDieMatchQuery( + TypeQuery &query, DWARFDIE die, + llvm::function_ref<bool(DWARFDIE die)> callback) { + // Nothing to match from query + if (query.GetContextRef().size() <= 1) + return callback(die); + + std::vector<lldb_private::CompilerContext> die_context; + if (query.GetModuleSearch()) + die_context = die.GetDeclContext(); + else + die_context = die.GetTypeLookupContext(); + + if (!query.ContextMatches(die_context)) + 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..d87aca8ef8bf63 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h @@ -64,6 +64,13 @@ class DWARFIndex { virtual void GetNamespaces(ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) = 0; + /// Get type DIEs meeting requires of \a query. + /// 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 + GetTypesWithQuery(TypeQuery &query, + llvm::function_ref<bool(DWARFDIE die)> callback); virtual void GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf, const CompilerDeclContext &parent_decl_ctx, @@ -115,6 +122,11 @@ class DWARFIndex { bool GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die, llvm::function_ref<bool(DWARFDIE die)> callback); + + /// Check if the type \a die can meet the requirements of \a query. + bool + ProcessTypeDieMatchQuery(TypeQuery &query, 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..0c036d8a780d7e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -301,7 +301,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,6 +375,21 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( m_fallback.GetFullyQualifiedType(context, callback); } +bool DebugNamesDWARFIndex::SameAsEntryContext( + const CompilerContext &query_context, + const DebugNames::Entry &entry) const { + // TODO: check dwarf tag matches. + // 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 query_context.name == + unit->PeekDIEName(unit->GetOffset() + *maybe_dieoffset); +} + bool DebugNamesDWARFIndex::SameParentChain( llvm::ArrayRef<llvm::StringRef> parent_names, llvm::ArrayRef<DebugNames::Entry> parent_entries) const { @@ -402,6 +418,49 @@ bool DebugNamesDWARFIndex::SameParentChain( return true; } +bool DebugNamesDWARFIndex::SameParentChain( + llvm::ArrayRef<CompilerContext> parent_contexts, + llvm::ArrayRef<DebugNames::Entry> parent_entries) const { + if (parent_entries.size() != parent_contexts.size()) + return false; + + // If the AT_name of any parent fails to match the expected name, we don't + // have a match. + for (auto [parent_context, parent_entry] : + llvm::zip_equal(parent_contexts, parent_entries)) + if (!SameAsEntryContext(parent_context, parent_entry)) + return false; + return true; +} + +bool DebugNamesDWARFIndex::WithinParentChain( + llvm::ArrayRef<CompilerContext> query_contexts, + llvm::ArrayRef<DebugNames::Entry> parent_chain) const { + if (query_contexts.size() == parent_chain.size()) + return SameParentChain(query_contexts, parent_chain); + + size_t query_idx = 0, chain_idx = 0; + while (query_idx < query_contexts.size() && chain_idx < parent_chain.size()) { + if (query_contexts.size() - query_idx > parent_chain.size() - chain_idx) { + // Parent chain has not enough entries, we can't possibly have a match. + return false; + } + + if (SameAsEntryContext(query_contexts[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; + } + } + return query_idx == query_contexts.size(); +} + void DebugNamesDWARFIndex::GetTypes( ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) { for (const DebugNames::Entry &entry : @@ -444,6 +503,73 @@ void DebugNamesDWARFIndex::GetNamespaces( m_fallback.GetNamespaces(name, callback); } +llvm::SmallVector<CompilerContext> +DebugNamesDWARFIndex::GetTypeQueryParentContexts(TypeQuery &query) { + std::vector<lldb_private::CompilerContext> &query_decl_context = + query.GetContextRef(); + llvm::SmallVector<CompilerContext> parent_contexts; + if (!query_decl_context.empty()) { + auto rbegin = query_decl_context.rbegin(), rend = query_decl_context.rend(); + // Reverse the query decl context to match parent chain. + // Skip the last entry, it is the type we are looking for. + for (auto rit = rbegin + 1; rit != rend; ++rit) + // Skip any context without name because .debug_names might not encode + // them. (e.g. annonymous namespace) + if ((rit->kind & CompilerContextKind::AnyType) != + CompilerContextKind::Invalid && + !rit->name.IsEmpty()) + parent_contexts.push_back(*rit); + } + return parent_contexts; +} + +void DebugNamesDWARFIndex::GetTypesWithQuery( + TypeQuery &query, llvm::function_ref<bool(DWARFDIE die)> callback) { + ConstString name = query.GetTypeBasename(); + std::vector<lldb_private::CompilerContext> query_context = + query.GetContextRef(); + if (query_context.size() <= 1) + return GetTypes(name, callback); + + llvm::SmallVector<CompilerContext> parent_contexts = + GetTypeQueryParentContexts(query); + // 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; + + 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 ProcessTypeDieMatchQuery(query, die, callback); + })) + return; + continue; + } + + // If parent_chain is shorter than parent_contexts, we can't possibly match. + if (parent_chain->size() < parent_contexts.size()) + continue; + else if (WithinParentChain(parent_contexts, *parent_chain) && + !ProcessEntry(entry, [&](DWARFDIE die) { + // After .debug_names filtering still sending to base class for + // further filtering before calling the callback. + return ProcessTypeDieMatchQuery(query, die, callback); + })) + return; + } + m_fallback.GetTypesWithQuery(query, 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..d728427b3a5d3f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -52,6 +52,10 @@ 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 + GetTypesWithQuery(TypeQuery &query, + llvm::function_ref<bool(DWARFDIE die)> callback) override; + void GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf, const CompilerDeclContext &parent_decl_ctx, @@ -118,6 +122,36 @@ class DebugNamesDWARFIndex : public DWARFIndex { bool SameParentChain(llvm::ArrayRef<llvm::StringRef> parent_names, llvm::ArrayRef<DebugNames::Entry> parent_entries) const; + bool SameParentChain(llvm::ArrayRef<CompilerContext> parent_names, + llvm::ArrayRef<DebugNames::Entry> parent_entries) const; + + /// Returns true if \a parent_contexts 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 \a parent_contexts by + /// client. + /// + /// \param[in] parent_contexts + /// The list of parent contexts 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_contexts entries are can be sequentially found + /// inside + /// \a parent_chain, otherwise False. + bool WithinParentChain(llvm::ArrayRef<CompilerContext> parent_contexts, + llvm::ArrayRef<DebugNames::Entry> parent_chain) const; + + /// Returns true if .debug_names pool entry \p entry matches \p query_context. + bool SameAsEntryContext(const CompilerContext &query_context, + const DebugNames::Entry &entry) const; + + llvm::SmallVector<CompilerContext> + GetTypeQueryParentContexts(TypeQuery &query); + 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..9ee92171658552 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2748,8 +2748,9 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); + TypeQuery query_full(query); bool have_index_match = false; - m_index->GetTypes(type_basename, [&](DWARFDIE die) { + m_index->GetTypesWithQuery(query_full, [&](DWARFDIE die) { // Check the language, but only if we have a language filter. if (query.HasLanguage()) { if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) @@ -2813,7 +2814,7 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { auto type_basename_simple = query_simple.GetTypeBasename(); // 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) { + m_index->GetTypesWithQuery(query_simple, [&](DWARFDIE die) { // Check the language, but only if we have a language filter. if (query.HasLanguage()) { if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits