[lldb] Support simplified template names

[aeubanks]

Summary

See https://discourse.llvm.org/t/dwarf-using-simplified-template-names/58417 for background on simplified template names.

lldb doesn't work with simplified template names because it uses DW_AT_name which doesn't contain template parameters under simplified template names.

Two major changes are required to make lldb work with simplified template names.

1. When building clang ASTs for struct-like dies, we use the name as a cache key. To distinguish between different instantiations of a template class, we need to add in the template parameters.

2. When looking up types, if the requested type name contains '<' and we didn't initially find any types from the index searching the name, strip the template parameters and search the index, then filter out results with non-matching template parameters. This takes advantage of the clang AST's ability to print full names rather than doing it by ourself.

An alternative is to fix up the names in the index to contain the fully qualified name, but that doesn't respect .debug_names.

[aeubanks]

comments

[aeubanks]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

801 ↗

(On Diff #461963)

moved since now we need the clang AST to rebuild the fully qualified name
the only other caller is some logging

[dblaikie]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

230–231 ↗

(On Diff #461963)

Oh, nifty place to test this - I'd been testing with the default changed in clang itself.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1660–1662

I think there's some existing problems with lldb here (& gdb, as it happens) - this doesn't specify where the parameter pack is in the argument list (looks like it assumes it's the only template parameters?) - and doesn't handle the possibility of multiple parameter packs in a single parameter list, which I think is possible in some corner cases.

I think maybe the existing lldb code can handle some non-pack parameters followed by a single pack, but this code assumes it's either non-pack or a pack, never both - so might be more restrictive than the existing limitations? But maybe not - I might've misread/misremembered the other lldb code.

1666

OK, so this line uses Clang's AST printing (avoiding having to reimplement all the type printing in lldb, like I've done in llvm-dwarfdump/libDebugInfoDWARF) - but I guess there's a reason we can't do that at a higher level for the whole template, but have to do it per template argument?

It'd be nice if we could do it for the whole parameter list/template specialization, to avoid having to have this <>, etc handling.

I guess it's a circular problem - have to build the name to look up the AST if we already have it... so can't build the name /from/ the AST, as such. Fair enough.

1687

What's incorrect about it at the moment?

Oh, I see this code has just moved around.

1692–1693

1707–1710

1822–1840

Maybe? But the named variable and less indentation's probably good too.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2508–2512

Maybe a comment here? I guess what's happening here is that if the name found isn't a template then it isn't relevant/can't match the original query that did have template parameters?

2515–2518

And this checks the stringified template params match exactly - so there's opportunity for improvement in the future to compare with more fuzzy matching (I guess we can't use clang's AST because that'd involve making two instances of the type somehow which doesn't make a lot of sense) so that users don't have to spell the template spec exactly the same way clang does (eg: x<int, int> versus x<int,int> - or other more complicated situations with function pointers, parentheses, casts, etc.

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1652

Any thoughts on making this a private method of DWARFASTParserClang instead of a lambda?

1687

Why move this into ParseStructureLikeDIE? Just to remove the need for a mutable std::string& param? I guess access to die? Might be easier to review (and maintain) as a helper function still

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2497

Minor: might be easier for future readers to cross-reference

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

9356–9359

Minor

Wonder if it's worth commenting why the template_basename may or may not contain a <.

[aeubanks]

address comments

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1660–1662

you're right, fixed to check both, and added test case

1687

so that we have access to ParseTemplateParameterInfos, which calls ParseTemplateDIE which creates clang types and stuffs them into TemplateParameterInfos, which we use to create the template parameters string.

moved into helper function

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2515–2518

lldb already requires exact name matching when looking up classes

e.g.

$ cat /tmp/a.cc
template<class T>struct Foo {};
int main() {
        Foo<int> a;
        Foo<long> b;
        Foo<float> c;
}
template<class T>struct Foo {};
int main() {
        Foo<int> a;
        Foo<long> b;
        Foo<float> c;
}
~/repos/llvm-project/build/cmake$ ./bin/lldb /tmp/a
(lldb) target create "/tmp/a"
Current executable set to '/tmp/a' (x86_64).
(lldb) im loo -t 'Foo< int>'
(lldb) im loo -t 'Foo<int>'
1 match found in /tmp/a:
id = {0x00000058}, name = "Foo<int>", byte-size = 1, decl = a.cc:1, compiler_type = "template<> struct Foo<int> {
}"

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1528

Can this affect operator< at all? Haven't thought much about it but that's one construct that comes to mind that can contain <

$ cat operator.cpp 
template<typename T>
struct Foo {
    int x = 0;

    template<typename U>
    friend bool operator<(Foo<U> const& lhs, Foo<U> const& rhs) {
        return lhs.x < rhs.x;
    }
};

int main() {
    Foo<int> x;
    Foo<int> y;
    return x < y;
}

0x000000c8:   DW_TAG_subprogram                                    
                DW_AT_low_pc    (0x0000000100003f5c)               
                DW_AT_high_pc   (0x0000000100003f8c)               
                DW_AT_APPLE_omit_frame_ptr      (true)        
                DW_AT_frame_base        (DW_OP_reg31 WSP)          
                DW_AT_linkage_name      ("_ZltIiEbRK3FooIT_ES4_")  
                DW_AT_name      ("operator<<int>")                 
                DW_AT_decl_file ("/Users/michaelbuch/operator.cpp")
                DW_AT_decl_line (6)                                
                DW_AT_type      (0x0000000000000135 "bool")        
                DW_AT_external  (true)

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1528

Looks like -gsimple-template-names leaves the template params in DW_AT_name in this case so we're fine? Would be nice to test though

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1528

Oh this wouldn't apply anyway since we only care about type names, ignore this comment

[Michael137]

lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py

2

I think it would be good to test this with -gsimple-template-names/-gno-simple-template-names

You can pass in a dictionary into self.build() and override Makefile flags

E..g, in lldb/test/API/lang/c/forward/TestForwardDeclaration.py we do this for -gdwarf-5.

So you could have two test_XXX methods like:

def do_test(self, debug_flags):
    self.build(dictionary=debug_flags)
    ... actual test ...

def test_simple_template_name(self):
    do_test(self, dict(CFLAGS_EXTRAS="-gsimple-template-names")

def test_no_simple_template_name(self):
    do_test(self, dict(CFLAGS_EXTRAS="-gno-simple-template-names")

[aeubanks]

address comments

lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py

2

thanks, that's very useful to know! done

[dblaikie]

Looking pretty good to me FWIW

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1561

Sorry, when I gave feedback on some pieces of this that were just moved around rather than new code written in this review - I don't mind the code moving around without changes (and optionally before/after the move making improvements, but not necessary).

If possible, might simplify the code review to move the code first/separately from this change, if the move isn't too meaningless independent of the rest of the changes?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2515–2518

Yeah, sorry - I understand it requires exact matching currently (because the index has the exact string in it) - my comment was forward-looking/idle thought that now that with simplified template names we can/have to lookup by base name, we have the option to do fuzzier matching when filtering all the base name matches - not suggesting that it's a regression that this currently does exact matching.

[aeubanks]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1561

https://reviews.llvm.org/D135979

[aeubanks]

rebase

[rnk]

@Michael137, are you comfortable approving this, or can you reroute to a reviewer who can help us with this?

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2500

Is there some other way to determine whether the debug-info was generated from -gsimple-template-names? Then we wouldn't have to check the existence of a < in the name in multiple places and wouldn't have to do this lookup speculatively. With this change, if we fail to find a template type in the index, we would do the lookup all over again, only to not find it again. Could that get expensive? Just trying to figure out if we can avoid doing this GetTypes call twice.

There have been talks about doing a base-name search by default followed by fuzzy matching on template parameters (though in the context of function names). The simple-template-names came up as a good motivator/facilitator for doing so. But for that idea to work here we'd have to be able to retrieve from the index by basename, so perhaps out of the scope of what we're trying to do here

tagging people involved in that discussion: @dblaikie @aprantl @jingham @labath

Other than this, generally LGTM

[labath]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1528

reverse the condition and use early exit?

1541

dump is a "debugging aid". I guess you should call print and pass the appropriate PrintingPolicy. It might just be the default, though maybe we could also try to make an effort to match the output in the non-simplified-names case.

1551

When can this be empty? Should we still include the <> in those cases?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2497

AT_name ?

2498

I don't think this can be keyed off of Empty() for two reasons:

• I believe the user can pass a non-empty type list into this function, expecting it to append to it
• just because you found one templated type above, it doesn't mean that there can't be another non-templated type somewhere (if only a part of the binary was built with simplified names).

I think we should just unconditionally (possibly guarded by a max_matches check) try both.

2500

Negative matches should be fast, and since typically only one of the branches will produce any results, I think this should be fine. Filtering matches in the simplified case would be slower, since you'd have build the full name of all potential candidates (and that could be thousands of instantiations), but I don't know how slow. But that's the price we pay for better filtering (which we don't do right now, but we could start doing afterwards).

2577

Do we need to do something similar in this FindTypes overload as well?

[aeubanks]

address comments

rebasing to ToT, TestCPPBreakpointsLocations.py is failing when using -gsimple-template-names. still working on that

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1551

it was empty when we're working with a non-templated die, so a lot of cases. I've added an early exit and changed this to an assert

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2500

Done.

I believe the only difference with -gsimple-template-names is that the name doesn't contain the template parameters, so I don't think there's any other way of determining if the debug info was generated from -gsimple-template-names.

2577

I was wondering when this code path is even taken, turns out it's only called from lldb-test, and only in two tests:

lldb-shell :: SymbolFile/DWARF/x86/compilercontext.ll
lldb-shell :: SymbolFile/DWARF/x86/module-ownership.mm

so probably doesn't need to be updated?

[aeubanks]

revert breakpoint test change
we can leave that to a later change, so I think this change is ready to go

[labath]

I can't say I fully understand all of this code, but I also don't know who would, so I guess I'll just say it "looks good" :)

I am wondering about the testing situation though. If I understand correctly, you've run the test suite with hardcoded simplified names, and it all passed (?)

I am definitely not suggesting we add a new test suite mode for that, but maybe we could extend this one test case with extra check that look at the type names in other contexts than in name lookup (e.g. expression evaluation, backtraces, ???) -- just to make sure that something doesn't break there in the future. WDYT?

lldb/test/API/lang/cpp/unique-types2/main.cpp

20

Would it be interesting to test nested types as well (Foo<T>::Bar<U>) ?

[aeubanks]

fix nested types by introducing Type::GetBaseName()
add expr eval tests

[aeubanks]

regarding the failure in TestCPPBreakpointLocations.py (added recently in https://reviews.llvm.org/D135921, seems like it's catching real issues with this patch), it looks like the manual dwarf index needs a similar fix to this

In D134378#3888462, @labath wrote:

I can't say I fully understand all of this code, but I also don't know who would, so I guess I'll just say it "looks good" :)

I am wondering about the testing situation though. If I understand correctly, you've run the test suite with hardcoded simplified names, and it all passed (?)

yes, that was true until https://reviews.llvm.org/D135921, which catches some missing functionality in this patch around breakpoints (I'd still like to defer that to a separate patch)

I am definitely not suggesting we add a new test suite mode for that, but maybe we could extend this one test case with extra check that look at the type names in other contexts than in name lookup (e.g. expression evaluation, backtraces, ???) -- just to make sure that something doesn't break there in the future. WDYT?

I've added expression evaluation to the test. do you have examples of backtrace tests?

lldb/test/API/lang/cpp/unique-types2/main.cpp

20

yes, and that caught a bug, thanks for the suggestion

I've added a Type::GetBaseName(), could you take a look at that?

[Michael137]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

3831

Could we just re-use GetTypeName and add an optional printing policy argument to it?

lldb/test/API/lang/cpp/unique-types2/main.cpp

20

Out of curiosity, what was the issue with using the qualified name?

[aeubanks]

combine GetBaseName into GetTypeName

[aeubanks]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

3809

this was mostly copied from GetTypeName above, not sure if these two should be factored out?

lldb/test/API/lang/cpp/unique-types2/main.cpp

20

GetQualifiedName returns the name including any scopes, so in this case it gave Foo<int>::Nested<char> rather than Nested<char> and the '<' string logic ended up comparing <int>::Nested<char> to <char>

[labath]

In D134378#3890302, @aeubanks wrote:

regarding the failure in TestCPPBreakpointLocations.py (added recently in https://reviews.llvm.org/D135921, seems like it's catching real issues with this patch), it looks like the manual dwarf index needs a similar fix to this

In D134378#3888462, @labath wrote:

I can't say I fully understand all of this code, but I also don't know who would, so I guess I'll just say it "looks good" :)

I am wondering about the testing situation though. If I understand correctly, you've run the test suite with hardcoded simplified names, and it all passed (?)

yes, that was true until https://reviews.llvm.org/D135921, which catches some missing functionality in this patch around breakpoints (I'd still like to defer that to a separate patch)

Yes, sounds good.

Can you elaborate on the manual index issue. My impression that it just takes the strings from inside DW_AT_name, and so it should behave similarly (identically?) to a compiler index -- though, in this case, we have direct control over it, so we could e.g. make it always store simplified names regardless of the debug info.

I am definitely not suggesting we add a new test suite mode for that, but maybe we could extend this one test case with extra check that look at the type names in other contexts than in name lookup (e.g. expression evaluation, backtraces, ???) -- just to make sure that something doesn't break there in the future. WDYT?

I've added expression evaluation to the test. do you have examples of backtrace tests?

I don't have anything particular in mind. I figured you'd just stop the middle of some templated member function and run bt. Though, when I think of it now, these names come from the demangler, so they're probably not particularly interesting here. (Unless you build with -gsce, but in that case, they should come from the same code that is doing the type templatification).

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

2509

This isn't a qualified name anymore. I guess every instance of the string "qual" in this block should be replaced or deleted.

[aeubanks]

rename variables

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

bool ParseTemplateDIE(const DWARFDIE &die,
                         lldb_private::TypeSystemClang::TemplateParameterInfos
                             &template_param_infos);
 
   bool ParseTemplateParameterInfos(
       const DWARFDIE &parent_die,
       lldb_private::TypeSystemClang::TemplateParameterInfos
           &template_param_infos);
 
+  /// Get the template parameters of a die as a string if the die name does not
+  /// already contain them. This happens with -gsimple-template-names.
+  std::string GetTemplateParametersString(const DWARFDIE &die);
+
   std::string GetCPlusPlusQualifiedName(const DWARFDIE &die);
 
   bool ParseChildMembers(
       const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
       std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
       std::vector<DWARFDIE> &member_function_dies,
       DelayedPropertyList &delayed_properties,
       const lldb::AccessType default_accessibility,

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
   if (symbol_context_scope != nullptr)
     type_sp->SetSymbolContextScope(symbol_context_scope);
 
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
   return type_sp;
 }
 
 std::string
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+    return std::string();
+  TypeSystemClang::TemplateParameterInfos template_param_infos;
+  if (!ParseTemplateParameterInfos(die, template_param_infos))
+    return std::string();
+  std::string all_template_names;
+  llvm::SmallVector<clang::TemplateArgument, 2> args =
+      template_param_infos.args;
+  if (template_param_infos.hasParameterPack())
+    args.append(template_param_infos.packed_args->args);
+  if (args.empty())
+    return std::string();
+  for (auto &arg : args) {
+    std::string template_name;
+    llvm::raw_string_ostream os(template_name);
+
+    // Are these LangOptions sufficient?
+    clang::LangOptions LO;
+    LO.CPlusPlus = true;
+    LO.Bool = true;
+    clang::PrintingPolicy pp(LO);
+    arg.print(pp, os, true);
+
+    if (!template_name.empty()) {
+      if (all_template_names.empty()) {
+        all_template_names.append("<");
+      } else {
+        all_template_names.append(", ");
+      }
+      all_template_names.append(template_name);
+    }
+  }
+  assert(!all_template_names.empty() && "no template parameters?");
+  // Spacing doesn't matter as long as we're consistent because we're only using
+  // this to deduplicate C++ symbols.
+  all_template_names.append(">");
+  return all_template_names;
+}
+
+std::string
 DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
   if (!die.IsValid())
     return "";
   const char *name = die.GetName();
   if (!name)
     return "";
   std::string qualified_name;
   DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
   // TODO: change this to get the correct decl context parent....
   while (parent_decl_ctx_die) {
+    // The name may not contain template parameters due to simplified template
+    // names; we must reconstruct the full name from child template parameter
+    // dies via GetTemplateParametersString().
     const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
     switch (parent_tag) {
     case DW_TAG_namespace: {
       if (const char *namespace_name = parent_decl_ctx_die.GetName()) {
         qualified_name.insert(0, "::");
         qualified_name.insert(0, namespace_name);
       } else {
         qualified_name.insert(0, "(anonymous namespace)::");
       }
       parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
       break;
     }
 
     case DW_TAG_class_type:
     case DW_TAG_structure_type:
     case DW_TAG_union_type: {
       if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
+        qualified_name.insert(0,
+                              GetTemplateParametersString(parent_decl_ctx_die));
         qualified_name.insert(0, "::");
         qualified_name.insert(0, class_union_struct_name);
       }
       parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
       break;
     }
 
     default:
       parent_decl_ctx_die.Clear();
       break;
     }
   }
 
   if (qualified_name.empty())
     qualified_name.append("::");
 
   qualified_name.append(name);
+  qualified_name.append(GetTemplateParametersString(die));
 
   return qualified_name;
 }
 
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
                                            const DWARFDIE &die,
                                            ParsedDWARFTypeAttributes &attrs) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
       // For C++, we rely solely upon the one definition rule that says
       // only one thing can exist at a given decl context. We ignore the
       // file and line that things are declared on.
       std::string qualified_name = GetCPlusPlusQualifiedName(die);
       if (!qualified_name.empty())
         unique_typename = ConstString(qualified_name);
       unique_decl.Clear();
     }
 
     if (dwarf->GetUniqueDWARFASTTypeMap().Find(
             unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
             *unique_ast_entry_up)) {
       type_sp = unique_ast_entry_up->m_type_sp;
       if (type_sp) {
         dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
         LinkDeclContextToDIE(
             GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
         return type_sp;
       }
     }
   }
 
   DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
                DW_TAG_value_to_name(tag), type_name_cstr);
 
   int tag_decl_kind = -1;
   AccessType default_accessibility = eAccessNone;
   if (tag == DW_TAG_structure_type) {
     tag_decl_kind = clang::TTK_Struct;
     default_accessibility = eAccessPublic;
   } else if (tag == DW_TAG_union_type) {
     tag_decl_kind = clang::TTK_Union;
     default_accessibility = eAccessPublic;
   } else if (tag == DW_TAG_class_type) {
     tag_decl_kind = clang::TTK_Class;
     default_accessibility = eAccessPrivate;
   }
 
   if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
       !die.HasChildren() && cu_language == eLanguageTypeObjC) {
     // Work around an issue with clang at the moment where forward
     // declarations for objective C classes are emitted as:
     //  DW_TAG_structure_type [2]
     //  DW_AT_name( "ForwardObjcClass" )
     //  DW_AT_byte_size( 0x00 )
     //  DW_AT_decl_file( "..." )
     //  DW_AT_decl_line( 1 )
     //
     // Note that there is no DW_AT_declaration and there are no children,
     // and the byte size is zero.
     attrs.is_forward_declaration = true;
   }
 
   if (attrs.class_language == eLanguageTypeObjC ||
       attrs.class_language == eLanguageTypeObjC_plus_plus) {
     if (!attrs.is_complete_objc_class &&
         die.Supports_DW_AT_APPLE_objc_complete_type()) {
       // We have a valid eSymbolTypeObjCClass class symbol whose name
       // matches the current objective C class that we are trying to find
       // and this DIE isn't the complete definition (we checked
       // is_complete_objc_class above and know it is false), so the real
       // definition is in here somewhere
       type_sp =
           dwarf->FindCompleteObjCDefinitionTypeForDIE(die, attrs.name, true);
 
       if (!type_sp) {
         SymbolFileDWARFDebugMap *debug_map_symfile =
             dwarf->GetDebugMapSymfile();
         if (debug_map_symfile) {
           // We weren't able to find a full declaration in this DWARF,
           // see if we have a declaration anywhere else...
           type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
               die, attrs.name, true);
         }
       }
if (attrs.accessibility == eAccessNone && decl_ctx) {
       if (DeclKindIsCXXClass(containing_decl_kind))
         attrs.accessibility = default_accessibility;
     }
 
     ClangASTMetadata metadata;
     metadata.SetUserID(die.GetID());
     metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
 
-    if (attrs.name.GetStringRef().contains('<')) {
-      TypeSystemClang::TemplateParameterInfos template_param_infos;
-      if (ParseTemplateParameterInfos(die, template_param_infos)) {
-        clang::ClassTemplateDecl *class_template_decl =
-            m_ast.ParseClassTemplateDecl(
-                decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-                attrs.name.GetCString(), tag_decl_kind, template_param_infos);
-        if (!class_template_decl) {
-          if (log) {
-            dwarf->GetObjectFile()->GetModule()->LogMessage(
-                log,
-                "SymbolFileDWARF(%p) - 0x%8.8x: %s type \"%s\" "
-                "clang::ClassTemplateDecl failed to return a decl.",
-                static_cast<void *>(this), die.GetOffset(),
-                DW_TAG_value_to_name(tag), attrs.name.GetCString());
-          }
-          return TypeSP();
-        }
+    if (ParseTemplateParameterInfos(die, template_param_infos) &&
+        (!template_param_infos.args.empty() ||
+         template_param_infos.packed_args)) {
 
-        clang::ClassTemplateSpecializationDecl *class_specialization_decl =
-            m_ast.CreateClassTemplateSpecializationDecl(
-                decl_ctx, GetOwningClangModule(die), class_template_decl,
-                tag_decl_kind, template_param_infos);
-        clang_type = m_ast.CreateClassTemplateSpecializationType(
-            class_specialization_decl);
-        clang_type_was_created = true;
 
-        m_ast.SetMetadata(class_template_decl, metadata);
-        m_ast.SetMetadata(class_specialization_decl, metadata);
-      }
-    }
 
     if (!clang_type_was_created) {
       clang_type_was_created = true;
       clang_type = m_ast.CreateRecordType(
           decl_ctx, GetOwningClangModule(die), attrs.accessibility,
           attrs.name.GetCString(), tag_decl_kind, attrs.class_language,
           &metadata, attrs.exports_symbols);
     }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (!matching_type)
       return true;
 
     // We found a type pointer, now find the shared pointer form our type
     // list
     types.InsertUnique(matching_type->shared_from_this());
     return types.GetSize() < max_matches;
   });
 
+  // With -gsimple-template-names, a templated type's DW_AT_name will not
+  // contain the template parameters. Try again stripping '<' and anything
+  // after, filtering out entries with template parameters that don't match.
+  if (types.GetSize() < max_matches) {
+    const llvm::StringRef nameRef = name.GetStringRef();
+    auto it = nameRef.find('<');
+    if (it != llvm::StringRef::npos) {
+      const llvm::StringRef nameNoTemplateParams = nameRef.slice(0, it);
+      const llvm::StringRef templateParams = nameRef.slice(it, nameRef.size());
+      m_index->GetTypes(ConstString(nameNoTemplateParams), [&](DWARFDIE die) {
+        if (!DIEInDeclContext(parent_decl_ctx, die))
+          return true; // The containing decl contexts don't match
+
+        TypeSP Ty = GetTypeForDIE(die);
+        llvm::StringRef qualName = Ty->GetQualifiedName().AsCString();
+        auto it = qualName.find('<');
+        // If the candidate qualified name doesn't have '<', it doesn't have
+        // template params to compare.
+        if (it == llvm::StringRef::npos)
+          return true;
+
+        // Filter out non-matching instantiations by comparing template params.
+        llvm::StringRef qualNameTemplateParams =
+            qualName.slice(it, qualName.size());
+        if (templateParams != qualNameTemplateParams)
+          return true;
+
+        Type *matching_type = ResolveType(die, true, true);
+        if (!matching_type)
+          return true;
+
+        // We found a type pointer, now find the shared pointer form our type
+        // list.
+        types.InsertUnique(matching_type->shared_from_this());
+        return types.GetSize() < max_matches;
+      });
+    }
+  }
+
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
   if (types.GetSize() < max_matches) {
     UpdateExternalModuleListIfNeeded();
 
     for (const auto &pair : m_external_type_modules)
       if (ModuleSP external_module_sp = pair.second)
void SymbolFileDWARF::FindTypes(
   // Make sure we haven't already searched this SymbolFile before.
   if (!searched_symbol_files.insert(this).second)
     return;
 
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   if (pattern.empty())
     return;
 
   ConstString name = pattern.back().name;
 
   if (!name)
     return;
 
   m_index->GetTypes(name, [&](DWARFDIE die) {
     if (!languages[GetLanguageFamily(*die.GetCU())])
       return true;
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

ConstString TypeSystemClang::GetTypeName(lldb::opaque_compiler_type_t type) {
   if (const auto *typedef_type = qual_type->getAs<clang::TypedefType>()) {
     const clang::TypedefNameDecl *typedef_decl = typedef_type->getDecl();
     return ConstString(GetTypeNameForDecl(typedef_decl));
   }
 
   return ConstString(qual_type.getAsString(GetTypePrintingPolicy()));
 }
 
 ConstString
 TypeSystemClang::GetDisplayTypeName(lldb::opaque_compiler_type_t type) {
   if (!type)
     return ConstString();
 
   clang::QualType qual_type(GetQualType(type));
   clang::PrintingPolicy printing_policy(getASTContext().getPrintingPolicy());
   printing_policy.SuppressTagKeyword = true;
   printing_policy.SuppressScope = false;
   printing_policy.SuppressUnwrittenScope = true;
   printing_policy.SuppressInlineNamespace = true;
   return ConstString(qual_type.getAsString(printing_policy));
 }
 
 uint32_t
 TypeSystemClang::GetTypeInfo(lldb::opaque_compiler_type_t type,
                              CompilerType *pointee_or_element_clang_type) {
   if (!type)
     return 0;
 
   if (pointee_or_element_clang_type)
     pointee_or_element_clang_type->Clear();
 
   clang::QualType qual_type =
       RemoveWrappingTypes(GetQualType(type), {clang::Type::Typedef});
 
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();
   switch (type_class) {
   case clang::Type::Attributed:
     return GetTypeInfo(qual_type->castAs<clang::AttributedType>()
                            ->getModifiedType()

 }
 
 clang::ClassTemplateDecl *TypeSystemClang::ParseClassTemplateDecl(
     clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
     lldb::AccessType access_type, const char *parent_name, int tag_decl_kind,
     const TypeSystemClang::TemplateParameterInfos &template_param_infos) {
   if (template_param_infos.IsValid()) {
     std::string template_basename(parent_name);
-    template_basename.erase(template_basename.find('<'));
+    // With -gsimple-template-names we may omit template parameters in the name.
+    if (auto i = template_basename.find('<'); i != std::string::npos)
+      template_basename.erase(i);
 
     return CreateClassTemplateDecl(decl_ctx, owning_module, access_type,
                                    template_basename.c_str(), tag_decl_kind,
                                    template_param_infos);
   }
   return nullptr;
 }
 
 void TypeSystemClang::CompleteTagDecl(clang::TagDecl *decl) {

lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py

def test_forward_declarations(self):
         self.assertCompleteWithVar("DefinedClass")
         self.assertCompleteWithVar("DefinedClassTypedef")
         self.assertCompleteWithVar("DefinedTemplateClass<int>")
         self.assertIsNotPresent("DefinedTemplateClass<long>")
 
         # Record types without a defining declaration are not complete.
         self.assertPointeeIncomplete("FwdClass *", "fwd_class")
         self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-        self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+        self.assertPointeeIncomplete("FwdTemplateClass<int> *", "fwd_template_class")
 
         # A pointer type is complete even when it points to an incomplete type.
         fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
         self.assertTrue(fwd_class_ptr.GetType().IsTypeComplete())
 
     @no_debug_info_test
     def test_builtin_types(self):
         """ Tests builtin types and types derived from them. """

lldb/test/API/lang/cpp/unique-types2/Makefile

+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py

+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+    def do_test(self, debug_flags):
+        """Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+        self.build(dictionary=debug_flags)
+        lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+        self.expect("image lookup -A -t 'Foo<char>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<Foo<int> >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<float>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+        self.expect("image lookup -A -t 'FooPack<char>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<Foo<int> >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<char, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<char, float>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int, int, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<float>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+        self.expect("image lookup -A -t 'FooPack<float, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_simple_template_name(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_no_simple_template_name(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
+

lldb/test/API/lang/cpp/unique-types2/main.cpp

+template <class T> class Foo {
+  T t;
+};
+
+template <class T, class... Ss> class FooPack {
+  T t;
+};
+
+int main() {
+  Foo<char> t1;
+  Foo<int> t2;
+  Foo<Foo<int>> t3;
+
+  FooPack<char> p1;
+  FooPack<int> p2;
+  FooPack<Foo<int>> p3;
+  FooPack<char, int> t4;
+  FooPack<char, float> t5;
+  FooPack<int, int> t6;
+  FooPack<int, int, int> t7;
+  // Set breakpoint here
+}