std: avoid aliasing violations when wrapping opaque C types

[joboet]

Fixes #160815 (and some other instances of the same problem)

See the new documentation of COpaque for a detailed description of the kinds of issues solved by this. In short: creating mutable references to the opaque types from pthread is unsound since some platforms (at least AIX) store an intrinsically list of these types and the creation of the mutable reference (e.g. in the drop glue) invalidates the other pointers to the type.

This doesn't just apply to the internal pthread Condvar and Mutex abstraction as described in the issue, but also to all other opaque types – nobody is promising us that these are not internally aliased. Hence this PR adds an internal COpaque helper type which uses a combination of UnsafePinned and MaybeUninit to relax all relevant requirements added by Rust's operational semantics.

CC @RalfJung I'd love to hear your opinion on this
r? libs

[ais523]

I noticed something interesting while working on the opsem of this, and it might make sense to call it out in the documentation in order to help make the safety proofs easier to understand.

Normally in Rust, if you convert a reference into a pointer, you cannot use the pointer outside the lifetime of the reference without causing aliasing violations. It's as though raw pointers have lifetimes, and copy them from the reference they're created from (but you get UB rather than a borrow checker error if you violate them). This is the root cause behind #160815; a reference was converted to a pointer, but the pointer outlived the reference. (Although UnsafeCell often helps with this, because all the &UnsafeCell can often be inferred to have the same lifetime and thus the pointers can often last as long as the UnsafeCell does, this mechanism fails in situations where an &mut is created, like in drop.)

With UnsafePinned, and wrappers for it like COpaque, this is no longer true: if you convert a reference to it into a pointer, the pointer is opsem-usable for as long as the containing memory is not used via anything other than UnsafePinned types, and that's the reason why code like Condvar is sound after the change.

In addition to mentioning this in COpaque's documentation, it should probably be mentioned in the safety comments of the uses of the variables (i.e. "this is sound because a pointer obtained via a COpaque lasts until the memory is repurposed, we ask the OS to stop using the memory in our destructor, and because self is Pin our destructor is guaranteed to run before the memory is repurposed"). It took me a long time to understand why this is sound, so having it in a code comment would likely help other people to figure out why it is sound.

[RalfJung]

With UnsafePinned, and wrappers for it like COpaque, this is no longer true: if you convert a reference to it into a pointer, the pointer is opsem-usable for as long as the containing memory is not used via anything other than UnsafePinned types, and that's the reason why code like Condvar is sound after the change.

Yeah, this is the exact same thing that already happens with UnsafeCell for shared references.

[RalfJung]

CC @RalfJung I'd love to hear your opinion on this

Using UnsafePinned here makes a lot of sense.
I am not sure if MaybeUninit is needed, but it can't hurt.

[joboet]

I am not sure if MaybeUninit is needed, but it can't hurt.

I just checked, POSIX actually says that:

The pthread_mutex_destroy() function shall destroy the mutex object referenced by mutex; the mutex object becomes, in effect, uninitialized. An implementation may cause pthread_mutex_destroy() to set the object referenced by mutex to an invalid value.

so the current Mutex and Condvar abstractions are of questionable soundness (though as the destruction happens in Drop and there are no further loads of the pthread_mutex_t/pthread_cond_t this is sound under rust-lang/unsafe-code-guidelines#414).

[tbu-]

Hyphenated things in doc blocks probably don't work correctly.

View changes since the review

[nia-e]

This looks correct to me. I worry slightly that there's a lot of other similar patterns in std that we're not looking at, but this is basically just a line-by-line change to the more relaxed semantics, so there's little to get wrong here ^^ r=me once the doc nit above is addressed

[joboet]

@bors r=@nia-e

[rust-bors]

📌 Commit 3b2afd1 has been approved by nia-e

It is now in the queue for this repository.