5225225
5225225@furry.engineer
rust unsafe code quiz: can you spot the issue.
fn main() { unsafe {
let src = [1_u8, 2, 3, 4];
let mut dest = std::mem::MaybeUninit::<[u8; 4]>::uninit();
std::ptr::copy_nonoverlapping(
&src,
dest.as_mut_ptr(),
std::mem::size_of::<[u8; 4]>());
} }
this code is blatantly wrong, once you know what it is. (also, before you ask, no it's not an argument order issue, that wouldn't compile in this case.)
rustc doesn't lint on this at all. (clippy does have a lint for this)
7
1
0
Δ-44203 'Carbon'
1@mastodon.vierkantor.com
SYN @5225225, hypothesis: does the size_of call return the size of a pointer to four u8s, not of the memory pointed to?
1
0
0
Squeakable 
[
]
squeakable@furry.engineer
Show content
@5225225 ...Oh. The `count` argument takes a number of `T`, not a number of bytes.
That was very hard to spot, especially given that the docs for `copy_nonoverlapping` say that it's semantically equivalent to `memcpy`!
0
0
0
@5225225 lmao and the panic in debug builds is not obvious at all either, this is great
i wasn't really sure what was going on until i went and looked at how the check is implemented which also is not a great sign ( https://github.com/rust-lang/rust/blob/c26db435bf8aee2efc397aab50f3a21eb351d6e5/library/core/src/intrinsics/mod.rs#L4327 )
1
0
0
@iximeow better then the alternative of no abort at all and having silent UB, at least, which is what most other languages do :P
(if you ran this code under miri you'd get a much more clear indication of the issue. or address sanitizer.)
also, the runtime abort wouldn't even happen if this was just copy. the only way it can even notice something is wrong here is that the ranges overlap.
1
0
0
@5225225 yeah and in reality it's probably unlikely to copy from the stack to the stack like this, so you wouldn't have the lucky colocated copies to correctly-but-confusing detect as overlapping...
1
0
0
@iximeow yeah, this kinda thing is just hard to notice if you don't have metadata like "what allocation does this pointer belong to and how big is it?"
the checks absolutely do help when they can though, especially when they're just on by default for everyone.
0
0
0
@1 SYN-ACK
No, size_of here just takes a type (in this case, [u8; 4]) and returns the size of that type in bytes. In this case, that would be 4.
1
0
0
@luna yeah. lmao.
hey, clippy does catch this (with a deny by default lint). so moral of the story: run clippy ig.
(honestly, imo this lint would be a candidate for uplifting into rustc itself because it's so obviously wrong to do this, and wrong in a very subtle way.)
0
0
0
Δ-44203 'Carbon'
1@mastodon.vierkantor.com
ACK @5225225, understood!
from reading the other responses, this one now sees why the intention behind the code is indeed incorrectly expressed. thank it for providing this puzzle. 
0
0
0
🦊ΘΔ
Show content
@5225225 OH it's a count not a size for third argument of copy_nonoverlapping 💀💀💀
At least it's reassuring to think that unless you have an IDE that suggests these functions to you and you guess it, you should see a "count" variable name and think for a second. Still, that's an easy footgun
EDIT: and of course here it silently works as intended because of u8
1
0
0
Show content
1
0
0
Show content
@SharpLimefox (untyped in that it doesnt need to be a valid T)
see https://github.com/rust-lang/unsafe-code-guidelines/issues/330 for more context
0
0
0
Show content
@5225225 I've been puzzled by this for a while.
Is the bug that copy_nonoverlapping will copy size of [u8;4] x size of [u8;4], so 16 bytes, instead of 4 bytes as intended ?
Because copy_nonoverlapping will copy count (arg3) x size of T, and T here is [u8;4] instead of u8 ?
1
0
0
Show content
@Sobex yep :3
it's subtle but yeah, copy_nonoverlapping takes a count, you don't need to calculate size yourself. the correct thing would be to just pass a constant 1
0
0
0