Skip to content

Commit

Permalink
[lldb/DWARF] Remove parsing recursion when searching for definition D…
Browse files Browse the repository at this point in the history 
…IEs (#96484)

If ParseStructureLikeDIE (or ParseEnum) encountered a declaration DIE,
it would call FindDefinitionTypeForDIE. This returned a fully formed
type, which it achieved by recursing back into ParseStructureLikeDIE
with the definition DIE.

This obscured the control flow and caused us to repeat some work (e.g.
the UniqueDWARFASTTypeMap lookup), but it mostly worked until we tried
to delay the definition search in #90663. After this patch, the two
ParseStructureLikeDIE calls were no longer recursive, but rather the
second call happened as a part of the CompleteType() call. This opened
the door to inconsistencies, as the second ParseStructureLikeDIE call
was not aware it was called to process a definition die for an existing
type.

To make that possible, this patch removes the recusive type resolution
from this function, and leaves just the "find definition die"
functionality. After finding the definition DIE, we just go back to the
original ParseStructureLikeDIE call, and have it finish the parsing
process with the new DIE.

While this patch is motivated by the work on delaying the definition
searching, I believe it is also useful on its own.
  • Loading branch information
@labath
labath authored Jun 25, 2024
1 parent 2d84e0f commit 8395f9c
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 218 deletions.
227 changes: 119 additions & 108 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
View file

Large diffs are not rendered by default.

187 changes: 91 additions & 96 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3045,118 +3045,113 @@ TypeSP SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE(
return type_sp;
}

TypeSP
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
TypeSP type_sp;
DWARFDIE
SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) {
if (!die.GetName())
return {};

if (die.GetName()) {
const dw_tag_t tag = die.Tag();
const dw_tag_t tag = die.Tag();

Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
if (log) {
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(tag={0} "
"({1}), name='{2}')",
DW_TAG_value_to_name(tag), tag, die.GetName());
}
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
if (log) {
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::FindDefinitionDIE(tag={0} "
"({1}), name='{2}')",
DW_TAG_value_to_name(tag), tag, die.GetName());
}

// Get the type system that we are looking to find a type for. We will
// use this to ensure any matches we find are in a language that this
// type system supports
const LanguageType language = GetLanguage(*die.GetCU());
TypeSystemSP type_system = nullptr;
if (language != eLanguageTypeUnknown) {
auto type_system_or_err = GetTypeSystemForLanguage(language);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Cannot get TypeSystem for language {1}: {0}",
Language::GetNameForLanguageType(language));
} else {
type_system = *type_system_or_err;
}
// Get the type system that we are looking to find a type for. We will
// use this to ensure any matches we find are in a language that this
// type system supports
const LanguageType language = GetLanguage(*die.GetCU());
TypeSystemSP type_system = nullptr;
if (language != eLanguageTypeUnknown) {
auto type_system_or_err = GetTypeSystemForLanguage(language);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Cannot get TypeSystem for language {1}: {0}",
Language::GetNameForLanguageType(language));
} else {
type_system = *type_system_or_err;
}
}

// See comments below about -gsimple-template-names for why we attempt to
// compute missing template parameter names.
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(); });

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())
// See comments below about -gsimple-template-names for why we attempt to
// compute missing template parameter names.
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(); });

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;
};
DWARFDIE result;
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
// are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
if (type_system &&
!type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
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
// are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
if (type_system &&
!type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
return true;

if (!die_matches(type_die)) {
if (log) {
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
"FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), "
"name='{2}') ignoring die={3:x16} ({4})",
DW_TAG_value_to_name(tag), tag, die.GetName(),
type_die.GetOffset(), type_die.GetName());
}
return true;
}

if (!die_matches(type_die)) {
if (log) {
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
"FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), name='{2}') "
"trying die={3:x16} ({4})",
"SymbolFileDWARF::FindDefinitionDIE(tag={0} ({1}), "
"name='{2}') ignoring die={3:x16} ({4})",
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
type_dwarf_decl_ctx.GetQualifiedName());
type_die.GetName());
}
return true;
}

Type *resolved_type = ResolveType(type_die, false);
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
return true;
if (log) {
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::FindDefinitionTypeDIE(tag={0} ({1}), name='{2}') "
"trying die={3:x16} ({4})",
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
type_dwarf_decl_ctx.GetQualifiedName());
}

type_sp = resolved_type->shared_from_this();
return false;
});
}
return type_sp;
result = type_die;
return false;
});
return result;
}

TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die,
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ class SymbolFileDWARF : public SymbolFileCommon {

SymbolFileDWARFDebugMap *GetDebugMapSymfile();

virtual lldb::TypeSP
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
virtual DWARFDIE FindDefinitionDIE(const DWARFDIE &die);

virtual lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
const DWARFDIE &die, ConstString type_name, bool must_be_implementation);
Expand Down
11 changes: 5 additions & 6 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,14 +1147,13 @@ SymbolFileDWARFDebugMap::ParseCallEdgesInFunction(
return {};
}

TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext(
const DWARFDIE &die) {
TypeSP type_sp;
DWARFDIE SymbolFileDWARFDebugMap::FindDefinitionDIE(const DWARFDIE &die) {
DWARFDIE result;
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die);
return type_sp ? IterationAction::Stop : IterationAction::Continue;
result = oso_dwarf->FindDefinitionDIE(die);
return result ? IterationAction::Stop : IterationAction::Continue;
});
return type_sp;
return result;
}

bool SymbolFileDWARFDebugMap::Supports_DW_AT_APPLE_objc_complete_type(
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {

CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);

lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
DWARFDIE FindDefinitionDIE(const DWARFDIE &die);

bool Supports_DW_AT_APPLE_objc_complete_type(SymbolFileDWARF *skip_dwarf_oso);

Expand Down
5 changes: 2 additions & 3 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,8 @@ UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
return GetBaseSymbolFile().GetUniqueDWARFASTTypeMap();
}

lldb::TypeSP
SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(die);
DWARFDIE SymbolFileDWARFDwo::FindDefinitionDIE(const DWARFDIE &die) {
return GetBaseSymbolFile().FindDefinitionDIE(die);
}

lldb::TypeSP SymbolFileDWARFDwo::FindCompleteObjCDefinitionTypeForDIE(
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {

UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override;

lldb::TypeSP
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) override;
DWARFDIE FindDefinitionDIE(const DWARFDIE &die) override;

lldb::TypeSP
FindCompleteObjCDefinitionTypeForDIE(const DWARFDIE &die,
Expand Down

0 comments on commit 8395f9c

Please sign in to comment.