[PATCH] D73714: [DWARF] Do not cut 64-bit values when dumping CIEs and FDEs.

Fri Jan 31 07:59:47 PST 2020

probinson added inline comments.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:291
+     << format(" %0*" PRIx64, IsDWARF64 ? 16 : 8, Length)
+     << format(" %0*" PRIx64, IsDWARF64 && !IsEH ? 16 : 8,
+               IsEH ? 0ULL : (IsDWARF64 ? DW64_CIE_ID : (uint64_t)DW_CIE_ID))
----------------
ikudrin wrote:
> probinson wrote:
> > Why the exception for .eh_frame?
> CIE ID in `.eh_frame`, unlike `.debug_frame`, is always 4 bytes long, see https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
That description diverges from what my copy of the AMD64 psABI spec says (v0.99.8 from 2015), which has nothing in it to specify that CIE_id/CIE_pointer are always 4-byte.  I agree that the CIE_id is not actually fixed; AFAICT the DWARF spec doesn't say what it should be, in normative text, although it uses 0xffffffff in an example.  The psABI says that a position-independent .eh_frame should set CIE_id to 1.

Clearly we should be dumping the actual CIE_id field from the file, not pretending it's a particular fixed value.  We also need to clear up under what conditions the field is 4 bytes versus 8; apparently it's not as simple as this patch suggests.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:321
+     << format(" %0*" PRIx64, FieldWidth, Length)
+     << format(" %0*" PRIx64, FieldWidth, LinkedCIEOffset)
+     << format(" FDE cie=%0*" PRIx64, FieldWidth, LinkedCIEOffset)
----------------
The FDE's offset-to-CIE field must exactly overlay the CIE's CIE_id field.  This treats it as strictly format-dependent, which is not consistent with the Linux document you pointed to.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73714/new/

https://reviews.llvm.org/D73714