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

Fri Jan 31 12:18:35 PST 2020

probinson added a comment.

The table and page numbering in my copy of the psABI does not match yours, but I do see the values you cite.  In my version, the CIE table also specifies that the Length field is 4 bytes (not optionally 12 bytes, or a 4-byte reserved value and 8-byte extended length, however you want to think about it).  But likely the Linux folks have added that possibility, while maintaining a 4-byte CIE_id/pointer field, which (as I describe below) doesn't actually get the .eh_frame section into trouble.

What I was mentioning before is (in my copy) in section 3.7, 2nd paragraph.  "Position independence: In order to avoid load time relocations for position independent code, the FDE CIE offset pointer should be stored relative to the start of CIE table entry. Frames using this extension of the DWARF standard must set the CIE identifier tag to 1."  This appears to disagree with the table, which says the value is always 0; but I *think* the descriptions of the CIE offset/pointer here and in the table are trying to say the same thing.  So I am not sure what the value 1 is about.

Aha, found the part of the DWARF spec that specifies the CIE id as 0xffffffff.  Section 7.24 in DWARF v5, and something nearby in earlier versions.  So it is normative after all.

I've been thinking about how the .debug_frame and .eh_frame definitions differ.  In .debug_frame, the FDE's CIE pointer is an offset from the beginning of the .debug_frame section; thus, it must be 64-bit in the DWARF-64 format.  In .eh_frame, the FDE's CIE pointer is the distance from the FDE to the CIE (loosely speaking) and so can be 32-bit so long as no individual set of CIE+FDEs exceeds 32-bits in length (likely a safe assumption).  This means that .eh_frame can in fact support a DWARF-64-like overall length, while still having only a 4-byte CIE pointer.

We could rely on the section name to distinguish these formats; there is a verification opportunity here, as .debug_frame should have CIE_id = 0xffffffff[ffffffff], while .eh_frame should have CIE_id = 0 (4-byte) even given a 12-byte initial length field.

Regarding what to do with this patch, I still think we should print the CIE_id as read from the file, not hard-code values, because hard-coded values mean that mistakes in setting CIE_id will never be noticed.  And you never know, things can change in the future.

================
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:
> > 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
I'm going to move my reply to the out-of-line comment section where it will be easier (for me at least) to read & follow.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73714