Page MenuHomePhabricator

[DebugInfo] Report the format of debug info tables.
ClosedPublic

Authored by ikudrin on May 25 2020, 8:39 AM.

Diff Detail

Event Timeline

ikudrin created this revision.May 25 2020, 8:39 AM

I vote for this patch (explicit format = DWARF{32,64})

jhenderson accepted this revision.May 27 2020, 1:00 AM

Thanks, LGTM, but obviously wait for others to give their input.

llvm/test/DebugInfo/dwarfdump-pubnames.test
5 ↗(On Diff #266026)

Aside: it's weird that some outputs use commas to separate things and others don't.

This revision is now accepted and ready to land.May 27 2020, 1:00 AM

I believe GNU readelf and friends tend to put this info after the length, e.g. "Length: 0x1234 (64-bit)" or "Length: 0x1234 (32-bit)" so it might be worth being vaguely similar.

I believe GNU readelf and friends tend to put this info after the length, e.g. "Length: 0x1234 (64-bit)" or "Length: 0x1234 (32-bit)" so it might be worth being vaguely similar.

We do not follow their dumping format, so we should be free to choose our own way. Note also that we already report the format of .debug_str_offsets sections like format = DWARF32/64. Personally, I do not have any strong preferences for any dumping format. I just need some reliable way to write tests for producers.

ikudrin marked 2 inline comments as done.May 29 2020, 8:39 AM
ikudrin added inline comments.
llvm/test/DebugInfo/dwarfdump-pubnames.test
5 ↗(On Diff #266026)

I've prepared D80806 for that.

ikudrin updated this revision to Diff 267610.Jun 1 2020, 7:15 AM
ikudrin marked an inline comment as done.
  • Rebased on the tip.
probinson accepted this revision.Jun 1 2020, 8:00 AM

LGTM with the suggested helper.

llvm/include/llvm/BinaryFormat/Dwarf.h
484 ↗(On Diff #267610)

And a helper taking bool IsDWARF64) here, so DWARFDebugFrame isn't using it's own idea of the format strings.

StringRef FormatString(bool IsDWARF64) {
  return FormatString(IsDWARF64 ? DWARF64 : DWARF32);
}
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
318 ↗(On Diff #267610)

Use FormatString(bool) helper here.

354 ↗(On Diff #267610)

Use FormatString(bool) helper here.

ikudrin marked 3 inline comments as done.Jun 1 2020, 9:47 PM

Thanks to all reviewers! It looks like we have reached the consensus at this variant, so I am going to land it, splitting into smaller parts for individual tables.

This revision was automatically updated to reflect the committed changes.