New lint: `_.as_ptr() as *mut _` is Undefined Behavior

[Shnatsel]

Casting a *const T to *mut T may lead to memory corruption since it allows mutation of shared state. Even if the *const T happened to be unique, it is still undefined behavior and the optimizer may break such code in interesting ways. In a nutshell, this is as bad as transmuting a & into &mut.

Update: turns out there are cases when this is not immediately trigger UB, but in those cases you shouldn't be doing these casts anyway.

This often occurs when people try to consume a data structure and create a new one from it, e.g.

let new_slice = core::slice::from_raw_parts_mut(old_slice.as_ptr() as *mut B, len)

in which case the proper solution is to rewrite it as

let new_slice = core::slice::from_raw_parts_mut(old_slice.as_mut_ptr(), len)

This also may happen when people try to mutate shared state through a &, in which case they need a Cell, RefCell or an UnsafeCell instead.

Playground with a real-world snippet that passes Clippy as of version 0.0.212 but fails MIRI: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=b28a15e3d99616b03caafdd794550946

This pattern seems to be quite widespread - quoting @RalfJung on Zulip:

I have seen at least 2 or 3 cases over the last few weeks for a const-to-mut raw ptr cast being the give-away for mutation of shared data

Rust stdlib also had this bug: https://blog.rust-lang.org/2017/02/09/Rust-1.15.1.html

[Lokathor]

This is basically covered by #4771 and the discussion there

[scottmcm]

I think this would be better phrased as a lint for casting something when there's a method that just gives you what you want. The cast here isn't UB, but I agree that you shouldn't be doing .as_ptr() as *mut T -- but nor should you be doing .as_mut_ptr() as *const T.

[Shnatsel]

#4771 would be allow-by-default while this is more specific and can be deny-by-default

[Shnatsel]

Wow, Rust standard library itself had this bug: https://blog.rust-lang.org/2017/02/09/Rust-1.15.1.html

Also turns out this might not be instant UB depending on how the underlying slice was obtained, but it's still a really bad idea. I'll update the description with this info.

[RalfJung]

Casting a *const T to *mut T may lead to memory corruption since it allows mutation of shared state. Even if the *const T happened to be unique, it is still undefined behavior and the optimizer may break such code in interesting ways. In a nutshell, this is as bad as transmuting a & into &mut.

This is not entirely correct... something like &mut foo as *mut T as *const T as *mut T is entirely harmless. What is relevant is the initial cast, when a reference is turned to a raw pointer. I think of the pointer as "crossing into another domain", that of uncontrolled raw accesses. If that initial transition is a *const, then the entire "domain" gets marked as read-only (modulo UnsafeCell). The raw ptrs basically "remember" the way that the first raw ptr got created.

Oh, looks like you found that yourself already. Might be good to update the initial paragraph then. ;)

[Shnatsel]

FWIW I've also opened a request for such lint in the compiler, but the resolution was "start out the lint in Clippy and then uplift it based on how the implementation looks".

[CAD97]

To be perfectly clear: (_: *const T) as *mut T is not UB. (The title of the issue should be adjusted for this fact.) All this does is change variance. In fact, this is perfectly safe code that doesn't require unsafe to be written, so it'd be unsound if this were UB.

What is UB is using a pointer whose provenance is a shared &_ borrow in a mutable manner; e.g. ptr::write(_, ...), &mut *_, or *_ =.

In fact _.as_ptr() as *mut _ occurs a lot, correctly, in a couple pointer-heavy libraries I wrote recently. The reason is for (A)Rc to ptr::NonNull conversion: they only stably expose into_raw() -> *const _ currently, and into_raw_non_null() -> ptr::NonNull<T> is not yet stable.

In conclusion, it would definitely be correct to warn-by-default use of _.as_ptr() as *mut _ and _.as_mut_ptr() as *const _ when both methods are present. This can be correct for both cases (suboptimal APIs exist and sometimes it is about variance), but these two are weird enough and likely-to-be-wrong enough to deserve acknowledgement.

This might be able to generically apply to any &_ -> *const _ function, because (|p: &*mut T| -> *const T { *p })(_) as *mut T is perfectly allowed but quite strange. Accessors typically should return the pointer type that communicates its provinence as well as possible.

[Lokathor]

Shocking that it hasn't been said yet: C FFI often needs to accept a *mut T that it's only going to read from. Because the C folks aren't as careful about const* vs * as we are in Rust.

[RalfJung]

In fact _.as_ptr() as *mut _ occurs a lot, correctly, in a couple pointer-heavy libraries I wrote recently. The reason is for (A)Rc to ptr::NonNull conversion: they only stably expose into_raw() -> *const _ currently, and into_raw_non_null() -> ptr::NonNull is not yet stable.

Notice that Rc::into_raw returns a ptr with "shared ref" provenance. So actually writing to that pointer is UB. (In fact, this likely makes Rc::from_raw(Rc::into_raw(rc)).get_mut() unsound .)

Shocking that it hasn't been said yet: C FFI often needs to accept a mut T that it's only going to read from. Because the C folks aren't as careful about const vs * as we are in Rust.

Where does a const-to-mut cast occur here? Is it when Rust code calls C code? But then one could also choose the C function signature in Rust to use *const if one is anyway sure that this is read-only.

[Lokathor]

You can PR winapi to offer something other than exactly and precisely what the official windows SDK headers declare, and we'll see how that goes ;3

[RalfJung]

Sure, if they choose a certain coding stale that might mean more lints that show up for them. I am just saying that there's nothing fundamental about FFI here.

[llogiq]

Do I get that right? We want to lint sequences of casts (with no other expressions in between) where the original expression is an immutable ref or ptr and the result is a mutable ref or ptr?