Auto merge of #145608 - Darksonn:derefmut-pin-fix, r=lcnr

Prevent downstream `impl DerefMut for Pin<LocalType>`

The safety requirements for [`PinCoerceUnsized`](https://doc.rust-lang.org/stable/std/pin/trait.PinCoerceUnsized.html) are essentially that the type does not have a malicious `Deref` or `DerefMut` impl. However, the `Pin` type is fundamental, so the end-user can provide their own implementation of `DerefMut` for `Pin<&SomeLocalType>`, so it's possible for `Pin` to have a malicious `DerefMut` impl. This unsoundness is known as rust-lang/rust#85099.

Unfortunately, this means that the implementation of `PinCoerceUnsized` for `Pin` is currently unsound. To fix that, modify the impl so that it becomes impossible for downstream crates to provide their own implementation of `DerefMut` for `Pin` by abusing a hidden struct that is not fundamental.

This PR is a breaking change, but it fixes rust-lang/rust#85099. The PR supersedes rust-lang/rust#144896.

r? lcnr

compiler/rustc_span/src/symbol.rs

@@ -309,6 +309,7 @@ symbols! {
         PathBuf,
         Pending,
         PinCoerceUnsized,
+        PinDerefMutHelper,
         Pointer,
         Poll,
         ProcMacro,

compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

@@ -3476,6 +3476,24 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                     // can do about it. As far as they are concerned, `?` is compiler magic.
                     return;
                 }
+                if tcx.is_diagnostic_item(sym::PinDerefMutHelper, parent_def_id) {
+                    let parent_predicate =
+                        self.resolve_vars_if_possible(data.derived.parent_trait_pred);
+
+                    // Skip PinDerefMutHelper in suggestions, but still show downstream suggestions.
+                    ensure_sufficient_stack(|| {
+                        self.note_obligation_cause_code(
+                            body_id,
+                            err,
+                            parent_predicate,
+                            param_env,
+                            &data.derived.parent_code,
+                            obligated_types,
+                            seen_requirements,
+                        )
+                    });
+                    return;
+                }
                 let self_ty_str =
                     tcx.short_string(parent_trait_pred.skip_binder().self_ty(), err.long_ty_path());
                 let trait_name = tcx.short_string(

library/core/src/pin.rs

@@ -1689,9 +1689,89 @@ impl<Ptr: [const] Deref> const 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
+    /// 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
+    /// `#[fundamental]`, so stdlib promises to never implement traits for `Pin` that it does not
+    /// implement today.
+    ///
+    /// However, this is problematic. Downstream crates could implement `DerefMut` for
+    /// `Pin<&LocalType>`, and they could do so maliciously. To prevent this, the implementation for
+    /// `Pin` delegates to this helper module. Since `helper::Pin` is not `#[fundamental]`, the
+    /// orphan rules assume that stdlib might implement `helper::DerefMut` for `helper::Pin<&_>` in
+    /// the future. Because of this, downstream crates can no longer provide an implementation of
+    /// `DerefMut` for `Pin<&_>`, as it might overlap with a trait impl that, according to the
+    /// orphan rules, the stdlib could introduce without a breaking change in a future release.
+    ///
+    /// See <https://github.com/rust-lang/rust/issues/85099> for the issue this fixes.
+    #[repr(transparent)]
+    #[unstable(feature = "pin_derefmut_internals", issue = "none")]
+    #[allow(missing_debug_implementations)]
+    pub struct PinHelper<Ptr> {
+        pointer: Ptr,
+    }
+
+    #[unstable(feature = "pin_derefmut_internals", issue = "none")]
+    #[rustc_const_unstable(feature = "const_convert", issue = "143773")]
+    #[rustc_diagnostic_item = "PinDerefMutHelper"]
+    pub const trait PinDerefMutHelper {
+        type Target: ?Sized;
+        fn deref_mut(&mut self) -> &mut Self::Target;
+    }
+
+    #[unstable(feature = "pin_derefmut_internals", issue = "none")]
+    #[rustc_const_unstable(feature = "const_convert", issue = "143773")]
+    impl<Ptr: [const] super::DerefMut> const PinDerefMutHelper for PinHelper<Ptr>
+    where
+        Ptr::Target: crate::marker::Unpin,
+    {
+        type Target = Ptr::Target;
+
+        #[inline(always)]
+        fn deref_mut(&mut self) -> &mut Ptr::Target {
+            &mut self.pointer
+        }
+    }
+}
+
 #[stable(feature = "pin", since = "1.33.0")]
 #[rustc_const_unstable(feature = "const_convert", issue = "143773")]
-impl<Ptr: [const] DerefMut<Target: Unpin>> const DerefMut for Pin<Ptr> {
+#[cfg(not(doc))]
+impl<Ptr> const DerefMut for Pin<Ptr>
+where
+    Ptr: [const] Deref,
+    helper::PinHelper<Ptr>: [const] helper::PinDerefMutHelper<Target = Self::Target>,
+{
+    #[inline]
+    fn deref_mut(&mut self) -> &mut Ptr::Target {
+        // SAFETY: Pin and PinHelper have the same layout, so this is equivalent to
+        // `&mut self.pointer` which is safe because `Target: Unpin`.
+        helper::PinDerefMutHelper::deref_mut(unsafe {
+            &mut *(self as *mut Pin<Ptr> as *mut helper::PinHelper<Ptr>)
+        })
+    }
+}
+
+/// The `Target` type is restricted to `Unpin` types as it's not safe to obtain a mutable reference
+/// to a pinned value.
+///
+/// For soundness reasons, implementations of `DerefMut` for `Pin<T>` are rejected even when `T` is
+/// a local type not covered by this impl block. (Since `Pin` is [fundamental], such implementations
+/// would normally be possible.)
+///
+/// [fundamental]: ../../reference/items/implementations.html#r-items.impl.trait.fundamental
+#[stable(feature = "pin", since = "1.33.0")]
+#[rustc_const_unstable(feature = "const_convert", issue = "143773")]
+#[cfg(doc)]
+impl<Ptr> const DerefMut for Pin<Ptr>
+where
+    Ptr: [const] DerefMut,
+    <Ptr as Deref>::Target: Unpin,
+{
     fn deref_mut(&mut self) -> &mut Ptr::Target {
         Pin::get_mut(Pin::as_mut(self))
     }

tests/mir-opt/inline_coroutine_body.run2-{closure#0}.Inline.panic-abort.diff

@@ -63,30 +63,28 @@
 +                             let mut _44: &mut std::future::Ready<()>;
 +                             let mut _45: &mut std::pin::Pin<&mut std::future::Ready<()>>;
 +                             scope 14 (inlined <Pin<&mut std::future::Ready<()>> as DerefMut>::deref_mut) {
-+                                 scope 15 (inlined Pin::<&mut std::future::Ready<()>>::as_mut) {
++                                 let mut _46: *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>>;
-+                                     let mut _46: &mut &mut std::future::Ready<()>;
++                                 let mut _47: *mut std::pin::Pin<&mut std::future::Ready<()>>;
-+                                     scope 16 (inlined Pin::<&mut std::future::Ready<()>>::new_unchecked) {
++                                 scope 15 (inlined <pin::helper::PinHelper<&mut std::future::Ready<()>> as pin::helper::PinDerefMutHelper>::deref_mut) {
-+                                     }
++                                     let mut _48: &mut &mut std::future::Ready<()>;
-+                                     scope 18 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
++                                     scope 16 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
 +                                     }
 +                                 }
-+                                 scope 17 (inlined Pin::<&mut std::future::Ready<()>>::get_mut) {
++                             }
++                             scope 17 (inlined Option::<()>::take) {
++                                 let mut _49: std::option::Option<()>;
++                                 scope 18 (inlined std::mem::replace::<Option<()>>) {
++                                     scope 19 {
 +                                     }
 +                                 }
-+                             scope 19 (inlined Option::<()>::take) {
++                             }
-+                                 let mut _47: std::option::Option<()>;
++                             scope 20 (inlined #[track_caller] Option::<()>::expect) {
-+                                 scope 20 (inlined std::mem::replace::<Option<()>>) {
++                                 let mut _50: isize;
++                                 let mut _51: !;
 +                                 scope 21 {
 +                                 }
 +                             }
 +                         }
-+                             scope 22 (inlined #[track_caller] Option::<()>::expect) {
-+                                 let mut _48: isize;
-+                                 let mut _49: !;
-+                                 scope 23 {
-+                                 }
-+                             }
-+                         }
 +                     }
 +                     scope 10 (inlined ready::<()>) {
 +                         let mut _40: std::option::Option<()>;
@@ -217,18 +215,23 @@
 +         _22 = &mut (*_23);
 +         StorageDead(_24);
 +         StorageLive(_44);
-+         StorageLive(_49);
++         StorageLive(_46);
++         StorageLive(_51);
 +         StorageLive(_41);
 +         StorageLive(_42);
-+         _44 = copy (_19.0: &mut std::future::Ready<()>);
 +         StorageLive(_47);
-+         _47 = Option::<()>::None;
++         _47 = &raw mut _19;
-+         _42 = copy ((*_44).0: std::option::Option<()>);
++         _46 = copy _47 as *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>> (PtrToPtr);
-+         ((*_44).0: std::option::Option<()>) = copy _47;
 +         StorageDead(_47);
-+         StorageLive(_48);
++         _44 = copy ((*_46).0: &mut std::future::Ready<()>);
-+         _48 = discriminant(_42);
++         StorageLive(_49);
-+         switchInt(move _48) -> [0: bb11, 1: bb12, otherwise: bb5];
++         _49 = Option::<()>::None;
++         _42 = copy ((*_44).0: std::option::Option<()>);
++         ((*_44).0: std::option::Option<()>) = copy _49;
++         StorageDead(_49);
++         StorageLive(_50);
++         _50 = discriminant(_42);
++         switchInt(move _50) -> [0: bb11, 1: bb12, otherwise: bb5];
       }
 + 
 +     bb5: {
@@ -291,16 +294,17 @@
 +     }
 + 
 +     bb11: {
-+         _49 = option::expect_failed(const "`Ready` polled after completion") -> unwind unreachable;
++         _51 = option::expect_failed(const "`Ready` polled after completion") -> unwind unreachable;
 +     }
 + 
 +     bb12: {
 +         _41 = move ((_42 as Some).0: ());
-+         StorageDead(_48);
++         StorageDead(_50);
 +         StorageDead(_42);
 +         _18 = Poll::<()>::Ready(move _41);
 +         StorageDead(_41);
-+         StorageDead(_49);
++         StorageDead(_51);
++         StorageDead(_46);
 +         StorageDead(_44);
 +         StorageDead(_22);
 +         StorageDead(_19);

tests/mir-opt/inline_coroutine_body.run2-{closure#0}.Inline.panic-unwind.diff

@@ -65,30 +65,28 @@
 +                             let mut _46: &mut std::future::Ready<()>;
 +                             let mut _47: &mut std::pin::Pin<&mut std::future::Ready<()>>;
 +                             scope 14 (inlined <Pin<&mut std::future::Ready<()>> as DerefMut>::deref_mut) {
-+                                 scope 15 (inlined Pin::<&mut std::future::Ready<()>>::as_mut) {
++                                 let mut _48: *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>>;
-+                                     let mut _48: &mut &mut std::future::Ready<()>;
++                                 let mut _49: *mut std::pin::Pin<&mut std::future::Ready<()>>;
-+                                     scope 16 (inlined Pin::<&mut std::future::Ready<()>>::new_unchecked) {
++                                 scope 15 (inlined <pin::helper::PinHelper<&mut std::future::Ready<()>> as pin::helper::PinDerefMutHelper>::deref_mut) {
-+                                     }
++                                     let mut _50: &mut &mut std::future::Ready<()>;
-+                                     scope 18 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
++                                     scope 16 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
 +                                     }
 +                                 }
-+                                 scope 17 (inlined Pin::<&mut std::future::Ready<()>>::get_mut) {
++                             }
++                             scope 17 (inlined Option::<()>::take) {
++                                 let mut _51: std::option::Option<()>;
++                                 scope 18 (inlined std::mem::replace::<Option<()>>) {
++                                     scope 19 {
 +                                     }
 +                                 }
-+                             scope 19 (inlined Option::<()>::take) {
++                             }
-+                                 let mut _49: std::option::Option<()>;
++                             scope 20 (inlined #[track_caller] Option::<()>::expect) {
-+                                 scope 20 (inlined std::mem::replace::<Option<()>>) {
++                                 let mut _52: isize;
++                                 let mut _53: !;
 +                                 scope 21 {
 +                                 }
 +                             }
 +                         }
-+                             scope 22 (inlined #[track_caller] Option::<()>::expect) {
-+                                 let mut _50: isize;
-+                                 let mut _51: !;
-+                                 scope 23 {
-+                                 }
-+                             }
-+                         }
 +                     }
 +                     scope 10 (inlined ready::<()>) {
 +                         let mut _42: std::option::Option<()>;
@@ -234,18 +232,23 @@
 +         _22 = &mut (*_23);
 +         StorageDead(_24);
 +         StorageLive(_46);
-+         StorageLive(_51);
++         StorageLive(_48);
++         StorageLive(_53);
 +         StorageLive(_43);
 +         StorageLive(_44);
-+         _46 = copy (_19.0: &mut std::future::Ready<()>);
 +         StorageLive(_49);
-+         _49 = Option::<()>::None;
++         _49 = &raw mut _19;
-+         _44 = copy ((*_46).0: std::option::Option<()>);
++         _48 = copy _49 as *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>> (PtrToPtr);
-+         ((*_46).0: std::option::Option<()>) = copy _49;
 +         StorageDead(_49);
-+         StorageLive(_50);
++         _46 = copy ((*_48).0: &mut std::future::Ready<()>);
-+         _50 = discriminant(_44);
++         StorageLive(_51);
-+         switchInt(move _50) -> [0: bb16, 1: bb17, otherwise: bb7];
++         _51 = Option::<()>::None;
++         _44 = copy ((*_46).0: std::option::Option<()>);
++         ((*_46).0: std::option::Option<()>) = copy _51;
++         StorageDead(_51);
++         StorageLive(_52);
++         _52 = discriminant(_44);
++         switchInt(move _52) -> [0: bb16, 1: bb17, otherwise: bb7];
       }
   
 -     bb6 (cleanup): {
@@ -332,16 +335,17 @@
 +     }
 + 
 +     bb16: {
-+         _51 = option::expect_failed(const "`Ready` polled after completion") -> bb11;
++         _53 = option::expect_failed(const "`Ready` polled after completion") -> bb11;
 +     }
 + 
 +     bb17: {
 +         _43 = move ((_44 as Some).0: ());
-+         StorageDead(_50);
++         StorageDead(_52);
 +         StorageDead(_44);
 +         _18 = Poll::<()>::Ready(move _43);
 +         StorageDead(_43);
-+         StorageDead(_51);
++         StorageDead(_53);
++         StorageDead(_48);
 +         StorageDead(_46);
 +         StorageDead(_22);
 +         StorageDead(_19);

tests/ui/deref/pin-impl-deref.rs

@@ -22,7 +22,7 @@ impl MyPinType {
 fn impl_deref_mut(_: impl DerefMut) {}
 fn unpin_impl_ref(r_unpin: Pin<&MyUnpinType>) {
     impl_deref_mut(r_unpin)
-    //~^ ERROR: the trait bound `Pin<&MyUnpinType>: DerefMut` is not satisfied
+    //~^ ERROR: the trait bound `&MyUnpinType: DerefMut` is not satisfied
 }
 fn unpin_impl_mut(r_unpin: Pin<&mut MyUnpinType>) {
     impl_deref_mut(r_unpin)
@@ -30,7 +30,7 @@ fn unpin_impl_mut(r_unpin: Pin<&mut MyUnpinType>) {
 fn pin_impl_ref(r_pin: Pin<&MyPinType>) {
     impl_deref_mut(r_pin)
     //~^ ERROR: `PhantomPinned` cannot be unpinned
-    //~| ERROR: the trait bound `Pin<&MyPinType>: DerefMut` is not satisfied
+    //~| ERROR: the trait bound `&MyPinType: DerefMut` is not satisfied
 }
 fn pin_impl_mut(r_pin: Pin<&mut MyPinType>) {
     impl_deref_mut(r_pin)

tests/ui/deref/pin-impl-deref.stderr

@@ -1,40 +1,34 @@
-error[E0277]: the trait bound `Pin<&MyUnpinType>: DerefMut` is not satisfied
+error[E0277]: the trait bound `&MyUnpinType: DerefMut` is not satisfied
   --> $DIR/pin-impl-deref.rs:24:20
    |
 LL |     impl_deref_mut(r_unpin)
-   |     -------------- ^^^^^^^ the trait `DerefMut` is not implemented for `Pin<&MyUnpinType>`
+   |     -------------- ^^^^^^^ the trait `DerefMut` is not implemented for `&MyUnpinType`
    |     |
    |     required by a bound introduced by this call
    |
+   = note: `DerefMut` is implemented for `&mut MyUnpinType`, but not for `&MyUnpinType`
    = note: required for `Pin<&MyUnpinType>` to implement `DerefMut`
 note: required by a bound in `impl_deref_mut`
   --> $DIR/pin-impl-deref.rs:22:27
    |
 LL | fn impl_deref_mut(_: impl DerefMut) {}
    |                           ^^^^^^^^ required by this bound in `impl_deref_mut`
-help: consider mutably borrowing here
-   |
-LL |     impl_deref_mut(&mut r_unpin)
-   |                    ++++
 
-error[E0277]: the trait bound `Pin<&MyPinType>: DerefMut` is not satisfied
+error[E0277]: the trait bound `&MyPinType: DerefMut` is not satisfied
   --> $DIR/pin-impl-deref.rs:31:20
    |
 LL |     impl_deref_mut(r_pin)
-   |     -------------- ^^^^^ the trait `DerefMut` is not implemented for `Pin<&MyPinType>`
+   |     -------------- ^^^^^ the trait `DerefMut` is not implemented for `&MyPinType`
    |     |
    |     required by a bound introduced by this call
    |
+   = note: `DerefMut` is implemented for `&mut MyPinType`, but not for `&MyPinType`
    = note: required for `Pin<&MyPinType>` to implement `DerefMut`
 note: required by a bound in `impl_deref_mut`
   --> $DIR/pin-impl-deref.rs:22:27
    |
 LL | fn impl_deref_mut(_: impl DerefMut) {}
    |                           ^^^^^^^^ required by this bound in `impl_deref_mut`
-help: consider mutably borrowing here
-   |
-LL |     impl_deref_mut(&mut r_pin)
-   |                    ++++
 
 error[E0277]: `PhantomPinned` cannot be unpinned
   --> $DIR/pin-impl-deref.rs:31:20

tests/ui/typeck/pin-unsound-issue-85099-derefmut.rs

@@ -1,5 +1,4 @@
-//@ check-pass
+//@ check-fail
-//@ known-bug: #85099
 
 // Should fail. Can coerce `Pin<T>` into `Pin<U>` where
 // `T: Deref<Target: Unpin>` and `U: Deref<Target: !Unpin>`, using the
@@ -43,6 +42,7 @@ impl<'a, Fut: Future<Output = ()>> SomeTrait<'a, Fut> for Fut {
 }
 
 impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
+//~^ ERROR: conflicting implementations of trait `DerefMut`
     fn deref_mut<'c>(
         self: &'c mut Pin<&'b dyn SomeTrait<'a, Fut>>,
     ) -> &'c mut (dyn SomeTrait<'a, Fut> + 'b) {

tests/ui/typeck/pin-unsound-issue-85099-derefmut.stderr

@@ -0,0 +1,14 @@
+error[E0119]: conflicting implementations of trait `DerefMut` for type `Pin<&dyn SomeTrait<'_, _>>`
+  --> $DIR/pin-unsound-issue-85099-derefmut.rs:44:1
+   |
+LL | impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: conflicting implementation in crate `core`:
+           - impl<Ptr> DerefMut for Pin<Ptr>
+             where <pin::helper::PinHelper<Ptr> as pin::helper::PinDerefMutHelper>::Target == <Pin<Ptr> as Deref>::Target, Ptr: Deref, pin::helper::PinHelper<Ptr>: pin::helper::PinDerefMutHelper, pin::helper::PinHelper<Ptr>: ?Sized;
+   = note: upstream crates may add a new impl of trait `std::pin::helper::PinDerefMutHelper` for type `std::pin::helper::PinHelper<&dyn SomeTrait<'_, _>>` in future versions
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0119`.