Page MenuHomePhabricator

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

Authored by ikudrin on Jan 30 2020, 7:46 AM.

Details

Summary

This fixes printing long values that might reside in CIE and FDE, including offsets, lengths, and addresses.

Diff Detail

Event Timeline

ikudrin created this revision.Jan 30 2020, 7:46 AM
probinson added inline comments.Jan 30 2020, 9:49 AM
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
291

Why the exception for .eh_frame?

ikudrin marked an inline comment as done.Jan 30 2020, 5:48 PM
ikudrin added inline comments.
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
291

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

probinson added inline comments.Jan 31 2020, 7:55 AM
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
291

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.

321

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.

ikudrin marked 2 inline comments as done.Jan 31 2020, 8:33 AM
ikudrin added inline comments.
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
291

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

321

Oh, thanks! I'll fix that with the next update.

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.

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
291

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.

ikudrin updated this revision to Diff 242053.Feb 3 2020, 6:33 AM
  • Fix the printing field length of CIE pointer for FDEs in .eh_frame.
  • Keep the actual value of the CIE Id field.

I have to mention that I fell a bit uncomfortable storing a constant value that can be easily reconstructed. I prepared another patch which uses another approach to guarantee that the reading and dumping values are the same. Please, take a look at D73887 and tell me which variant you like better; I will abandon the other one.

ikudrin abandoned this revision.Feb 12 2020, 11:58 PM

Abandoning in favor of D73887.