[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 12:24:23 PDT 2024
https://github.com/DmT021 updated https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 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 1/2] 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 | 68 +++++++++++++++++++++
lldb/source/Expression/IRExecutionUnit.cpp | 55 +++++++++--------
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++++++++++++++--------
6 files changed, 197 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..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ 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) {
+ 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;
+ };
+
+ auto hinted_module = search_hint ? search_hint->module_sp : nullptr;
+ if (hinted_module) {
+ assert(std::find(m_modules.begin(), m_modules.end(), hinted_module) !=
+ m_modules.end());
+ if (FindInModule(hinted_module) == IterationAction::Stop)
+ return;
+ }
+ std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+ for (const ModuleSP &module_sp : m_modules) {
+ 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 +567,39 @@ 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::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+ auto hinted_module = search_hint ? search_hint->module_sp : nullptr;
+ if (hinted_module) {
+ assert(std::find(m_modules.begin(), m_modules.end(), hinted_module) !=
+ m_modules.end());
+ if (FindInModule(hinted_module) == IterationAction::Stop)
+ return;
+ }
+ for (const ModuleSP &module_sp : m_modules) {
+ 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/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp
index 9b0042ffdb4bfe..e3f5b6477741b4 100644
--- a/lldb/source/Symbol/Symbol.cpp
+++ b/lldb/source/Symbol/Symbol.cpp
@@ -261,6 +261,14 @@ void Symbol::GetDescription(
s->PutCStringColorHighlighted(mangled_name.GetStringRef(), settings);
s->PutCString("\"");
}
+ if (this->m_is_debug)
+ s->PutCString(", debug");
+ if (this->m_is_external)
+ s->PutCString(", external");
+ if (this->m_is_synthetic)
+ s->PutCString(", synthetic");
+ if (this->m_is_weak)
+ s->PutCString(", weak");
}
void Symbol::Dump(Stream *s, Target *target, uint32_t index,
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
}
>From a58f5fc5379336d30ea3c03d8504aaf8d1253a1b Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov <dmt021 at gmail.com>
Date: Sun, 18 Aug 2024 17:31:42 +0200
Subject: [PATCH 2/2] Temporary commit to check if it fixed the timed out tests
---
lldb/source/Core/ModuleList.cpp | 4 +++-
lldb/source/Expression/IRExecutionUnit.cpp | 23 ++++++++--------------
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 9b45334fb65a15..693cebab60dde5 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -475,6 +475,8 @@ void ModuleList::FindFunctions(
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()) {
@@ -485,6 +487,7 @@ void ModuleList::FindFunctions(
return IterationAction::Continue;
};
+ // std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
auto hinted_module = search_hint ? search_hint->module_sp : nullptr;
if (hinted_module) {
assert(std::find(m_modules.begin(), m_modules.end(), hinted_module) !=
@@ -492,7 +495,6 @@ void ModuleList::FindFunctions(
if (FindInModule(hinted_module) == IterationAction::Stop)
return;
}
- std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
for (const ModuleSP &module_sp : m_modules) {
if (module_sp != hinted_module) {
if (FindInModule(module_sp) == IterationAction::Stop)
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index 56ba76ebfe0fbf..4218ff5c2e1399 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -791,12 +791,10 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
function_options.include_symbols = true;
function_options.include_inlines = false;
- const ModuleList &images = target->GetImages();
-
for (const ConstString &name : names) {
lldb::addr_t result_addr = LLDB_INVALID_ADDRESS;
- images.FindFunctions(
+ sc.target_sp->GetImages().FindFunctions(
name, lldb::eFunctionNameTypeFull, function_options, &sc,
[&](const lldb::ModuleSP &module,
const SymbolContextList &partial_result) {
@@ -809,18 +807,13 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
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;
+ 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;
+ }
lldb::addr_t best_internal_load_address =
resolver.GetBestInternalLoadAddress();
More information about the lldb-commits
mailing list