Page MenuHomePhabricator

LLDB: Fix resolving nested template parameters
Needs ReviewPublic

Authored by PatriosTheGreat on May 30 2022, 10:14 AM.

Details

Summary

Right now LLDB ignores nested template type:

echo "
template <typename T>
struct A {};
int main() {
  A<A<int>> s;
}
" > sample.cc

clang++ sample.cc -g -O0
lldb-15 a.out -o "breakpoint set -l 6 -f sample.cc" -o "run" -o "frame variable"

The result:
(A<A<>>) s = {}

It looks like this CL introduced this behavior: https://reviews.llvm.org/D92425
Before the LLDB was resolving this type correctly:

lldb-11 a.out -o "breakpoint set -l 6 -f sample.cc" -o "run" -o "frame variable"
(A<A<int>) s = {}

I discussed this issue with Raphael in discord:
https://discord.com/channels/636084430946959380/636732809708306432/980825811714191371

Apparently in this case Clang emits A<int> as a forward declaration:

./llvm-dwarfdump a.out

0x000005b4:   DW_TAG_base_type
                DW_AT_name      ("int")
                DW_AT_encoding  (DW_ATE_signed)
                DW_AT_byte_size (0x04)
0x000005bb:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("A<A<int> >")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/home/teemperor/test/args.cpp")
                DW_AT_decl_line (2)

0x000005c4:     DW_TAG_template_type_parameter
                  DW_AT_type    (0x000005ce "A<int>")
                  DW_AT_name    ("T")

0x000005cd:     NULL

0x000005ce:   DW_TAG_structure_type
                DW_AT_name      ("A<int>")
                DW_AT_declaration       (true)

0x000005d3:   NULL

So for LLDB it looks the same as template with empty arguments.
Turning back the old logic is fixing this issue. Other tests from LLVM test suite on Linux seems to be green.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald Transcript
PatriosTheGreat requested review of this revision.May 30 2022, 10:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2022, 10:14 AM
PatriosTheGreat edited the summary of this revision. (Show Details)May 30 2022, 10:17 AM
PatriosTheGreat added a reviewer: werat.
PatriosTheGreat edited the summary of this revision. (Show Details)May 31 2022, 9:51 AM
PatriosTheGreat edited the summary of this revision. (Show Details)May 31 2022, 9:55 AM
labath added a subscriber: labath.Jun 10 2022, 3:19 AM

I don't really have the full context here, but I am wondering if we shouldn't somehow take the DW_AT_declaration attribute into account here. It seems like that should give a more definitive answer as to whether we can expect to see a full set of template parameters or not.

teemperor added a comment.EditedJun 13 2022, 12:08 AM

I don't really have the full context here, but I am wondering if we shouldn't somehow take the DW_AT_declaration attribute into account here. It seems like that should give a more definitive answer as to whether we can expect to see a full set of template parameters or not.

I think that is actually a good approach. I think the current patch rejects forward declarations (which is fine as we don't have the required information to make a template) and empty parameter packs (which is bad as this currently works).

Example: This is currently rejected with this patch (-> we end up creating a class named X<int>) even though we have all the required information:

template<typename ...T>
struct X {};

int main() {
  X<int> x;
}

->

0x00000059:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("X<int>")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/home/teemperor/test/someoneshouldreallypaymeforthis.cpp")
                DW_AT_decl_line (2)

0x00000062:     DW_TAG_GNU_template_parameter_pack
                  DW_AT_name    ("T")

0x00000067:       DW_TAG_template_type_parameter
                    DW_AT_type  (0x00000052 "int")

0x0000006c:       NULL

0x0000006d:     NULL

0x0000006e:   NULL

So I would say we just bail out if we're asked to parse any template declaration and just do the fallback approach that creates a fake class.