llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.26k stars 12.09k forks source link

[LLDB] Unreliable CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo #111291

Open mentlerd opened 1 month ago

mentlerd commented 1 month ago

CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo fails to extract the callable information from an std::function which is constructed from a lambda inside a function which has multiple parameters due to this part of it's implementation:

https://github.com/llvm/llvm-project/blob/8ebd4019500cbec28426f19e6440484d53ecaecd/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp#L293-L311

As a consequence, std::function summary cannot print the target function's description, and stepping into operator() does not work.

I have augmented LLDB's logging locally to confirm that this is indeed where things go wrong:

#include <functional>

std::function<void()> GetFunc() {
    return []{ printf("func\n"); };
}
std::function<void()> GetFunc1(int a) {
    return [=]{ printf("func1(%d)\n", a); };
}
std::function<void()> GetFunc2(int a, int b) {
    return [=]{ printf("func2(%d,%d)\n", a, b); };
}

int main() {
    auto f1 = GetFunc();
    auto f2 = GetFunc1(10);
    auto f3 = GetFunc2(10, 10);

    printf("Breakpoint here\n");
    /*
        vtable_name='vtable for std::__1::__function::__func<GetFunc()::$_0, std::__1::allocator<GetFunc()::$_0>, void ()>'
        first_template_parameter='GetFunc()::$_0'

        vtable_name='vtable for std::__1::__function::__func<GetFunc1(int)::$_0, std::__1::allocator<GetFunc1(int)::$_0>, void ()>'
        first_template_parameter='GetFunc1(int)::$_0'

        vtable_name='vtable for std::__1::__function::__func<GetFunc2(int, int)::$_0, std::__1::allocator<GetFunc2(int, int)::$_0>, void ()>'
        first_template_parameter='GetFunc2(int'
    */
    return 0;
}
mentlerd commented 1 month ago

I have been interested in LLDB for a while now, and this looks like a fairly isolated and low impact problem that I would like to try to fix it.

At a first glance some naive algorithm that considers parentheses could work here, but I think somehow using the SBType system to extract the first template argument type would be much more robust. WDYT?

(My experience in interacting LLDB is through writing synthetic children providers for various custom container types written in C++. Some pointers as to how one would go about fixing this would be much appreciated!)

llvmbot commented 1 month ago

@llvm/issue-subscribers-lldb

Author: David Mentler (mentlerd)

`CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo` fails to extract the callable information from an `std::function` which is constructed from a lambda inside a function which has multiple parameters due to this part of it's implementation: https://github.com/llvm/llvm-project/blob/8ebd4019500cbec28426f19e6440484d53ecaecd/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp#L293-L311 As a consequence, `std::function` summary cannot print the target function's description, and stepping into `operator()` does not work. I have augmented LLDB's logging locally to confirm that this is indeed where things go wrong: ```c++ #include <functional> std::function<void()> GetFunc() { return []{ printf("func\n"); }; } std::function<void()> GetFunc1(int a) { return [=]{ printf("func1(%d)\n", a); }; } std::function<void()> GetFunc2(int a, int b) { return [=]{ printf("func2(%d,%d)\n", a, b); }; } int main() { auto f1 = GetFunc(); auto f2 = GetFunc1(10); auto f3 = GetFunc2(10, 10); printf("Breakpoint here\n"); /* vtable_name='vtable for std::__1::__function::__func<GetFunc()::$_0, std::__1::allocator<GetFunc()::$_0>, void ()>' first_template_parameter='GetFunc()::$_0' vtable_name='vtable for std::__1::__function::__func<GetFunc1(int)::$_0, std::__1::allocator<GetFunc1(int)::$_0>, void ()>' first_template_parameter='GetFunc1(int)::$_0' vtable_name='vtable for std::__1::__function::__func<GetFunc2(int, int)::$_0, std::__1::allocator<GetFunc2(int, int)::$_0>, void ()>' first_template_parameter='GetFunc2(int' */ return 0; } ```
Michael137 commented 1 month ago

Good catch, thanks for reporting!

At a first glance some naive algorithm that considers parentheses could work here, but I think somehow using the SBType system to extract the first template argument type would be much more robust. WDYT?

Agreed, I don't think we want to get into the business of parsing C++ in yet another place (we already have the CPlusPlusNameParser, and some ad-hoc parsing here-and-there). If feasible, using a lldb_private::CompilerType to get the template argument names would be great. There's also a recently added GetVTableInfo API, which one could try to use to simplify some of the existing logic in FindLibCppStdFunctionCallableInfo, though haven't looked at it closely to know if it would help much here.

Taking a step back, is there some way the compiler can help us here? By making all these heuristics easier to determine. Or better yet, use DW_AT_trampoline to step through to the callable (maybe using https://github.com/llvm/llvm-project/pull/110188)?

mentlerd commented 1 month ago

If feasible, using a lldb_private::CompilerType to get the template argument names would be great. There's also a recently added GetVTableInfo API, which one could try to use to simplify some of the existing logic in FindLibCppStdFunctionCallableInfo, though haven't looked at it closely to know if it would help much here.

GetVTableInfo lead me to ValueObjectVTable which made getting the virtual functions corresponding to the stored __value_func's type very convenient and succint! However I am a bit stuck with obtaining the class type for these member functions..

I have stepped through Function::GetCompilerType() which at some point seems to partially (?) reconstruct the ClassTemplateSpecializationDecl containing the virtual function, including the fact that this class has the template parameters we are interested in. However it does not seem like I can get to this representation from the returned CompilerType.

Taking a step back, is there some way the compiler can help us here? By making all these heuristics easier to determine. Or better yet, use DW_AT_trampoline to step through to the callable (maybe using https://github.com/llvm/llvm-project/pull/110188)?

That attribute looks interesting - but sounds like a broader subject to teach stepping thread plans to handle trampolines properly.

Michael137 commented 1 month ago

If feasible, using a lldb_private::CompilerType to get the template argument names would be great. There's also a recently added GetVTableInfo API, which one could try to use to simplify some of the existing logic in FindLibCppStdFunctionCallableInfo, though haven't looked at it closely to know if it would help much here.

GetVTableInfo lead me to ValueObjectVTable which made getting the virtual functions corresponding to the stored __value_func's type very convenient and succint! However I am a bit stuck with obtaining the class type for these member functions..

Sounds like a good refactor to do in a separate NFC commit (if it all works as before)!

I have stepped through Function::GetCompilerType() which at some point seems to partially (?) reconstruct the ClassTemplateSpecializationDecl containing the virtual function, including the fact that this class has the template parameters we are interested in. However it does not seem like I can get to this representation from the returned CompilerType.

Can you share the AST that you're dealing with? Note, we do a "best effort" reconstruction of the ClassTemplateSpecializationDecl, but we only have the DWARF representation to work with, so it will never be full fidelity. Which means we sometimes (especially for function templates) don't represent the AST in the exact same way that Clang would do. But as long as we can get to the template parameter we should be ok in this case. Can you use, e.g., GetTypeTemplateArgument to get to the template argument?

Taking a step back, is there some way the compiler can help us here? By making all these heuristics easier to determine. Or better yet, use DW_AT_trampoline to step through to the callable (maybe using #110188)?

That attribute looks interesting - but sounds like a broader subject to teach stepping thread plans to handle trampolines properly.

Yea, would be good to track this as an eventual goal.

Michael137 commented 1 month ago

That attribute looks interesting - but sounds like a broader subject to teach stepping thread plans to handle trampolines properly.

@augusto2112 will be able to comment on whether the thread plan infrastructure is already in place to deal with the debug_transparent attribute

mentlerd commented 1 month ago

Can you share the AST that you're dealing with? Note, we do a "best effort" reconstruction of the ClassTemplateSpecializationDecl, but we only have the DWARF representation to work with, so it will never be full fidelity. Which means we sometimes (especially for function templates) don't represent the AST in the exact same way that Clang would do. But as long as we can get to the template parameter we should be ok in this case. Can you use, e.g., GetTypeTemplateArgument to get to the template argument?

In the meantime I have managed to get to a point where I could obtain a CompilerDeclContext for the underlying CXXMethodDecl of the virtual functions, but got stuck here again. There does not seem to be a type system agnostic API to walk upwards to the enclosing context, and convert it back into a CompilerType to ask for template arguments :/

Is this an intentional design decision, or simply the lack of need so far? What would be the appropriate conversion functions to add?

As far as reconstructed AST goes, this is the most recent output I got while exploring in the debugger:

(lldb) e clang_decl_ctx_of_resolved_func->getParent()->dumpAsDecl();
ClassTemplateSpecializationDecl 0x15598cd18 <<invalid sloc>> <invalid sloc> <undeserialized declarations> class __func explicit_specialization
|-TemplateArgument type '(unnamed class)'
| `-RecordType 0x144711c20 '(unnamed class)'
|   `-CXXRecord 0x144711b70 ''                                        <--- Trouble ahead?
|-TemplateArgument type 'std::allocator<(unnamed class)>'
| `-RecordType 0x1540bd5c0 'std::allocator<(unnamed class)>'
|   `-ClassTemplateSpecialization 0x1540bd4c8 'allocator'
|-TemplateArgument type 'int (int)'
| `-FunctionProtoType 0x144712040 'int (int)' cdecl
|   |-BuiltinType 0x14470dd10 'int'
|   `-BuiltinType 0x14470dd10 'int'
|-AccessSpecDecl 0x15598d078 <<invalid sloc>> <invalid sloc> public
`-CXXMethodDecl 0x15598cf08 <<invalid sloc>> <invalid sloc> __clone 'std::__function::__base<int (int)> *() const' virtual
  `-AsmLabelAttr 0x15598cfb0 <<invalid sloc>> Implicit "_ZNKSt3__110__function6__funcIZ4foo25DummyIbiEE3$_0NS_9allocatorIS4_EEFiiEE7__cloneEv"

This is the relevant part of the testcase that I have tweaked to break, and I am trying to fix:

template<typename = bool, typename = int>
struct Dummy {
  // Used to make lambda host function's symbol more complex
};

int foo2(Dummy<> dummy = {}) {
   auto f = [](int x) {
       return x+1;
   };

   std::function<int (int)> foo2_f = f;

   return foo2_f(10); // Set break point at this line.
}
jimingham commented 1 month ago

If feasible, using a lldb_private::CompilerType to get the template argument names would be great. There's also a recently added GetVTableInfo API, which one could try to use to simplify some of the existing logic in FindLibCppStdFunctionCallableInfo, though haven't looked at it closely to know if it would help much here.

GetVTableInfo lead me to ValueObjectVTable which made getting the virtual functions corresponding to the stored __value_func's type very convenient and succint! However I am a bit stuck with obtaining the class type for these member functions..

I have stepped through Function::GetCompilerType() which at some point seems to partially (?) reconstruct the ClassTemplateSpecializationDecl containing the virtual function, including the fact that this class has the template parameters we are interested in. However it does not seem like I can get to this representation from the returned CompilerType.

Taking a step back, is there some way the compiler can help us here? By making all these heuristics easier to determine. Or better yet, use DW_AT_trampoline to step through to the callable (maybe using #110188)?

That attribute looks interesting - but sounds like a broader subject to teach stepping thread plans to handle trampolines properly.

I'm not sure what you mean here?

The "debug_transparent" algorithm that Augusto introduced is for cases where if you just stepi through the function you've arrived in, you will step directly to the "interesting" function that the transparent one was wrapping. IIRC, when you do std::function you get a bunch of frames on the stack between you and where you want to go. We would rather not have algorithms that just stepi for some indeterminate number of steps in the hopes of getting somewhere useful - that can be very slow, particularly for remote sessions where latency is an issue. So I don't think that's appropriate for these purposes.

There's already a generic architecture to handle stepping through trampolines. Sometimes that has to be a dynamic calculation (e.g. ObjC message dispatch which you really can't know where it will go till you try and can change through the execution of the program - even for the same object) and others times the trampoline recognizer is static - like the "step through cross-library symbols" which just looks at the symbol.

The routine that handles stepping through trampolines for std::function is CPPLanguageRuntime::GetStepThroughTrampolinePlan which uses the Callable detection to figure out the target. If the std::function invocation that starts the process is in code for which we have debug information, then shortcutting that detection by using the value of the DW_AT_trampoline would be fine. It would be a shame to only be able to step through std::function dispatches for which we have debug info, however. So I don't think this should supplant the direct inspection method, however.

Michael137 commented 1 month ago

In the meantime I have managed to get to a point where I could obtain a CompilerDeclContext for the underlying CXXMethodDecl of the virtual functions, but got stuck here again. There does not seem to be a type system agnostic API to walk upwards to the enclosing context, and convert it back into a CompilerType to ask for template arguments :/

Is this an intentional design decision, or simply the lack of need so far? What would be the appropriate conversion functions to add?

As far as reconstructed AST goes, this is the most recent output I got while exploring in the debugger:

(lldb) e clang_decl_ctx_of_resolved_func->getParent()->dumpAsDecl();
ClassTemplateSpecializationDecl 0x15598cd18 <<invalid sloc>> <invalid sloc> <undeserialized declarations> class __func explicit_specialization
|-TemplateArgument type '(unnamed class)'
| `-RecordType 0x144711c20 '(unnamed class)'
|   `-CXXRecord 0x144711b70 ''                                        <--- Trouble ahead?
|-TemplateArgument type 'std::allocator<(unnamed class)>'
| `-RecordType 0x1540bd5c0 'std::allocator<(unnamed class)>'
|   `-ClassTemplateSpecialization 0x1540bd4c8 'allocator'
|-TemplateArgument type 'int (int)'
| `-FunctionProtoType 0x144712040 'int (int)' cdecl
|   |-BuiltinType 0x14470dd10 'int'
|   `-BuiltinType 0x14470dd10 'int'
|-AccessSpecDecl 0x15598d078 <<invalid sloc>> <invalid sloc> public
`-CXXMethodDecl 0x15598cf08 <<invalid sloc>> <invalid sloc> __clone 'std::__function::__base<int (int)> *() const' virtual
  `-AsmLabelAttr 0x15598cfb0 <<invalid sloc>> Implicit "_ZNKSt3__110__function6__funcIZ4foo25DummyIbiEE3$_0NS_9allocatorIS4_EEFiiEE7__cloneEv"

Hmmm yea the DWARF for this looks a bit unhelpful if we wanted to determine whether the template parameter is a lambda here:

0x000005f4:           DW_TAG_subprogram
                        DW_AT_linkage_name      ("_ZNKSt3__110__function6__funcIZ4foo25DummyIbiEE3$_0NS_9allocatorIS4_EEFiiEE7__cloneEPNS0_6__baseIS7_EE")
                        DW_AT_name      ("__clone")
                        DW_AT_virtuality        (DW_VIRTUALITY_virtual)
                        DW_AT_vtable_elem_location      (DW_OP_constu 0x3)
                        DW_AT_containing_type   (0x0000055a "std::__1::__function::__func<(lambda at lambda.cpp:9:14), std::__1::allocator<(lambda at lambda.cpp:9:14)>, int (int)>")

0x0000055a:         DW_TAG_class_type
                      DW_AT_containing_type     (0x000006a0 "std::__1::__function::__base<int (int)>")
                      DW_AT_name        ("__func<(lambda at lambda.cpp:9:14), std::__1::allocator<(lambda at lambda.cpp:9:14)>, int (int)>")

0x00000565:           DW_TAG_template_type_parameter
                        DW_AT_type      (0x00002e11 "class ")
                        DW_AT_name      ("_FD")

0x00002e11:     DW_TAG_class_type
                  DW_AT_calling_convention      (DW_CC_pass_by_value)
                  DW_AT_byte_size       (0x01)
                  DW_AT_decl_file       ("/Users/michaelbuch/lambda.cpp")
                  DW_AT_decl_line       (9)

0x00002e17:       DW_TAG_subprogram
                    DW_AT_name  ("operator()")
                    DW_AT_decl_file     ("/Users/michaelbuch/lambda.cpp")
                    DW_AT_decl_line     (9)
                    DW_AT_type  (0x00002dea "int")
                    DW_AT_declaration   (true)

This is why we end up with the CXXRecords with empty names. So really the only evidence we're dealing with a lambda is the lambda at lambda.cpp bit, which we probably don't want to rely upon as it's a demangler implementation detail.

With GCC the lambda actually gets a DW_AT_name <lambda>:

0x000011d9:       DW_TAG_subprogram
                    DW_AT_name  ("<lambda>")
                    DW_AT_artificial    (true)
                    DW_AT_declaration   (true)
                    DW_AT_object_pointer    (0x000011e6)
                    DW_AT_sibling   (0x000011f1)

(which we might actually want to adopt in Clang since it would make LLDB's life slightly easier (CC @dwblaikie)).

But even if we determine that this is a lambda, later down the line in FindLibCppStdFunctionCallableInfo we use the template parameter name to do a FindFunctions call and find the lambda's operator(). What do you get when you try to dump this decl CXXRecord 0x144711b70 ''. Does it have the operator() attached to it?

mentlerd commented 1 month ago

In the meantime I have managed to get to a point where I could obtain a CompilerDeclContext for the underlying CXXMethodDecl of the virtual functions, but got stuck here again. There does not seem to be a type system agnostic API to walk upwards to the enclosing context, and convert it back into a CompilerType to ask for template arguments :/

Is this an intentional design decision, or simply the lack of need so far? What would be the appropriate conversion functions to add?

For the time being I added these (possibly too powerful/clang type system specific) APIs:

virtual CompilerDecl TypeSystem::DeclContextGetDecl(void *opaque_decl_ctx) {
  return CompilerDecl();
}

CompilerDecl TypeSystemClang::DeclContextGetDecl(void *opaque_decl_ctx) override; 

CompilerDecl CompilerDeclContext::GetDecl() const;

This allows me to get to the member function declaration's enclosing class, which luckily contains pretty much everything to be able to tell apart the various types that std::function can wrap, including the generic callable case which the current tests call out as unsupported 🚀

https://github.com/llvm/llvm-project/blob/35684fa4bc8d2288d479cb8aa9d275b14bfefead/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp#L50

// f1
int (*)(int, int)

// f2
class  {
    int acc;
    std::function<int (int, int)> f1;
public:
    int operator()(int) const;
}

// f3
int (*)(int, int)

// f4
struct Bar {
    int add_num2(int);
    int operator()();
    int add_num(int) const;
}

// f5
int (struct Bar::*)(int)

This is why we end up with the CXXRecords with empty names. So really the only evidence we're dealing with a lambda is the lambda at lambda.cpp bit, which we probably don't want to rely upon as it's a demangler implementation detail.

I think the lack of class name combined with the sole call operator member function is good enough to call something a lambda until clang starts emitting a DW_AT_name for the class.

Michael137 commented 1 month ago

In the meantime I have managed to get to a point where I could obtain a CompilerDeclContext for the underlying CXXMethodDecl of the virtual functions, but got stuck here again. There does not seem to be a type system agnostic API to walk upwards to the enclosing context, and convert it back into a CompilerType to ask for template arguments :/ Is this an intentional design decision, or simply the lack of need so far? What would be the appropriate conversion functions to add?

For the time being I added these (possibly too powerful/clang type system specific) APIs:

virtual CompilerDecl TypeSystem::DeclContextGetDecl(void *opaque_decl_ctx) {
  return CompilerDecl();
}

CompilerDecl TypeSystemClang::DeclContextGetDecl(void *opaque_decl_ctx) override; 

CompilerDecl CompilerDeclContext::GetDecl() const;

This allows me to get to the member function declaration's enclosing class, which luckily contains pretty much everything to be able to tell apart the various types that std::function can wrap, including the generic callable case which the current tests call out as unsupported 🚀

https://github.com/llvm/llvm-project/blob/35684fa4bc8d2288d479cb8aa9d275b14bfefead/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp#L50

Sounds promising, thanks! I'd suggest you put up a draft PR and we can iterate from there. Hard to say exactly which APIs do or do not exist without seeing the usage