Page MenuHomePhabricator

[DebugInfo] Fix emitting DWARF64 DWO compilation units and string offset tables (9/19).
ClosedPublic

Authored by ikudrin on Wed, Sep 2, 6:22 AM.

Details

Summary

These two fixes are better to go together because llvm-dwarfdump is unable to dump a table when another one is malformed.

Diff Detail

Event Timeline

ikudrin created this revision.Wed, Sep 2, 6:22 AM
ikudrin requested review of this revision.Wed, Sep 2, 6:22 AM
ikudrin retitled this revision from [DebugInfo] Fix emitting DWARF64 DWO compilation units and string offset tables (11/19). to [DebugInfo] Fix emitting DWARF64 DWO compilation units and string offset tables (9/19)..Thu, Sep 3, 7:40 AM
dblaikie accepted this revision.Thu, Sep 3, 12:35 PM
dblaikie added inline comments.
llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

Perhaps the abbr_offset should be printed with more digits to reflect the longer encoding size? (I think that's been the general approach elsewhere in the dump output format, yes?)

This revision is now accepted and ready to land.Thu, Sep 3, 12:35 PM
jhenderson added inline comments.Fri, Sep 4, 1:08 AM
llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

Optional more concise alternative to the existing regex, making use of FileCheck's new precision for numeric variables (strictly speaking, this pattern would allow a 17-digit number with no leading 0s, but that might be beyond what's needed). Not sure if you'd prefer it or not - happy either way.

ikudrin added inline comments.Fri, Sep 4, 2:19 AM
llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

As I don't care about the exact length that much, and as the abbr_offset was reported as 4-digit before my changes, and as abbreviation tables are not expected to be longer in DWARF64 compared to DWARF32, I did not change the format of that field. For me, the most reliable part is the format reported directly, and also the fact that the parser can correctly read all the expected information.

8

Maybe it is better to simplify this CHECK line even more? We do not care about the exact value of the length anyway.

jhenderson added inline comments.Fri, Sep 4, 2:46 AM
llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

Works for me too, if we've got some coverage showing the dumping code correctly formats the length field for a compile unit using 64-bit DWARF?

ikudrin added inline comments.Fri, Sep 4, 3:09 AM
llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

You mean, like llvm/test/tools/llvm-dwarfdump/X86/debug_info_min_dwarf64.s?

jhenderson accepted this revision.Fri, Sep 4, 3:36 AM

LGTM, with your suggested changes to the length check.

llvm/test/DebugInfo/X86/debug-info-dwo-dwarf64.ll
8

That should suffice, yes :-)

ikudrin updated this revision to Diff 289940.Fri, Sep 4, 6:50 AM
  • Relaxed checking in the test. The line is a bit longer than I suggested because the test has to validate emitting the abbr_offset field.