[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
Dmitrii Galimzianov via lldb-commits
lldb-commits at lists.llvm.org
Sun Aug 18 13:44:09 PDT 2024
https://github.com/DmT021 updated https://github.com/llvm/llvm-project/pull/102835
>From c647b26ad534bb998063722f930ddd07162bfee7 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov <dmt021 at gmail.com>
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h | 43 ++++++++++++
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp | 80 ++++++++++++++++++++++
lldb/source/Expression/IRExecutionUnit.cpp | 55 ++++++++-------
lldb/source/Symbol/SymbolContext.cpp | 71 ++++++++++++-------
5 files changed, 201 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified in
+ /// the symbol context, that module will be searched first followed by all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref<IterationAction(
+ const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result)>
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified in
+ /// the symbol context, that module will be searched first followed by all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref<IterationAction(
+ const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result)>
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable<collection, SymbolContext, vector_adapter>
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..f190679fcdf7c9 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,48 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref<IterationAction(const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result)>
+ partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+ if (!module)
+ return IterationAction::Continue;
+ module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+ if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+ }
+ return IterationAction::Continue;
+ };
+
+ std::vector<ModuleSP> modules_copy;
+ {
+ std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+ modules_copy = m_modules;
+ }
+
+ auto hinted_module = search_hint ? search_hint->module_sp : nullptr;
+ if (hinted_module) {
+ assert(std::find(modules_copy.begin(), modules_copy.end(), hinted_module) !=
+ modules_copy.end());
+ if (FindInModule(hinted_module) == IterationAction::Stop)
+ return;
+ }
+ for (const ModuleSP &module_sp : modules_copy) {
+ if (module_sp != hinted_module) {
+ if (FindInModule(module_sp) == IterationAction::Stop)
+ return;
+ }
+ }
+}
+
void ModuleList::FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
SymbolContextList &sc_list) {
@@ -532,6 +574,44 @@ void ModuleList::FindSymbolsWithNameAndType(ConstString name,
module_sp->FindSymbolsWithNameAndType(name, symbol_type, sc_list);
}
+void ModuleList::FindSymbolsWithNameAndType(
+ ConstString name, lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref<IterationAction(const ModuleSP &,
+ const SymbolContextList &)>
+ partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+ module->FindSymbolsWithNameAndType(name, symbol_type, sc_list_partial);
+ if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+ }
+ return IterationAction::Continue;
+ };
+
+
+ std::vector<ModuleSP> modules_copy;
+ {
+ std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+ modules_copy = m_modules;
+ }
+ auto hinted_module = search_hint ? search_hint->module_sp : nullptr;
+ if (hinted_module) {
+ assert(std::find(modules_copy.begin(), modules_copy.end(), hinted_module) !=
+ modules_copy.end());
+ if (FindInModule(hinted_module) == IterationAction::Stop)
+ return;
+ }
+ for (const ModuleSP &module_sp : modules_copy) {
+ if (module_sp != hinted_module) {
+ if (FindInModule(module_sp) == IterationAction::Stop)
+ return;
+ }
+ }
+}
+
void ModuleList::FindSymbolsMatchingRegExAndType(
const RegularExpression ®ex, lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const {
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index f220704423627d..56ba76ebfe0fbf 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -704,7 +704,7 @@ class LoadAddressResolver {
LoadAddressResolver(Target *target, bool &symbol_was_missing_weak)
: m_target(target), m_symbol_was_missing_weak(symbol_was_missing_weak) {}
- std::optional<lldb::addr_t> Resolve(SymbolContextList &sc_list) {
+ std::optional<lldb::addr_t> Resolve(const SymbolContextList &sc_list) {
if (sc_list.IsEmpty())
return std::nullopt;
@@ -791,31 +791,36 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
function_options.include_symbols = true;
function_options.include_inlines = false;
- for (const ConstString &name : names) {
- if (sc.module_sp) {
- SymbolContextList sc_list;
- sc.module_sp->FindFunctions(name, CompilerDeclContext(),
- lldb::eFunctionNameTypeFull, function_options,
- sc_list);
- if (auto load_addr = resolver.Resolve(sc_list))
- return *load_addr;
- }
-
- if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name, lldb::eFunctionNameTypeFull,
- function_options, sc_list);
- if (auto load_addr = resolver.Resolve(sc_list))
- return *load_addr;
- }
+ const ModuleList &images = target->GetImages();
- if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindSymbolsWithNameAndType(
- name, lldb::eSymbolTypeAny, sc_list);
- if (auto load_addr = resolver.Resolve(sc_list))
- return *load_addr;
- }
+ for (const ConstString &name : names) {
+ lldb::addr_t result_addr = LLDB_INVALID_ADDRESS;
+
+ images.FindFunctions(
+ name, lldb::eFunctionNameTypeFull, function_options, &sc,
+ [&](const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result) {
+ if (auto load_addr = resolver.Resolve(partial_result)) {
+ result_addr = *load_addr;
+ return IterationAction::Stop;
+ }
+ return IterationAction::Continue;
+ });
+ if (result_addr != LLDB_INVALID_ADDRESS)
+ return result_addr;
+
+ images.FindSymbolsWithNameAndType(
+ name, lldb::eSymbolTypeAny, &sc,
+ [&](const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result) {
+ if (auto load_addr = resolver.Resolve(partial_result)) {
+ result_addr = *load_addr;
+ return IterationAction::Stop;
+ }
+ return IterationAction::Continue;
+ });
+ if (result_addr != LLDB_INVALID_ADDRESS)
+ return result_addr;
lldb::addr_t best_internal_load_address =
resolver.GetBestInternalLoadAddress();
diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp
index 8f26e41d192044..9402b4700e182b 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -895,31 +895,54 @@ const Symbol *SymbolContext::FindBestGlobalDataSymbol(ConstString name,
}
};
- if (module) {
- SymbolContextList sc_list;
- module->FindSymbolsWithNameAndType(name, eSymbolTypeAny, sc_list);
- const Symbol *const module_symbol = ProcessMatches(sc_list, error);
-
- if (!error.Success()) {
- return nullptr;
- } else if (module_symbol) {
- return module_symbol;
- }
- }
-
- {
- SymbolContextList sc_list;
- target.GetImages().FindSymbolsWithNameAndType(name, eSymbolTypeAny,
- sc_list);
- const Symbol *const target_symbol = ProcessMatches(sc_list, error);
-
- if (!error.Success()) {
- return nullptr;
- } else if (target_symbol) {
- return target_symbol;
- }
- }
+ const Symbol *result_symbol = nullptr;
+ target.GetImages().FindSymbolsWithNameAndType(
+ name, eSymbolTypeAny, this,
+ [&error, ProcessMatches, &result_symbol, &target, name,
+ this](const lldb::ModuleSP &module,
+ const SymbolContextList &partial_result) {
+ const Symbol *const processed_symbol =
+ ProcessMatches(partial_result, error);
+ if (!error.Success()) {
+ return IterationAction::Stop;
+ }
+ if (!processed_symbol)
+ return IterationAction::Continue;
+ if (module == this->module_sp) {
+ // When the found symbol is in the hinted module use it even
+ // if it's an internal one.
+ result_symbol = processed_symbol;
+ return IterationAction::Stop;
+ }
+ if (!result_symbol) {
+ result_symbol = processed_symbol;
+ return IterationAction::Continue;
+ }
+ if (result_symbol->IsExternal() == processed_symbol->IsExternal()) {
+ assert(processed_symbol != result_symbol &&
+ "The same symbol is found in two modules");
+ StreamString ss;
+ bool areBothExternal = result_symbol->IsExternal();
+ ss.Printf("Multiple %s symbols found for '%s'\n",
+ areBothExternal ? "external" : "internal",
+ name.AsCString());
+ result_symbol->GetDescription(&ss, eDescriptionLevelVerbose, &target);
+ ss.PutChar('\n');
+ processed_symbol->GetDescription(&ss, eDescriptionLevelVerbose,
+ &target);
+ ss.PutChar('\n');
+ error.SetErrorString(ss.GetData());
+ return IterationAction::Stop;
+ }
+ if (!result_symbol->IsExternal() && processed_symbol->IsExternal())
+ result_symbol = processed_symbol;
+ return IterationAction::Continue;
+ });
+ if (!error.Success())
+ return nullptr;
+ if (result_symbol)
+ return result_symbol;
return nullptr; // no error; we just didn't find anything
}
More information about the lldb-commits
mailing list