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

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.

Reviewed By: Michael137

Differential Revision: https://reviews.llvm.org/D140240

diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index 0de1bec..9a010c2 100644 (file)
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -193,6 +193,8 @@ public:
 
   bool IsScalarType() const;
 
+  bool IsTemplateType() const;
+
   bool IsTypedefType() const;
 
   bool IsVoidType() const;

diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index 362db0a..ff0d0c3 100644 (file)
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -143,6 +143,9 @@ public:
 
   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; }

diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 69682d1..02dafda 100644 (file)
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -357,6 +357,8 @@ public:
                                 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);
 

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 4479f68..3716e9e 100644 (file)
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2501,6 +2501,11 @@ void SymbolFileDWARF::FindTypes(
   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
@@ -2509,6 +2514,13 @@ void SymbolFileDWARF::FindTypes(
     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());
@@ -2519,11 +2531,11 @@ void SymbolFileDWARF::FindTypes(
   // 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

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index bd017da..f54f775 100644 (file)
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7131,6 +7131,17 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
   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) {

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 9cf7cd3..85d706d 100644 (file)
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -826,6 +826,8 @@ public:
                                 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;
 

diff --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp
index 20e63f6..ff0a383 100644 (file)
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -260,6 +260,13 @@ bool CompilerType::IsScalarType() const {
   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())

diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index 048e37f..b17ff50 100644 (file)
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -393,6 +393,10 @@ bool Type::IsAggregateType() {
   return GetForwardCompilerType().IsAggregateType();
 }
 
+bool Type::IsTemplateType() {
+  return GetForwardCompilerType().IsTemplateType();
+}
+
 lldb::TypeSP Type::GetTypedefType() {
   lldb::TypeSP type_sp;
   if (IsTypedef()) {

diff --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp
index d1d78fc..e71f8ea 100644 (file)
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -116,6 +116,10 @@ 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;

diff --git a/lldb/test/API/lang/cpp/unique-types4/Makefile b/lldb/test/API/lang/cpp/unique-types4/Makefile
new file mode 100644 (file)
index 0000000..99998b2
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types4/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff --git a/lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py b/lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
new file mode 100644 (file)
index 0000000..ccc3833
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
@@ -0,0 +1,31 @@
+"""
+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"))

diff --git a/lldb/test/API/lang/cpp/unique-types4/main.cpp b/lldb/test/API/lang/cpp/unique-types4/main.cpp
new file mode 100644 (file)
index 0000000..8306352
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types4/main.cpp
@@ -0,0 +1,23 @@
+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
+}