shafik created this revision.
shafik added reviewers: jasonmolenda, aam, jingham.
Herald added a subscriber: christof.
This PR depends on D67111 <https://reviews.llvm.org/D67111>
Performance issues lead to the libc++ std::function formatter to be disabled.
We addressed some of those performance issues by adding caching see D67111
<https://reviews.llvm.org/D67111>
This PR fixes the first lookup performance by not using
`FindSymbolsMatchingRegExAndType(...)` and instead finding the compilation unit
the `std::function` wrapped callable should be in and then searching for the
callable directly in the CU.
https://reviews.llvm.org/D69913
Files:
include/lldb/Symbol/CompileUnit.h
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Symbol/CompileUnit.cpp
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -568,6 +568,11 @@
"weak_ptr synthetic children",
ConstString("^(std::__[[:alnum:]]+::)weak_ptr<.+>(( )?&)?$"),
stl_synth_flags, true);
+ AddCXXSummary(cpp_category_sp,
+ lldb_private::formatters::LibcxxFunctionSummaryProvider,
+ "libc++ std::function summary provider",
+ ConstString("^std::__[[:alnum:]]+::function<.+>$"),
+ stl_summary_flags, true);
stl_summary_flags.SetDontShowChildren(false);
stl_summary_flags.SetSkipPointers(false);
Index: source/Symbol/CompileUnit.cpp
===================================================================
--- source/Symbol/CompileUnit.cpp
+++ source/Symbol/CompileUnit.cpp
@@ -12,6 +12,7 @@
#include "lldb/Symbol/SymbolFile.h"
#include "lldb/Symbol/VariableList.h"
#include "lldb/Target/Language.h"
+#include "lldb/Utility/Timer.h"
using namespace lldb;
using namespace lldb_private;
@@ -81,6 +82,31 @@
return;
}
+lldb::FunctionSP CompileUnit::FindFunction(
+ llvm::function_ref<bool(const llvm::StringRef)> matching_lambda) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat, "CompileUnit::FindFunction");
+
+ lldb::ModuleSP module = CalculateSymbolContextModule();
+
+ if (!module)
+ return {};
+
+ SymbolFile *symbol_file = module->GetSymbolFile();
+
+ if (!symbol_file)
+ return {};
+
+ // m_functions_by_uid is filled in lazily but we need all the entries.
+ symbol_file->ParseFunctions(*this);
+
+ for (auto &p : m_functions_by_uid) {
+ if (matching_lambda(p.second->GetName().GetStringRef()))
+ return p.second;
+ }
+ return {};
+}
+
// Dump the current contents of this object. No functions that cause on demand
// parsing of functions, globals, statics are called, so this is a good
// function to call to get an idea of the current contents of the CompileUnit
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -826,6 +826,8 @@
}
size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat, "SymbolFileDWARF::ParseFunctions");
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
if (!dwarf_cu)
Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===================================================================
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -21,6 +21,7 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/UniqueCStringMap.h"
#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Target/ABI.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/RegisterContext.h"
@@ -28,6 +29,7 @@
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/ThreadPlanRunToAddress.h"
#include "lldb/Target/ThreadPlanStepInRange.h"
+#include "lldb/Utility/Timer.h"
using namespace lldb;
using namespace lldb_private;
@@ -61,6 +63,10 @@
CPPLanguageRuntime::LibCppStdFunctionCallableInfo
CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
lldb::ValueObjectSP &valobj_sp) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat,
+ "CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo");
+
LibCppStdFunctionCallableInfo optional_info;
if (!valobj_sp)
@@ -93,7 +99,7 @@
// this entry and lookup operator()() and obtain the line table entry.
// 3) a callable object via operator()(). We will obtain the name of the
// object from the first template parameter from __func's vtable. We will
- // look up the objectc operator()() and obtain the line table entry.
+ // look up the objects operator()() and obtain the line table entry.
// 4) a member function. A pointer to the function will stored after the
// we will obtain the name from this pointer.
// 5) a free function. A pointer to the function will stored after the vtable
@@ -130,6 +136,12 @@
if (status.Fail())
return optional_info;
+ lldb::addr_t vtable_address_first_entry =
+ process->ReadPointerFromMemory(vtable_address + address_size, status);
+
+ if (status.Fail())
+ return optional_info;
+
lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
// As commened above we may not have a function pointer but if we do we will
// need it.
@@ -144,9 +156,15 @@
if (target.GetSectionLoadList().IsEmpty())
return optional_info;
+ Address vtable_first_entry_resolved;
+
+ if (!target.GetSectionLoadList().ResolveLoadAddress(
+ vtable_address_first_entry, vtable_first_entry_resolved))
+ return optional_info;
+
Address vtable_addr_resolved;
SymbolContext sc;
- Symbol *symbol;
+ Symbol *symbol = nullptr;
if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
vtable_addr_resolved))
@@ -192,10 +210,10 @@
function_address_resolved, eSymbolContextEverything, sc);
symbol = sc.symbol;
}
-
- auto contains_lambda_identifier = []( llvm::StringRef & str_ref ) {
- return str_ref.contains("$_") || str_ref.contains("'lambda'");
- };
+
+ auto contains_lambda_identifier = [](llvm::StringRef &str_ref) {
+ return str_ref.contains("$_") || str_ref.contains("'lambda'");
+ };
// Case 4 or 5
// We eliminate these cases early because they don't need the potentially
@@ -219,8 +237,7 @@
//
// we want to append ::operator()()
if (contains_lambda_identifier(first_template_parameter))
- return llvm::Regex::escape(first_template_parameter.str()) +
- R"(::operator\(\)\(.*\))";
+ return first_template_parameter.str() + R"(::operator()()";
if (symbol != nullptr &&
symbol->GetName().GetStringRef().contains("__invoke")) {
@@ -234,14 +251,13 @@
//
// We want to slice off __invoke(...) and append operator()()
std::string lambda_operator =
- llvm::Regex::escape(symbol_name.slice(0, pos2 + 1).str()) +
- R"(operator\(\)\(.*\))";
+ symbol_name.slice(0, pos2 + 1).str() + R"(operator()()";
return lambda_operator;
}
// Case 3
- return first_template_parameter.str() + R"(::operator\(\)\(.*\))";
+ return first_template_parameter.str() + R"(::operator()()";
;
};
@@ -253,8 +269,41 @@
SymbolContextList scl;
- target.GetImages().FindSymbolsMatchingRegExAndType(
- RegularExpression{R"(^)" + func_to_match}, eSymbolTypeAny, scl, true);
+ CompileUnit *vtable_cu =
+ vtable_first_entry_resolved.CalculateSymbolContextCompileUnit();
+ llvm::StringRef name_to_use = func_to_match;
+ bool is_lambda = contains_lambda_identifier(name_to_use);
+
+ // Case 3, we have a callable object instead of a lambda
+ //
+ // TODO
+ // We currently don't support this case a callable object may have multiple
+ // operator()() varying on const/non-const and number of arguments and we
+ // don't have a way to currently distinguish them so we will bail out now.
+ if (!is_lambda)
+ return {};
+
+ size_t pos = llvm::StringRef::npos;
+ pos = name_to_use.find("::operator");
+
+ if (vtable_cu) {
+ llvm::StringRef sliced_name = name_to_use.slice(0, pos);
+ auto find_function_matcher = [sliced_name](llvm::StringRef name) {
+ if (name.startswith(sliced_name) && name.contains("operator"))
+ if (name.contains("$_") || name.contains("'lambda'"))
+ return true;
+
+ return false;
+ };
+
+ lldb::FunctionSP fsp = vtable_cu->FindFunction(find_function_matcher);
+
+ if (fsp) {
+ SymbolContext sc;
+ fsp->CalculateSymbolContext(&sc);
+ scl.Append(sc);
+ }
+ }
// Case 1,2 or 3
if (scl.GetSize() >= 1) {
Index: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
@@ -32,7 +32,9 @@
return f_mem(bar1) + // Set break point at this line.
f1(acc,acc) + // Source main invoking f1
f2(acc) + // Set break point at this line.
- f3(acc+1,acc+2) + // Set break point at this line.
- f4() + // Set break point at this line.
+ f3(acc+1,acc+2) + // Set break point at this line.
+ // TODO reenable this case when std::function formatter supports
+ // general callable object case.
+ //f4() + // Set break point at this line.
f5(bar1, 10); // Set break point at this line.
}
Index: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
===================================================================
--- packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
+++ packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
@@ -59,10 +59,12 @@
self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
process.Continue()
- thread.StepInto()
- self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_operator_line ) ;
- self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
- process.Continue()
+ # TODO reenable this case when std::function formatter supports
+ # general callable object case.
+ #thread.StepInto()
+ #self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_operator_line ) ;
+ #self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
+ #process.Continue()
thread.StepInto()
self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_add_num_line ) ;
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
+++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
@@ -22,19 +22,16 @@
self.expect("frame variable " + variable,
substrs=[variable + " = " + result_to_match])
self.expect("log timers dump",
- substrs=["lldb_private::Module::FindSymbolsMatchingRegExAndType"])
+ substrs=["lldb_private::CompileUnit::FindFunction"])
self.runCmd("log timers reset")
self.expect("frame variable " + variable,
substrs=[variable + " = " + result_to_match])
self.expect("log timers dump",
matching=False,
- substrs=["lldb_private::Module::FindSymbolsMatchingRegExAndType"])
+ substrs=["lldb_private::CompileUnit::FindFunction"])
- # Temporarily skipping for everywhere b/c we are disabling the std::function formatter
- # due to performance issues but plan on turning it back on once they are addressed.
- @skipIf
@add_test_categories(["libc++"])
def test(self):
"""Test that std::function as defined by libc++ is correctly printed by LLDB"""
@@ -62,7 +59,9 @@
self.run_frame_var_check_cache_use("f2", "Lambda in File main.cpp at Line 43")
self.run_frame_var_check_cache_use("f3", "Lambda in File main.cpp at Line 47")
- self.run_frame_var_check_cache_use("f4", "Function in File main.cpp at Line 16")
+ # TODO reenable this case when std::function formatter supports
+ # general callable object case.
+ #self.run_frame_var_check_cache_use("f4", "Function in File main.cpp at Line 16")
# These cases won't hit the cache at all but also don't require
# an expensive lookup.
Index: include/lldb/Symbol/CompileUnit.h
===================================================================
--- include/lldb/Symbol/CompileUnit.h
+++ include/lldb/Symbol/CompileUnit.h
@@ -163,6 +163,16 @@
void ForeachFunction(
llvm::function_ref<bool(const lldb::FunctionSP &)> lambda) const;
+ /// Find a function in the compile unit based on the predicate matching_lambda
+ ///
+ /// \param[in] matching_lambda
+ /// A predicate used to evaluate each function name.
+ ///
+ /// \return
+ /// The FunctionSP containing the first matching entry.
+ lldb::FunctionSP
+ FindFunction(llvm::function_ref<bool(const llvm::StringRef)> matching_lambda);
+
/// Dump the compile unit contents to the stream \a s.
///
/// \param[in] s
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
