Casting constness can be risky, here's a simple fix

I just realized it may be risky to cast consts. Consider this code:

// AtomicPtr is a good real life example of the need to cast pointers
static FOO: AtomicPtr<BigBar> = AtomicPtr::new(ptr::null_mut());

// assume this code is far away from FOO declaration above
fn do_stuff() {
   // load returns *mut _
   let const_ptr =  FOO.load(Ordering::Relaxed) as *const BigBar;
   unsafe { do_something_with_const_big_bar(const_ptr); }
}

Now imagine someone refactoring FOO by e.g. changing BigBar into SmallBar (SmallBar having smaller size in memory than BigBar) and forgetting to change the cast in do_stuff and subsequent call. There would be no compile errorbut a silent conversion. do_something_with_const_big_bar would be called and possibly read out-of-bounds memory - UB (there are other ways to UB, this is just one example).

The solution is simple - define this function:

fn ptr_mut_to_const<T>(ptr: *mut T) -> *const T {
    ptr as _
}

And use it instead of raw cast.

Great, now you know how to avoid the risk but maybe it'd make sense to go further? We could add this function and it's converse (const_to_mut) to core::ptr and have a (clippy) lint to suggest it instead of as. This would save those people who didn't read this post.

What do you think? Is it worth adding to core? Is there a better way? I'm willing to make a PR if experienced Rust devs agree this is a good idea.

11 Likes

Apparently for *mut T to *const T, there's already an implicit coercion in place, so you don't need to use the as at all, and then you're safe for if the type changes.

Rust Playground

or

Rust Playground

For the converse, *const T to *mut T, a conversion method is quite useful in my opinion, probably as a method of *mut T. A (clippy) lint seems like a reasonable idea, too. For consistency, a *mut T to *const T method can make sense, too. After all, e.g. &String to &str converts implicitly, too, yet there's also a method for it.

8 Likes

I fully agree with basically everything you wrote. :slight_smile: as on raw pointers gives me the jeebies, we should avoid it wherever we can.

We already have cast which lets one cast the type while preserving mutability, this would basically be dual.

11 Likes

@steffahn :open_mouth: that's interesting! Agree with having both methods.

@RalfJung thanks a lot for feedback! Hearing it from you is very reassuring. I guess it's safe to make a PR now. :slight_smile:

1 Like

It's worth noting that cast only works for Sized target types, so for ?Sized types it's still required to use as. Pointer metadata might allow us to loosen cast to actually say "has the same metadata," depending on the final shape of the feature.

1 Like

Though you would commonly want to adjust the metadata value, e.g. *const [u16] -> *const [u8] needs to double the length.

3 Likes

And we also recently got (on nightly) to_bits and from_bits (https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.to_bits) as a way to avoid a different kind of as cast.

So I also think it makes sense to add methods for the *mut<->*const casts.

Do people have any thoughts on the names? as_const and as_mut are the first that jump to mind to me, but it might be interesting to find a longer name for the const-to-mut version than for the other way around, to help make it stand out.

Aw man, to_bits and from_bits aren't const? That's unfortunate. And several other pointer methods are const-unstable, despite their tracking issue being closed.

Anyway, my request here: please make as_const/as_mut const.

As for names, I think as_mut is great. I'd prefer we don't try to inflate the name just to make it stand out more.

as_mut already exists. I think as_mut_ptr makes sense, which is a method we'd need to steal from *mut [T]/#![feature(slice_ptr_get)] (where IMO the name is inappropriate/confusing anyways, *mut [T] already is a mutable ptr; it comes from the slice::as_mut_ptr name). As for the inverse, for consistency then as_const_ptr. Or as_const or as_ptr could also work.

Edit: Actually, as_mut only exists for *mut T, but still IMO, the confusion potential when re-using it is somewhat high.

Edit2: For comparison, NonNull uses as_ptr and as_mut_ptr naming for conversion to *const T and *mut T. No, that's wrong, it uses as_ptr to return *mut T.

2 Likes

They aren't and they won't, at least for to_bits. Casting between pointers and integers is unsupportable at const-time since we cannot know at which address the object will be placed when the program is being run.

All the others can and should be const fn though, true.

2 Likes

I don't want to sidetrack this too much, but my use case is actually about statics. Non-const fns can't be called when initializing a static, which makes statics kinda annoying to work with (particularly in FFI contexts) (example; I know I can just use &'static _ but sometimes an API requires a usize and it'd be nice to use the right type from the beginning instead of having to cast it repeatedly; I also know this is orthogonal to this thread since you couldn't even really pass the static address to a const fn for further evaluation/casting, since that value is only known at link time and not compile time; sorry for the tangent and massive parenthetical, I just wish I could make a static usize from a static variable's address).

Anyway, I'm happy to hear it's not contentious for these proposed methods to be const.

As I was going to write the PR, I noticed that cast only disallows destination to be unsized but allows source to be unsized. playground

Is this intended? It doesn't seem to do anything bad but doesn't look right either.

I would guess it is indeed intended. Casting pointers with as always supports converting from any ?Sized type to any Sized type, but not to arbitrary ?Sized target types. Unsized to unsized pointer casts (with as) only work if the type of the metadata matches, which is the case if both types are containing1 slices (string slice, or any [T] size), or if both types are containing1 trait objects for the same trait (with potentially different + Send / + Sync / etc additions).

1By "containing" I mean, that the type Mutex<dyn Trait> in a pointer like *const Mutex<dyn Trait> contains a dyn Trait, and its pointer has consequently the same type of metadata. Or a struct Foo { x: [u8] } will make *const Foo have the same metadata as *const [u8], or *mut str, or other slices..

This relation is impossible to express at type level (at least with stable standard library API), so cast methods cannot support them.

Yeah, that occurred to me, what I'm unsure of is whether

  • casting unsized pointer to sized is well-defined (guaranteed to drop metadata etc)
  • cast was written with the intention to support this conversion

Submitted the PR: Implemented const casts of raw pointers by Kixunil · Pull Request #92657 · rust-lang/rust · GitHub