Page MenuHomePhabricator

[llvm-dwarfdump] - Dump the older versions of .eh_frame/.debug_frame correctly.
ClosedPublic

Authored by grimar on Nov 23 2018, 6:17 AM.

Details

Summary

The issue is the following.

DWARF 2 used version 1 for .debug_frame.
(Appendix G, p. 416 http://dwarfstd.org/doc/DWARF5.pdf)

lib/MC now always sets version 1 for .eh_frame (and sets 1-4 versions for .debug_frame correctly):
https://github.com/llvm-mirror/llvm/blob/master/lib/MC/MCDwarf.cpp#L1530
https://github.com/llvm-mirror/llvm/blob/master/lib/MC/MCDwarf.cpp#L1562
https://github.com/llvm-mirror/llvm/blob/master/lib/MC/MCDwarf.cpp#L1602

In version 1, return_address_register was defined as ubyte, while other versions
switched to uleb128.
(p 62, http://www.dwarfstd.org/doc/dwarf-2.0.0.pdf)

Patch teaches llvm-dwarfdump about this difference.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Nov 23 2018, 6:17 AM
grimar edited the summary of this revision. (Show Details)Nov 23 2018, 6:18 AM

btw, I am not sure why do we still emit .eh_frame of version 1 in lib/MC.
For example, LLD supports versions 1 and 3 from the beginning I think, and it seems we could switch to 3.

Though at the same time I do not know if this "Return address register" can be > 7 bits,
so I guess having it as the ubyte is perhaps fine and then it is ok to have .eh_frame of version 1.

Seems plausible - though I'll leave it to @probinson for final sign off as I don't know too much about eh/debug_frame things.

grimar added a comment.Dec 3 2018, 2:38 AM

Paul, what do you think? (Ping)

probinson accepted this revision.Dec 3 2018, 9:49 AM

The indentation in the test file looks a little funny; please make sure there are no hard tabs in the file. Aside from that LGTM.

FTR, the format of .eh_frame is specified by the ELF spec, and has not been updated to reflect newer DWARF versions, that I'm aware of. If we can get the ELF spec updated, then we wouldn't need this fussy stuff. The format of .debug_frame of course is under the control of the DWARF spec, and so (even though it's really kind of stupid) the content does have to vary slightly depending on which section name we use.

This revision is now accepted and ready to land.Dec 3 2018, 9:49 AM
This revision was automatically updated to reflect the committed changes.