[Lldb-commits] [lldb] c0ac25f - [lldb] Fix simple template names interaction with debug info declarations
Arthur Eubanks via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 8 09:41:14 PST 2022
Author: Arthur Eubanks
Date: 2022-12-08T09:40:43-08:00
New Revision: c0ac25f1acc4dae97f420cd6bdf1c00c632563f1
URL: https://github.com/llvm/llvm-project/commit/c0ac25f1acc4dae97f420cd6bdf1c00c632563f1
DIFF: https://github.com/llvm/llvm-project/commit/c0ac25f1acc4dae97f420cd6bdf1c00c632563f1.diff
LOG: [lldb] Fix simple template names interaction with debug info declarations
Without checking template parameters, we would sometimes lookup the
wrong type definition for a type declaration because different
instantiations of the same template class had the same debug info name.
The added GetForwardDeclarationDIETemplateParams() shouldn't need a
cache because we'll cache the results of the declaration -> definition
lookup anyway. (DWARFASTParserClang::ParseStructureLikeDIE()
is_forward_declaration branch)
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D138834
Added:
lldb/test/API/lang/cpp/unique-types3/Makefile
lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
lldb/test/API/lang/cpp/unique-types3/a.cpp
lldb/test/API/lang/cpp/unique-types3/a.h
lldb/test/API/lang/cpp/unique-types3/main.cpp
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index a6dba10c3057a..412616acecafc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -55,6 +55,9 @@ class DWARFASTParser {
virtual void EnsureAllDIEsInDeclContextHaveBeenParsed(
lldb_private::CompilerDeclContext decl_context) = 0;
+ virtual lldb_private::ConstString
+ GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
+
static llvm::Optional<lldb_private::SymbolFile::ArrayInfo>
ParseChildArrayInfo(const DWARFDIE &parent_die,
const lldb_private::ExecutionContext *exe_ctx = nullptr);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 905df91ecaeb7..d88ff00296633 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -740,6 +740,39 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
return type_sp;
}
+ConstString
+DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
+ if (llvm::StringRef(die.GetName()).contains("<"))
+ return ConstString();
+
+ clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die, nullptr);
+ TypeSystemClang::TemplateParameterInfos template_param_infos;
+ if (ParseTemplateParameterInfos(die, template_param_infos) &&
+ (!template_param_infos.args.empty() ||
+ template_param_infos.packed_args)) {
+ // Most of the parameters here don't matter, but we make sure the base name
+ // is empty so when we print the name we only get the template parameters.
+ clang::ClassTemplateDecl *class_template_decl =
+ m_ast.ParseClassTemplateDecl(decl_ctx, GetOwningClangModule(die),
+ eAccessPublic, "", clang::TTK_Struct,
+ template_param_infos);
+ if (!class_template_decl)
+ return ConstString();
+
+ clang::ClassTemplateSpecializationDecl *class_specialization_decl =
+ m_ast.CreateClassTemplateSpecializationDecl(
+ decl_ctx, GetOwningClangModule(die), class_template_decl,
+ clang::TTK_Struct, template_param_infos);
+ if (!class_specialization_decl)
+ return ConstString();
+ CompilerType clang_type =
+ m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+ ConstString name = clang_type.GetTypeName(/*BaseOnly*/ true);
+ return name;
+ }
+ return ConstString();
+}
+
TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
@@ -1500,41 +1533,6 @@ TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
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);
- arg.print(m_ast.getASTContext().getPrintingPolicy(), 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())
@@ -1566,8 +1564,8 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
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, GetDIEClassTemplateParams(parent_decl_ctx_die).AsCString(""));
qualified_name.insert(0, "::");
qualified_name.insert(0, class_union_struct_name);
}
@@ -1585,7 +1583,7 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
qualified_name.append("::");
qualified_name.append(name);
- qualified_name.append(GetTemplateParametersString(die));
+ qualified_name.append(GetDIEClassTemplateParams(die).AsCString(""));
return qualified_name;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index d1ca8278f26e7..b14e9e648eb72 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -88,6 +88,24 @@ class DWARFASTParserClang : public DWARFASTParser {
ExtractIntFromFormValue(const lldb_private::CompilerType &int_type,
const DWARFFormValue &form_value) const;
+ /// Returns the template parameters of a class DWARFDIE as a string.
+ ///
+ /// This is mostly useful for -gsimple-template-names which omits template
+ /// parameters from the DIE name and instead always adds template parameter
+ /// children DIEs.
+ ///
+ /// Currently this is only called in two places, when uniquing C++ classes and
+ /// when looking up the definition for a declaration (which is then cached).
+ /// If this is ever called more than twice per DIE, we need to start caching
+ /// the results to prevent unbounded growth of the created clang AST nodes.
+ ///
+ /// \param die The struct/class DWARFDIE containing template parameters.
+ /// \return A string, including surrounding '<>', of the template parameters.
+ /// If the DIE's name already has '<>', returns an empty ConstString because
+ /// it's assumed that the caller is using the DIE name anyway.
+ lldb_private::ConstString
+ GetDIEClassTemplateParams(const DWARFDIE &die) override;
+
protected:
/// Protected typedefs and members.
/// @{
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index bc23a53be8a7b..4479f687823e8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2978,6 +2978,15 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
}
}
+ // See comments below about -gsimple-template-names for why we attempt to
+ // compute missing template parameter names.
+ ConstString template_params;
+ if (type_system) {
+ DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
+ if (dwarf_ast)
+ template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+ }
+
m_index->GetTypes(GetDWARFDeclContext(die), [&](DWARFDIE type_die) {
// Make sure type_die's language matches the type system we are
// looking for. We don't want to find a "Foo" type from Java if we
@@ -3049,6 +3058,27 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
return true;
+ // With -gsimple-template-names, the DIE name may not contain the template
+ // parameters. If the declaration has template parameters but doesn't
+ // contain '<', check that the child template parameters match.
+ if (template_params) {
+ llvm::StringRef test_base_name =
+ GetTypeForDIE(type_die)->GetBaseName().GetStringRef();
+ auto i = test_base_name.find('<');
+
+ // Full name from clang AST doesn't contain '<' so this type_die isn't
+ // a template parameter, but we're expecting template parameters, so
+ // bail.
+ if (i == llvm::StringRef::npos)
+ return true;
+
+ llvm::StringRef test_template_params =
+ test_base_name.slice(i, test_base_name.size());
+ // Bail if template parameters don't match.
+ if (test_template_params != template_params.GetStringRef())
+ return true;
+ }
+
type_sp = resolved_type->shared_from_this();
return false;
});
diff --git a/lldb/test/API/lang/cpp/unique-types3/Makefile b/lldb/test/API/lang/cpp/unique-types3/Makefile
new file mode 100644
index 0000000000000..1672544f5f660
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types3/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp a.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py b/lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
new file mode 100644
index 0000000000000..fababa1f95d32
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
@@ -0,0 +1,27 @@
+"""
+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 UniqueTypesTestCase3(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("a.cpp"))
+ self.expect_expr("a", result_type="S<int>")
+
+ @skipIf(compiler=no_match("clang"))
+ @skipIf(compiler_version=["<", "15.0"])
+ def test_simple_template_names(self):
+ # Can't directly set CFLAGS_EXTRAS here because the Makefile can't
+ # override an environment variable.
+ self.do_test(dict(TEST_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-types3/a.cpp b/lldb/test/API/lang/cpp/unique-types3/a.cpp
new file mode 100644
index 0000000000000..e4f1a8ca60c6c
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types3/a.cpp
@@ -0,0 +1,5 @@
+#include "a.h"
+
+void f(S<int> &a) {
+ (void)a; // Set breakpoint here
+}
diff --git a/lldb/test/API/lang/cpp/unique-types3/a.h b/lldb/test/API/lang/cpp/unique-types3/a.h
new file mode 100644
index 0000000000000..4bc15769df572
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types3/a.h
@@ -0,0 +1,3 @@
+template <typename T> struct S {
+ T t;
+};
diff --git a/lldb/test/API/lang/cpp/unique-types3/main.cpp b/lldb/test/API/lang/cpp/unique-types3/main.cpp
new file mode 100644
index 0000000000000..44816bc70a006
--- /dev/null
+++ b/lldb/test/API/lang/cpp/unique-types3/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+S<double> a1;
+S<int> a2;
+S<float> a3;
+
+void f(S<int> &);
+
+int main() { f(a2); }
More information about the lldb-commits
mailing list