[Lldb-commits] [lldb] d483d48 - [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes
Arthur Eubanks via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 20 10:10:22 PST 2022
Author: Arthur Eubanks
Date: 2022-12-20T10:10:07-08:00
New Revision: d483d488ca5d04534105b67d83f3c16d142e7a05
URL: https://github.com/llvm/llvm-project/commit/d483d488ca5d04534105b67d83f3c16d142e7a05
DIFF: https://github.com/llvm/llvm-project/commit/d483d488ca5d04534105b67d83f3c16d142e7a05.diff
LOG: [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
Added:
lldb/test/API/lang/cpp/unique-types4/Makefile
lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
lldb/test/API/lang/cpp/unique-types4/main.cpp
Modified:
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/TypeSystem.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index 0de1bec4d733a..9a010c23aee2c 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -193,6 +193,8 @@ class CompilerType {
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 362db0ac054ad..ff0d0c3516da4 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -143,6 +143,9 @@ class Type : public std::enable_shared_from_this<Type>, public UserID {
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 69682d121f2fd..02dafda67eec3 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -357,6 +357,8 @@ class TypeSystem : public PluginInterface,
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 4479f687823e8..3716e9e800d74 100644
--- 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 bd017dab20bf0..f54f77593560b 100644
--- 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 9cf7cd339e92a..85d706de62ab3 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -826,6 +826,8 @@ class TypeSystemClang : public TypeSystem {
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 20e63f65584c3..ff0a383300e34 100644
--- 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 048e37fb1db2f..b17ff507557da 100644
--- 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 d1d78fc1eec52..e71f8ea28d7f2 100644
--- 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
index 0000000000000..99998b20bcb05
--- /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
index 0000000000000..ccc38337dd4f3
--- /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
index 0000000000000..830635202a750
--- /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
+}
More information about the lldb-commits
mailing list