Add as_mut_ptr

[kornelski]

Fixes #24

[sfackler]

I would rather fix the definition of Opaque, if that is actually incorrect.

[kornelski]

I'm not sure if it's even possible for the Opaque type to be correct.

1. Opaque(PhantomData<UnsafeCell<*mut ()>>). PhantomData doesn't exist in memory and is always-zero sized. Miri checks memory address ranges, and I don't expect PhantomData to ever own a memory range. The type in it is not stored, only used for type checking. Miri isn't just a type checker.
2. Opaque(UnsafeCell<*mut ()>) holds a wrong type (UnsafeCell is shallow). It doesn't cover memory range of the target type, only sizeof(usize) number of bytes. Getting the right size unfortunately makes the opaque type not opaque.
3. Opaque(UnsafeCell<CType>) it's not enough for UnsafeCell to exist and own some memory to allow mutable aliasing. It's not passive, and it requires use of get() to obtain a blessed pointer. The docs explicitly say that mutable dereference of ptr as *const UnsafeCell<T> as *mut T is UB. And I think that's exactly what as_ptr does now.
4. It's also UB to mutate through *mut T while any &T of the same instance is alive. I don't know if type casting from &T to *mut U makes it technically allowed, but this API doesn't help prevent it anyway.

A &self -> *const T &mut self -> *mut T API would at least help with const-correct C APIs. It's hard to handle mutable aliasing and data races in C and FFI, but the &self -> *mut API makes the wrong thing easier.

[sfackler]

The definition of Opaque came from eddyb, who I trust to have an accurate understanding of the compiler's memory model (at least as of 2017 when it was designed).

As I mentioned in #24, Rust's concept of mutability and C's concept of const-correctness do not align. The & -> *mut transform must work for nontrivial C bindings to work.

[kornelski]

Memory model of Rust has changed since then, and is still not fully defined. However, as_ptr currently is very similar to a case that is explicitly documented as UB:

unsafe fn not_allowed<T>(ptr: &UnsafeCell<T>) -> &mut T {
  let t = ptr as *const UnsafeCell<T> as *mut T;
  // This is undefined behavior, because the `*mut T` pointer
  // was not obtained through `.get()` nor `.raw_get()`:
  unsafe { &mut *t }
}

[sfackler]

"Is similar to" is not the same thing as "is".

[kornelski]

The point is that casting a pointer to an UnsafeCell type is not enough to get a pointer that allows mutation of the data behind it. You're not using get(), so UnsafeCell doesn't give you a valid pointer.

[sfackler]

If we are operating under the belief that Opaque's structure was valid in 2017, I do not believe these concerns are realistic. This library is downloaded ~140,000 times a day. The Rust developers are not going to change the memory model and its implementation in the compiler in a way that will break foreign-types overnight.

I would like to see either:

1. A concrete example of how the foreign-types API is unsound today.
2. Someone from the lang team stating explicitly that they intend to make this pattern unsound in the future and what the correct pattern is to handle this use case.

[madsmtm]

3. Opaque(UnsafeCell<CType>) it's not enough for UnsafeCell to exist and own some memory to allow mutable aliasing. It's not passive, and it requires use of get() to obtain a blessed pointer. The docs explicitly say that mutable dereference of ptr as *const UnsafeCell<T> as *mut T is UB. And I think that's exactly what as_ptr does now.

I traced that documentation comment back to this PR and this Zulip stream, in which there is talk about mostly disallowing this for now, but wanting to allow it in the future. Not sure what the conclusion here should be, just noting.