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.

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

Add RunTimeEndian #219

Merged
merged 3 commits into from
Jul 13, 2017
Merged

Add RunTimeEndian #219

merged 3 commits into from
Jul 13, 2017

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Jul 12, 2017

Fixes #84

No noticeable change in benchmarks when not using RunTimeEndian. I didn't measure with it.

I changed dwarfdump to use it since performance doesn't matter much there, to test it and to see what the size change was:

Before:

   text	   data	    bss	    dec	    hex	filename
 680902	  22249	   3704	 706855	  ac927	target/release/examples/dwarfdump

After:

   text	   data	    bss	    dec	    hex	filename
 593190	  22025	   3704	 618919	  971a7	target/release/examples/dwarfdump

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 93.857% when pulling 20dc85d on philipc:endian into 414f8a1 on gimli-rs:master.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Great to finally handle this :)

src/endianity.rs Outdated

/// Little endian byte order.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct LittleEndian();
Copy link
Member

Choose a reason for hiding this comment

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

Why

pub struct LittleEndian();

instead of

pub struct LittleEndian;

?

I think the latter is more idiomatic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll fix that up before merging, thanks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a way to avoid needing to create NativeEndian values as NativeEndian {}?

Copy link
Member

Choose a reason for hiding this comment

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

This should do it:

#[cfg(target_endian = "little")]
pub use self::LittleEndian as NativeEndian;

#[cfg(target_endian = "big")]
pub use self::BigEndian as NativeEndian;

However, when I try this I start getting errors in tests/parse_self.rs (?!?!??!)

failures:

---- test_parse_self_debug_aranges stdout ----
	Reading section "debug_aranges" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_aranges"
thread 'test_parse_self_debug_aranges' panicked at 'Should parse arange OK: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_aranges
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_pubtypes stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
Reading section "debug_pubtypes" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_pubtypes"
thread 'test_parse_self_debug_pubtypes' panicked at 'Should parse pubtype OK: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_pubtypes
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_pubnames stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
Reading section "debug_pubnames" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_pubnames"
thread 'test_parse_self_debug_pubnames' panicked at 'Should parse pubname OK: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_pubnames
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_eh_frame stdout ----
	Reading section "eh_frame" at path "/Users/fitzgen/src/gimli/fixtures/self/eh_frame"
thread 'test_parse_self_eh_frame' panicked at 'Should parse CFI entry OK: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_eh_frame
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_line stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
Reading section "debug_line" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_line"
Reading section "debug_str" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_str"
thread 'test_parse_self_debug_line' panicked at 'Should parse compilation unit: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_line
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_info stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
thread 'test_parse_self_debug_info' panicked at 'Should parse compilation unit: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_info
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_ranges stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
Reading section "debug_ranges" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_ranges"
thread 'test_parse_self_debug_ranges' panicked at 'Should parse compilation unit: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_ranges
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic

---- test_parse_self_debug_loc stdout ----
	Reading section "debug_info" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_info"
Reading section "debug_abbrev" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_abbrev"
Reading section "debug_loc" at path "/Users/fitzgen/src/gimli/fixtures/self/debug_loc"
thread 'test_parse_self_debug_loc' panicked at 'Should parse compilation unit: UnexpectedEof', src/libcore/result.rs:860
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::expect
  10: parse_self::test_parse_self_debug_loc
  11: <F as test::FnBox<T>>::call_box
  12: __rust_maybe_catch_panic


failures:
    test_parse_self_debug_aranges
    test_parse_self_debug_info
    test_parse_self_debug_line
    test_parse_self_debug_loc
    test_parse_self_debug_pubnames
    test_parse_self_debug_pubtypes
    test_parse_self_debug_ranges
    test_parse_self_eh_frame

philipc added 3 commits July 13, 2017 11:56
This means LittleEndian and BigEndian are now structs, and no
longer implement byteorder::ByteOrder.
Also store in some variants of AttributeValue.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 93.856% when pulling c23c747 on philipc:endian into 414f8a1 on gimli-rs:master.

@philipc philipc merged commit a2ef012 into gimli-rs:master Jul 13, 2017
@philipc philipc deleted the endian branch July 13, 2017 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants