Rust-clippy: cast_ptr_alignment is acceptable in a narrow set of contexts

[Cocalus]

Clippy may not be able to see when this is acceptable but if so there are several functions that can handle unaligned ptrs. If the unaligned pointer is only used as an input to those then it ideally wouldn't be flagged.

The obvious
std::ptr::read_unaligned
std::ptr::write_unaligned

Anything with memcpy or memmove semantics
std::ptr::copy
std::ptr::copy_nonoverlapping

I'm unsure about std::ptr::swap and std::ptr::swap_nonoverlapping

And in
std::arch::x86_64
std::arch::x86
storeu
loadu
lddqu

There might be some others.

[danburkert]

Example from something I'm working on:

error: casting from `*const u8` to a more-strictly-aligned pointer (`*const i16`)
  --> src/<>.rs:<>:<>
   |
69 |                 (data as *const i16).read_unaligned().to_le()
   |

which is obviously fine. It seems appropriate to at least mention this in the 'Known problems' section of cast_ptr_alignment.

[Valloric]

I've only ever seen cast_ptr_alignment lint being pretty useless; I have similar code to @danburkert.

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`)
   --> src/btree/raw_page.rs:160:13
    |
160 |             (key_entry_start.as_ptr() as *mut u16).read_unaligned();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_ptr_alignment

and also

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`)
   --> src/btree/raw_page.rs:220:28
    |
220 |       ptr::write_unaligned(entry_start as *mut u16, key_bytes_to_copy);
    |                            ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_ptr_alignment

[strega-nil]

This also breaks in the context of allocation functions - malloc always returns a 16-byte aligned pointer, iirc, so there's no need to lint the cast of the return value.

[Cocalus]

Malloc returns pointers that aligned to size of the allocation (rounding upto the next highest power of 2) or sufficiently large enough max alignment (often 16 bytes). Most allocators have a minimum alignment as well. Rust's default allocator jemalloc has a minimum alignment of 8 bytes by default.

[strega-nil]

@Cocalus oh, also, to be clear, the _only_ functions in rust which can deal with unaligned pointers are those that end in _unaligned - copy and copy_nonoverlapping do expect aligned pointers.

[Cocalus]

If that's true then the documentation is wrong, or the name should be fixed.

From https://doc.rust-lang.org/beta/std/ptr/fn.copy_nonoverlapping.html

copy_nonoverlapping is semantically equivalent to C's memcpy

Memcpy just move bytes from one place to another. And is generally the standards correct way to realign data in C.

I'm actively using it in unsafe code to fix the alignment, of a few types. So If that's wrong what is the correct way of copying a bunch of bytes to a particular alignment.

[strega-nil]

@Cocalus note: https://doc.rust-lang.org/nightly/std/intrinsics/fn.copy_nonoverlapping.html#safety

The correct way is to either 1) use read_unaligned and write_unaligned in a loop, or 2) use copy_nonoverlapping::<u8>

[strega-nil]

I guess this has been a learning experience, so this is likely a good warning. I'd like to do the following:

• Change from error -> warning
• Add a note about the std::ptr functions

are people okay with this?

[Cocalus]

Thanks, the alignment requirement has been added to the nightly docs since I last checked. I think just casting both sides to *u8 and doing the sizeof calculation by hand should work for my needs.

Adding the note that there are some functions that support unaligned is a definite improvement. If clippy can't white list the functions directly, then listing some/all of them should be helpful. I wonder if the pointer types could be extended to have a unaligned type. Then there could be "safe" casts that verify alignment.

For my needs I still want the alignment check in general, since it's easy to mess up in unsafe code. I've been adding allow(cast_ptr_alignment) to the individual calls that I hand verify. Since I don't allow clippy warnings the change from error to warning doesn't really affect my workflow. I'm fine leaving it as an error with a description of the rare exceptions, since it seems far more likely to misused. But I don't know what clippy's standards are for that.

[dtolnay]

+1 for recognizing the specific pattern of ptr::read_unaligned(ptr as *const T) as correct. Example that came up in Chrome OS and required suppressing this lint: platform/crosvm/crosvm_plugin/src/lib.rs

[vks]

A note about the std::ptr functions was added, but the lint was not converted to a warning yet.

[kornelski]

Same here. I've discovered it because I had a real bug (yay!), but the warning didn't go away when I used read_unaligned.