[lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

[aeubanks]

Summary

The provided test case was crashing because of confusion attempting to find types for ns::Foo under -gsimple-template-names. (This looks broken normally because it's attempting to find ns::Foo rather than ns::Foo<T>)

Looking up types can't give false positives, as opposed to looking up functions as mentioned in https://reviews.llvm.org/D137098.

[aeubanks]

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

2522

I'm looking into whether or not this can be sped up by checking if the type is templated some other way

[aeubanks]

check if type is template by looking at underlying clang Type, rather than looking for a '<'
properly handle typedefs

[aeubanks]

lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py

16

this makes handling of -gsimple-template-names the same as the current handling of -gno-simple-template-names

I haven't dug too deep into the existing difference between FooDouble/FooInt vs the others

for the "no member named" errors, I believe clang is attempting to get the uninstantiated declarations of Foo/Bar, which the dwarf knows nothing about

[Michael137]

lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py

16

Yup that's exactly what it is. For expr Foo<double>, clang::Sema's template lookup would ask LLDB to give it type Foo, but we can't service that request from DWARF currently

[Michael137]

look reasonable to me

[Michael137]

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

2521

You could consider using CompilerType::GetTypeInfo instead of introducing CompilerType::IsTemplateType. Though that function looks a bit chunky

[aeubanks]

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

2521

just tried this but it's recognizing the type as just a normal record instead of a template instantiation

lldb/include/lldb/Symbol/CompilerType.h

public:
 
   bool IsReferenceType(CompilerType *pointee_type = nullptr,
                        bool *is_rvalue = nullptr) const;
 
   bool ShouldTreatScalarValueAsAddress() const;
 
   bool IsScalarType() const;
 
+  bool IsTemplateType() const;
+
   bool IsTypedefType() const;
 
   bool IsVoidType() const;
   /// \}
 
   /// Type Completion.
   /// \{
   bool GetCompleteType() const;

lldb/include/lldb/Symbol/Type.h

public:
   ConstString GetBaseName();
 
   llvm::Optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope);
 
   uint32_t GetNumChildren(bool omit_empty_base_classes);
 
   bool IsAggregateType();
 
+  // Returns if the type is a templated decl. Does not look through typedefs.
+  bool IsTemplateType();
+
   bool IsValidType() { return m_encoding_uid_type != eEncodingInvalid; }
 
   bool IsTypedef() { return m_encoding_uid_type == eEncodingIsTypedefUID; }
 
   lldb::TypeSP GetTypedefType();
 
   ConstString GetName() const { return m_name; }
 

lldb/include/lldb/Symbol/TypeSystem.h

#endif
   // TODO: Return all matches for a given name by returning a
   // vector<vector<uint32_t>>
   // so we catch all names that match a given child name, not just the first.
   virtual size_t
   GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
                                 const char *name, bool omit_empty_base_classes,
                                 std::vector<uint32_t> &child_indexes) = 0;
 
+  virtual bool IsTemplateType(lldb::opaque_compiler_type_t type);
+
   virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
                                          bool expand_pack);
 
   virtual lldb::TemplateArgumentKind
   GetTemplateArgumentKind(lldb::opaque_compiler_type_t type, size_t idx,
                           bool expand_pack);
   virtual CompilerType
   GetTypeTemplateArgument(lldb::opaque_compiler_type_t type, size_t idx,

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

else
           "SymbolFileDWARF::FindTypes (sc, name=\"%s\", parent_decl_ctx = "
           "NULL, max_matches=%u, type_list)",
           name.GetCString(), max_matches);
   }
 
   if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
     return;
 
+  // Unlike FindFunctions(), FindTypes() following cannot produce false
+  // positives.
+
+  const llvm::StringRef name_ref = name.GetStringRef();
+  auto name_bracket_index = name_ref.find('<');
   m_index->GetTypes(name, [&](DWARFDIE die) {
     if (!DIEInDeclContext(parent_decl_ctx, die))
       return true; // The containing decl contexts don't match
 
     Type *matching_type = ResolveType(die, true, true);
     if (!matching_type)
       return true;
 
+    // With -gsimple-template-names, a templated type's DW_AT_name will not
+    // contain the template parameters. Make sure that if the original query
+    // didn't contain a '<', we filter out entries with template parameters.
+    if (name_bracket_index == llvm::StringRef::npos &&
+        matching_type->IsTemplateType())
+      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 name_ref = name.GetStringRef();
-    auto it = name_ref.find('<');
-    if (it != llvm::StringRef::npos) {
-      const llvm::StringRef name_no_template_params = name_ref.slice(0, it);
-      const llvm::StringRef template_params = name_ref.slice(it, name_ref.size());
+    if (name_bracket_index != llvm::StringRef::npos) {
+      const llvm::StringRef name_no_template_params =
+          name_ref.slice(0, name_bracket_index);
+      const llvm::StringRef template_params =
+          name_ref.slice(name_bracket_index, name_ref.size());
       m_index->GetTypes(ConstString(name_no_template_params), [&](DWARFDIE die) {
         if (!DIEInDeclContext(parent_decl_ctx, die))
           return true; // The containing decl contexts don't match
 
         const llvm::StringRef base_name = GetTypeForDIE(die)->GetBaseName().AsCString();
         auto it = base_name.find('<');
         // If the candidate qualified name doesn't have '<', it doesn't have
         // template params to compare.

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

#endif
   // TODO: Return all matches for a given name by returning a
   // vector<vector<uint32_t>>
   // so we catch all names that match a given child name, not just the first.
   size_t
   GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
                                 const char *name, bool omit_empty_base_classes,
                                 std::vector<uint32_t> &child_indexes) override;
 
+  bool IsTemplateType(lldb::opaque_compiler_type_t type) override;
+
   size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
                                  bool expand_pack) override;
 
   lldb::TemplateArgumentKind
   GetTemplateArgumentKind(lldb::opaque_compiler_type_t type, size_t idx,
                           bool expand_pack) override;
   CompilerType GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,
                                        size_t idx, bool expand_pack) override;

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

if (type && name && name[0]) {
 
     default:
       break;
     }
   }
   return UINT32_MAX;
 }
 
+bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) {
+  if (!type)
+    return false;
+  CompilerType ct(weak_from_this(), type);
+  const clang::Type *clang_type = ClangUtil::GetQualType(ct).getTypePtr();
+  if (auto *cxx_record_decl = dyn_cast<clang::TagType>(clang_type))
+    return isa<clang::ClassTemplateSpecializationDecl>(
+        cxx_record_decl->getDecl());
+  return false;
+}
+
 size_t
 TypeSystemClang::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
                                          bool expand_pack) {
   if (!type)
     return 0;
 
   clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();

lldb/source/Symbol/CompilerType.cpp

 
 bool CompilerType::IsScalarType() const {
   if (IsValid())
     if (auto type_system_sp = GetTypeSystem())
       return type_system_sp->IsScalarType(m_type);
   return false;
 }
 
+bool CompilerType::IsTemplateType() const {
+  if (IsValid())
+    if (auto type_system_sp = GetTypeSystem())
+      return type_system_sp->IsTemplateType(m_type);
+  return false;
+}
+
 bool CompilerType::IsTypedefType() const {
   if (IsValid())
     if (auto type_system_sp = GetTypeSystem())
       return type_system_sp->IsTypedefType(m_type);
   return false;
 }
 
 bool CompilerType::IsVoidType() const {

lldb/source/Symbol/Type.cpp

 uint32_t Type::GetNumChildren(bool omit_empty_base_classes) {
   return GetForwardCompilerType().GetNumChildren(omit_empty_base_classes, nullptr);
 }
 
 bool Type::IsAggregateType() {
   return GetForwardCompilerType().IsAggregateType();
 }
 
+bool Type::IsTemplateType() {
+  return GetForwardCompilerType().IsTemplateType();
+}
+
 lldb::TypeSP Type::GetTypedefType() {
   lldb::TypeSP type_sp;
   if (IsTypedef()) {
     Type *typedef_type = m_symbol_file->ResolveTypeUID(m_encoding_uid);
     if (typedef_type)
       type_sp = typedef_type->shared_from_this();
   }
   return type_sp;

lldb/source/Symbol/TypeSystem.cpp

 CompilerType TypeSystem::GetBuiltinTypeByName(ConstString name) {
   return CompilerType();
 }
 
 CompilerType TypeSystem::GetTypeForFormatters(void *type) {
   return CompilerType(weak_from_this(), type);
 }
 
+bool TypeSystem::IsTemplateType(lldb::opaque_compiler_type_t type) {
+  return false;
+}
+
 size_t TypeSystem::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
                                            bool expand_pack) {
   return 0;
 }
 
 TemplateArgumentKind
 TypeSystem::GetTemplateArgumentKind(opaque_compiler_type_t type, size_t idx,
                                     bool expand_pack) {

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

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

lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.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 UniqueTypesTestCase4(TestBase):
+    def do_test(self, debug_flags):
+        """Test that we display the correct template instantiation."""
+        self.build(dictionary=debug_flags)
+        lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+        # FIXME: these should successfully print the values
+        self.expect("print ns::Foo<double>::value", substrs=["no member named"], error=True)
+        self.expect("print ns::Foo<int>::value", substrs=["no member named"], error=True)
+        self.expect("print ns::Bar<double>::value", substrs=["no member named"], error=True)
+        self.expect("print ns::Bar<int>::value", substrs=["no member named"], error=True)
+        self.expect("print ns::FooDouble::value", substrs=["Couldn't lookup symbols"], error=True)
+        self.expect("print ns::FooInt::value", substrs=["Couldn't lookup symbols"], error=True)
+
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_simple_template_names(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_names(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))

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

+namespace ns {
+
+template <typename T> struct Foo {
+  static T value;
+};
+
+template <typename T> using Bar = Foo<T>;
+
+using FooInt = Foo<int>;
+using FooDouble = Foo<double>;
+
+} // namespace ns
+
+ns::Foo<double> a;
+ns::Foo<int> b;
+ns::Bar<double> c;
+ns::Bar<int> d;
+ns::FooInt e;
+ns::FooDouble f;
+
+int main() {
+  // Set breakpoint here
+}