Skip to content

UnsafePinned: also include the effects of UnsafeCell #140638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 11 additions & 19 deletions library/core/src/pin/unsafe_pinned.rs
Original file line number Diff line number Diff line change
@@ -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<T>` is not guaranteed to be a unique pointer.
/// This also subsumes the effects of `UnsafeCell`, i.e., `&UnsafePinned<T>` 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
Expand All @@ -25,30 +28,19 @@ use crate::{fmt, ptr};
#[repr(transparent)]
#[unstable(feature = "unsafe_pinned", issue = "125735")]
pub struct UnsafePinned<T: ?Sized> {
value: T,
value: UnsafeCell<T>,
}

// Override the manual `!Sync` in `UnsafeCell`.
#[unstable(feature = "unsafe_pinned", issue = "125735")]
unsafe impl<T: ?Sized + Sync> Sync for UnsafePinned<T> {}

/// 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<T: ?Sized> !Unpin for UnsafePinned<T> {}

/// 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<T: Copy> Copy for UnsafePinned<T> {}

#[unstable(feature = "unsafe_pinned", issue = "125735")]
impl<T: Copy> Clone for UnsafePinned<T> {
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
Expand All @@ -63,7 +55,7 @@ impl<T> UnsafePinned<T> {
#[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`.
Expand All @@ -72,7 +64,7 @@ impl<T> UnsafePinned<T> {
#[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()
}
}

Expand Down
25 changes: 25 additions & 0 deletions src/tools/miri/tests/fail/async-shared-mutable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! FIXME: This test should pass! However, `async fn` does not yet use `UnsafePinned`.
//! This is a regression test for <https://github.com/rust-lang/rust/issues/137750>:
//! `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);
}
43 changes: 43 additions & 0 deletions src/tools/miri/tests/fail/async-shared-mutable.stack.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
error: Undefined Behavior: attempting a write access using <TAG> 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 <TAG> 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: <TAG> 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: <TAG> 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 `<std::future::PollFn<{closure@tests/fail/async-shared-mutable.rs:LL:CC}> 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

47 changes: 47 additions & 0 deletions src/tools/miri/tests/fail/async-shared-mutable.tree.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: Undefined Behavior: write access through <TAG> at ALLOC[OFFSET] is forbidden
--> tests/fail/async-shared-mutable.rs:LL:CC
|
LL | *x = 1;
| ^^^^^^ write access through <TAG> 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 <TAG> has state Frozen which forbids this child write access
help: the accessed tag <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 <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 <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 `<std::future::PollFn<{closure@tests/fail/async-shared-mutable.rs:LL:CC}> 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

16 changes: 16 additions & 0 deletions src/tools/miri/tests/pass/both_borrows/unsafe_pinned.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@revisions: stack tree
//@[tree]compile-flags: -Zmiri-tree-borrows
#![feature(unsafe_pinned)]

use std::pin::UnsafePinned;

fn mutate(x: &UnsafePinned<i32>) {
let ptr = x as *const _ as *mut i32;
Copy link
Contributor

@Sky9x Sky9x May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this go through UnsafeCell/UnsafePinned::get? https://doc.rust-lang.org/nightly/std/cell/struct.UnsafeCell.html#memory-layout

also maybe UnsafePinned::get should return a *mut T now...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're testing the aliasing model here so we want to see everything that happens in the code. This is not a doctest. :)

unsafe { ptr.write(42) };
}

fn main() {
let x = UnsafePinned::new(0);
mutate(&x);
assert_eq!(x.into_inner(), 42);
}
Loading