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][DebugInfo] Refactor some code for easier sharing. #82153

Merged
merged 4 commits into from
Feb 22, 2024
Merged

[LLVM][DebugInfo] Refactor some code for easier sharing. #82153

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a25058a
[LLVM][DebugInfo] Refactor some code for easier sharing.
cmtice Feb 18, 2024
7106757
[LLVM][DebugInfo] Refactor some code for easier sharing.
cmtice Feb 18, 2024
7c3d097
[LLVM][DebugInfo] Refactor some code for easier sharing.
cmtice Feb 20, 2024
2a4edad
[LLVM][DebugInfo] Refactor some code for easier sharing.
cmtice Feb 21, 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
28 changes: 21 additions & 7 deletions llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,17 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
uint64_t getEntryOffset() const { return EntryOffset; }
};

// Offsets for the start of various important tables from the start of the
// section.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use Doxygen cmments ///?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

struct DWARFDebugNamesOffsets {
uint64_t CUsBase;
uint64_t BucketsBase;
uint64_t HashesBase;
uint64_t StringOffsetsBase;
uint64_t EntryOffsetsBase;
uint64_t EntriesBase;
};

/// Represents a single accelerator table within the DWARF v5 .debug_names
/// section.
class NameIndex {
Expand All @@ -572,12 +583,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
// Base of the whole unit and of various important tables, as offsets from
// the start of the section.
uint64_t Base;
uint64_t CUsBase;
uint64_t BucketsBase;
uint64_t HashesBase;
uint64_t StringOffsetsBase;
uint64_t EntryOffsetsBase;
uint64_t EntriesBase;
DWARFDebugNamesOffsets Offsets;

void dumpCUs(ScopedPrinter &W) const;
void dumpLocalTUs(ScopedPrinter &W) const;
Expand Down Expand Up @@ -638,7 +644,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
/// Returns the Entry at the relative `Offset` from the start of the Entry
/// pool.
Expected<Entry> getEntryAtRelativeOffset(uint64_t Offset) const {
auto OffsetFromSection = Offset + this->EntriesBase;
auto OffsetFromSection = Offset + this->Offsets.EntriesBase;
return getEntry(&OffsetFromSection);
}

Expand Down Expand Up @@ -793,6 +799,14 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
const NameIndex *getCUNameIndex(uint64_t CUOffset);
};

// Calculates the starting offsets for various sections within the
// debug_names section. This has been pulled out of DWARFDebugNames, to
// allow easier sharing of the code with other tools, such as LLD.
Copy link
Collaborator

Choose a reason for hiding this comment

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

///

Copy link
Contributor

Choose a reason for hiding this comment

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

This has been pulled out of DWARFDebugNames, to
// allow easier sharing of the code with other tools, such as LLD.

This is more of a review comment than documentation, I suggest removing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

uint64_t FindDebugNamesOffsets(DWARFDebugNames::DWARFDebugNamesOffsets &Offsets,
uint64_t HdrSize,
const dwarf::DwarfFormat Format,
const DWARFDebugNames::Header &Hdr);

/// If `Name` is the name of a templated function that includes template
/// parameters, returns a substring of `Name` containing no template
/// parameters.
Expand Down
71 changes: 46 additions & 25 deletions llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ DWARFDebugNames::Abbrev DWARFDebugNames::AbbrevMapInfo::getTombstoneKey() {

Expected<DWARFDebugNames::AttributeEncoding>
DWARFDebugNames::NameIndex::extractAttributeEncoding(uint64_t *Offset) {
if (*Offset >= EntriesBase) {
if (*Offset >= Offsets.EntriesBase) {
return createStringError(errc::illegal_byte_sequence,
"Incorrectly terminated abbreviation table.");
}
Expand All @@ -536,7 +536,7 @@ DWARFDebugNames::NameIndex::extractAttributeEncodings(uint64_t *Offset) {

Expected<DWARFDebugNames::Abbrev>
DWARFDebugNames::NameIndex::extractAbbrev(uint64_t *Offset) {
if (*Offset >= EntriesBase) {
if (*Offset >= Offsets.EntriesBase) {
return createStringError(errc::illegal_byte_sequence,
"Incorrectly terminated abbreviation table.");
}
Expand All @@ -552,32 +552,52 @@ DWARFDebugNames::NameIndex::extractAbbrev(uint64_t *Offset) {
return Abbrev(Code, dwarf::Tag(Tag), AbbrevOffset, std::move(*AttrEncOr));
}

uint64_t
llvm::FindDebugNamesOffsets(DWARFDebugNames::DWARFDebugNamesOffsets &Offsets,
uint64_t HdrSize, dwarf::DwarfFormat Format,
const DWARFDebugNames::Header &Hdr) {
uint32_t DwarfSize = (Format == llvm::dwarf::DwarfFormat::DWARF32) ? 4 : 8;
uint64_t Offset = HdrSize;
Offsets.CUsBase = Offset;
Offset += Hdr.CompUnitCount * DwarfSize;
Offset += Hdr.LocalTypeUnitCount * DwarfSize;
Offset += Hdr.ForeignTypeUnitCount * 8;

Offsets.BucketsBase = Offset;
Offset += Hdr.BucketCount * 4;

Offsets.HashesBase = Offset;
if (Hdr.BucketCount > 0)
Offset += Hdr.NameCount * 4;

Offsets.StringOffsetsBase = Offset;
Offset += Hdr.NameCount * DwarfSize;

Offsets.EntryOffsetsBase = Offset;
Offset += Hdr.NameCount * DwarfSize;

Offset += Hdr.AbbrevTableSize;
Offsets.EntriesBase = Offset;

return Offset;
}

Error DWARFDebugNames::NameIndex::extract() {
const DWARFDataExtractor &AS = Section.AccelSection;
uint64_t Offset = Base;
if (Error E = Hdr.extract(AS, &Offset))
return E;

const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
CUsBase = Offset;
Offset += Hdr.CompUnitCount * SectionOffsetSize;
Offset += Hdr.LocalTypeUnitCount * SectionOffsetSize;
Offset += Hdr.ForeignTypeUnitCount * 8;
BucketsBase = Offset;
Offset += Hdr.BucketCount * 4;
HashesBase = Offset;
if (Hdr.BucketCount > 0)
Offset += Hdr.NameCount * 4;
StringOffsetsBase = Offset;
Offset += Hdr.NameCount * SectionOffsetSize;
EntryOffsetsBase = Offset;
Offset += Hdr.NameCount * SectionOffsetSize;
Offset = FindDebugNamesOffsets(Offsets, Offset, Hdr.Format, Hdr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is strange that we assign to Offset, but then overwrite it on the next line?

So what's the return value for? Perhaps if it's needed by the future callers but not right now - maybe it should be in the Offsets structure, and that Offsets structure should be returned by this function, rather than as an input parameter?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, something feels off here. Suggestion:

  1. rename Offset to EndOfHeaderOffset
  2. remove the return value from FindDebugNamesOffset
  3. declare Offset in the line below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did something very similar to what felipe recommended.


Offset = Offsets.EntryOffsetsBase + (Hdr.NameCount * SectionOffsetSize);

if (!AS.isValidOffsetForDataOfSize(Offset, Hdr.AbbrevTableSize))
return createStringError(errc::illegal_byte_sequence,
"Section too small: cannot read abbreviations.");

EntriesBase = Offset + Hdr.AbbrevTableSize;
Offsets.EntriesBase = Offset + Hdr.AbbrevTableSize;

for (;;) {
auto AbbrevOr = extractAbbrev(&Offset);
Expand Down Expand Up @@ -679,7 +699,7 @@ void DWARFDebugNames::Entry::dumpParentIdx(
return;
}

auto AbsoluteOffset = NameIdx->EntriesBase + FormValue.getRawUValue();
auto AbsoluteOffset = NameIdx->Offsets.EntriesBase + FormValue.getRawUValue();
W.getOStream() << "Entry @ 0x" + Twine::utohexstr(AbsoluteOffset);
}

Expand Down Expand Up @@ -708,22 +728,23 @@ std::error_code DWARFDebugNames::SentinelError::convertToErrorCode() const {
uint64_t DWARFDebugNames::NameIndex::getCUOffset(uint32_t CU) const {
assert(CU < Hdr.CompUnitCount);
const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
uint64_t Offset = CUsBase + SectionOffsetSize * CU;
uint64_t Offset = Offsets.CUsBase + SectionOffsetSize * CU;
return Section.AccelSection.getRelocatedValue(SectionOffsetSize, &Offset);
}

uint64_t DWARFDebugNames::NameIndex::getLocalTUOffset(uint32_t TU) const {
assert(TU < Hdr.LocalTypeUnitCount);
const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
uint64_t Offset = CUsBase + SectionOffsetSize * (Hdr.CompUnitCount + TU);
uint64_t Offset =
Offsets.CUsBase + SectionOffsetSize * (Hdr.CompUnitCount + TU);
return Section.AccelSection.getRelocatedValue(SectionOffsetSize, &Offset);
}

uint64_t DWARFDebugNames::NameIndex::getForeignTUSignature(uint32_t TU) const {
assert(TU < Hdr.ForeignTypeUnitCount);
const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
uint64_t Offset =
CUsBase +
Offsets.CUsBase +
SectionOffsetSize * (Hdr.CompUnitCount + Hdr.LocalTypeUnitCount) + 8 * TU;
return Section.AccelSection.getU64(&Offset);
}
Expand Down Expand Up @@ -759,28 +780,28 @@ DWARFDebugNames::NameIndex::getNameTableEntry(uint32_t Index) const {
assert(0 < Index && Index <= Hdr.NameCount);
const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
uint64_t StringOffsetOffset =
StringOffsetsBase + SectionOffsetSize * (Index - 1);
Offsets.StringOffsetsBase + SectionOffsetSize * (Index - 1);
uint64_t EntryOffsetOffset =
EntryOffsetsBase + SectionOffsetSize * (Index - 1);
Offsets.EntryOffsetsBase + SectionOffsetSize * (Index - 1);
const DWARFDataExtractor &AS = Section.AccelSection;

uint64_t StringOffset =
AS.getRelocatedValue(SectionOffsetSize, &StringOffsetOffset);
uint64_t EntryOffset = AS.getUnsigned(&EntryOffsetOffset, SectionOffsetSize);
EntryOffset += EntriesBase;
EntryOffset += Offsets.EntriesBase;
return {Section.StringSection, Index, StringOffset, EntryOffset};
}

uint32_t
DWARFDebugNames::NameIndex::getBucketArrayEntry(uint32_t Bucket) const {
assert(Bucket < Hdr.BucketCount);
uint64_t BucketOffset = BucketsBase + 4 * Bucket;
uint64_t BucketOffset = Offsets.BucketsBase + 4 * Bucket;
return Section.AccelSection.getU32(&BucketOffset);
}

uint32_t DWARFDebugNames::NameIndex::getHashArrayEntry(uint32_t Index) const {
assert(0 < Index && Index <= Hdr.NameCount);
uint64_t HashOffset = HashesBase + 4 * (Index - 1);
uint64_t HashOffset = Offsets.HashesBase + 4 * (Index - 1);
return Section.AccelSection.getU32(&HashOffset);
}

Expand Down
Loading