Skip to content

[lldb][DWARF] Change GetAttributes to always visit current DIE before recursing #123261

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

Merged
merged 9 commits into from
Jan 17, 2025
83 changes: 61 additions & 22 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,22 +281,34 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
return !ranges.empty();
}

// Get all attribute values for a given DIE, including following any
// specification or abstract origin attributes and including those in the
// results. Any duplicate attributes will have the first instance take
// precedence (this can happen for declaration attributes).
void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
DWARFAttributes &attributes,
Recurse recurse,
uint32_t curr_depth) const {
const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
if (!abbrevDecl) {
attributes.Clear();
return;
}
/// Helper for the public \ref DWARFDebugInfoEntry::GetAttributes API.
/// Adds all attributes of the DIE at the top of the \c worklist to the
/// \c attributes list. Specifcations and abstract origins are added
/// to the \c worklist if the referenced DIE has not been seen before.
static bool GetAttributes(llvm::SmallVector<DWARFDIE> &worklist,
llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
DWARFAttributes &attributes) {
assert(!worklist.empty() && "Need at least one DIE to visit.");
assert(seen.size() >= 1 &&
"Need to have seen at least the currently visited entry.");

DWARFDIE current = worklist.pop_back_val();

const auto *cu = current.GetCU();
assert(cu);

const auto *entry = current.GetDIE();
assert(entry);

const auto *abbrevDecl =
entry->GetAbbreviationDeclarationPtr(current.GetCU());
if (!abbrevDecl)
return false;

const DWARFDataExtractor &data = cu->GetData();
lldb::offset_t offset = GetFirstAttributeOffset();
lldb::offset_t offset = current.GetDIE()->GetFirstAttributeOffset();

const bool is_first_die = seen.size() == 1;

for (const auto &attribute : abbrevDecl->attributes()) {
DWARFFormValue form_value(cu);
Expand All @@ -309,10 +321,10 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
switch (attr) {
case DW_AT_sibling:
case DW_AT_declaration:
if (curr_depth > 0) {
if (!is_first_die) {
// This attribute doesn't make sense when combined with the DIE that
// references this DIE. We know a DIE is referencing this DIE because
// curr_depth is not zero
// we've visited more than one DIE already.
break;
}
[[fallthrough]];
Expand All @@ -321,13 +333,12 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
break;
}

if (recurse == Recurse::yes &&
((attr == DW_AT_specification) || (attr == DW_AT_abstract_origin))) {
if (attr == DW_AT_specification || attr == DW_AT_abstract_origin) {
if (form_value.ExtractValue(data, &offset)) {
DWARFDIE spec_die = form_value.Reference();
if (spec_die)
spec_die.GetDIE()->GetAttributes(spec_die.GetCU(), attributes,
recurse, curr_depth + 1);
if (DWARFDIE spec_die = form_value.Reference()) {
if (seen.insert(spec_die.GetDIE()).second)
worklist.push_back(spec_die);
}
}
} else {
const dw_form_t form = form_value.Form();
Expand All @@ -339,6 +350,34 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
DWARFFormValue::SkipValue(form, data, &offset, cu);
}
}

return true;
}

DWARFAttributes DWARFDebugInfoEntry::GetAttributes(const DWARFUnit *cu,
Recurse recurse) const {
// FIXME: use ElaboratingDIEIterator to follow specifications/abstract origins
// instead of maintaining our own worklist/seen list.

DWARFAttributes attributes;

llvm::SmallVector<DWARFDIE, 3> worklist;
worklist.emplace_back(cu, this);

// Keep track if DIEs already seen to prevent infinite recursion.
// Value of '3' was picked for the same reason that
// DWARFDie::findRecursively does.
llvm::SmallSet<DWARFDebugInfoEntry const *, 3> seen;
seen.insert(this);

do {
if (!::GetAttributes(worklist, seen, attributes)) {
attributes.Clear();
break;
}
} while (!worklist.empty() && recurse == Recurse::yes);

return attributes;
}

// GetAttributeValue
Expand Down
32 changes: 22 additions & 10 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,28 @@ class DWARFDebugInfoEntry {
lldb::offset_t *offset_ptr);

using Recurse = DWARFBaseDIE::Recurse;
DWARFAttributes GetAttributes(DWARFUnit *cu,
Recurse recurse = Recurse::yes) const {
DWARFAttributes attrs;
GetAttributes(cu, attrs, recurse, 0 /* curr_depth */);
return attrs;
}

/// Get all attribute values for a given DIE, optionally following any
/// specifications and abstract origins and including their attributes
/// in the result too.
///
/// When following specifications/abstract origins, the attributes
/// on the referring DIE are guaranteed to be visited before the attributes of
/// the referenced DIE.
///
/// \param[in] cu DWARFUnit that this entry belongs to.
///
/// \param[in] recurse If set to \c Recurse::yes, will include attributes
/// on DIEs referenced via \c DW_AT_specification and \c DW_AT_abstract_origin
/// (including across multiple levels of indirection).
///
/// \returns DWARFAttributes that include all attributes found on this DIE
/// (and possibly referenced DIEs). Attributes may appear multiple times
/// (e.g., if a declaration and definition both specify the same attribute).
/// On failure, the returned DWARFAttributes will be empty.
///
DWARFAttributes GetAttributes(const DWARFUnit *cu,
Recurse recurse = Recurse::yes) const;

dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
DWARFFormValue &formValue,
Expand Down Expand Up @@ -178,10 +194,6 @@ class DWARFDebugInfoEntry {
/// A copy of the DW_TAG value so we don't have to go through the compile
/// unit abbrev table
dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;

private:
void GetAttributes(DWARFUnit *cu, DWARFAttributes &attrs, Recurse recurse,
uint32_t curr_depth) const;
};
} // namespace dwarf
} // namespace lldb_private::plugin
Expand Down
5 changes: 4 additions & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3423,7 +3423,10 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
mangled = form_value.AsCString();
break;
case DW_AT_type:
type_die_form = form_value;
// DW_AT_type on declaration may be less accurate than
// that of definition, so don't overwrite it.
if (!type_die_form.IsValid())
type_die_form = form_value;
break;
case DW_AT_external:
is_external = form_value.Boolean();
Expand Down
Loading
Loading