Auto merge of rust-lang#120248 - WaffleLapkin:bonk-ptr-object-casts, r=<try>

Make casts of pointers to trait objects stricter

This is an attempt to `fix` rust-lang#120222 and rust-lang#120217.

cc `@oli-obk` `@compiler-errors` `@lcnr`

Author: bors

compiler/rustc_hir_typeck/src/cast.rs

@@ -42,7 +42,7 @@ use rustc_middle::ty::cast::{CastKind, CastTy};
 use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::{self, Ty, TypeAndMut, TypeVisitableExt, VariantDef};
 use rustc_session::lint;
-use rustc_span::def_id::{DefId, LOCAL_CRATE};
+use rustc_span::def_id::LOCAL_CRATE;
 use rustc_span::symbol::sym;
 use rustc_span::Span;
 use rustc_trait_selection::infer::InferCtxtExt;
@@ -72,7 +72,7 @@ enum PointerKind<'tcx> {
     /// No metadata attached, ie pointer to sized type or foreign type
     Thin,
     /// A trait object
-    VTable(Option<DefId>),
+    VTable(Option<ty::Binder<'tcx, ty::ExistentialTraitRef<'tcx>>>),
     /// Slice
     Length,
     /// The unsize info of this projection or opaque type
@@ -100,7 +100,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         Ok(match *t.kind() {
             ty::Slice(_) | ty::Str => Some(PointerKind::Length),
-            ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
+            ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal())),
             ty::Adt(def, args) if def.is_struct() => match def.non_enum_variant().tail_opt() {
                 None => Some(PointerKind::Thin),
                 Some(f) => {
@@ -611,14 +611,39 @@ impl<'a, 'tcx> CastCheck<'tcx> {
         } else {
             match self.try_coercion_cast(fcx) {
                 Ok(()) => {
-                    if self.expr_ty.is_unsafe_ptr() && self.cast_ty.is_unsafe_ptr() {
-                        // When casting a raw pointer to another raw pointer, we cannot convert the cast into
-                        // a coercion because the pointee types might only differ in regions, which HIR typeck
-                        // cannot distinguish. This would cause us to erroneously discard a cast which will
-                        // lead to a borrowck error like #113257.
-                        // We still did a coercion above to unify inference variables for `ptr as _` casts.
-                        // This does cause us to miss some trivial casts in the trival cast lint.
-                        debug!(" -> PointerCast");
+                    if let ty::RawPtr(src_pointee) = self.expr_ty.kind()
+                        && let ty::RawPtr(tgt_pointee) = self.cast_ty.kind()
+                    {
+                        if let Ok(Some(src_kind)) = fcx.pointer_kind(src_pointee.ty, self.expr_span)
+                            && let Ok(Some(tgt_kind)) =
+                                fcx.pointer_kind(tgt_pointee.ty, self.cast_span)
+                        {
+                            match (src_kind, tgt_kind) {
+                                // When casting a raw pointer to another raw pointer, we cannot convert the cast into
+                                // a coercion because the pointee types might only differ in regions, which HIR typeck
+                                // cannot distinguish. This would cause us to erroneously discard a cast which will
+                                // lead to a borrowck error like #113257.
+                                // We still did a coercion above to unify inference variables for `ptr as _` casts.
+                                // This does cause us to miss some trivial casts in the trivial cast lint.
+                                (PointerKind::Thin, PointerKind::Thin)
+                                | (PointerKind::Length, PointerKind::Length) => {
+                                    debug!(" -> PointerCast");
+                                }
+
+                                // If we are not casting pointers to sized types or slice-ish DSTs
+                                // (handled above), we need to make a coercion cast. This prevents
+                                // casts like `*const dyn Trait<'a> -> *const dyn Trait<'b>` which
+                                // are unsound.
+                                //
+                                // See <https://github.com/rust-lang/rust/issues/120217>
+                                (_, _) => {
+                                    debug!(" -> CoercionCast");
+                                    fcx.typeck_results
+                                        .borrow_mut()
+                                        .set_coercion_cast(self.expr.hir_id.local_id);
+                                }
+                            }
+                        }
                     } else {
                         self.trivial_cast_lint(fcx);
                         debug!(" -> CoercionCast");

library/std/src/thread/mod.rs

@@ -164,6 +164,7 @@ use crate::ffi::{CStr, CString};
 use crate::fmt;
 use crate::io;
 use crate::marker::PhantomData;
+use crate::mem::transmute;
 use crate::mem::{self, forget};
 use crate::num::NonZeroU64;
 use crate::num::NonZeroUsize;
@@ -545,10 +546,11 @@ impl Builder {
             scope_data.increment_num_running_threads();
         }
 
-        let main = Box::new(main);
+        let main: Box<dyn FnOnce() + '_> = Box::new(main);
         // SAFETY: dynamic size and alignment of the Box remain the same. See below for why the
         // lifetime change is justified.
-        let main = unsafe { Box::from_raw(Box::into_raw(main) as *mut (dyn FnOnce() + 'static)) };
+        let main =
+            unsafe { transmute::<Box<dyn FnOnce() + '_>, Box<dyn FnOnce() + 'static>>(main) };
 
         Ok(JoinInner {
             // SAFETY:

tests/ui/cast/cast-rfc0401-vtable-kinds.rs

@@ -16,13 +16,6 @@ impl<T> Foo<T> for () {}
 impl Foo<u32> for u32 { fn foo(&self, _: u32) -> u32 { self+43 } }
 impl Bar for () {}
 
-unsafe fn round_trip_and_call<'a>(t: *const (dyn Foo<u32>+'a)) -> u32 {
-    let foo_e : *const dyn Foo<u16> = t as *const _;
-    let r_1 = foo_e as *mut dyn Foo<u32>;
-
-    (&*r_1).foo(0)
-}
-
 #[repr(C)]
 struct FooS<T:?Sized>(T);
 #[repr(C)]
@@ -38,11 +31,6 @@ fn tuple_i32_to_u32<T:?Sized>(u: *const (i32, T)) -> *const (u32, T) {
 
 
 fn main() {
-    let x = 4u32;
-    let y : &dyn Foo<u32> = &x;
-    let fl = unsafe { round_trip_and_call(y as *const dyn Foo<u32>) };
-    assert_eq!(fl, (43+4));
-
     let s = FooS([0,1,2]);
     let u: &FooS<[u32]> = &s;
     let u: *const FooS<[u32]> = u;

tests/ui/cast/ptr-to-ptr-different-regions.rs

@@ -11,12 +11,6 @@ fn extend_lifetime_very_very_safely<'a>(v: *const Foo<'a>) -> *const Foo<'static
     v as *const Foo<'static>
 }
 
-trait Trait {}
-
-fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
-    ptr as _
-}
-
 fn main() {
     let unit = ();
     let foo = Foo { a: &unit };

tests/ui/cast/ptr-to-trait-obj-different-args.rs

@@ -0,0 +1,30 @@
+// check-fail
+//
+// issue: <https://github.com/rust-lang/rust/issues/120222>
+
+
+trait A {}
+impl<T> A for T {}
+trait B {}
+impl<T> B for T {}
+
+trait Trait<G> {}
+struct X;
+impl<T> Trait<X> for T {}
+struct Y;
+impl<T> Trait<Y> for T {}
+
+fn main() {
+    let a: *const dyn A = &();
+    let b: *const dyn B = a as _; //~ error: casting `*const dyn A` as `*const dyn B` is invalid
+
+    let x: *const dyn Trait<X> = &();
+    let y: *const dyn Trait<Y> = x as _; //~ error: casting `*const dyn Trait<X>` as `*const dyn Trait<Y>` is invalid
+
+    _ = (b, y);
+}
+
+fn generic<T>(x: *const dyn Trait<X>, t: *const dyn Trait<T>) {
+    let _: *const dyn Trait<T> = x as _; //~ error: casting `*const (dyn Trait<X> + 'static)` as `*const dyn Trait<T>` is invalid
+    let _: *const dyn Trait<X> = t as _; //~ error: casting `*const (dyn Trait<T> + 'static)` as `*const dyn Trait<X>` is invalid
+}

tests/ui/cast/ptr-to-trait-obj-different-args.stderr

@@ -0,0 +1,35 @@
+error[E0606]: casting `*const dyn A` as `*const dyn B` is invalid
+  --> $DIR/ptr-to-trait-obj-different-args.rs:19:27
+   |
+LL |     let b: *const dyn B = a as _;
+   |                           ^^^^^^
+   |
+   = note: vtable kinds may not match
+
+error[E0606]: casting `*const dyn Trait<X>` as `*const dyn Trait<Y>` is invalid
+  --> $DIR/ptr-to-trait-obj-different-args.rs:22:34
+   |
+LL |     let y: *const dyn Trait<Y> = x as _;
+   |                                  ^^^^^^
+   |
+   = note: vtable kinds may not match
+
+error[E0606]: casting `*const (dyn Trait<X> + 'static)` as `*const dyn Trait<T>` is invalid
+  --> $DIR/ptr-to-trait-obj-different-args.rs:28:34
+   |
+LL |     let _: *const dyn Trait<T> = x as _;
+   |                                  ^^^^^^
+   |
+   = note: vtable kinds may not match
+
+error[E0606]: casting `*const (dyn Trait<T> + 'static)` as `*const dyn Trait<X>` is invalid
+  --> $DIR/ptr-to-trait-obj-different-args.rs:29:34
+   |
+LL |     let _: *const dyn Trait<X> = t as _;
+   |                                  ^^^^^^
+   |
+   = note: vtable kinds may not match
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0606`.

tests/ui/cast/ptr-to-trait-obj-different-regions.rs

@@ -0,0 +1,31 @@
+// check-fail
+//
+// issue: <https://github.com/rust-lang/rust/issues/120217>
+
+#![feature(arbitrary_self_types)]
+
+trait Static<'a> {
+    fn proof(self: *const Self, s: &'a str) -> &'static str;
+}
+
+fn bad_cast<'a>(x: *const dyn Static<'static>) -> *const dyn Static<'a> {
+    x as _ //~ error: lifetime may not live long enough
+}
+
+impl Static<'static> for () {
+    fn proof(self: *const Self, s: &'static str) -> &'static str {
+        s
+    }
+}
+
+fn extend_lifetime(s: &str) -> &'static str {
+    bad_cast(&()).proof(s)
+}
+
+fn main() {
+    let s = String::from("Hello World");
+    let slice = extend_lifetime(&s);
+    println!("Now it exists: {slice}");
+    drop(s);
+    println!("Now it’s gone: {slice}");
+}

tests/ui/cast/ptr-to-trait-obj-different-regions.stderr

@@ -0,0 +1,10 @@
+error: lifetime may not live long enough
+  --> $DIR/ptr-to-trait-obj-different-regions.rs:12:5
+   |
+LL | fn bad_cast<'a>(x: *const dyn Static<'static>) -> *const dyn Static<'a> {
+   |             -- lifetime `'a` defined here
+LL |     x as _
+   |     ^^^^^^ returning this value requires that `'a` must outlive `'static`
+
+error: aborting due to 1 previous error
+