-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. #90663
[lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. #90663
Changes from 2 commits
4e80000
751a58d
c0a539e
515c85d
40b87f0
6086993
0481675
4f2fbfb
c7edf15
3414ce2
a8d45fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,6 +182,8 @@ class SymbolFileDWARF : public SymbolFileCommon { | |
void FindFunctions(const RegularExpression ®ex, bool include_inlines, | ||
SymbolContextList &sc_list) override; | ||
|
||
DWARFDIE FindDefinitionDIE(const DWARFDIE &die); | ||
|
||
void | ||
GetMangledNamesForFunction(const std::string &scope_qualified_name, | ||
std::vector<ConstString> &mangled_names) override; | ||
|
@@ -342,6 +344,11 @@ class SymbolFileDWARF : public SymbolFileCommon { | |
return m_forward_decl_compiler_type_to_die; | ||
} | ||
|
||
typedef llvm::DenseMap<const DWARFDebugInfoEntry *, DIERef> DIEToDIE; | ||
virtual DIEToDIE &GetDeclarationDIEToDefinitionDIE() { | ||
return m_die_to_def_die; | ||
} | ||
|
||
typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP> | ||
DIEToVariableSP; | ||
|
||
|
@@ -533,9 +540,16 @@ class SymbolFileDWARF : public SymbolFileCommon { | |
NameToOffsetMap m_function_scope_qualified_name_map; | ||
std::unique_ptr<DWARFDebugRanges> m_ranges; | ||
UniqueDWARFASTTypeMap m_unique_ast_type_map; | ||
// A map from DIE to lldb_private::Type. For record type, the key might be | ||
// either declaration DIE or definition DIE. | ||
DIEToTypePtr m_die_to_type; | ||
DIEToVariableSP m_die_to_variable_sp; | ||
// A map from CompilerType to the struct/class/union/enum DIE (might be a | ||
// declaration or a definition) that is used to construct it. | ||
CompilerTypeToDIE m_forward_decl_compiler_type_to_die; | ||
// A map from a struct/class/union/enum DIE (might be a declaration or a | ||
// definition) to its definition DIE. | ||
DIEToDIE m_die_to_def_die; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to avoid creating the extra map, for example by modifying the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I also can't help but wonder why do we need that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
llvm::DenseMap<dw_offset_t, std::unique_ptr<SupportFileList>> | ||
m_type_unit_support_files; | ||
std::vector<uint32_t> m_lldb_cu_to_dwarf_unit; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf; | |
bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die, | ||
const lldb_private::Declaration &decl, | ||
const int32_t byte_size, | ||
bool is_forward_declaration, | ||
UniqueDWARFASTType &entry) const { | ||
for (const UniqueDWARFASTType &udt : m_collection) { | ||
// Make sure the tags match | ||
if (udt.m_die.Tag() == die.Tag()) { | ||
// Validate byte sizes of both types only if both are valid. | ||
if (udt.m_byte_size < 0 || byte_size < 0 || | ||
udt.m_byte_size == byte_size) { | ||
// Make sure the file and line match | ||
if (udt.m_declaration == decl) { | ||
// The type has the same name, and was defined on the same file and | ||
// line. Now verify all of the parent DIEs match. | ||
DWARFDIE parent_arg_die = die.GetParent(); | ||
DWARFDIE parent_pos_die = udt.m_die.GetParent(); | ||
bool match = true; | ||
bool done = false; | ||
while (!done && match && parent_arg_die && parent_pos_die) { | ||
const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); | ||
const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); | ||
if (parent_arg_tag == parent_pos_tag) { | ||
switch (parent_arg_tag) { | ||
case DW_TAG_class_type: | ||
case DW_TAG_structure_type: | ||
case DW_TAG_union_type: | ||
case DW_TAG_namespace: { | ||
const char *parent_arg_die_name = parent_arg_die.GetName(); | ||
if (parent_arg_die_name == | ||
nullptr) // Anonymous (i.e. no-name) struct | ||
{ | ||
// If they are not both definition DIEs or both declaration DIEs, then | ||
// don't check for byte size and declaration location, because declaration | ||
// DIEs usually don't have those info. | ||
bool matching_size_declaration = | ||
udt.m_is_forward_declaration != is_forward_declaration | ||
? true | ||
: (udt.m_byte_size < 0 || byte_size < 0 || | ||
udt.m_byte_size == byte_size) && | ||
udt.m_declaration == decl; | ||
if (matching_size_declaration) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For readability, can we do:
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
// The type has the same name, and was defined on the same file and | ||
// line. Now verify all of the parent DIEs match. | ||
DWARFDIE parent_arg_die = die.GetParent(); | ||
DWARFDIE parent_pos_die = udt.m_die.GetParent(); | ||
bool match = true; | ||
bool done = false; | ||
while (!done && match && parent_arg_die && parent_pos_die) { | ||
const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); | ||
const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); | ||
if (parent_arg_tag == parent_pos_tag) { | ||
switch (parent_arg_tag) { | ||
case DW_TAG_class_type: | ||
case DW_TAG_structure_type: | ||
case DW_TAG_union_type: | ||
case DW_TAG_namespace: { | ||
const char *parent_arg_die_name = parent_arg_die.GetName(); | ||
if (parent_arg_die_name == | ||
nullptr) // Anonymous (i.e. no-name) struct | ||
{ | ||
match = false; | ||
} else { | ||
const char *parent_pos_die_name = parent_pos_die.GetName(); | ||
if (parent_pos_die_name == nullptr || | ||
((parent_arg_die_name != parent_pos_die_name) && | ||
strcmp(parent_arg_die_name, parent_pos_die_name))) | ||
match = false; | ||
} else { | ||
const char *parent_pos_die_name = parent_pos_die.GetName(); | ||
if (parent_pos_die_name == nullptr || | ||
((parent_arg_die_name != parent_pos_die_name) && | ||
strcmp(parent_arg_die_name, parent_pos_die_name))) | ||
match = false; | ||
} | ||
} break; | ||
|
||
case DW_TAG_compile_unit: | ||
case DW_TAG_partial_unit: | ||
done = true; | ||
break; | ||
default: | ||
break; | ||
} | ||
} break; | ||
|
||
case DW_TAG_compile_unit: | ||
case DW_TAG_partial_unit: | ||
done = true; | ||
break; | ||
default: | ||
break; | ||
} | ||
parent_arg_die = parent_arg_die.GetParent(); | ||
parent_pos_die = parent_pos_die.GetParent(); | ||
} | ||
parent_arg_die = parent_arg_die.GetParent(); | ||
parent_pos_die = parent_pos_die.GetParent(); | ||
} | ||
|
||
if (match) { | ||
entry = udt; | ||
return true; | ||
} | ||
if (match) { | ||
entry = udt; | ||
return true; | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to erase the iterator before calling FindDefinitionDIE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, we change
m_forward_decl_compiler_type_to_die
to be updated with definition DIE. So, we may need to erase twice, because the first erase is always necessary if we failed to find definition DIE for it. The second erase is because calling FindDefinitionDIE might add new entry to the definition DIE because the first one removed it.