You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
False positives from invalid_reference_casting #124685
Starting with rustc 1.78.0 (because of #118983) this produces the following
error: casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused
--> src/lib.rs:9:9
|
5 | let a1 = &mut array[offset];
| ------------- backing allocation comes from here
6 | let a2 = a1 as *mut u8;
7 | let a3 = a2 as *mut u64;
| -------------- casting happend here
8 | unsafe {
9 | ptr::write_unaligned(a3, ptr::read_unaligned(a3) | mask);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: casting from `u8` (1 bytes) to `u64` (8 bytes)
= note: `#[deny(invalid_reference_casting)]` on by default
Perhaps I'm missing something but this is not undefined behavior as far as I can tell. The lint is wrong about the backing allocation and I suspect the false warning is downstream from that?
Under the memory model checked by Miri by default, a &mut u8 is not permitted to access more than the one byte it points to, even if you got it from a larger object. Reborrowing to a reference type shrinks the allowed access range. Use array[offset..(offset + 8)].as_mut_ptr() instead to obtain a pointer that is valid to write to the correct region. array[offset..].as_mut_ptr() would also be valid, as that region contains the correct bytes, but it's less precise about its intent.
The important bit from the error message is that the code is trying to access alloc1270[0x0..0x8] (the intended 8 bytes), but the pointer does not have permission for that range, only [0x0..0x1] (the first byte).
I think that the invalid_reference_casting lint's wording should be changed though, "backing allocation" is not precise and does not correctly describe what it's detecting. cc #118983
Your comment and the lint are misleading. There is no accepted memory model. And the lint message is pointing to code which does not create an allocation, so at best the lint has grabbed the wrong span. If this lint is supposed to engage in provenance-based reasoning, it would need to indicate that. Otherwise it is simply buggy.
True, "the current" is not quite right. "The memory model currently checked by Miri by default" is perhaps a better wording for what I wanted to say, I will edit my comment.
Agreed that the lint is definitely sketchy wording at best, and I'm suspicious of its implementation given the incorrect wording.
And the lint message is pointing to code which does not create an allocation, so at best the lint has grabbed the wrong span. If this lint is supposed to engage in provenance-based reasoning, it would need to indicate that. Otherwise it is simply buggy.
The lint is not supposed to engage in "provenance-based reasoning", it is just supposed to peel all the reference/raw pointer casting until it finds an allocation, and then it should compare the size of type and report an error if the target is bigger than the source. Nothing else, nothing more.
So as @saethlin correctly mentions it, nothing in this code creates an "allocation", so I think the lint shouldn't have fired here. (whenever there is actual UB or not is irrelevant here)
Activity
asquared31415 commentedon May 4, 2024
Under the memory model checked by Miri by default, a
&mut u8is not permitted to access more than the one byte it points to, even if you got it from a larger object. Reborrowing to a reference type shrinks the allowed access range. Usearray[offset..(offset + 8)].as_mut_ptr()instead to obtain a pointer that is valid to write to the correct region.array[offset..].as_mut_ptr()would also be valid, as that region contains the correct bytes, but it's less precise about its intent.@rustbot label -regression-from-stable-to-stable -C-bug -I-prioritize +C-discussion
asquared31415 commentedon May 4, 2024
Miri also agrees that the code as written contains undefined behavior, in the top right of the page select "Tools > Miri" to run and you'll see that it says that there's undefined behavior.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e2d5ba429bbd2fb8401599c04d046213
The important bit from the error message is that the code is trying to access
alloc1270[0x0..0x8](the intended 8 bytes), but the pointer does not have permission for that range, only[0x0..0x1](the first byte).asquared31415 commentedon May 4, 2024
I think that the
invalid_reference_castinglint's wording should be changed though, "backing allocation" is not precise and does not correctly describe what it's detecting. cc #118983@rustbot label +A-diagnostics +D-papercut
saethlin commentedon May 4, 2024
Your comment and the lint are misleading. There is no accepted memory model. And the lint message is pointing to code which does not create an allocation, so at best the lint has grabbed the wrong span. If this lint is supposed to engage in provenance-based reasoning, it would need to indicate that. Otherwise it is simply buggy.
asquared31415 commentedon May 4, 2024
True, "the current" is not quite right. "The memory model currently checked by Miri by default" is perhaps a better wording for what I wanted to say, I will edit my comment.
Agreed that the lint is definitely sketchy wording at best, and I'm suspicious of its implementation given the incorrect wording.
[-]False positive invalid_reference_casting?[/-][+]False positives from invalid_reference_casting[/+]Urgau commentedon May 4, 2024
The lint is not supposed to engage in "provenance-based reasoning", it is just supposed to peel all the reference/raw pointer casting until it finds an allocation, and then it should compare the size of type and report an error if the target is bigger than the source. Nothing else, nothing more.
I tried describing the intent of the change in #118983 (comment).
So as @saethlin correctly mentions it, nothing in this code creates an "allocation", so I think the lint shouldn't have fired here. (whenever there is actual UB or not is irrelevant here)
Rollup merge of rust-lang#124761 - Urgau:ref-casting_bigger_slice_ind…
Unrolled build for rust-lang#124761