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

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 2 inline comments as done.
ikudrin 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))
----------------
probinson wrote:
> 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.
I am not sure what is the "CIE identifier tag" they talk about in the "Position independence" section you cited, but please take a look at a description for `CIE id` on page 62:

> Table 4.6: Common Information Entry (CIE)
> CIE id, 4, Value 0 for .eh_frame (used to distinguish CIEs and FDEs when scanning the section)

And a description for `CIE pointer` on page 64:

> Table 4.8: Frame Descriptor Entry (FDE)
> CIE pointer, 4, Distance from this field to the nearest preceding CIE (the value is subtracted from the current address). This value can never be zero and thus can be used to distinguish CIE’s and FDE’s when scanning the `.eh_frame` section


================
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)
----------------
probinson wrote:
> 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.
Oh, thanks! I'll fix that with the next update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73714