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 <dmt...@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 43d931a844740..171850e59baa3 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 0bc707070f850..66622b5c15f7c 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 b03490bacf959..9b45334fb65a1 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 f220704423627..56ba76ebfe0fb 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 9b0042ffdb4bf..e3f5b6477741b 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 8f26e41d19204..9402b4700e182 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 b5d317cfe34cf596fb71268da59e133e56e5fb36 Mon Sep 17 00:00:00 2001 From: Dmitrii Galimzianov <dmt...@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 9b45334fb65a1..091bcba5239eb 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 56ba76ebfe0fb..4218ff5c2e139 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(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits