cranelift / Issue #1340 Fix remaining clippy warnings · git-cranelift

Stream: git-cranelift

Topic: cranelift / Issue #1340 Fix remaining clippy warnings

GitHub (Jan 13 2020 at 17:45):

@philipc, based on your comment https://github.com/bytecodealliance/cranelift/pull/1324#discussion_r365509394 I added 1195612 as well.

GitHub (Jan 14 2020 at 19:13):

abrown commented on Issue #1340:

@yurydelendik, the clippy error comes at line 255:

error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u32`) (1 < 4 bytes)
   --> cranelift-codegen/src/isa/x86/fde.rs:255:48
    |
255 |     let fde_offset = unsafe { ptr::read::<u32>(bytes.as_ptr() as *const u32) } as usize + 4;
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[deny(clippy::cast_ptr_alignment)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment

How do you think this should be fixed? Do we know that bytes is aligned to a 4-byte boundary or can we get there?

GitHub (Jan 14 2020 at 19:17):

abrown commented on Issue #1340:

Or perhaps https://doc.rust-lang.org/std/vec/struct.Vec.html#method.align_to and assert that the prefix has zero length?

GitHub (Jan 14 2020 at 19:57):

abrown edited a comment on Issue #1340:

Or perhaps https://doc.rust-lang.org/std/vec/struct.Vec.html#method.align_to and assert that the prefix has zero length?

[edit: 6eb09da does just that]

GitHub (Jan 16 2020 at 03:10):

yurydelendik commented on Issue #1340:

error: casting from *const u8 to a more-strictly-aligned pointer (*const u32) (1 < 4 bytes)
--> cranelift-codegen/src/isa/x86/fde.rs:255:48
How do you think this should be fixed? Do we know that bytes is aligned to a 4-byte boundary or can we get there?

There was a thread probably related to this: https://github.com/bytecodealliance/cranelift/pull/1320#discussion_r366016546 -- u32::from_le_bytes(bytes.as_slice()[..8].try_into() may not need alignment?

GitHub (Jan 16 2020 at 17:06):

abrown commented on Issue #1340:

@yurydelendik, @iximeow: is 0252c77 what you meant? The tests will likely fail (they fail on my system) so I don't think I understood exactly what was intended here.

GitHub (Jan 16 2020 at 17:41):

iximeow commented on Issue #1340:

@abrown my point was to remove the unsafe ptr::read entirely, and replace it with the from_le_bytes expression. mentally conflating the pointer size and u32 size is why i wrote the wrong slice length [..8] instead of [..4] :)
let fde_offset = u32::from_le_bytes(bytes.as_slice()[..4]
    .try_into()
    .expect("4 bytes at the beginning of the allocated FDE") as usize + 4;
is what i was thinking should be a good substitute. i'm pretty sure we can actually be confident there will be a u32 of bytes there, so the expect ought to never fail.

GitHub (Jan 16 2020 at 17:42):

iximeow edited a comment on Issue #1340:

@abrown my point was to remove the unsafe ptr::read entirely, and replace it with the from_le_bytes expression. mentally conflating the pointer size and u32 size is why i wrote the wrong slice length [..8] instead of [..4] :)
let fde_offset = u32::from_le_bytes(bytes.as_slice()[..4]
    .try_into()
    .expect("4 bytes at the beginning of the allocated FDE") as usize + 4;
is what i was thinking should be a good substitute. i'm pretty sure we can actually be confident there will be a u32 of bytes there, so the expect ought to never fail.

edit: I see we raced and you _just_ pushed a commit with that change!

GitHub (Jan 17 2020 at 20:03):

yurydelendik commented on Issue #1340:

Thank you for the patch

Last updated: Nov 22 2024 at 17:03 UTC