aeubanks created this revision.
Herald added a reviewer: shafik.
Herald added a project: All.
aeubanks updated this revision to Diff 461963.
aeubanks added a comment.
aeubanks added a reviewer: dblaikie.
aeubanks published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
comments
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:801
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
- std::string &storage) const {
----------------
moved since now we need the clang AST to rebuild the fully qualified name
the only other caller is some logging
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134378
Files:
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
lldb/test/API/lang/cpp/unique-types2/Makefile
lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
lldb/test/API/lang/cpp/unique-types2/main.cpp
Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -0,0 +1,11 @@
+
+template <class T> class Foo {
+ T t;
+};
+
+int main() {
+ Foo<char> t1;
+ Foo<int> t2;
+ Foo<Foo<int>> t3;
+ // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -0,0 +1,18 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+ def test(self):
+ """Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+ self.build()
+ 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)
Index: lldb/test/API/lang/cpp/unique-types2/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===================================================================
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -51,7 +51,7 @@
# 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 *")
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9323,7 +9323,9 @@
const TypeSystemClang::TemplateParameterInfos &template_param_infos) {
if (template_param_infos.IsValid()) {
std::string template_basename(parent_name);
- template_basename.erase(template_basename.find('<'));
+ auto i = template_basename.find('<');
+ if (i != std::string::npos)
+ template_basename.erase(i);
return CreateClassTemplateDecl(decl_ctx, owning_module, access_type,
template_basename.c_str(), tag_decl_kind,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2475,6 +2475,43 @@
return types.GetSize() < max_matches;
});
+ // If we've come up empty and the name contains '<' meaning it contains
+ // templates parameters, try again with the template parameters stripped since
+ // with simplified template names the DT_name may only contain the base name.
+ if (types.Empty()) {
+ 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 (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.
@@ -2943,8 +2980,6 @@
if (!try_resolving_type) {
if (log) {
- std::string qualified_name;
- type_die.GetQualifiedName(qualified_name);
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
@@ -2952,7 +2987,7 @@
"qualified-name='%s') ignoring die=0x%8.8x (%s)",
DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
- qualified_name.c_str());
+ type_die.GetName());
}
return true;
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
const char *GetPubname(const DWARFUnit *cu) const;
- const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
- const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
- std::string &storage) const;
-
bool GetDIENamesAndRanges(
DWARFUnit *cu, const char *&name, const char *&mangled,
DWARFRangeList &rangeList, int &decl_file, int &decl_line,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
return DWARFDIE();
}
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
- std::string &storage) const {
- DWARFAttributes attributes;
- GetAttributes(cu, attributes, Recurse::yes);
- return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
- const DWARFAttributes &attributes,
- std::string &storage) const {
-
- const char *name = GetName(cu);
-
- if (name) {
- DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
- storage.clear();
- // TODO: change this to get the correct decl context parent....
- while (parent_decl_ctx_die) {
- const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
- switch (parent_tag) {
- case DW_TAG_namespace: {
- const char *namespace_name = parent_decl_ctx_die.GetName();
- if (namespace_name) {
- storage.insert(0, "::");
- storage.insert(0, namespace_name);
- } else {
- storage.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: {
- const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
- if (class_union_struct_name) {
- storage.insert(0, "::");
- storage.insert(0, class_union_struct_name);
- }
- parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
- } break;
-
- default:
- parent_decl_ctx_die.Clear();
- break;
- }
- }
-
- if (storage.empty())
- storage.append("::");
-
- storage.append(name);
- }
- if (storage.empty())
- return nullptr;
- return storage.c_str();
-}
-
lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -30,8 +30,6 @@
const char *GetPubname() const;
- const char *GetQualifiedName(std::string &storage) const;
-
using DWARFBaseDIE::GetName;
void GetName(lldb_private::Stream &s) const;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -215,13 +215,6 @@
return nullptr;
}
-const char *DWARFDIE::GetQualifiedName(std::string &storage) const {
- if (IsValid())
- return m_die->GetQualifiedName(m_cu, storage);
- else
- return nullptr;
-}
-
// GetName
//
// Get value of the DW_AT_name attribute and place that value into the supplied
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1542,8 +1542,94 @@
// 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;
- if (die.GetQualifiedName(qualified_name))
+
+ // The name may not contain template parameters due to simplified template
+ // names, we must reconstruct the full name from child template parameter
+ // dies.
+ if (die.IsValid()) {
+ auto get_template_params_string = [this](DWARFDIE die) {
+ if (llvm::StringRef(die.GetName()).contains("<"))
+ return std::string();
+ TypeSystemClang::TemplateParameterInfos template_param_infos;
+ if (ParseTemplateParameterInfos(die, template_param_infos)) {
+ std::string all_template_names;
+ llvm::SmallVector<clang::TemplateArgument, 2> &args =
+ template_param_infos.args;
+ if (template_param_infos.hasParameterPack()) {
+ args = template_param_infos.packed_args->args;
+ }
+ for (auto &arg : args) {
+ std::string template_name;
+ llvm::raw_string_ostream os(template_name);
+ arg.dump(os);
+ if (!template_name.empty()) {
+ if (all_template_names.empty()) {
+ all_template_names.append("<");
+ } else {
+ all_template_names.append(", ");
+ }
+ all_template_names.append(template_name);
+ }
+ }
+ if (!all_template_names.empty()) {
+ all_template_names.append(">");
+ // does the spacing here matter?
+ }
+ return all_template_names;
+ }
+ return std::string();
+ };
+ const char *name = die.GetName();
+ if (name) {
+ DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+ // TODO: change this to get the correct decl context parent....
+ while (parent_decl_ctx_die) {
+ const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
+ switch (parent_tag) {
+ case DW_TAG_namespace: {
+ const char *namespace_name = parent_decl_ctx_die.GetName();
+ if (namespace_name) {
+ 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: {
+ const char *class_union_struct_name =
+ parent_decl_ctx_die.GetName();
+
+ if (class_union_struct_name) {
+ qualified_name.insert(
+ 0, get_template_params_string(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(get_template_params_string(die));
+ }
unique_typename = ConstString(qualified_name);
+ }
unique_decl.Clear();
}
@@ -1716,36 +1802,40 @@
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();
+ bool is_template = false;
+ TypeSystemClang::TemplateParameterInfos template_param_infos;
+ if (ParseTemplateParameterInfos(die, template_param_infos)) {
+ is_template = !template_param_infos.args.empty() ||
+ template_param_infos.packed_args;
+ }
+
+ if (is_template) {
+ 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();
+ }
- 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;
+ 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);
- }
+ m_ast.SetMetadata(class_template_decl, metadata);
+ m_ast.SetMetadata(class_specialization_decl, metadata);
}
if (!clang_type_was_created) {
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===================================================================
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -227,7 +227,8 @@
DEBUG_INFO_FLAG ?= -gdwarf
endif
-DEBUG_INFO_FLAG ?= -g
+# DO NOT SUBMIT
+DEBUG_INFO_FLAG ?= -g -gsimple-template-names
CFLAGS ?= $(DEBUG_INFO_FLAG) -O0
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
