Page MenuHomePhabricator

[DebugInfo] Dump fields according to their formats.
ClosedPublic

Authored by ikudrin on Fri, May 15, 5:18 AM.

Details

Summary

This patch changes dumping fields of debugging tables which have different sizes in DWARF32 and DWARF64 formats, and which were reported as 8-digit hex values so that they use widths of 8 or 16 digits according to the format of the corresponding table. Other cases are left unaffected by this change, in particular:

  • Fields of .debug_names, because they are printed with floating width without leading zeroes;
  • Abbreviation and type offsets in compilation and type units, because they use the width of 4 digits in dumping.

Diff Detail

Event Timeline

ikudrin created this revision.Fri, May 15, 5:18 AM
ikudrin updated this revision to Diff 264213.Fri, May 15, 5:29 AM
  • Add test DebugInfo/X86/debug-frame-dwarf64.s
dblaikie accepted this revision.Fri, May 15, 12:24 PM

Looks good - might be good/better to commit per-section, to isolate the changes/testing/etc.

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
486 ↗(On Diff #264213)

maybe pull this 16/8 computation out to the top, as you have in other parts of this patch? and/or refactor it into a utility function that gives the offset length, given the DWARF format.

This revision is now accepted and ready to land.Fri, May 15, 12:24 PM
ikudrin marked an inline comment as done.Sun, May 17, 11:27 PM

Looks good - might be good/better to commit per-section, to isolate the changes/testing/etc.

Thanks! I'll split it on commit.

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
486 ↗(On Diff #264213)

maybe pull this 16/8 computation out to the top, as you have in other parts of this patch?

OK.

and/or refactor it into a utility function that gives the offset length, given the DWARF format.

I would not add that separate function; that would just bloat the common code. I think, that if not an explicit expression, I would prefer to use 2 * dwarf::getDwarfOffsetByteSize(Format).

jhenderson accepted this revision.Mon, May 18, 1:24 AM

All makes sense to me. LGTM.

llvm/test/DebugInfo/dwarfdump-64-bit-dwarf.test
4–5 ↗(On Diff #264213)

This comment (and maybe even the test as a whole) looks a little stale nowadays...

ikudrin marked an inline comment as done.Mon, May 18, 2:51 AM
ikudrin added inline comments.
llvm/test/DebugInfo/dwarfdump-64-bit-dwarf.test
4–5 ↗(On Diff #264213)

You are probably right. I'll prepare a separate review for that.

This revision was automatically updated to reflect the committed changes.