Skip to content

Introduce a PinSafePointer trait that generalizes PinCoerceUnsized #156935

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.

Sign up for GitHub

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

Merged
rust-bors merged 1 commit into from
Aug 9, 2026
Merged

Introduce a PinSafePointer trait that generalizes PinCoerceUnsized #156935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 21 additions & 2 deletions library/alloc/src/boxed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ use core::ops::{
};
#[cfg(not(no_global_oom_handling))]
use core::ops::{Residual, Try};
use core::pin::{Pin, PinCoerceUnsized};
use core::pin::{Pin, PinSafePointer};
use core::ptr::{self, NonNull, Unique};
use core::task::{Context, Poll};

Expand Down Expand Up @@ -2365,8 +2365,27 @@ impl<Args: Tuple, F: AsyncFn<Args> + ?Sized, A: Allocator> AsyncFn<Args> for Box
#[unstable(feature = "coerce_unsized", issue = "18598")]
impl<T: ?Sized + Unsize<U>, U: ?Sized, A: Allocator> CoerceUnsized<Box<U, A>> for Box<T, A> {}

// A pointer can only be pin safe if it does not implement certain safe traits
// maliciously. Since `Box` is fundamental, downstream crates may be able to
// implement those traits for `Box<LocalType>`, so we must carefully check that
// this is not a problem for each trait.
//
// The `Box` type always implements `Deref` and `DerefMut`, so despite being
// fundamental, downstream crates cannot implement these traits for
// `Box<LocalType>`.
//
// Conversely, downstream crates are able to implement `Clone`, `Debug`, and
// `Display` for `Box<LocalType>` as long as `LocalType` does not implement
// said trait. However, the `Box<T>` type does not treat the existence of an
// `&Box<T>` as evidence that the `T` is not pinned, so this is not
// problematic.
//
// Finally, even if downstream crates provide their own implementation of
// `Clone` for `Box<LocalType>`, it is not problematic for the cloned box to be
// wrapped in `Pin`, since the same conversion could have been carried out
// safely as `Box::pin((*p).clone())`.
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<T: ?Sized, A: Allocator> PinCoerceUnsized for Box<T, A> {}
unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for Box<T, A> {}

@Darksonn Darksonn May 25, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added A: 'static bounds to these impls since you generally cannot pin values in memory from a non-static allocator.

View changes since the review

@theemathas theemathas May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can create a Pin<Box<T, A>> for a non-'static allocator by first pinning using MyAllocator<'static>, and then use subtyping to turn that into MyAllocator<'a>.

@Darksonn Darksonn May 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But that's kind of awkward, then. In that scenario, we have a non-static Pin<Box<T, A<'a>>>, we clone the inner box to create a new Box<T, A<'a>>, and then we wrap the clone in Pin to get a second Pin<Box<T, A<'a>>>. But you can normally only wrap a box in Pin if the allocator is 'static, so the argument I'm using for Box doesn't quite apply anymore.

@theemathas theemathas May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh dear.......

@theemathas theemathas May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This feels unsound, but I haven't figured out yet how.

@theemathas theemathas May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's unsound (even without using the PinCoerceUnsized impl). I filed this as #157089.


// It is quite crucial that we only allow the `Global` allocator here.
// Handling arbitrary custom allocators (which can affect the `Box` layout heavily!)
Expand Down
13 changes: 10 additions & 3 deletions library/alloc/src/rc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ use core::ops::{Residual, Try};
use core::panic::{RefUnwindSafe, UnwindSafe};
#[cfg(not(no_global_oom_handling))]
use core::pin::Pin;
use core::pin::PinCoerceUnsized;
use core::pin::PinSafePointer;
use core::ptr::{self, NonNull, drop_in_place};
#[cfg(not(no_global_oom_handling))]
use core::slice::from_raw_parts_mut;
Expand Down Expand Up @@ -2446,12 +2446,19 @@ impl<T: ?Sized, A: Allocator> Deref for Rc<T, A> {
}
}

// The API of this pointer type enforces that if the `T` is pinned, then *all*
// clones of this `Rc<T>` are wrapped as `Pin<Rc<T>>`. Since an `&Rc<T>` could
// be used to obtain an `Rc<T>` that is not wrapped in `Pin` (and later used
// with `Rc::get_mut`), this means that this type treats `&Rc<T>` as evidence
// that the `T` is not pinned. The implementations of various traits are written
// accordingly. Since this type is not fundamental, downstream crates cannot
// provide malicious implementations of any of the traits relevant for `Pin`.

@RalfJung RalfJung May 27, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this relies on something about the orphan rule, right? That makes me uneasy.^^
An explicit impl !DerefMut for Rc would be a lot better IMO.

View changes since the review

@Darksonn Darksonn May 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, by the orphan rule downstream crates cannot implement DerefMut for Rc<_> under any scenario.

@RalfJung RalfJung May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Those are the current orphan rules. How do we know they will never be relaxed?

The only promise the orphan rules are making is "there will never be more than one impl for the same trait + type". I don't think we should rely on any property that goes beyond this, and I don't see how the argument here follows from that property.

@Darksonn Darksonn May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can add a negative impl to simplify the reasoning for Rc, but I think there is no way out for the #145608 case unless someone comes up with an entirely different solution. It's simply unsound to add untrusted code to the coherence domain of core because it could implement DerefMut for Pin<LocalType> maliciously.

@Darksonn Darksonn May 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, even if I add the impl !DerefMut for Rc, what about this one?

impl<T: ?Sized + fmt::Debug, A: Allocator> fmt::Debug for Rc<T, A>

I do not think we can get out of this without orphan-rules based reasoning.

@RalfJung RalfJung May 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, we need new language features to support this properly I think. Maybe if we could have a "dual" negative impl?

impl<T: ?Sized + !fmt::Debug, A> !fmt::Debug for Rc<T, A>

Since every concrete type either impls Debug or not, we can always use one or the other impl as justification.

#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<T: ?Sized, A: Allocator> PinCoerceUnsized for Rc<T, A> {}
unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for Rc<T, A> {}

//#[unstable(feature = "unique_rc_arc", issue = "112566")]
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<T: ?Sized, A: Allocator> PinCoerceUnsized for UniqueRc<T, A> {}
unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for UniqueRc<T, A> {}

#[unstable(feature = "deref_pure_trait", issue = "87121")]
unsafe impl<T: ?Sized, A: Allocator> DerefPure for Rc<T, A> {}
Expand Down
14 changes: 11 additions & 3 deletions library/alloc/src/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use core::ops::{CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn, Lega
#[cfg(not(no_global_oom_handling))]
use core::ops::{Residual, Try};
use core::panic::{RefUnwindSafe, UnwindSafe};
use core::pin::{Pin, PinCoerceUnsized};
use core::pin::{Pin, PinSafePointer};
use core::ptr::{self, NonNull};
#[cfg(not(no_global_oom_handling))]
use core::slice::from_raw_parts_mut;
Expand Down Expand Up @@ -2453,8 +2453,16 @@ impl<T: ?Sized, A: Allocator> Deref for Arc<T, A> {
}
}

// The API of this pointer type enforces that if the `T` is pinned, then *all*
// clones of this `Arc<T>` are wrapped as `Pin<Arc<T>>`. Since an `&Arc<T>`
// could be used to obtain an `Arc<T>` that is not wrapped in `Pin` (and later
// used with `Arc::get_mut`), this means that this type treats `&Arc<T>` as
// evidence that the `T` is not pinned. The implementations of various traits
// are written accordingly. Since this type is not fundamental, downstream
// crates cannot provide malicious implementations of any of the traits relevant
// for `Pin`.
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<T: ?Sized, A: Allocator> PinCoerceUnsized for Arc<T, A> {}
unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for Arc<T, A> {}

#[unstable(feature = "deref_pure_trait", issue = "87121")]
unsafe impl<T: ?Sized, A: Allocator> DerefPure for Arc<T, A> {}
Expand Down Expand Up @@ -4914,7 +4922,7 @@ impl<T: ?Sized, A: Allocator> Deref for UniqueArc<T, A> {

// #[unstable(feature = "unique_rc_arc", issue = "112566")]
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<T: ?Sized> PinCoerceUnsized for UniqueArc<T> {}
unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for UniqueArc<T, A> {}

#[unstable(feature = "unique_rc_arc", issue = "112566")]
impl<T: ?Sized, A: Allocator> DerefMut for UniqueArc<T, A> {
Expand Down
12 changes: 9 additions & 3 deletions library/core/src/cell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ use crate::marker::{Destruct, PhantomData, Unsize};
use crate::mem::{self, ManuallyDrop};
use crate::ops::{self, CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn};
use crate::panic::const_panic;
use crate::pin::PinCoerceUnsized;
use crate::pin::PinSafePointer;
use crate::ptr::{self, NonNull};
use crate::range;

Expand Down Expand Up @@ -2710,8 +2710,14 @@ fn assert_coerce_unsized(
let _: RefCell<&dyn Send> = d;
}

// The implementations of Deref/DerefMut are not malicious, so we can allow the
// user to perform unsizing coercions with `Pin<Ref<'b, T>>` pointers if they
// can manage to create one.
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<'b, T: ?Sized> PinCoerceUnsized for Ref<'b, T> {}
unsafe impl<'b, T: ?Sized> PinSafePointer for Ref<'b, T> {}

// The implementations of Deref/DerefMut are not malicious, so we can allow the
// user to perform unsizing coercions with `Pin<RefMut<'b, T>>` pointers if they
// can manage to create one.
#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
unsafe impl<'b, T: ?Sized> PinCoerceUnsized for RefMut<'b, T> {}
unsafe impl<'b, T: ?Sized> PinSafePointer for RefMut<'b, T> {}
184 changes: 150 additions & 34 deletions library/core/src/pin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,9 +1080,8 @@ pub use self::unsafe_pinned::UnsafePinned;
/// [subtle-details]: self#subtle-details-and-the-drop-guarantee "pin subtle details"
/// [`unsafe`]: ../../std/keyword.unsafe.html "keyword unsafe"
//
// Note: the `Clone` derive below causes unsoundness as it's possible to implement
// `Clone` for mutable references.
// See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311> for more details.
// Note: the `Clone` derive below is sound because either `Ptr: PinSafePointer`
// or the pointee is `Unpin`.
#[stable(feature = "pin", since = "1.33.0")]
#[lang = "pin"]
#[fundamental]
Expand All @@ -1097,7 +1096,8 @@ pub struct Pin<Ptr> {
// issues. `&self.pointer` should not be accessible to untrusted trait
// implementations.
//
// See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73> for more details.
// See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73> and the
// `PinSafePointer` trait for more details.

#[stable(feature = "pin_trait_impls", since = "1.41.0")]
impl<Ptr: Deref, Q: Deref> PartialEq<Pin<Q>> for Pin<Ptr>
Expand Down Expand Up @@ -1230,11 +1230,15 @@ impl<Ptr: Deref> Pin<Ptr> {
/// points to is pinned, that is a violation of the API contract and may lead to undefined
/// behavior in later (even safe) operations.
///
/// By using this method, you are also making a promise about the [`Deref`],
/// [`DerefMut`], and [`Drop`] implementations of `Ptr`, if they exist. Most importantly, they
/// must not move out of their `self` arguments: `Pin::as_mut` and `Pin::as_ref`
/// will call `DerefMut::deref_mut` and `Deref::deref` *on the pointer type `Ptr`*
/// and expect these methods to uphold the pinning invariants.
/// By using this method, you are also making a promise about several trait
/// implementations of `Ptr` itself, if they exist. Most importantly, they

@Mark-Simulacrum Mark-Simulacrum Aug 2, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm wondering if there's some formulation of "if they exist" like "if they exist now or could exist in the future"...

View changes since the review

@Darksonn Darksonn Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I could add an:

By using this method, you are also making a promise about several trait implementations of Ptr itself, if they exist. For fundamental types, this applies even if a downstream crate implements the trait.

@Mark-Simulacrum Mark-Simulacrum Aug 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That seems like a good idea to me.

/// must not move out of their `self` arguments: `Pin::as_mut` and
/// `Pin::as_ref` will call `DerefMut::deref_mut` and `Deref::deref` *on the
/// pointer type `Ptr`* and expect these methods to uphold the pinning
/// invariants. These requirements are specified in more detail on the
/// [`PinSafePointer`] trait, and `Ptr` must abide by the safety
/// requirements of that trait.
///
/// Moreover, by calling this method you promise that the reference `Ptr`
/// dereferences to will not be moved out of again; in particular, it
/// must not be possible to obtain a `&mut Ptr::Target` and then
Expand Down Expand Up @@ -1690,7 +1694,7 @@ const impl<Ptr: [const] Deref> Deref for Pin<Ptr> {
mod helper {
/// Helper that prevents downstream crates from implementing `DerefMut` for `Pin`.
///
/// The `Pin` type implements the unsafe trait `PinCoerceUnsized`, which essentially requires
/// The `Pin` type implements the unsafe trait `PinSafePointer`, which essentially requires
/// that the type does not have a malicious `Deref` or `DerefMut` impl. However, without this
/// helper module, downstream crates are able to write `impl DerefMut for Pin<LocalType>` as
/// long as it does not overlap with the impl provided by stdlib. This is because `Pin` is
Expand Down Expand Up @@ -1781,6 +1785,10 @@ unsafe impl<Ptr: DerefPure> DerefPure for Pin<Ptr> {}
#[unstable(feature = "legacy_receiver_trait", issue = "none")]
impl<Ptr: LegacyReceiver> LegacyReceiver for Pin<Ptr> {}

// The following implementations allow untrusted trait implementations to access
// `&self.pointer`, which is only sound because these traits are mentioned in
// the safety requirements of `PinSafePointer`.

#[stable(feature = "pin", since = "1.33.0")]
impl<Ptr: fmt::Debug> fmt::Debug for Pin<Ptr> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down Expand Up @@ -1810,49 +1818,157 @@ impl<Ptr: fmt::Pointer> fmt::Pointer for Pin<Ptr> {
#[stable(feature = "pin", since = "1.33.0")]
impl<Ptr, U> CoerceUnsized<Pin<U>> for Pin<Ptr>
where
Ptr: CoerceUnsized<U> + PinCoerceUnsized,
U: PinCoerceUnsized,
Ptr: CoerceUnsized<U> + PinSafePointer,
U: PinSafePointer,
{
}

#[stable(feature = "pin", since = "1.33.0")]
impl<Ptr, U> DispatchFromDyn<Pin<U>> for Pin<Ptr>
where
Ptr: DispatchFromDyn<U> + PinCoerceUnsized,
U: PinCoerceUnsized,
Ptr: DispatchFromDyn<U> + PinSafePointer,
U: PinSafePointer,
{
}

#[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")]
/// Trait that indicates that this is a pointer or a wrapper for one, where
/// unsizing can be performed on the pointee when it is pinned.
/// Trait that indicates that this is a pointer that does not misbehave when
/// combined with [`Pin`].
///
/// Note that for backwards compatibility reasons, it is possible to create a
/// [`Pin<P>`] for pointer types `P` that do not implement this trait. However,
/// this can only be done safely if `<P as Deref>::Target` implements `Unpin`,
/// which means that pinning has no effect.
///
/// # Safety
///
/// Given a pointer of this type, the concrete type returned by its
/// `deref` method and (if it implements `DerefMut`) its `deref_mut` method
/// must be the same type and must not change without a modification.
/// The following operations are not considered modifications:
///
/// * Moving the pointer.
/// * Performing unsizing coercions on the pointer.
/// * Performing dynamic dispatch with the pointer.
/// * Calling `deref` or `deref_mut` on the pointer.
///
/// The concrete type of a trait object is the type that the vtable corresponds
/// to. The concrete type of a slice is an array of the same element type and
/// the length specified in the metadata. The concrete type of a sized type
/// is the type itself.
pub unsafe trait PinCoerceUnsized: Deref {}
/// Types that implement this trait must not provide "malicious" implementations
/// of any safe traits used by [`Pin`].
///
/// ## The pointer must always reference the same object
///
/// Calls to [`deref`]/[`deref_mut`] on the same `Pin<P>` instance must always
/// refer to the same object. That is, the address returned by these methods
/// must not change. This applies even if the pointer type is moved.
Comment on lines +1850 to +1852

@RalfJung RalfJung May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this is still doing the thing where it talks about a DerefMut impl but DerefMut is not a supertrait of this trait. What are the valid ways in which an unsafe impl PinSafePointer can satisfy this requirement?

  • There's a DerefMut for the same set of types in the same crate: that's easy, we can just check the condition.
  • There's a negative DerefMut for the same set of types in the same crate: that's easy, no impl can exist so the condition is trivially satisfied.
  • Neither of these hold true. Now we... uh, look at the orphan rule and hope for the best? I am not sure that's a good idea, this is exactly what has bitten us in the past...

The same concern applies to the requirements on Clone, the formatting traits, and any others that I missed. (IMO the docs should call out more explicitly: If this type also implements Foo, then blah. If it also implements Bar, then blah. etc)

View changes since the review

@Darksonn Darksonn May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree that it's best to avoid this kind of safety requirement, but I think we have no choice. Luckily we do not need to worry about dyn PinSafePointer.

This PR is adding comments to every impl of PinSafePointer explaining why the various trait implementations are compatible or cannot exist for the particular type in question. Please see the comments for various ways we can make these arguments.

@RalfJung RalfJung May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah in std we can do that because we have negative impls. But our users can't really do that... and the moment one wants to impl PinSafePointer for MyType<T> where MyType<T>: DerefMut for some but not all T, I don't see any way to make this work.

@Darksonn Darksonn May 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Negative impls are only required for fundamental types, so I'm not too concerned about that.

@Darksonn Darksonn May 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With regards to types where MyType<T>: DerefMut for some but not all T, we do in fact have one instance of exactly that. Please see #145608 for the trick to make that sound.

@RalfJung RalfJung May 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have seen that PR and ran away screaming.^^

I really hope the reasoning is sound, but I do not have a coherent proof in my head -- it relies too much on orphan rule details.

Comment on lines +1851 to +1852

@theemathas theemathas May 30, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the address returned by these methods must not change

This seems to imply that it's OK for DerefMut to return different fat references with the same address but different metadata (and therefore different underlying concrete types).

View changes since the review

@Darksonn Darksonn May 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True, the part about concrete types should not be behind an "if the smart pointer can participate in unsizing coercions" and should just be unconditional.

@Mark-Simulacrum Mark-Simulacrum Aug 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this still pending an edit? I admit I don't quite understand why we have such a strong condition, but if we think it's reasonable then I don't have direct objections. My main concern with all these rules is that I am not sure how to audit for them effectively (especially as we seem to keep changing them as we find more potential problems).

@Darksonn Darksonn Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated now.

///
/// These coercions must also not change the underlying concrete type. Here, the
/// concrete type of a trait object is the type that the vtable corresponds to.
/// The concrete type of a slice is an array of the same element type and the
/// length specified in the metadata. The concrete type of a sized type is the
/// type itself.
///
/// As an example, after unsizing coercing a pinned pointer, `deref_mut` must
/// not return a `#[repr(transparent)]` wrapper around the value it referenced
/// before being unsized, even if the address is unchanged.

@Mark-Simulacrum Mark-Simulacrum Aug 2, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this accurate? Or is changing to repr(transparent) wrapper fine as long as the impls are all the same on both, i.e., not actually malicious?

View changes since the review

@Darksonn Darksonn Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can choose to disallow obscure cases even if they are sound.

I mean, if the #[repr(transparent)] wrapper behaves identically to the original object, then I suppose it's ok. But I really don't see any reason to allow this kind of thing. In general, I'd expect that if I have a MyBox<MyStruct> and I coerce that to MyBox<dyn MyTrait>, then I would be able to deref it to &dyn MyTrait and invoke Any::downcast to get back a &MyStruct. But if MyBox somehow returns a &dyn MyTrait that actually corresponds to Wrapper<MyStruct>, then that downcast would fail. This kind of type change on coercion seems extremely surprising.

///
/// ## The pointer must not move its pointee
///
/// The [`deref_mut`] method and the pointer type's destructor are called with a
/// `&mut self` receiver, but they must behave as-if it was a `self: Pin<&mut
/// Self>` receiver. That is, they must not move out of the underlying value.
///
/// As an example, `deref_mut` must not invoke `swap` on the inner value.
///
/// ## Shared access to the pointer
///
/// If this pointer type uses `&P` references as evidence that this value is not
/// pinned, then it must not treat the `&self` argument passed to [`Clone`] or
/// the formatting traits ([`fmt::Debug`], [`fmt::Display`], [`fmt::Pointer`])
/// as such evidence.
///
/// As an example, given a `Pin<Arc<T>>` there is no way to obtain an `&Arc<T>`
/// (note that `Deref` just gives a `&T`). Because of this, the [`Arc`] type can
/// assume that an `&Arc<T>` value can only exist if the `T` is not pinned,
/// which justifies the soundness of the [`Arc::get_mut`] method.
Comment on lines +1874 to +1882

@RalfJung RalfJung May 27, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't realize get_mut becomes even more subtle when pinning gets involved. Impressive.

View changes since the review

///
/// ## Cloning pinned pointers
///
/// When a [`Pin<P>`] is cloned, the `P` pointer value returned by `clone` is
/// passed to [`Pin::new_unchecked`]. The implementation of [`Clone`] must
/// return a value such that this is sound.
///
/// For example, when a `Pin<&T>` is cloned, the resulting `&T` points at the
/// same value. The value is known to be pinned since a `Pin<&T>` to it exists,
/// so it is safe to wrap the `&T` returned by `clone` in `Pin`.
///
/// [`deref`]: Deref::deref
/// [`deref_mut`]: DerefMut::deref_mut
/// [`clone`]: Clone::clone
/// [`Arc`]: ../../std/sync/struct.Arc.html "Arc"
/// [`Arc::get_mut`]: ../../std/sync/struct.Arc.html#method.get_mut "Arc::get_mut"
pub unsafe trait PinSafePointer: Deref + Sized {}

@Darksonn Darksonn May 25, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a Sized bound here due to #156920 (comment). This doesn't actually affect any use-cases since Pin<P> already requires P: Sized, but seemed like a good idea nonetheless.

View changes since the review


// A pointer can only be pin safe if it does not implement certain safe traits
// maliciously. Since `&T` is fundamental, downstream crates may be able to
// implement those traits for `&LocalType`, so we must carefully check that
// this is not a problem for each trait.
//
// The `&T` type always implements [`Deref`] and [`Clone`], so despite being
// fundamental, downstream crates cannot implement these traits for
// `&LocalType`.
//
// The `&T` type has a negative blanket implementations for [`DerefMut`], so
// downstream crates cannot implement `DerefMut` for `&LocalType`.
//
// Conversely, downstream crates are able to implement `Debug` and `Display` for
// `&LocalType` as long as `LocalType` does not implement said trait. However,
// the existence of an `&&T` cannot be treated as evidence that the `T` is not
// pinned, so this is not problematic.
#[stable(feature = "pin", since = "1.33.0")]
unsafe impl<'a, T: ?Sized> PinCoerceUnsized for &'a T {}
unsafe impl<'a, T: ?Sized> PinSafePointer for &'a T {}

// A pointer can only be pin safe if it does not implement certain safe traits
// maliciously. Since `&mut T` is fundamental, downstream crates may be able to
// implement those traits for `&mut LocalType`, so we must carefully check that
// this is not a problem for each trait.
//
// The `&mut T` type always implements [`Deref`] and [`DerefMut`], so despite
// being fundamental, downstream crates cannot implement these traits for `&mut
// LocalType`.
//
// The `&mut T` type has a negative blanket implementations for [`Clone`], so
// downstream crates cannot implement `Clone` for `&mut LocalType`.
//
// Conversely, downstream crates are able to implement `Debug` and `Display`
// for `&mut LocalType` as long as `LocalType` does not implement said trait.
// However, the existence of an `&&mut T` cannot be treated as evidence that the
// `T` is not pinned, so this is not problematic.
#[stable(feature = "pin", since = "1.33.0")]
unsafe impl<'a, T: ?Sized> PinCoerceUnsized for &'a mut T {}
unsafe impl<'a, T: ?Sized> PinSafePointer for &'a mut T {}

// A pointer can only be pin safe if it does not implement certain safe traits
// maliciously. `Pin` implements these traits by forwarding to `P`, which also
// asserts that these implementations are not malicious, so the implementations
// provided by `core` are ok. However, since `Pin<T>` is fundamental,
// downstream crates may be able to implement those traits for `Pin<LocalType>`
// directly, so we must carefully check that if a downstream crate can
// implement these traits for `Pin<LocalType>`, then this does not lead to any
// problems for the `Pin<Pin<LocalType>>` type.
//
// The `Pin<P>` type only implements `Deref` when `P: Deref`, so downstream
// crates can implement `Deref` for `Pin<LocalType>` in cases where `LocalType:
// !Deref`. However, as `Deref` is a super-trait for `PinSafePointer`, we do
// not assert that `Pin<P>` is pin safe in that scenario.
//
// The `Pin<P>` type only implements `DerefMut` when `P: DerefMut` and
// `P::Target: Unpin`, so normally downstream crates would be able to provide
// an implementation of `DerefMut` for `Pin<LocalType>` when `LocalType` does
// not satisfy those conditions. However, a special hack is used to prevent
// such downstream implementations, so this is not a problem. See
// [#145608](https://github.com/rust-lang/rust/pull/145608) for details.
//
// Conversely, downstream crates are able to implement `Clone`, `Debug` and
// `Display` for `Pin<LocalType>` as long as `LocalType` does not implement
// said trait. However, the existence of an `&Pin<P>` cannot be treated as
// evidence that the value is not pinned, so this is not problematic.
//
// Furthermore, in the case of `Clone`, cloning a `Pin<Pin<P>>` will utilize
// `Pin::new_unchecked` to convert from `Pin<P>` to `Pin<Pin<P>>`. However,
// given that the implementation of `Clone` returned a `Pin<P>`, we know that
// the target value is pinned, so this conversion is okay even if `Clone` was
// implemented for `Pin<P>` by a downstream crate.
#[stable(feature = "pin", since = "1.33.0")]
unsafe impl<T: PinCoerceUnsized> PinCoerceUnsized for Pin<T> {}
unsafe impl<P: PinSafePointer> PinSafePointer for Pin<P> {}

/// Constructs a <code>[Pin]<[&mut] T></code>, by pinning a `value: T` locally.
///
Expand Down
Loading