Decide what to do about UnsafePinned and safe Pin::as_ref #137750
Description
As context, this entirely safe code is reported as UB by Miri under both Stacked Borrows and Tree Borrows:
use core::{ops::Deref, pin::{pin, Pin}, task::{Context, Poll, Waker}}; fn main() { let mut f = pin!(async move { let x = &mut 0u8; core::future::poll_fn(move |_| { *x = 1; //~^ ERROR write access is forbidden Poll::<()>::Pending }).await }); let mut cx = Context::from_waker(&Waker::noop()); assert_eq!(f.as_mut().poll(&mut cx), Poll::Pending); _ = <Pin<&mut _> as Deref>::deref(&f); assert_eq!(f.as_mut().poll(&mut cx), Poll::Pending); }Output
Under TB:error: Undefined Behavior: write access through <1666> at alloc969[0x8] is forbidden --> src/main.rs:7:13 | 7 | *x = 1; | ^^^^^^ write access through <1666> at alloc969[0x8] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental = help: the accessed tag <1666> has state Frozen which forbids this child write access help: the accessed tag <1666> was created here, in the initial state Reserved --> src/main.rs:6:9 | 6 | / core::future::poll_fn(move |_| { 7 | | *x = 1; 8 | | Poll::<()>::Pending 9 | | }).await | |________________^ help: the accessed tag <1666> later transitioned to Active due to a child write access at offsets [0x8..0x9] --> src/main.rs:7:13 | 7 | *x = 1; | ^^^^^^ = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference help: the accessed tag <1666> later transitioned to Frozen due to a reborrow (acting as a foreign read access) at offsets [0x0..0x10] --> src/main.rs:13:9 | 13 | _ = <Pin<&mut _> as Deref>::deref(&f); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: this transition corresponds to a loss of write permissions = note: BACKTRACE (of the first span): = note: inside closure at src/main.rs:7:13: 7:19 --> src/main.rs:9:12 | 9 | }).await | ^^^^^ note: inside `main` --> src/main.rs:14:16 | 14 | assert_eq!(f.as_mut().poll(&mut cx), Poll::Pending); | ^^^^^^^^^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace error: aborting due to 1 previous errorThis isn't a bug in Miri really. It's a bug... elsewhere. Part of that is described by:
But at the same time, this is relevant to the post-RFC work on UnsafePinned:
The idea of UnsafePinned is that we use it to wrap the fields subject to self-borrows so that given a mutable reference to such fields, the compiler (and our model) won't assume that a foreign write can't occur.
But the RFC suggested that while &mut UnsafePinned<T> was special, &UnsafePinned<T> wasn't, and would therefore act like &T. This is where we've hit problems due to safe Pin::deref, as @RalfJung has described:
- Tracking Issue for RFC 3467: UnsafePinned #125735 (comment)
- Initial
UnsafePinnedimplementation [Part 1: Libs] #137043 (comment) - !Send Future violates stacked borrow rules miri#3796 (comment)
In the above example, if the compiler were to insert:
let x: &mut UnsafePinned<0u8> = ..;Then we still end up having a problem, under the RFC's model and our current model behavior, which is (quoting @RalfJung):
when a shared reference gets created, do a read of everything the reference points to (except
UnsafeCell) to ensure things are in a sane state
What happens in the example is that:
- We create and store a mutable self-reference to the
u8owned by the future. - We activate that self-reference with a write, marking it
Uniqueunder TB. - Then, due to the
derefcreating a reference to the future and our behavior to treat that as a read of the entire future, a foreign read occurs, marking the mutable reference asFrozen. - Next time we try to write, we get UB.
About this, @RalfJung has said:
In the RFC we had the question whether
UnsafePinnedshould also act like anUnsafeCell. I am now fairly sure that the answer is "yes, it must". The reason is this: due toPin::derefbeing safe, it is at any moment possible to create a shared reference to a future that is halted at a suspension point. If there is noUnsafeCell, this acts like a read of the entire future data, and that read conflicts with mutable references that the future may hold to its own data. This is not a necessary property of aliasing with mutable reference, but it is a necessary consequence of the decision thatPin::derefshould be safe.
And also that:
One could imagine aliasing models that don't do such a read, though then
dereferenceablebecomes more questionable and I don't know how this would impact optimizations.
Anyway, it seems we should have a canonical issue to use to decide about how to proceed about this and how either our model or RFC 3467 would need to be adjusted before the stabilization of UnsafePinned, so here is that issue.
2025-04-26:
As a slight refinement here, note that we actually don't even need deref, as Pin::as_ref is enough.
use core::{ pin::{pin, Pin}, task::{Context, Poll, Waker}, }; fn main() { let mut f = pin!(async move { let x = &mut 0u8; core::future::poll_fn(move |_| { *x = 1; //~^ ERROR write access is forbidden Poll::<()>::Pending }) .await }); let mut cx = Context::from_waker(&Waker::noop()); assert_eq!(f.as_mut().poll(&mut cx), Poll::Pending); let _: Pin<&_> = f.as_ref(); // Or: `f.as_mut().into_ref()`. assert_eq!(f.as_mut().poll(&mut cx), Poll::Pending); }cc @RalfJung @WaffleLapkin @rust-lang/opsem @rust-lang/lang
@rustbot labels +T-lang +T-opsem +C-discussion +F-unsafe_pinned