shafik updated this revision to Diff 228533.
shafik marked 6 inline comments as done.
shafik added a comment.
After updating branch the stepping test started failing. It was failing because
the __invoke case was subtly broken in this patch. Fixing that case opened up a
lot of refactoring opportunities. So this patch ends up being a large refactor
and in many ways a simplification now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69913/new/
https://reviews.llvm.org/D69913
Files:
lldb/include/lldb/Symbol/CompileUnit.h
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/CompileUnit.cpp
Index: lldb/source/Symbol/CompileUnit.cpp
===================================================================
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/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: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -828,6 +828,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: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/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;
@@ -58,9 +60,53 @@
return false;
}
+bool contains_lambda_identifier(llvm::StringRef &str_ref) {
+ return str_ref.contains("$_") || str_ref.contains("'lambda'");
+};
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+line_entry_helper(Target &target, const SymbolContext &sc, Symbol *symbol,
+ llvm::StringRef first_template_param_sref,
+ bool has___invoke) {
+
+ CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
+
+ AddressRange range;
+ sc.GetAddressRange(eSymbolContextEverything, 0, false, range);
+
+ Address address = range.GetBaseAddress();
+
+ Address addr;
+ if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
+ addr)) {
+ LineEntry line_entry;
+ addr.CalculateSymbolContextLineEntry(line_entry);
+
+ if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+ // Case 1 and 2
+ optional_info.callable_case = lldb_private::CPPLanguageRuntime::
+ LibCppStdFunctionCallableCase::Lambda;
+ } else {
+ // Case 3
+ optional_info.callable_case = lldb_private::CPPLanguageRuntime::
+ LibCppStdFunctionCallableCase::CallableObject;
+ }
+
+ optional_info.callable_symbol = *symbol;
+ optional_info.callable_line_entry = line_entry;
+ optional_info.callable_address = addr;
+ }
+
+ return optional_info;
+}
+
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 +139,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
@@ -113,6 +159,9 @@
optional_info.member__f_pointer_value = member__f_pointer_value;
+ if (!member__f_pointer_value)
+ return optional_info;
+
ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
Process *process = exe_ctx.GetProcessPtr();
@@ -130,8 +179,14 @@
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
+ // As commented above we may not have a function pointer but if we do we will
// need it.
lldb::addr_t possible_function_address =
process->ReadPointerFromMemory(address_after_vtable, status);
@@ -144,9 +199,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))
@@ -159,7 +220,7 @@
if (symbol == nullptr)
return optional_info;
- llvm::StringRef vtable_name(symbol->GetName().GetCString());
+ llvm::StringRef vtable_name(symbol->GetName().GetStringRef());
bool found_expected_start_string =
vtable_name.startswith("vtable for std::__1::__function::__func<");
@@ -172,6 +233,11 @@
// ... __func<main::$_0, std::__1::allocator<main::$_0> ...
// ^^^^^^^^^
//
+ // We could see names such as:
+ // main::$_0
+ // Bar::add_num2(int)::'lambda'(int)
+ // Bar
+ //
// We do this by find the first < and , and extracting in between.
//
// This covers the case of the lambda known at compile time.
@@ -192,17 +258,29 @@
function_address_resolved, eSymbolContextEverything, sc);
symbol = sc.symbol;
}
-
- auto contains_lambda_identifier = []( llvm::StringRef & str_ref ) {
- return str_ref.contains("$_") || str_ref.contains("'lambda'");
- };
+
+ // These conditions are used several times to simplify statements later on.
+ bool has___invoke =
+ (symbol ? symbol->GetName().GetStringRef().contains("__invoke") : false);
+ auto calculate_symbol_context_helper = [](auto &t,
+ SymbolContextList &sc_list) {
+ SymbolContext sc;
+ t->CalculateSymbolContext(&sc);
+ sc_list.Append(sc);
+ };
+
+ // Case 2
+ if (has___invoke) {
+ SymbolContextList scl;
+ calculate_symbol_context_helper(symbol, scl);
+
+ return line_entry_helper(target, scl[0], symbol, first_template_parameter,
+ has___invoke);
+ }
// Case 4 or 5
- // We eliminate these cases early because they don't need the potentially
- // expensive lookup through the symbol table.
if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for") &&
- !contains_lambda_identifier(first_template_parameter) &&
- !symbol->GetName().GetStringRef().contains("__invoke")) {
+ !contains_lambda_identifier(first_template_parameter) && !has___invoke) {
optional_info.callable_case =
LibCppStdFunctionCallableCase::FreeOrMemberFunction;
optional_info.callable_address = function_address_resolved;
@@ -211,41 +289,7 @@
return optional_info;
}
- auto get_name = [&first_template_parameter, &symbol, contains_lambda_identifier]() {
- // Given case 1:
- //
- // main::$_0
- // Bar::add_num2(int)::'lambda'(int)
- //
- // we want to append ::operator()()
- if (contains_lambda_identifier(first_template_parameter))
- return llvm::Regex::escape(first_template_parameter.str()) +
- R"(::operator\(\)\(.*\))";
-
- if (symbol != nullptr &&
- symbol->GetName().GetStringRef().contains("__invoke")) {
-
- llvm::StringRef symbol_name = symbol->GetName().GetStringRef();
- size_t pos2 = symbol_name.find_last_of(':');
-
- // Given case 2:
- //
- // main::$_1::__invoke(...)
- //
- // 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\(\)\(.*\))";
-
- return lambda_operator;
- }
-
- // Case 3
- return first_template_parameter.str() + R"(::operator\(\)\(.*\))";
- ;
- };
-
- std::string func_to_match = get_name();
+ std::string func_to_match = first_template_parameter.str();
auto it = CallableLookupCache.find(func_to_match);
if (it != CallableLookupCache.end())
@@ -253,41 +297,40 @@
SymbolContextList scl;
- target.GetImages().FindSymbolsMatchingRegExAndType(
- RegularExpression{R"(^)" + func_to_match}, eSymbolTypeAny, scl);
+ CompileUnit *vtable_cu =
+ vtable_first_entry_resolved.CalculateSymbolContextCompileUnit();
+ llvm::StringRef name_to_use = func_to_match;
- // Case 1,2 or 3
- if (scl.GetSize() >= 1) {
- SymbolContext sc2 = scl[0];
-
- AddressRange range;
- sc2.GetAddressRange(eSymbolContextEverything, 0, false, range);
-
- Address address = range.GetBaseAddress();
-
- Address addr;
- if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
- addr)) {
- LineEntry line_entry;
- addr.CalculateSymbolContextLineEntry(line_entry);
-
- if (contains_lambda_identifier(first_template_parameter) ||
- (symbol != nullptr &&
- symbol->GetName().GetStringRef().contains("__invoke"))) {
- // Case 1 and 2
- optional_info.callable_case = LibCppStdFunctionCallableCase::Lambda;
- } else {
- // Case 3
- optional_info.callable_case =
- LibCppStdFunctionCallableCase::CallableObject;
- }
-
- optional_info.callable_symbol = *symbol;
- optional_info.callable_line_entry = line_entry;
- optional_info.callable_address = addr;
+ // 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 (!contains_lambda_identifier(name_to_use))
+ return optional_info;
+
+ if (vtable_cu && !has___invoke) {
+ auto find_function_matcher = [name_to_use](llvm::StringRef name) {
+ if (name.startswith(name_to_use) && name.contains("operator"))
+ return true;
+
+ return false;
+ };
+
+ lldb::FunctionSP func_sp = vtable_cu->FindFunction(find_function_matcher);
+
+ if (func_sp) {
+ calculate_symbol_context_helper(func_sp, scl);
}
}
+ // Case 1 or 3
+ if (scl.GetSize() >= 1) {
+ optional_info = line_entry_helper(target, scl[0], symbol,
+ first_template_parameter, has___invoke);
+ }
+
CallableLookupCache[func_to_match] = optional_info;
return optional_info;
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -567,6 +567,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: lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
+++ lldb/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: lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
+++ lldb/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: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
@@ -17,24 +17,22 @@
# Run frame var for a variable twice. Verify we do not hit the cache
# the first time but do the second time.
- def run_frame_var_check_cache_use(self, variable, result_to_match):
+ def run_frame_var_check_cache_use(self, variable, result_to_match, skip_find_function=False):
self.runCmd("log timers reset")
self.expect("frame variable " + variable,
substrs=[variable + " = " + result_to_match])
- self.expect("log timers dump",
- substrs=["lldb_private::Module::FindSymbolsMatchingRegExAndType"])
+ if not skip_find_function:
+ self.expect("log timers dump",
+ 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"""
@@ -61,8 +59,10 @@
lldbutil.continue_to_breakpoint(self.process(), bkpt)
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")
+ self.run_frame_var_check_cache_use("f3", "Lambda in File main.cpp at Line 47", True)
+ # 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: lldb/include/lldb/Symbol/CompileUnit.h
===================================================================
--- lldb/include/lldb/Symbol/CompileUnit.h
+++ lldb/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
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
