Skip to content

Commit 52db8db

Browse files
committed
[lldb/DWARF] Remove parsing recursion when searching for definition DIEs
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 llvm#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 c053ec9 commit 52db8db

7 files changed

+208
-213
lines changed

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

Lines changed: 107 additions & 103 deletions
Large diffs are not rendered by default.

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

Lines changed: 91 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -3038,118 +3038,113 @@ TypeSP SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE(
30383038
return type_sp;
30393039
}
30403040

3041-
TypeSP
3042-
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
3043-
TypeSP type_sp;
3041+
DWARFDIE
3042+
SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) {
3043+
if (!die.GetName())
3044+
return {};
30443045

3045-
if (die.GetName()) {
3046-
const dw_tag_t tag = die.Tag();
3046+
const dw_tag_t tag = die.Tag();
30473047

3048-
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
3049-
if (log) {
3050-
GetObjectFile()->GetModule()->LogMessage(
3051-
log,
3052-
"SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(tag={0} "
3053-
"({1}), name='{2}')",
3054-
DW_TAG_value_to_name(tag), tag, die.GetName());
3055-
}
3048+
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
3049+
if (log) {
3050+
GetObjectFile()->GetModule()->LogMessage(
3051+
log,
3052+
"SymbolFileDWARF::FindDefinitionDIE(tag={0} "
3053+
"({1}), name='{2}')",
3054+
DW_TAG_value_to_name(tag), tag, die.GetName());
3055+
}
30563056

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

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

3122+
if (!die_matches(type_die)) {
31333123
if (log) {
3134-
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
31353124
GetObjectFile()->GetModule()->LogMessage(
31363125
log,
3137-
"SymbolFileDWARF::"
3138-
"FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), name='{2}') "
3139-
"trying die={3:x16} ({4})",
3126+
"SymbolFileDWARF::FindDefinitionDIE(tag={0} ({1}), "
3127+
"name='{2}') ignoring die={3:x16} ({4})",
31403128
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
3141-
type_dwarf_decl_ctx.GetQualifiedName());
3129+
type_die.GetName());
31423130
}
3131+
return true;
3132+
}
31433133

3144-
Type *resolved_type = ResolveType(type_die, false);
3145-
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
3146-
return true;
3134+
if (log) {
3135+
DWARFDeclContext type_dwarf_decl_ctx = type_die.GetDWARFDeclContext();
3136+
GetObjectFile()->GetModule()->LogMessage(
3137+
log,
3138+
"SymbolFileDWARF::FindDefinitionTypeDIE(tag={0} ({1}), name='{2}') "
3139+
"trying die={3:x16} ({4})",
3140+
DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
3141+
type_dwarf_decl_ctx.GetQualifiedName());
3142+
}
31473143

3148-
type_sp = resolved_type->shared_from_this();
3149-
return false;
3150-
});
3151-
}
3152-
return type_sp;
3144+
result = type_die;
3145+
return false;
3146+
});
3147+
return result;
31533148
}
31543149

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

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
351351

352352
SymbolFileDWARFDebugMap *GetDebugMapSymfile();
353353

354-
virtual lldb::TypeSP
355-
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
354+
virtual DWARFDIE FindDefinitionDIE(const DWARFDIE &die);
356355

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

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

Lines changed: 5 additions & 6 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 2 additions & 3 deletions
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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
7676

7777
UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override;
7878

79-
lldb::TypeSP
80-
FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) override;
79+
DWARFDIE FindDefinitionDIE(const DWARFDIE &die) override;
8180

8281
lldb::TypeSP
8382
FindCompleteObjCDefinitionTypeForDIE(const DWARFDIE &die,

0 commit comments

Comments
 (0)