diff --git a/library/core/src/pin/unsafe_pinned.rs b/library/core/src/pin/unsafe_pinned.rs index 5fb628c8adbc5..29783bdd6c5fb 100644 --- a/library/core/src/pin/unsafe_pinned.rs +++ b/library/core/src/pin/unsafe_pinned.rs @@ -1,10 +1,13 @@ +use crate::cell::UnsafeCell; use crate::marker::{PointerLike, Unpin}; use crate::ops::{CoerceUnsized, DispatchFromDyn}; use crate::pin::Pin; use crate::{fmt, ptr}; -/// This type provides a way to opt-out of typical aliasing rules; +/// This type provides a way to entirely opt-out of typical aliasing rules; /// specifically, `&mut UnsafePinned` is not guaranteed to be a unique pointer. +/// This also subsumes the effects of `UnsafeCell`, i.e., `&UnsafePinned` may point to data +/// that is being mutated. /// /// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still /// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work @@ -25,30 +28,19 @@ use crate::{fmt, ptr}; #[repr(transparent)] #[unstable(feature = "unsafe_pinned", issue = "125735")] pub struct UnsafePinned { - value: T, + value: UnsafeCell, } +// Override the manual `!Sync` in `UnsafeCell`. +#[unstable(feature = "unsafe_pinned", issue = "125735")] +unsafe impl Sync for UnsafePinned {} + /// When this type is used, that almost certainly means safe APIs need to use pinning to avoid the /// aliases from becoming invalidated. Therefore let's mark this as `!Unpin`. You can always opt /// back in to `Unpin` with an `impl` block, provided your API is still sound while unpinned. #[unstable(feature = "unsafe_pinned", issue = "125735")] impl !Unpin for UnsafePinned {} -/// The type is `Copy` when `T` is to avoid people assuming that `Copy` implies there is no -/// `UnsafePinned` anywhere. (This is an issue with `UnsafeCell`: people use `Copy` bounds to mean -/// `Freeze`.) Given that there is no `unsafe impl Copy for ...`, this is also the option that -/// leaves the user more choices (as they can always wrap this in a `!Copy` type). -// FIXME(unsafe_pinned): this may be unsound or a footgun? -#[unstable(feature = "unsafe_pinned", issue = "125735")] -impl Copy for UnsafePinned {} - -#[unstable(feature = "unsafe_pinned", issue = "125735")] -impl Clone for UnsafePinned { - fn clone(&self) -> Self { - *self - } -} - // `Send` and `Sync` are inherited from `T`. This is similar to `SyncUnsafeCell`, since // we eventually concluded that `UnsafeCell` implicitly making things `!Sync` is sometimes // unergonomic. A type that needs to be `!Send`/`!Sync` should really have an explicit @@ -63,7 +55,7 @@ impl UnsafePinned { #[must_use] #[unstable(feature = "unsafe_pinned", issue = "125735")] pub const fn new(value: T) -> Self { - UnsafePinned { value } + UnsafePinned { value: UnsafeCell::new(value) } } /// Unwraps the value, consuming this `UnsafePinned`. @@ -72,7 +64,7 @@ impl UnsafePinned { #[unstable(feature = "unsafe_pinned", issue = "125735")] #[rustc_allow_const_fn_unstable(const_precise_live_drops)] pub const fn into_inner(self) -> T { - self.value + self.value.into_inner() } } diff --git a/src/tools/miri/tests/fail/async-shared-mutable.rs b/src/tools/miri/tests/fail/async-shared-mutable.rs new file mode 100644 index 0000000000000..62780e7a11c96 --- /dev/null +++ b/src/tools/miri/tests/fail/async-shared-mutable.rs @@ -0,0 +1,25 @@ +//! FIXME: This test should pass! However, `async fn` does not yet use `UnsafePinned`. +//! This is a regression test for : +//! `UnsafePinned` must include the effects of `UnsafeCell`. +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +//@normalize-stderr-test: "\[0x[a-fx\d.]+\]" -> "[OFFSET]" + +use core::future::Future; +use core::pin::{Pin, pin}; +use core::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 + 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); +} diff --git a/src/tools/miri/tests/fail/async-shared-mutable.stack.stderr b/src/tools/miri/tests/fail/async-shared-mutable.stack.stderr new file mode 100644 index 0000000000000..8f53a55cc3ee3 --- /dev/null +++ b/src/tools/miri/tests/fail/async-shared-mutable.stack.stderr @@ -0,0 +1,43 @@ +error: Undefined Behavior: attempting a write access using at ALLOC[OFFSET], but that tag does not exist in the borrow stack for this location + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | *x = 1; + | ^^^^^^ + | | + | attempting a write access using at ALLOC[OFFSET], but that tag does not exist in the borrow stack for this location + | this error occurs as part of an access at ALLOC[OFFSET] + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a Unique retag at offsets [OFFSET] + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | / core::future::poll_fn(move |_| { +LL | | *x = 1; +LL | | Poll::<()>::Pending +LL | | }) +LL | | .await + | |______________^ +help: was later invalidated at offsets [OFFSET] by a SharedReadOnly retag + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | let _: Pin<&_> = f.as_ref(); // Or: `f.as_mut().into_ref()`. + | ^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside closure at tests/fail/async-shared-mutable.rs:LL:CC + = note: inside ` as std::future::Future>::poll` at RUSTLIB/core/src/future/poll_fn.rs:LL:CC +note: inside closure + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | .await + | ^^^^^ +note: inside `main` + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | 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 error + diff --git a/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr b/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr new file mode 100644 index 0000000000000..d1e66a0d043f5 --- /dev/null +++ b/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr @@ -0,0 +1,47 @@ +error: Undefined Behavior: write access through at ALLOC[OFFSET] is forbidden + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | *x = 1; + | ^^^^^^ write access through at ALLOC[OFFSET] 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 has state Frozen which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | / core::future::poll_fn(move |_| { +LL | | *x = 1; +LL | | Poll::<()>::Pending +LL | | }) +LL | | .await + | |______________^ +help: the accessed tag later transitioned to Active due to a child write access at offsets [OFFSET] + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | *x = 1; + | ^^^^^^ + = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference +help: the accessed tag later transitioned to Frozen due to a reborrow (acting as a foreign read access) at offsets [OFFSET] + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | let _: Pin<&_> = f.as_ref(); // Or: `f.as_mut().into_ref()`. + | ^^^^^^^^^^ + = help: this transition corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): + = note: inside closure at tests/fail/async-shared-mutable.rs:LL:CC + = note: inside ` as std::future::Future>::poll` at RUSTLIB/core/src/future/poll_fn.rs:LL:CC +note: inside closure + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | .await + | ^^^^^ +note: inside `main` + --> tests/fail/async-shared-mutable.rs:LL:CC + | +LL | 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 error + diff --git a/src/tools/miri/tests/pass/both_borrows/unsafe_pinned.rs b/src/tools/miri/tests/pass/both_borrows/unsafe_pinned.rs new file mode 100644 index 0000000000000..0c75a07bfa2af --- /dev/null +++ b/src/tools/miri/tests/pass/both_borrows/unsafe_pinned.rs @@ -0,0 +1,16 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(unsafe_pinned)] + +use std::pin::UnsafePinned; + +fn mutate(x: &UnsafePinned) { + let ptr = x as *const _ as *mut i32; + unsafe { ptr.write(42) }; +} + +fn main() { + let x = UnsafePinned::new(0); + mutate(&x); + assert_eq!(x.into_inner(), 42); +}