Skip to content

[BOLT][DWARF] Fix parent chain in debug_names entries with forward declaration. #93865

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 3 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions bolt/include/bolt/Core/DIEBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,9 @@ class DIEBuilder {
/// Along with current CU, and DIE being processed and the new DIE offset to
/// be updated, it takes in Parents vector that can be empty if this DIE has
/// no parents.
uint32_t
finalizeDIEs(DWARFUnit &CU, DIE &Die,
std::vector<std::optional<BOLTDWARF5AccelTableData *>> &Parents,
uint32_t &CurOffset);
uint32_t finalizeDIEs(DWARFUnit &CU, DIE &Die,
std::optional<BOLTDWARF5AccelTableData *> Parent,
uint32_t NumberParentsInChain, uint32_t &CurOffset);

void registerUnit(DWARFUnit &DU, bool NeedSort);

Expand Down
7 changes: 5 additions & 2 deletions bolt/include/bolt/Core/DebugNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,25 @@ class BOLTDWARF5AccelTableData : public DWARF5AccelTableData {
BOLTDWARF5AccelTableData(const uint64_t DieOffset,
const std::optional<uint64_t> DefiningParentOffset,
const unsigned DieTag, const unsigned UnitID,
const bool IsTU,
const bool IsParentRoot, const bool IsTU,
const std::optional<unsigned> SecondUnitID)
: DWARF5AccelTableData(DieOffset, DefiningParentOffset, DieTag, UnitID,
IsTU),
SecondUnitID(SecondUnitID) {}
SecondUnitID(SecondUnitID), IsParentRoot(IsParentRoot) {}

uint64_t getDieOffset() const { return DWARF5AccelTableData::getDieOffset(); }
unsigned getDieTag() const { return DWARF5AccelTableData::getDieTag(); }
unsigned getUnitID() const { return DWARF5AccelTableData::getUnitID(); }
bool isTU() const { return DWARF5AccelTableData::isTU(); }
bool isParentRoot() const { return IsParentRoot; }
std::optional<unsigned> getSecondUnitID() const { return SecondUnitID; }

void setPatchOffset(uint64_t PatchOffset) { OffsetVal = PatchOffset; }
uint64_t getPatchOffset() const { return std::get<uint64_t>(OffsetVal); }

private:
std::optional<unsigned> SecondUnitID;
bool IsParentRoot;
};

class DWARF5AcceleratorTable {
Expand All @@ -57,6 +59,7 @@ class DWARF5AcceleratorTable {
std::optional<BOLTDWARF5AccelTableData *>
addAccelTableEntry(DWARFUnit &Unit, const DIE &Die,
const std::optional<uint64_t> &DWOID,
const uint32_t NumberParentsInChain,
std::optional<BOLTDWARF5AccelTableData *> &Parent);
/// Set current unit being processed.
void setCurrentUnit(DWARFUnit &Unit, const uint64_t UnitStartOffset);
Expand Down
29 changes: 18 additions & 11 deletions bolt/lib/Core/DIEBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,32 +461,42 @@ getUnitForOffset(DIEBuilder &Builder, DWARFContext &DWCtx,
return nullptr;
}

uint32_t DIEBuilder::finalizeDIEs(
DWARFUnit &CU, DIE &Die,
std::vector<std::optional<BOLTDWARF5AccelTableData *>> &Parents,
uint32_t &CurOffset) {
uint32_t
DIEBuilder::finalizeDIEs(DWARFUnit &CU, DIE &Die,
std::optional<BOLTDWARF5AccelTableData *> Parent,
uint32_t NumberParentsInChain, uint32_t &CurOffset) {
getState().DWARFDieAddressesParsed.erase(Die.getOffset());
uint32_t CurSize = 0;
Die.setOffset(CurOffset);
std::optional<BOLTDWARF5AccelTableData *> NameEntry =
DebugNamesTable.addAccelTableEntry(
CU, Die, SkeletonCU ? SkeletonCU->getDWOId() : std::nullopt,
Parents.back());
NumberParentsInChain, Parent);
// It is possible that an indexed debugging information entry has a parent
// that is not indexed (for example, if its parent does not have a name
// attribute). In such a case, a parent attribute may point to a nameless
// index entry (that is, one that cannot be reached from any entry in the name
// table), or it may point to the nearest ancestor that does have an index
// entry.
// Skipping entry is not very useful for LLDB. This follows clang where
// children of forward declaration won't have DW_IDX_parent.
// https://github.com/llvm/llvm-project/pull/91808

// If Parent is nullopt and NumberParentsInChain is not zero, then forward
// declaration was encountered in this DF traversal. Propagating nullopt for
// Parent to children.
if (!Parent && NumberParentsInChain)
NameEntry = std::nullopt;
if (NameEntry)
Parents.push_back(std::move(NameEntry));
++NumberParentsInChain;
for (DIEValue &Val : Die.values())
CurSize += Val.sizeOf(CU.getFormParams());
CurSize += getULEB128Size(Die.getAbbrevNumber());
CurOffset += CurSize;

for (DIE &Child : Die.children()) {
uint32_t ChildSize = finalizeDIEs(CU, Child, Parents, CurOffset);
uint32_t ChildSize =
finalizeDIEs(CU, Child, NameEntry, NumberParentsInChain, CurOffset);
CurSize += ChildSize;
}
// for children end mark.
Expand All @@ -496,9 +506,6 @@ uint32_t DIEBuilder::finalizeDIEs(
}

Die.setSize(CurSize);
if (NameEntry)
Parents.pop_back();

return CurSize;
}

Expand All @@ -510,7 +517,7 @@ void DIEBuilder::finish() {
DebugNamesTable.setCurrentUnit(CU, UnitStartOffset);
std::vector<std::optional<BOLTDWARF5AccelTableData *>> Parents;
Parents.push_back(std::nullopt);
finalizeDIEs(CU, *UnitDIE, Parents, CurOffset);
finalizeDIEs(CU, *UnitDIE, std::nullopt, 0, CurOffset);

DWARFUnitInfo &CurUnitInfo = getUnitInfoByDwarfUnit(CU);
CurUnitInfo.UnitOffset = UnitStartOffset;
Expand Down
11 changes: 9 additions & 2 deletions bolt/lib/Core/DebugNames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ static uint64_t getEntryID(const BOLTDWARF5AccelTableData &Entry) {
std::optional<BOLTDWARF5AccelTableData *>
DWARF5AcceleratorTable::addAccelTableEntry(
DWARFUnit &Unit, const DIE &Die, const std::optional<uint64_t> &DWOID,
const uint32_t NumberParentsInChain,
std::optional<BOLTDWARF5AccelTableData *> &Parent) {
if (Unit.getVersion() < 5 || !NeedToCreate)
return std::nullopt;
Expand Down Expand Up @@ -312,8 +313,14 @@ DWARF5AcceleratorTable::addAccelTableEntry(
// Keeping memory footprint down.
if (ParentOffset)
EntryRelativeOffsets.insert({*ParentOffset, 0});
bool IsParentRoot = false;
// If there is no parent and no valid Entries in parent chain this is a root
// to be marked with a flag.
if (!Parent && !NumberParentsInChain)
IsParentRoot = true;
It.Values.push_back(new (Allocator) BOLTDWARF5AccelTableData(
Die.getOffset(), ParentOffset, DieTag, UnitID, IsTU, SecondIndex));
Die.getOffset(), ParentOffset, DieTag, UnitID, IsParentRoot, IsTU,
SecondIndex));
return It.Values.back();
};

Expand Down Expand Up @@ -462,7 +469,7 @@ void DWARF5AcceleratorTable::populateAbbrevsMap() {
Abbrev.addAttribute({dwarf::DW_IDX_die_offset, dwarf::DW_FORM_ref4});
if (std::optional<uint64_t> Offset = Value->getParentDieOffset())
Abbrev.addAttribute({dwarf::DW_IDX_parent, dwarf::DW_FORM_ref4});
else
else if (Value->isParentRoot())
Abbrev.addAttribute(
{dwarf::DW_IDX_parent, dwarf::DW_FORM_flag_present});
FoldingSetNodeID ID;
Expand Down
Loading
Loading