request: lint when raw pointer slicing implicitly creates a temporary reference #99437
Comments
|
This ties into the concept that has come up a few times of distinguishing an "unsafe place" (one created from a raw pointer deref) from a "safe place" (on created from a reference deref). |
|
I don't think we want to distinguish them in the AM semantics (just like |
|
To repeat my other comment, the most unexpected part IMO is that this is valid with a single index. All official sources say that index notation goes through the I agree that this particular case can be footgunny, though (you'd almost never want to directly use std::ptr::{self, addr_of_mut};
pub fn test(ptr: *mut [u8]) -> *mut [u8] {
let layout_size = 24;
unsafe { ptr::slice_from_raw_parts_mut(ptr.cast::<u8>(), layout_size) }
}But if the subslice is length-dependent (e.g., for a suffix), it's a lot more complicated, since use std::ptr::{self, addr_of_mut};
pub fn test(ptr: *mut [u8]) -> *mut [u8] {
assert!(
!ptr.is_null(),
"no stable way to get the len of a null slice ptr"
);
let layout_size = 24;
let len = unsafe { (*(ptr as *mut [()])).len() };
assert!(len >= layout_size, "slice too short");
let start = unsafe { ptr.cast::<u8>().add(len - layout_size) };
unsafe { ptr::slice_from_raw_parts_mut(start, layout_size) }
}(Weirdness like this is why I generally avoid slice pointers altogether.) |
|
I think the correct alternative is to use the methods tracked in #74265, though those don't do bounds checks. |
…ith slice This potential source of UB was discovered while upgrading the Rust toolchain, which upgrades us to a new version of Miri with stricter rules around raw pointers. Specifically, an expression like `addr_of_mut!((*(ptr))[offset..])` is deliberately attempting to operate only on raw pointers while avoiding any intermediate references, since references have invariants that raw pointers do not. However, there is in fact an implicit reference here that is created as a result of the indexing operation. This is both surprising and not surprising, for interesting reasons. First, it should not be surprising because indexing is governed by the Index traits, whose methods function return references, so their presence here is natural. On the other hand, it is surprising because Rust already special cases `(*ptr)[foo]` when `ptr` is a raw slice and `foo` is not a range to avoid the Index traits entirely, which allows it to avoid emitting an intermediate reference. The ideal solution here is for Rust to be smart enough to not introduce the intermediate reference here at all, which is tracked at rust-lang/rust#73987 . In addition, while investigating this issue I brought it up to the Unsafe Code Guidelines team, who saw fit to file rust-lang/rust#99437 as a more specific example of the potential perils of the current behavior.
…ith range This potential source of UB was discovered while upgrading the Rust toolchain, which upgrades us to a new version of Miri with stricter rules around raw pointers. Specifically, an expression like `addr_of_mut!((*(ptr))[offset..])` is deliberately attempting to operate only on raw pointers while avoiding any intermediate references, since references have invariants that raw pointers do not. However, there is in fact an implicit reference here that is created as a result of the indexing operation. This is both surprising and not surprising, for interesting reasons. First, it should not be surprising because indexing is governed by the Index traits, whose methods function return references, so their presence here is natural. On the other hand, it is surprising because Rust already special cases `(*ptr)[foo]` when `ptr` is a raw slice and `foo` is not a range to avoid the Index traits entirely, which allows it to avoid emitting an intermediate reference. The ideal solution here is for Rust to be smart enough to not introduce the intermediate reference here at all, which is tracked at rust-lang/rust#73987 . In addition, while investigating this issue I brought it up to the Unsafe Code Guidelines team, who saw fit to file rust-lang/rust#99437 as a more specific example of the potential perils of the current behavior. Signed-off-by: bstrie <[email protected]>
… with range This potential source of UB was discovered while upgrading the Rust toolchain, which upgrades us to a new version of Miri with stricter rules around raw pointers. Specifically, an expression like `addr_of_mut!((*(ptr))[offset..])` is deliberately attempting to operate only on raw pointers while avoiding any intermediate references, since references have invariants that raw pointers do not. However, there is in fact an implicit reference here that is created as a result of the indexing operation. This is both surprising and not surprising, for interesting reasons. First, it should not be surprising because indexing is governed by the Index traits, whose methods function return references, so their presence here is natural. On the other hand, it is surprising because Rust already special cases `(*ptr)[foo]` when `ptr` is a raw slice and `foo` is not a range to avoid the Index traits entirely, which allows it to avoid emitting an intermediate reference. The ideal solution here is for Rust to be smart enough to not introduce the intermediate reference here at all, which is tracked at rust-lang/rust#73987 . In addition, while investigating this issue I brought it up to the Unsafe Code Guidelines team, who saw fit to file rust-lang/rust#99437 as a more specific example of the potential perils of the current behavior. Signed-off-by: bstrie <[email protected]>
… with range This potential source of UB was discovered while upgrading the Rust toolchain, which upgrades us to a new version of Miri with stricter rules around raw pointers. Specifically, an expression like `addr_of_mut!((*(ptr))[offset..])` is deliberately attempting to operate only on raw pointers while avoiding any intermediate references, since references have invariants that raw pointers do not. However, there is in fact an implicit reference here that is created as a result of the indexing operation. This is both surprising and not surprising, for interesting reasons. First, it should not be surprising because indexing is governed by the Index traits, whose methods function return references, so their presence here is natural. On the other hand, it is surprising because Rust already special cases `(*ptr)[foo]` when `ptr` is a raw slice and `foo` is not a range to avoid the Index traits entirely, which allows it to avoid emitting an intermediate reference. The ideal solution here is for Rust to be smart enough to not introduce the intermediate reference here at all, which is tracked at rust-lang/rust#73987 . In addition, while investigating this issue I brought it up to the Unsafe Code Guidelines team, who saw fit to file rust-lang/rust#99437 as a more specific example of the potential perils of the current behavior. Signed-off-by: bstrie <[email protected]>
|
Rejecting code like that is probably going to be hard, but both this and #73987 can be mitigated by a lint I think. Concretely, what happens here is that the compiler desugars |
RalfJung
changed the title
|
@rustbot claim |
jyn514
added
C-enhancement
So let's say you know about aliasing models and have learned that to avoid all aliasing rules of Rust you need to use raw pointers throughout, no references. So for example you write:
Looks good, doesn't it?
Well sadly the MIR for this code looks as follows:
Note the
_4 = &mut (*_1): Rust "helpfully" created a reference for us to subslice into!This is terrible, because it introduces UB.
This code should either be rejected, or use raw pointers throughout.
Related to #73987.
The text was updated successfully, but these errors were encountered: