Skip to content
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

Relax Allocator bounds into pin-safe trait #94114

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Relax Allocator bounds into pin-safe trait
Because `Allocator` requires that returned memory blocks remain valid
until the instance and all clones are dropped, some types of allocators
cannot fulfill the safety requirements for the trait. This change
relaxes the safety requirements of `Allocator` to blocks remaining valid
until the allocator becomes unreachable and moves the bound into a
separate `PinSafeAllocator` trait.

It also relaxes some overly cautious bounds on the `Unpin` impls for
`Box` since unpinning a box doesn't require any special consideration.
  • Loading branch information
djkoloski committed Jul 24, 2022
commit fe3d3fc85c86283ba590bb3f0e3d2828ac05ec4e
7 changes: 7 additions & 0 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ extern "Rust" {
/// to the allocator registered with the `#[global_allocator]` attribute
/// if there is one, or the `std` crate’s default.
///
/// This type is always guaranteed to implement [`PinSafeAllocator`].
///
/// Note: while this type is unstable, the functionality it provides can be
/// accessed through the [free functions in `alloc`](self#functions).
#[unstable(feature = "allocator_api", issue = "32838")]
Expand Down Expand Up @@ -314,6 +316,11 @@ unsafe impl Allocator for Global {
}
}

// SAFETY: memory blocks allocated by `Global` are not invalidated when `Global` is dropped.
#[unstable(feature = "allocator_api", issue = "32838")]
#[cfg(not(test))]
unsafe impl PinSafeAllocator for Global {}

/// The allocator for unique pointers.
#[cfg(all(not(no_global_oom_handling), not(test)))]
#[lang = "exchange_malloc"]
Expand Down
41 changes: 20 additions & 21 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ use core::task::{Context, Poll};

#[cfg(not(no_global_oom_handling))]
use crate::alloc::{handle_alloc_error, WriteCloneIntoRaw};
use crate::alloc::{AllocError, Allocator, Global, Layout};
use crate::alloc::{AllocError, Allocator, Global, Layout, PinSafeAllocator};
#[cfg(not(no_global_oom_handling))]
use crate::borrow::Cow;
use crate::raw_vec::RawVec;
Expand Down Expand Up @@ -1123,8 +1123,12 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
// recognized as "releasing" the unique pointer to permit aliased raw accesses,
// so all raw pointer methods have to go through `Box::leak`. Turning *that* to a raw pointer
// behaves correctly.
let alloc = unsafe { ptr::read(&b.1) };
(Unique::from(Box::leak(b)), alloc)
let manually_drop = mem::ManuallyDrop::new(b);
// SAFETY: unique ownership of the memory block moves into `ptr`
let ptr = unsafe { &mut *manually_drop.0.as_ptr() };
// SAFETY: moving the allocator will not invalidate `ptr`
let alloc = unsafe { ptr::read(&manually_drop.1) };
(Unique::from(ptr), alloc)
}

/// Returns a reference to the underlying allocator.
Expand Down Expand Up @@ -1179,9 +1183,13 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
#[inline]
pub const fn leak<'a>(b: Self) -> &'a mut T
where
A: 'a,
A: ~const PinSafeAllocator,
{
unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() }
let (ptr, alloc) = Box::into_unique(b);
mem::forget(alloc);
// SAFETY: ptr will remain valid for any lifetime since `alloc` is never
// dropped
unsafe { &mut *ptr.as_ptr() }
}

/// Converts a `Box<T>` into a `Pin<Box<T>>`. If `T` does not implement [`Unpin`], then
Expand Down Expand Up @@ -1218,7 +1226,7 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
#[rustc_const_unstable(feature = "const_box", issue = "92521")]
pub const fn into_pin(boxed: Self) -> Pin<Self>
where
A: 'static,
A: ~const PinSafeAllocator,
{
// It's not possible to move or replace the insides of a `Pin<Box<T>>`
// when `T: !Unpin`, so it's safe to pin it directly without any
Expand Down Expand Up @@ -1454,9 +1462,9 @@ impl<T> From<T> for Box<T> {

#[stable(feature = "pin", since = "1.33.0")]
#[rustc_const_unstable(feature = "const_box", issue = "92521")]
impl<T: ?Sized, A: Allocator> const From<Box<T, A>> for Pin<Box<T, A>>
impl<T: ?Sized, A> const From<Box<T, A>> for Pin<Box<T, A>>
where
A: 'static,
A: ~const PinSafeAllocator,
{
/// Converts a `Box<T>` into a `Pin<Box<T>>`. If `T` does not implement [`Unpin`], then
/// `*boxed` will be pinned in memory and unable to be moved.
Expand Down Expand Up @@ -2033,13 +2041,10 @@ impl<T: ?Sized, A: Allocator> AsMut<T> for Box<T, A> {
*/
#[stable(feature = "pin", since = "1.33.0")]
#[rustc_const_unstable(feature = "const_box", issue = "92521")]
impl<T: ?Sized, A: Allocator> const Unpin for Box<T, A> where A: 'static {}
impl<T: ?Sized, A: Allocator> const Unpin for Box<T, A> {}

#[unstable(feature = "generator_trait", issue = "43122")]
impl<G: ?Sized + Generator<R> + Unpin, R, A: Allocator> Generator<R> for Box<G, A>
where
A: 'static,
{
impl<G: ?Sized + Generator<R> + Unpin, R, A: Allocator> Generator<R> for Box<G, A> {
type Yield = G::Yield;
type Return = G::Return;

Expand All @@ -2049,10 +2054,7 @@ where
}

#[unstable(feature = "generator_trait", issue = "43122")]
impl<G: ?Sized + Generator<R>, R, A: Allocator> Generator<R> for Pin<Box<G, A>>
where
A: 'static,
{
impl<G: ?Sized + Generator<R>, R, A: Allocator> Generator<R> for Pin<Box<G, A>> {
type Yield = G::Yield;
type Return = G::Return;

Expand All @@ -2062,10 +2064,7 @@ where
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<F: ?Sized + Future + Unpin, A: Allocator> Future for Box<F, A>
where
A: 'static,
{
impl<F: ?Sized + Future + Unpin, A: Allocator> Future for Box<F, A> {
type Output = F::Output;

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ impl<T> Rc<T> {
#[stable(feature = "pin", since = "1.33.0")]
#[must_use]
pub fn pin(value: T) -> Pin<Rc<T>> {
// SAFETY: Global is a pin-safe allocator.
unsafe { Pin::new_unchecked(Rc::new(value)) }
}

Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,13 +530,15 @@ impl<T> Arc<T> {
#[stable(feature = "pin", since = "1.33.0")]
#[must_use]
pub fn pin(data: T) -> Pin<Arc<T>> {
// SAFETY: Global is a pin-safe allocator.
unsafe { Pin::new_unchecked(Arc::new(data)) }
}

/// Constructs a new `Pin<Arc<T>>`, return an error if allocation fails.
#[unstable(feature = "allocator_api", issue = "32838")]
#[inline]
pub fn try_pin(data: T) -> Result<Pin<Arc<T>>, AllocError> {
// SAFETY: Global is a pin-safe allocator.
unsafe { Ok(Pin::new_unchecked(Arc::try_new(data)?)) }
}

Expand Down
5 changes: 4 additions & 1 deletion library/alloc/tests/boxed.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use core::alloc::{AllocError, Allocator, Layout};
use core::alloc::{AllocError, Allocator, Layout, PinSafeAllocator};
use core::cell::Cell;
use core::mem::MaybeUninit;
use core::ptr::NonNull;
Expand Down Expand Up @@ -151,6 +151,9 @@ unsafe impl const Allocator for ConstAllocator {
}
}

// SAFETY: Memory allocated by `ConstAllocator` is never invalidated.
unsafe impl const PinSafeAllocator for ConstAllocator {}

#[test]
fn const_box() {
const VALUE: u32 = {
Expand Down
28 changes: 24 additions & 4 deletions library/core/src/alloc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,14 @@ impl fmt::Display for AllocError {
/// # Safety
///
/// * Memory blocks returned from an allocator must point to valid memory and retain their validity
/// until the instance and all of its clones are dropped,
/// until the instance and all of its clones are dropped, forgotten, or otherwise rendered
/// inaccessible. The validity of memory block is tied directly to the validity of the allocator
/// group that it is allocated from.
///
/// * cloning or moving the allocator must not invalidate memory blocks returned from this
/// allocator. A cloned allocator must behave like the same allocator, and
/// * Cloning or moving the allocator must not invalidate memory blocks returned from this
/// allocator. A cloned allocator must behave like the same allocator.
///
/// * any pointer to a memory block which is [*currently allocated*] may be passed to any other
/// * Any pointer to a memory block which is [*currently allocated*] may be passed to any other
/// method of the allocator.
///
/// [*currently allocated*]: #currently-allocated-memory
Expand Down Expand Up @@ -408,3 +410,21 @@ where
unsafe { (**self).shrink(ptr, old_layout, new_layout) }
}
}

/// An [`Allocator`] which returns memory blocks that can safely be pinned.
///
/// Unlike `Allocator`, `PinSafeAllocator` guarantees that forgetting an instance will cause any
/// allocated memory to remain valid indefinitely.
///
/// # Safety
///
/// In addition to the safety guarantees of `Allocator`, memory blocks returned from a
/// `PinSafeAllocator` must retain their validity until the instance and all of its clones are
/// dropped.
#[unstable(feature = "allocator_api", issue = "32838")]
pub unsafe trait PinSafeAllocator: Allocator {}

#[unstable(feature = "allocator_api", issue = "32838")]
// SAFETY: Allocators that live forever never become unreachable, and so never invalidate their
// allocated memory blocks.
unsafe impl<A: Allocator + ?Sized> PinSafeAllocator for &'static A {}
1 change: 1 addition & 0 deletions src/test/ui/box/leak-alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ fn use_value(_: u32) {}
fn main() {
let alloc = Alloc {};
let boxed = Box::new_in(10, alloc.by_ref());
//~^ ERROR `alloc` does not live long enough
let theref = Box::leak(boxed);
drop(alloc);
//~^ ERROR cannot move out of `alloc` because it is borrowed
Expand Down
29 changes: 21 additions & 8 deletions src/test/ui/box/leak-alloc.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
error[E0597]: `alloc` does not live long enough
--> $DIR/leak-alloc.rs:24:33
|
LL | let boxed = Box::new_in(10, alloc.by_ref());
| ^^^^^^^^^^^^^^
| |
| borrowed value does not live long enough
| argument requires that `alloc` is borrowed for `'static`
...
LL | }
| - `alloc` dropped here while still borrowed

error[E0505]: cannot move out of `alloc` because it is borrowed
--> $DIR/leak-alloc.rs:26:10
--> $DIR/leak-alloc.rs:27:10
|
LL | let boxed = Box::new_in(10, alloc.by_ref());
| -------------- borrow of `alloc` occurs here
LL | let theref = Box::leak(boxed);
| --------------
| |
| borrow of `alloc` occurs here
| argument requires that `alloc` is borrowed for `'static`
...
LL | drop(alloc);
| ^^^^^ move out of `alloc` occurs here
LL |
LL | use_value(*theref)
| ------- borrow later used here

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0505`.
Some errors have detailed explanations: E0505, E0597.
For more information about an error, try `rustc --explain E0505`.
36 changes: 36 additions & 0 deletions src/test/ui/box/pin-safe-alloc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// run-pass
#![feature(allocator_api)]
#![feature(box_into_pin)]

use std::alloc::{AllocError, Allocator, Layout, PinSafeAllocator, System};
use std::ptr::NonNull;
use std::marker::PhantomPinned;
use std::boxed::Box;

struct Alloc {}

unsafe impl Allocator for Alloc {
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
System.allocate(layout)
}

unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
System.deallocate(ptr, layout)
}
}

unsafe impl PinSafeAllocator for Alloc {}

fn main() {
struct MyPinned {
_value: u32,
_pinned: PhantomPinned,
}

let value = MyPinned {
_value: 0,
_pinned: PhantomPinned,
};
let alloc = Alloc {};
let _ = Box::pin_in(value, alloc);
}
34 changes: 34 additions & 0 deletions src/test/ui/box/pin-unsafe-alloc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![feature(allocator_api)]
#![feature(box_into_pin)]

use std::alloc::{AllocError, Allocator, Layout, System};
use std::ptr::NonNull;
use std::marker::PhantomPinned;
use std::boxed::Box;

struct Alloc {}

unsafe impl Allocator for Alloc {
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
System.allocate(layout)
}

unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
System.deallocate(ptr, layout)
}
}

fn main() {
struct MyPinned {
_value: u32,
_pinned: PhantomPinned,
}

let value = MyPinned {
_value: 0,
_pinned: PhantomPinned,
};
let alloc = Alloc {};
let _ = Box::pin_in(value, alloc);
//~^ ERROR the trait bound `Alloc: PinSafeAllocator` is not satisfied
}
21 changes: 21 additions & 0 deletions src/test/ui/box/pin-unsafe-alloc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0277]: the trait bound `Alloc: PinSafeAllocator` is not satisfied
--> $DIR/pin-unsafe-alloc.rs:32:32
|
LL | let _ = Box::pin_in(value, alloc);
| ----------- ^^^^^ expected an implementor of trait `PinSafeAllocator`
| |
| required by a bound introduced by this call
|
note: required by a bound in `Box::<T, A>::pin_in`
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
|
LL | A: ~const Allocator + ~const PinSafeAllocator + ~const Drop + ~const Destruct,
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Box::<T, A>::pin_in`
help: consider borrowing here
|
LL | let _ = Box::pin_in(value, &alloc);
| +

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.