Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support all CIE versions in eh_frame #493

Merged
merged 1 commit into from
May 7, 2020
Merged

Support all CIE versions in eh_frame #493

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 11 additions & 49 deletions src/read/cfi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,15 +468,6 @@ pub enum CieOffsetEncoding {
U64,
}

// Ditto about being `pub`.
#[doc(hidden)]
#[allow(missing_docs)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ReturnAddressRegisterEncoding {
U8,
Uleb,
}

/// An offset into an `UnwindSection`.
//
// Needed to avoid conflicting implementations of `Into<T>`.
Expand Down Expand Up @@ -534,9 +525,6 @@ pub trait _UnwindSectionPrivate<R: Reader> {
/// underflows, return `None`.
fn resolve_cie_offset(&self, base: R::Offset, offset: R::Offset) -> Option<R::Offset>;

/// Return true if our parser is compatible with the given version.
fn compatible_version(version: u8) -> bool;

/// Does this version of this unwind section encode address and segment
/// sizes in its CIEs?
fn has_address_and_segment_sizes(version: u8) -> bool;
Expand All @@ -546,10 +534,6 @@ pub trait _UnwindSectionPrivate<R: Reader> {

/// The segment size to use if `has_address_and_segment_sizes` returns false.
fn segment_size(&self) -> u8;

/// What is the encoding used for the return address register in CIEs for
/// this unwind section?
fn return_address_register_encoding(version: u8) -> ReturnAddressRegisterEncoding;
}

/// A section holding unwind information: either `.debug_frame` or
Expand Down Expand Up @@ -734,16 +718,6 @@ impl<R: Reader> _UnwindSectionPrivate<R> for DebugFrame<R> {
Some(offset)
}

fn compatible_version(version: u8) -> bool {
// Version 1 of `.debug_frame` corresponds to DWARF 2, and then for
// DWARF 3 and 4, I think they decided to just match the standard's
// version.
match version {
1 | 3 | 4 => true,
_ => false,
}
}

fn has_address_and_segment_sizes(version: u8) -> bool {
version == 4
}
Expand All @@ -755,14 +729,6 @@ impl<R: Reader> _UnwindSectionPrivate<R> for DebugFrame<R> {
fn segment_size(&self) -> u8 {
self.segment_size
}

fn return_address_register_encoding(version: u8) -> ReturnAddressRegisterEncoding {
if version == 1 {
ReturnAddressRegisterEncoding::U8
} else {
ReturnAddressRegisterEncoding::Uleb
}
}
}

impl<R: Reader> UnwindSection<R> for DebugFrame<R> {
Expand Down Expand Up @@ -792,10 +758,6 @@ impl<R: Reader> _UnwindSectionPrivate<R> for EhFrame<R> {
base.checked_sub(offset)
}

fn compatible_version(version: u8) -> bool {
version == 1
}

fn has_address_and_segment_sizes(_version: u8) -> bool {
false
}
Expand All @@ -807,10 +769,6 @@ impl<R: Reader> _UnwindSectionPrivate<R> for EhFrame<R> {
fn segment_size(&self) -> u8 {
0
}

fn return_address_register_encoding(_version: u8) -> ReturnAddressRegisterEncoding {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they are both the same now, we should delete this method from the trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, updated. Would you go even further and also support CIE v4; effectively removing the compatible_version method, too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you delete the pub enum ReturnAddressRegisterEncoding definition too, and it's probably worth squashing these commits. Otherwise this looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, must have missed the warning. Amended.

ReturnAddressRegisterEncoding::Uleb
}
}

impl<R: Reader> UnwindSection<R> for EhFrame<R> {
Expand Down Expand Up @@ -1281,8 +1239,13 @@ impl<R: Reader> CommonInformationEntry<R> {
mut rest: R,
) -> Result<CommonInformationEntry<R>> {
let version = rest.read_u8()?;
if !Section::compatible_version(version) {
return Err(Error::UnknownVersion(u64::from(version)));

// Version 1 of `.debug_frame` corresponds to DWARF 2, and then for
// DWARF 3 and 4, I think they decided to just match the standard's
// version.
match version {
1 | 3 | 4 => (),
_ => return Err(Error::UnknownVersion(u64::from(version))),
}

let mut augmentation_string = rest.read_null_terminated_slice()?;
Expand All @@ -1298,11 +1261,10 @@ impl<R: Reader> CommonInformationEntry<R> {
let code_alignment_factor = rest.read_uleb128()?;
let data_alignment_factor = rest.read_sleb128()?;

let return_address_register = match Section::return_address_register_encoding(version) {
ReturnAddressRegisterEncoding::U8 => Register(rest.read_u8()?.into()),
ReturnAddressRegisterEncoding::Uleb => {
rest.read_uleb128().and_then(Register::from_u64)?
}
let return_address_register = if version == 1 {
Register(rest.read_u8()?.into())
} else {
rest.read_uleb128().and_then(Register::from_u64)?
};

let augmentation = if augmentation_string.is_empty() {
Expand Down