Support all CIE versions in eh_frame #493
Conversation
|
I think this bug is simply a result of bad documentation. The LSB spec says "Version: A 1 byte value that identifies the version number of the frame information structure. This value shall be 1.", and it doesn't give the return address encoding. We did notice problems in #244, so I guess this PR fixes that. |
| @@ -808,8 +814,12 @@ impl<R: Reader> _UnwindSectionPrivate<R> for EhFrame<R> { | |||
| 0 | |||
| } | |||
|
|
|||
| fn return_address_register_encoding(_version: u8) -> ReturnAddressRegisterEncoding { | |||
There was a problem hiding this comment.
Since they are both the same now, we should delete this method from the trait.
There was a problem hiding this comment.
Good point, updated. Would you go even further and also support CIE v4; effectively removing the compatible_version method, too?
There was a problem hiding this comment.
Yeah, given that we already know the doc is wrong, we should do what other consumers do. I haven't recently looked at gdb, but I think that it handles eh_frame and debug_frame identically for version checks.
There was a problem hiding this comment.
Can you delete the pub enum ReturnAddressRegisterEncoding definition too, and it's probably worth squashing these commits. Otherwise this looks good.
There was a problem hiding this comment.
Ah yes, must have missed the warning. Amended.
jan-auer
force-pushed
the
fix/eh-frame-cie-3
branch
from
8b6fa8a
to
70c2490
Compare
jan-auer
force-pushed
the
fix/eh-frame-cie-3
branch
from
70c2490
to
44614c6
Compare
jan-auer
changed the title
|
@philipc would it be possible to release a patch version? |
|
I would prefer to release a minor version with all of the fixes since the last release (e.g. #487 too), if that works for you. |
While
.eh_framealways has version 1, it can contain CIEs with version 3. It also seems that CIE v1 always has U8 encoding for the return address, as opposed to the currently used LEB128. This would make it equivalent to.debug_frame. I am not sure why it was implemented like this in the first place, but considering that the RA is rarely >= 128, this might just have been coincidental.My guess is that version 4 could potentially occur, although I've not seen it in the wild and would conservatively not implement it at the moment. LLVM seems to treat CIEs coming from debug_frame and eh_frame the same.
Fixes #244
See https://www.airs.com/blog/archives/460