Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[llvm/DWARF] Recursively resolve DW_AT_signature references #97423

Merged
merged 2 commits into from
Jul 18, 2024
Merged

[llvm/DWARF] Recursively resolve DW_AT_signature references #97423

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30c7cef
[llvm/DWARF] Recursively resolve DW_AT_signature references
labath Jul 2, 2024
85daf40
test
labath Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,6 @@ class DWARFDie {
DWARFDie getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const;
DWARFDie getAttributeValueAsReferencedDie(const DWARFFormValue &V) const;

DWARFDie resolveTypeUnitReference() const;

/// Extract the range base attribute from this DIE as absolute section offset.
///
/// This is a utility function that checks for either the DW_AT_rnglists_base
Expand Down
43 changes: 15 additions & 28 deletions llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ static void dumpLocationExpr(raw_ostream &OS, const DWARFFormValue &FormValue,
.print(OS, DumpOpts, U);
}

static DWARFDie resolveReferencedType(DWARFDie D, DWARFFormValue F) {
return D.getAttributeValueAsReferencedDie(F).resolveTypeUnitReference();
}

static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die,
const DWARFAttribute &AttrValue, unsigned Indent,
DIDumpOptions DumpOpts) {
Expand Down Expand Up @@ -198,8 +194,8 @@ static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die,
DINameKind::LinkageName))
OS << Space << "\"" << Name << '\"';
} else if (Attr == DW_AT_type || Attr == DW_AT_containing_type) {
DWARFDie D = resolveReferencedType(Die, FormValue);
if (D && !D.isNULL()) {
if (DWARFDie D = Die.getAttributeValueAsReferencedDie(FormValue);
D && !D.isNULL()) {
OS << Space << "\"";
dumpTypeQualifiedName(D, OS);
OS << '"';
Expand Down Expand Up @@ -291,13 +287,12 @@ DWARFDie::findRecursively(ArrayRef<dwarf::Attribute> Attrs) const {
if (auto Value = Die.find(Attrs))
return Value;

if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin))
if (Seen.insert(D).second)
Worklist.push_back(D);

if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
if (Seen.insert(D).second)
Worklist.push_back(D);
for (dwarf::Attribute Attr :
{DW_AT_abstract_origin, DW_AT_specification, DW_AT_signature}) {
if (auto D = Die.getAttributeValueAsReferencedDie(Attr))
if (Seen.insert(D).second)
Worklist.push_back(D);
}
}

return std::nullopt;
Expand All @@ -312,27 +307,19 @@ DWARFDie::getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const {

DWARFDie
DWARFDie::getAttributeValueAsReferencedDie(const DWARFFormValue &V) const {
DWARFDie Result;
if (auto SpecRef = V.getAsRelativeReference()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit questionable that getAsRelativeReference returns a valid value for DW_FORM_ref_sig8 - it's not a CU-relative reference form... (eg: if one had called getAttributeValueAsReferencedDie on a ref_sig8 before this patch, it would have walked off to some garbage offset, right? Interpreting the sig8 as an offset)

So might be worth changing it to not do that and having an explicit/separate case here?

(aside: this case is probably untested? How'd you come across changing this function? Clang/LLVM doesn't produce direct references via ref_sig8, only via the skeleton type DIEs... ah, I see, the findRecursively change would cause us to call getAttributeValueAsReferencedDie(DW_AT_signature)? OK.

Incidentally this has probably added support for some DWARF that GCC produces - where it references the type unit directly from DW_AT_type, maybe? Currently llvm-dwarfdump doesn't resolve such references ( https://godbolt.org/z/E5boxcj1r - note the DW_TAG_variable's DW_AT_type (0xc949e2ea8b91cfb0) only has the signature, and I guess with this patch maybe we now get DW_AT_type (0xc949e2ea8b91cfb0 "t1")?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So might be worth changing it to not do that and having an explicit/separate case here?

Yeah, I was wondering about that. It did seem somewhat suboptimal, but I wasn't particularly keen on taking it on.

Now I am, and here's my attempt to do something about this: #98905

Incidentally this has probably added support for some DWARF that GCC produces - where it references the type unit directly from DW_AT_type, maybe? Currently llvm-dwarfdump doesn't resolve such references ( https://godbolt.org/z/E5boxcj1r - note the DW_TAG_variable's DW_AT_type (0xc949e2ea8b91cfb0) only has the signature, and I guess with this patch maybe we now get DW_AT_type (0xc949e2ea8b91cfb0 "t1")?)

Interesting. I'll look into this tomorrow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incidentally this has probably added support for some DWARF that GCC produces - where it references the type unit directly from DW_AT_type, maybe? Currently llvm-dwarfdump doesn't resolve such references ( https://godbolt.org/z/E5boxcj1r - note the DW_TAG_variable's DW_AT_type (0xc949e2ea8b91cfb0) only has the signature, and I guess with this patch maybe we now get DW_AT_type (0xc949e2ea8b91cfb0 "t1")?)

Interesting. I'll look into this tomorrow.

Yes, it really does fix it. I added a test (modifying an existing one) for that. PTAL.

if (SpecRef->Unit)
Result = SpecRef->Unit->getDIEForOffset(SpecRef->Unit->getOffset() +
SpecRef->Offset);
else if (auto SpecUnit =
U->getUnitVector().getUnitForOffset(SpecRef->Offset))
Result = SpecUnit->getDIEForOffset(SpecRef->Offset);
}
return Result;
}

DWARFDie DWARFDie::resolveTypeUnitReference() const {
if (auto Attr = find(DW_AT_signature)) {
if (std::optional<uint64_t> Sig = Attr->getAsReferenceUVal()) {
return SpecRef->Unit->getDIEForOffset(SpecRef->Unit->getOffset() +
SpecRef->Offset);
if (V.getForm() == dwarf::DW_FORM_ref_sig8) {
if (DWARFTypeUnit *TU = U->getContext().getTypeUnitForHash(
U->getVersion(), *Sig, U->isDWOUnit()))
U->getVersion(), SpecRef->Offset, U->isDWOUnit()))
return TU->getDIEForOffset(TU->getTypeOffset() + TU->getOffset());
}
if (auto *SpecUnit = U->getUnitVector().getUnitForOffset(SpecRef->Offset))
return SpecUnit->getDIEForOffset(SpecRef->Offset);
}
return *this;
return {};
}

std::optional<uint64_t> DWARFDie::getRangesBaseAttribute() const {
Expand Down
52 changes: 25 additions & 27 deletions llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,10 @@ void DWARFTypePrinter::appendArrayType(const DWARFDie &D) {
EndedWithTemplate = false;
}

static DWARFDie resolveReferencedType(DWARFDie D,
dwarf::Attribute Attr = DW_AT_type) {
return D.getAttributeValueAsReferencedDie(Attr).resolveTypeUnitReference();
}
static DWARFDie resolveReferencedType(DWARFDie D, DWARFFormValue F) {
return D.getAttributeValueAsReferencedDie(F).resolveTypeUnitReference();
}
DWARFDie DWARFTypePrinter::skipQualifiers(DWARFDie D) {
while (D && (D.getTag() == DW_TAG_const_type ||
D.getTag() == DW_TAG_volatile_type))
D = resolveReferencedType(D);
D = D.getAttributeValueAsReferencedDie(DW_AT_type);
return D;
}

Expand Down Expand Up @@ -103,7 +96,9 @@ DWARFTypePrinter::appendUnqualifiedNameBefore(DWARFDie D,
return DWARFDie();
}
DWARFDie InnerDIE;
auto Inner = [&] { return InnerDIE = resolveReferencedType(D); };
auto Inner = [&] {
return InnerDIE = D.getAttributeValueAsReferencedDie(DW_AT_type);
};
const dwarf::Tag T = D.getTag();
switch (T) {
case DW_TAG_pointer_type: {
Expand Down Expand Up @@ -134,7 +129,8 @@ DWARFTypePrinter::appendUnqualifiedNameBefore(DWARFDie D,
OS << '(';
else if (Word)
OS << ' ';
if (DWARFDie Cont = resolveReferencedType(D, DW_AT_containing_type)) {
if (DWARFDie Cont =
D.getAttributeValueAsReferencedDie(DW_AT_containing_type)) {
appendQualifiedName(Cont);
EndedWithTemplate = false;
OS << "::";
Expand Down Expand Up @@ -173,7 +169,8 @@ DWARFTypePrinter::appendUnqualifiedNameBefore(DWARFDie D,
case DW_TAG_base_type:
*/
default: {
const char *NamePtr = dwarf::toString(D.find(DW_AT_name), nullptr);
const char *NamePtr =
dwarf::toString(D.findRecursively(DW_AT_name), nullptr);
if (!NamePtr) {
appendTypeTagName(D.getTag());
return DWARFDie();
Expand Down Expand Up @@ -235,9 +232,9 @@ void DWARFTypePrinter::appendUnqualifiedNameAfter(
case DW_TAG_pointer_type: {
if (needsParens(Inner))
OS << ')';
appendUnqualifiedNameAfter(Inner, resolveReferencedType(Inner),
/*SkipFirstParamIfArtificial=*/D.getTag() ==
DW_TAG_ptr_to_member_type);
appendUnqualifiedNameAfter(
Inner, Inner.getAttributeValueAsReferencedDie(DW_AT_type),
/*SkipFirstParamIfArtificial=*/D.getTag() == DW_TAG_ptr_to_member_type);
break;
}
case DW_TAG_LLVM_ptrauth_type: {
Expand Down Expand Up @@ -341,7 +338,7 @@ bool DWARFTypePrinter::appendTemplateParameters(DWARFDie D,
appendTemplateParameters(C, FirstParameter);
}
if (C.getTag() == dwarf::DW_TAG_template_value_parameter) {
DWARFDie T = resolveReferencedType(C);
DWARFDie T = C.getAttributeValueAsReferencedDie(DW_AT_type);
Sep();
if (T.getTag() == DW_TAG_enumeration_type) {
OS << '(';
Expand Down Expand Up @@ -461,7 +458,7 @@ bool DWARFTypePrinter::appendTemplateParameters(DWARFDie D,
continue;
auto TypeAttr = C.find(DW_AT_type);
Sep();
appendQualifiedName(TypeAttr ? resolveReferencedType(C, *TypeAttr)
appendQualifiedName(TypeAttr ? C.getAttributeValueAsReferencedDie(*TypeAttr)
: DWARFDie());
}
if (IsTemplate && *FirstParameter && FirstParameter == &FirstParameterValue) {
Expand All @@ -473,15 +470,15 @@ bool DWARFTypePrinter::appendTemplateParameters(DWARFDie D,
void DWARFTypePrinter::decomposeConstVolatile(DWARFDie &N, DWARFDie &T,
DWARFDie &C, DWARFDie &V) {
(N.getTag() == DW_TAG_const_type ? C : V) = N;
T = resolveReferencedType(N);
T = N.getAttributeValueAsReferencedDie(DW_AT_type);
if (T) {
auto Tag = T.getTag();
if (Tag == DW_TAG_const_type) {
C = T;
T = resolveReferencedType(T);
T = T.getAttributeValueAsReferencedDie(DW_AT_type);
} else if (Tag == DW_TAG_volatile_type) {
V = T;
T = resolveReferencedType(T);
T = T.getAttributeValueAsReferencedDie(DW_AT_type);
}
}
}
Expand All @@ -491,10 +488,11 @@ void DWARFTypePrinter::appendConstVolatileQualifierAfter(DWARFDie N) {
DWARFDie T;
decomposeConstVolatile(N, T, C, V);
if (T && T.getTag() == DW_TAG_subroutine_type)
appendSubroutineNameAfter(T, resolveReferencedType(T), false, C.isValid(),
V.isValid());
appendSubroutineNameAfter(T, T.getAttributeValueAsReferencedDie(DW_AT_type),
false, C.isValid(), V.isValid());
else
appendUnqualifiedNameAfter(T, resolveReferencedType(T));
appendUnqualifiedNameAfter(T,
T.getAttributeValueAsReferencedDie(DW_AT_type));
}
void DWARFTypePrinter::appendConstVolatileQualifierBefore(DWARFDie N) {
DWARFDie C;
Expand All @@ -504,7 +502,7 @@ void DWARFTypePrinter::appendConstVolatileQualifierBefore(DWARFDie N) {
bool Subroutine = T && T.getTag() == DW_TAG_subroutine_type;
DWARFDie A = T;
while (A && A.getTag() == DW_TAG_array_type)
A = resolveReferencedType(A);
A = A.getAttributeValueAsReferencedDie(DW_AT_type);
bool Leading =
(!A || (A.getTag() != DW_TAG_pointer_type &&
A.getTag() != llvm::dwarf::DW_TAG_ptr_to_member_type)) &&
Expand Down Expand Up @@ -546,7 +544,7 @@ void DWARFTypePrinter::appendSubroutineNameAfter(
if (P.getTag() != DW_TAG_formal_parameter &&
P.getTag() != DW_TAG_unspecified_parameters)
return;
DWARFDie T = resolveReferencedType(P);
DWARFDie T = P.getAttributeValueAsReferencedDie(DW_AT_type);
if (SkipFirstParamIfArtificial && RealFirst && P.find(DW_AT_artificial)) {
FirstParamIfArtificial = T;
RealFirst = false;
Expand All @@ -567,7 +565,7 @@ void DWARFTypePrinter::appendSubroutineNameAfter(
if (DWARFDie P = FirstParamIfArtificial) {
if (P.getTag() == DW_TAG_pointer_type) {
auto CVStep = [&](DWARFDie CV) {
if (DWARFDie U = resolveReferencedType(CV)) {
if (DWARFDie U = CV.getAttributeValueAsReferencedDie(DW_AT_type)) {
Const |= U.getTag() == DW_TAG_const_type;
Volatile |= U.getTag() == DW_TAG_volatile_type;
return U;
Expand Down Expand Up @@ -653,7 +651,8 @@ void DWARFTypePrinter::appendSubroutineNameAfter(
if (D.find(DW_AT_rvalue_reference))
OS << " &&";

appendUnqualifiedNameAfter(Inner, resolveReferencedType(Inner));
appendUnqualifiedNameAfter(
Inner, Inner.getAttributeValueAsReferencedDie(DW_AT_type));
}
void DWARFTypePrinter::appendScopes(DWARFDie D) {
if (D.getTag() == DW_TAG_compile_unit)
Expand All @@ -666,7 +665,6 @@ void DWARFTypePrinter::appendScopes(DWARFDie D) {
return;
if (D.getTag() == DW_TAG_lexical_block)
return;
D = D.resolveTypeUnitReference();
if (DWARFDie P = D.getParent())
appendScopes(P);
appendUnqualifiedName(D);
Expand Down
Loading