Skip to content

Commit

Permalink
[lldb/DWARF] Fix type definition search with simple template names (l…
Browse files Browse the repository at this point in the history
…lvm#95905)

With simple template names the template arguments aren't embedded in the
DW_AT_name attribute of the type. The code in
FindDefinitionTypeForDWARFDeclContext was comparing the synthesized
template arguments on the leaf (most deeply nested) DIE, but was not
sufficient, as the difference get be at any level above that
(Foo<T>::Bar vs. Foo<U>::Bar). This patch makes sure we compare the
entire context.

As a drive-by I also remove the completely unnecessary
ConstStringification of the GetDIEClassTemplateParams result.
  • Loading branch information
labath authored and AlexisPerry committed Jun 27, 2024
1 parent 5186ed2 commit 220585e
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 46 deletions.
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class DWARFASTParser {
virtual void EnsureAllDIEsInDeclContextHaveBeenParsed(
CompilerDeclContext decl_context) = 0;

virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
virtual std::string GetDIEClassTemplateParams(const DWARFDIE &die) = 0;

static std::optional<SymbolFile::ArrayInfo>
ParseChildArrayInfo(const DWARFDIE &parent_die,
Expand Down
16 changes: 8 additions & 8 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,16 +838,16 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
return type_sp;
}

ConstString
std::string
DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
if (llvm::StringRef(die.GetName()).contains("<"))
return ConstString();
return {};

TypeSystemClang::TemplateParameterInfos template_param_infos;
if (ParseTemplateParameterInfos(die, template_param_infos)) {
return ConstString(m_ast.PrintTemplateParams(template_param_infos));
}
return ConstString();
if (ParseTemplateParameterInfos(die, template_param_infos))
return m_ast.PrintTemplateParams(template_param_infos);

return {};
}

TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
Expand Down Expand Up @@ -1632,7 +1632,7 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
case DW_TAG_union_type: {
if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
qualified_name.insert(
0, GetDIEClassTemplateParams(parent_decl_ctx_die).AsCString(""));
0, GetDIEClassTemplateParams(parent_decl_ctx_die));
qualified_name.insert(0, "::");
qualified_name.insert(0, class_union_struct_name);
}
Expand All @@ -1650,7 +1650,7 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
qualified_name.append("::");

qualified_name.append(name);
qualified_name.append(GetDIEClassTemplateParams(die).AsCString(""));
qualified_name.append(GetDIEClassTemplateParams(die));

return qualified_name;
}
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
///
/// \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
/// If the DIE's name already has '<>', returns an empty string because
/// it's assumed that the caller is using the DIE name anyway.
lldb_private::ConstString GetDIEClassTemplateParams(
std::string GetDIEClassTemplateParams(
const lldb_private::plugin::dwarf::DWARFDIE &die) override;

protected:
Expand Down
70 changes: 36 additions & 34 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3072,14 +3072,43 @@ 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);
std::vector<std::string> template_params;
DWARFDeclContext die_dwarf_decl_ctx;
DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() : nullptr;
for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
ctx_die = ctx_die.GetParentDeclContextDIE()) {
die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
template_params.push_back(
(ctx_die.IsStructUnionOrClass() && dwarf_ast)
? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
: "");
}
const bool any_template_params = llvm::any_of(
template_params, [](llvm::StringRef p) { return !p.empty(); });

const DWARFDeclContext die_dwarf_decl_ctx = die.GetDWARFDeclContext();
auto die_matches = [&](DWARFDIE type_die) {
// Resolve the type if both have the same tag or {class, struct} tags.
const bool tag_matches =
type_die.Tag() == tag ||
(IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
if (!tag_matches)
return false;
if (any_template_params) {
size_t pos = 0;
for (DWARFDIE ctx_die = type_die;
ctx_die && !isUnitType(ctx_die.Tag()) &&
pos < template_params.size();
ctx_die = ctx_die.GetParentDeclContextDIE(), ++pos) {
if (template_params[pos].empty())
continue;
if (template_params[pos] != dwarf_ast->GetDIEClassTemplateParams(ctx_die))
return false;
}
if (pos != template_params.size())
return false;
}
return true;
};
m_index->GetFullyQualifiedType(die_dwarf_decl_ctx, [&](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
Expand All @@ -3088,13 +3117,7 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
!type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
return true;

const dw_tag_t type_tag = type_die.Tag();
// Resolve the type if both have the same tag or {class, struct} tags.
const bool try_resolving_type =
type_tag == tag ||
(IsStructOrClassTag(type_tag) && IsStructOrClassTag(tag));

if (!try_resolving_type) {
if (!die_matches(type_die)) {
if (log) {
GetObjectFile()->GetModule()->LogMessage(
log,
Expand Down Expand Up @@ -3122,27 +3145,6 @@ 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;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// CHECK: (lldb) target variable
// CHECK-NEXT: (ReferencesBoth<'A'>) both_a = {
// CHECK-NEXT: (Outer<'A'>::Inner *) a = 0x{{[0-9A-Fa-f]*}} {}
// CHECK-NEXT: (Outer<'A'>::Inner *) b = 0x{{[0-9A-Fa-f]*}} {}
// CHECK-NEXT: (Outer<'B'>::Inner *) b = 0x{{[0-9A-Fa-f]*}} {}
// CHECK-NEXT: }
// CHECK-NEXT: (ReferencesBoth<'B'>) both_b = {
// CHECK-NEXT: (Outer<'A'>::Inner *) a = 0x{{[0-9A-Fa-f]*}} {}
Expand Down

0 comments on commit 220585e

Please sign in to comment.