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 <dmt...@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 } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits