Skip to content

Commit 8395f9c

Browse files
authored
[lldb/DWARF] Remove parsing recursion when searching for definition DIEs (#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.
1 parent 2d84e0f commit 8395f9c

7 files changed

+220
-218
lines changed

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

+119-108
Large diffs are not rendered by default.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

+91-96
Original file line numberDiff line numberDiff line change
@@ -3045,118 +3045,113 @@ TypeSP SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE(
30453045
return type_sp;
30463046
}
30473047

3048-
TypeSP
3049-
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
3050-
TypeSP type_sp;
3048+
DWARFDIE
3049+
SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) {
3050+
if (!die.GetName())
3051+
return {};
30513052

3052-
if (die.GetName()) {
3053-
const dw_tag_t tag = die.Tag();
3053+
const dw_tag_t tag = die.Tag();
30543054

3055-
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
3056-
if (log) {
3057-
GetObjectFile()->GetModule()->LogMessage(
3058-
log,
3059-
"SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(tag={0} "
3060-
"({1}), name='{2}')",
3061-
DW_TAG_value_to_name(tag), tag, die.GetName());
3062-
}
3055+
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
3056+
if (log) {
3057+
GetObjectFile()->GetModule()->LogMessage(
3058+
log,
3059+
"SymbolFileDWARF::FindDefinitionDIE(tag={0} "
3060+
"({1}), name='{2}')",
3061+
DW_TAG_value_to_name(tag), tag, die.GetName());
3062+
}
30633063

3064-
// Get the type system that we are looking to find a type for. We will
3065-
// use this to ensure any matches we find are in a language that this
3066-
// type system supports
3067-
const LanguageType language = GetLanguage(*die.GetCU());
3068-
TypeSystemSP type_system = nullptr;
3069-
if (language != eLanguageTypeUnknown) {
3070-
auto type_system_or_err = GetTypeSystemForLanguage(language);
3071-
if (auto err = type_system_or_err.takeError()) {
3072-
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
3073-
"Cannot get TypeSystem for language {1}: {0}",
3074-
Language::GetNameForLanguageType(language));
3075-
} else {
3076-
type_system = *type_system_or_err;
3077-
}
3064+
// Get the type system that we are looking to find a type for. We will
3065+
// use this to ensure any matches we find are in a language that this
3066+
// type system supports
3067+
const LanguageType language = GetLanguage(*die.GetCU());
3068+
TypeSystemSP type_system = nullptr;
3069+
if (language != eLanguageTypeUnknown) {
3070+
auto type_system_or_err = GetTypeSystemForLanguage(language);
3071+
if (auto err = type_system_or_err.takeError()) {
3072+
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
3073+
"Cannot get TypeSystem for language {1}: {0}",
3074+
Language::GetNameForLanguageType(language));
3075+
} else {
3076+
type_system = *type_system_or_err;
30783077
}
3078+
}
30793079

3080-
// See comments below about -gsimple-template-names for why we attempt to
3081-
// compute missing template parameter names.
3082-
std::vector<std::string> template_params;
3083-
DWARFDeclContext die_dwarf_decl_ctx;
3084-
DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() : nullptr;
3085-
for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
3086-
ctx_die = ctx_die.GetParentDeclContextDIE()) {
3087-
die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
3088-
template_params.push_back(
3089-
(ctx_die.IsStructUnionOrClass() && dwarf_ast)
3090-
? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
3091-
: "");
3092-
}
3093-
const bool any_template_params = llvm::any_of(
3094-
template_params, [](llvm::StringRef p) { return !p.empty(); });
3095-
3096-
auto die_matches = [&](DWARFDIE type_die) {
3097-
// Resolve the type if both have the same tag or {class, struct} tags.
3098-
const bool tag_matches =
3099-
type_die.Tag() == tag ||
3100-
(IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
3101-
if (!tag_matches)
3102-
return false;
3103-
if (any_template_params) {
3104-
size_t pos = 0;
3105-
for (DWARFDIE ctx_die = type_die;
3106-
ctx_die && !isUnitType(ctx_die.Tag()) &&
3107-
pos < template_params.size();
3108-
ctx_die = ctx_die.GetParentDeclContextDIE(), ++pos) {
3109-
if (template_params[pos].empty())
3110-
continue;
3111-
if (template_params[pos] != dwarf_ast->GetDIEClassTemplateParams(ctx_die))
3112-
return false;
3113-
}
3114-
if (pos != template_params.size())
3080+
// See comments below about -gsimple-template-names for why we attempt to
3081+
// compute missing template parameter names.
3082+
std::vector<std::string> template_params;
3083+
DWARFDeclContext die_dwarf_decl_ctx;
3084+
DWARFASTParser *dwarf_ast =
3085+
type_system ? type_system->GetDWARFParser() : nullptr;
3086+
for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
3087+
ctx_die = ctx_die.GetParentDeclContextDIE()) {
3088+
die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
3089+
template_params.push_back(
3090+
(ctx_die.IsStructUnionOrClass() && dwarf_ast)
3091+
? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
3092+
: "");
3093+
}
3094+
const bool any_template_params = llvm::any_of(
3095+
template_params, [](llvm::StringRef p) { return !p.empty(); });
3096+
3097+
auto die_matches = [&](DWARFDIE type_die) {
3098+
// Resolve the type if both have the same tag or {class, struct} tags.
3099+
const bool tag_matches =
3100+
type_die.Tag() == tag ||
3101+
(IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
3102+
if (!tag_matches)
3103+
return false;
3104+
if (any_template_params) {
3105+
size_t pos = 0;
3106+
for (DWARFDIE ctx_die = type_die; ctx_die && !isUnitType(ctx_die.Tag()) &&
3107+
pos < template_params.size();
3108+
ctx_die = ctx_die.GetParentDeclContextDIE(), ++pos) {
3109+
if (template_params[pos].empty())
3110+
continue;
3111+
if (template_params[pos] !=
3112+
dwarf_ast->GetDIEClassTemplateParams(ctx_die))
31153113
return false;
31163114
}
3115+
if (pos != template_params.size())
3116+
return false;
3117+
}
3118+
return true;
3119+
};
3120+
DWARFDIE result;
3121+
m_index->GetFullyQualifiedType(die_dwarf_decl_ctx, [&](DWARFDIE type_die) {
3122+
// Make sure type_die's language matches the type system we are
3123+
// looking for. We don't want to find a "Foo" type from Java if we
3124+
// are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
3125+
if (type_system &&
3126+
!type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
31173127
return true;
3118-
};
3119-
m_index->GetFullyQualifiedType(die_dwarf_decl_ctx, [&](DWARFDIE type_die) {
3120-
// Make sure type_die's language matches the type system we are
3121-
// looking for. We don't want to find a "Foo" type from Java if we
3122-
// are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
3123-
if (type_system &&
3124-
!type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
3125-
return true;
3126-
3127-
if (!die_matches(type_die)) {
3128-
if (log) {
3129-
GetObjectFile()->GetModule()->LogMessage(
3130-
log,
3131-
"SymbolFileDWARF::"
3132-
"FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), "
3133-
"name='{2}') ignoring die={3:x16} ({4})",
3134-
DW_TAG_value_to_name(tag), tag, die.GetName(),
3135-
type_die.GetOffset(), type_die.GetName());
3136-
}
3137-
return true;
3138-
}
31393128

3129+
if (!die_matches(type_die)) {
31403130
if (log) {
3141-
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
31423131
GetObjectFile()->GetModule()->LogMessage(
31433132
log,
3144-
"SymbolFileDWARF::"
3145-
"FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), name='{2}') "
3146-
"trying die={3:x16} ({4})",
3133+
"SymbolFileDWARF::FindDefinitionDIE(tag={0} ({1}), "
3134+
"name='{2}') ignoring die={3:x16} ({4})",
31473135
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
3148-
type_dwarf_decl_ctx.GetQualifiedName());
3136+
type_die.GetName());
31493137
}
3138+
return true;
3139+
}
31503140

3151-
Type *resolved_type = ResolveType(type_die, false);
3152-
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
3153-
return true;
3141+
if (log) {
3142+
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
3143+
GetObjectFile()->GetModule()->LogMessage(
3144+
log,
3145+
"SymbolFileDWARF::FindDefinitionTypeDIE(tag={0} ({1}), name='{2}') "
3146+
"trying die={3:x16} ({4})",
3147+
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
3148+
type_dwarf_decl_ctx.GetQualifiedName());
3149+
}
31543150

3155-
type_sp = resolved_type->shared_from_this();
3156-
return false;
3157-
});
3158-
}
3159-
return type_sp;
3151+
result = type_die;
3152+
return false;
3153+
});
3154+
return result;
31603155
}
31613156

31623157
TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die,

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
360360

361361
SymbolFileDWARFDebugMap *GetDebugMapSymfile();
362362

363-
virtual lldb::TypeSP
364-
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
363+
virtual DWARFDIE FindDefinitionDIE(const DWARFDIE &die);
365364

366365
virtual lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
367366
const DWARFDIE &die, ConstString type_name, bool must_be_implementation);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -1147,14 +1147,13 @@ SymbolFileDWARFDebugMap::ParseCallEdgesInFunction(
11471147
return {};
11481148
}
11491149

1150-
TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext(
1151-
const DWARFDIE &die) {
1152-
TypeSP type_sp;
1150+
DWARFDIE SymbolFileDWARFDebugMap::FindDefinitionDIE(const DWARFDIE &die) {
1151+
DWARFDIE result;
11531152
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
1154-
type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die);
1155-
return type_sp ? IterationAction::Stop : IterationAction::Continue;
1153+
result = oso_dwarf->FindDefinitionDIE(die);
1154+
return result ? IterationAction::Stop : IterationAction::Continue;
11561155
});
1157-
return type_sp;
1156+
return result;
11581157
}
11591158

11601159
bool SymbolFileDWARFDebugMap::Supports_DW_AT_APPLE_objc_complete_type(

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
277277

278278
CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);
279279

280-
lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
280+
DWARFDIE FindDefinitionDIE(const DWARFDIE &die);
281281

282282
bool Supports_DW_AT_APPLE_objc_complete_type(SymbolFileDWARF *skip_dwarf_oso);
283283

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,8 @@ UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
125125
return GetBaseSymbolFile().GetUniqueDWARFASTTypeMap();
126126
}
127127

128-
lldb::TypeSP
129-
SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
130-
return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(die);
128+
DWARFDIE SymbolFileDWARFDwo::FindDefinitionDIE(const DWARFDIE &die) {
129+
return GetBaseSymbolFile().FindDefinitionDIE(die);
131130
}
132131

133132
lldb::TypeSP SymbolFileDWARFDwo::FindCompleteObjCDefinitionTypeForDIE(

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
7878

7979
UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override;
8080

81-
lldb::TypeSP
82-
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) override;
81+
DWARFDIE FindDefinitionDIE(const DWARFDIE &die) override;
8382

8483
lldb::TypeSP
8584
FindCompleteObjCDefinitionTypeForDIE(const DWARFDIE &die,

0 commit comments

Comments
 (0)