Correctly handle address sizes for DW_CFA_set_loc

[mitsuhiko]

The current code does not correctly handle the the DW_CFA_set_loc instruction. It assumes the argument is uleb128 encoded but it's actually a fixed size integer depending on the address size. Mishandling this can cause the cfi iteration to consume garbage after an integer has been incorrectly read.

I was not sure however about how the current tests assume pointer sizes so for now I hardcoded this to be 8. This might have to become sizeof::<usize>().

This fixes #102

[coveralls]

Coverage increased (+0.004%) to 87.263% when pulling 3aa132c on mitsuhiko:bugfix/address-size-set-loc into 36522bd on gimli-rs:master.

[fitzgen]

This is great, thanks!

[fitzgen]

I think your intuition was on the money regarding address sizes. From CI:

---- read::cfi::tests::test_parse_fde_32_ok stdout ----
thread 'read::cfi::tests::test_parse_fde_32_ok' panicked at 'assertion failed: `(left == right)`
  left: `Ok(FrameDescriptionEntry { offset: 0, length: 27, format: Dwarf32, cie: CommonInformationEntry { offset: 0, length: 100, format: Dwarf32, version: 4, augmentation: None, address_size: 8, segment_size: 0, code_alignment_factor: 3, data_alignment_factor: 2, return_address_register: Register(1), initial_instructions: EndianSlice { slice: [], endian: LittleEndian }, phantom: PhantomData }, initial_segment: 0, initial_address: 4276993775, address_range: 39, address_size: 4, augmentation: None, instructions: EndianSlice { slice: [0, 0, 0, 0, 0, 0, 0], endian: LittleEndian } })`,
 right: `Ok(FrameDescriptionEntry { offset: 0, length: 27, format: Dwarf32, cie: CommonInformationEntry { offset: 0, length: 100, format: Dwarf32, version: 4, augmentation: None, address_size: 8, segment_size: 0, code_alignment_factor: 3, data_alignment_factor: 2, return_address_register: Register(1), initial_instructions: EndianSlice { slice: [], endian: LittleEndian }, phantom: PhantomData }, initial_segment: 0, initial_address: 4276993775, address_range: 39, address_size: 8, augmentation: None, instructions: EndianSlice { slice: [0, 0, 0, 0, 0, 0, 0], endian: LittleEndian } })`', src/read/cfi.rs:3832:9

https://travis-ci.org/gimli-rs/gimli/jobs/478405308#L975 etc

[fitzgen]

Oh also -- this doesn't resolve the segments part of #102 so either that bug should be fixed in this PR too, or we shouldn't have the "fixes" thing in the PR comment so that issue is left open.

[mitsuhiko]

Going to change the sizes. The segment part I did not understand. I looked at what llvm does and I did not see any obvious difference other than the size aspect of the argument.

[mitsuhiko]

Did some changes to the tests but this needs a thorough review before merging. One of the tests already intentionally tries to test for a 64bit addressed dwarf32 but the second part of the test seems to assume native endianness if I'm not mistaken.

[fitzgen]

This fixes the failing test:

diff --git a/src/read/cfi.rs b/src/read/cfi.rs
index 5601986..9828081 100644
--- a/src/read/cfi.rs
+++ b/src/read/cfi.rs
@@ -4093,7 +4093,7 @@ mod tests {
             offset: 0,
             length: 0,
             format: Format::Dwarf32,
-            address_size: 8,
+            address_size: cie1.address_size,
             cie: cie1.clone(),
             initial_segment: 0,
             initial_address: 0xfeed_beef,
@@ -4106,7 +4106,7 @@ mod tests {
             offset: 0,
             length: 0,
             format: Format::Dwarf32,
-            address_size: 8,
+            address_size: cie2.address_size,
             cie: cie2.clone(),
             initial_segment: 0,
             initial_address: 0xfeed_face,

The expected FDEs that we parse need to have address sizes that match their corresponding CIEs (which got updated in your second commit, but the FDEs did not).

Fold this into the PR and we should be good to merge!

[mitsuhiko]

@fitzgen since the address size is already on the cie anyways i will change it so tha ti always use that data.

[fitzgen]

CI had passed, and then the most recent commit broke it again:

---- read::cfi::tests::test_parse_cfi_instruction_set_loc stdout ----
thread 'read::cfi::tests::test_parse_cfi_instruction_set_loc' panicked at 'assertion failed: `(left == right)`
  left: `EndianSlice { slice: [0, 0, 0, 0, 1, 2, 3, 4], endian: LittleEndian }`,
 right: `EndianSlice { slice: [1, 2, 3, 4], endian: LittleEndian }`', src/read/cfi.rs:4293:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
    read::cfi::tests::test_parse_cfi_instruction_set_loc

[mitsuhiko]

I think the pass was spurious but i need to verify. I will fix this but i want to understand all test cases first to make sure they are correct :)

[philipc]

The segments support is low priority, and I'm not surprised LLVM doesn't support it. There's already some places in gimli where we return Error::UnsupportedSegmentSize for non-zero segment sizes.

[fitzgen]

Thanks for sticking with this one!