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. This is done by adding restrictions on casting pointers to trait objects. Currently the restriction is > When casting `*const X<dyn A>` -> `*const Y<dyn B>`, if `B` has a principal trait, `dyn A 'erased: Unsize<dyn B 'erased>` must hold This ensures that 1. Principal trait's generic arguments match (no `*const dyn Tr<A>` -> `*const dyn Tr<B>` casts, which are a problem for [rust-lang#120222](rust-lang#120222)) 2. Principal trait's lifetime arguments match (no `*const dyn Tr<'a>` -> `*const dyn Tr<'b>` casts, which are a problem for [rust-lang#120217](rust-lang#120217)) 3. No auto traits can be _added_ (this is a problem for arbitrary self types, see [this comment](rust-lang#120248 (comment))) Some notes: - We only care about the metadata/last field, so you can still cast `*const dyn T` to `*const WithHeader<dyn T>`, etc - The lifetime of the trait object itself (`dyn A 'lt`) is not checked, so you can still cast `*mut FnOnce() '_` to `*mut FnOnce() 'static`, etc - This feels fishy, but I couldn't come up with a reason it must be checked - The new checks are only done if `B` has a principal, so you can still do any kinds of cast, if the target only has auto traits - This is because auto traits are not enough to get unsoundness issues that this PR fixes - ...and so it makes sense to minimize breakage The plan is to, ~~once the checks are properly implemented~~, run crate to determine how much damage does this do. The diagnostics are currently not great, to say the least, but as far as I can tell this correctly fixes the issues. cc `@oli-obk` `@compiler-errors` `@lcnr`
rust-lang-ci · Feb 13, 2024 · 13cae60 · 13cae60
2 parents 74c3f5a 5f27951
commit 13cae60
Show file tree

Hide file tree

Showing 21 changed files with 495 additions and 70 deletions.
diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs
@@ -2358,7 2358,42 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
  let cast_ty_from = CastTy::from_ty(ty_from);
  let cast_ty_to = CastTy::from_ty(*ty);
  match (cast_ty_from, cast_ty_to) {
- (Some(CastTy::Ptr(_)), Some(CastTy::Ptr(_))) => (),
  (Some(CastTy::Ptr(src)), Some(CastTy::Ptr(dst))) => {
  let src_tail = tcx.struct_tail_without_normalization(src.ty);
  let dst_tail = tcx.struct_tail_without_normalization(dst.ty);
 
  if let ty::Dynamic(..) = src_tail.kind()
  && let ty::Dynamic(dst_tty, ..) = dst_tail.kind()
  && dst_tty.principal().is_some()
  {
  // Erase trait object lifetimes, to allow casts like `*mut dyn FnOnce()` -> `*mut dyn FnOnce() 'static`.
  let src_tail =
  erase_single_trait_object_lifetime(tcx, src_tail);
  let dst_tail =
  erase_single_trait_object_lifetime(tcx, dst_tail);
 
  let trait_ref = ty::TraitRef::from_lang_item(
  tcx,
  LangItem::Unsize,
  span,
  [src_tail, dst_tail],
  );
 
  self.prove_trait_ref(
  trait_ref,
  location.to_locations(),
  ConstraintCategory::Cast {
  unsize_to: Some(tcx.fold_regions(dst_tail, |r, _| {
  if let ty::ReVar(_) = r.kind() {
  tcx.lifetimes.re_erased
  } else {
  r
  }
  })),
  },
  );
  }
  }
  _ => {
  span_mirbug!(
  self,
@@ -2881,3 2916,15 @@ impl<'tcx> TypeOp<'tcx> for InstantiateOpaqueType<'tcx> {
  Ok(output)
  }
 }
 
 fn erase_single_trait_object_lifetime<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> {
  let &ty::Dynamic(tty, region, dyn_kind @ ty::Dyn) = ty.kind() else {
  bug!("expected trait object")
  };
 
  if region.is_erased() {
  return ty;
  }
 
  tcx.mk_ty_from_kind(ty::Dynamic(tty, tcx.lifetimes.re_erased, dyn_kind))
 }
diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs
@@ -32,17 32,17 @@ use super::FnCtxt;
 
 use crate::errors;
 use crate::type_error_struct;
-use hir::ExprKind;
 use rustc_errors::{codes::*, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
-use rustc_hir as hir;
 use rustc_hir::{self as hir, ExprKind, LangItem};
 use rustc_infer::traits::Obligation;
 use rustc_macros::{TypeFoldable, TypeVisitable};
 use rustc_middle::mir::Mutability;
 use rustc_middle::ty::adjustment::AllowTwoPhase;
 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(&'tcx ty::List<ty::Binder<'tcx, ty::ExistentialPredicate<'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)),
  ty::Adt(def, args) if def.is_struct() => match def.non_enum_variant().tail_opt() {
  None => Some(PointerKind::Thin),
  Some(f) => {
@@ -747,7 747,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
  Err(CastError::IllegalCast)
  }
 
- // ptr -> *
  // ptr -> ptr
  (Ptr(m_e), Ptr(m_c)) => self.check_ptr_ptr_cast(fcx, m_e, m_c), // ptr-ptr-cast
 
  // ptr-addr-cast
@@ -791,40 791,83 @@ impl<'a, 'tcx> CastCheck<'tcx> {
  fn check_ptr_ptr_cast(
  &self,
  fcx: &FnCtxt<'a, 'tcx>,
- m_expr: ty::TypeAndMut<'tcx>,
- m_cast: ty::TypeAndMut<'tcx>,
  m_src: ty::TypeAndMut<'tcx>,
  m_dst: ty::TypeAndMut<'tcx>,
  ) -> Result<CastKind, CastError> {
- debug!("check_ptr_ptr_cast m_expr={:?} m_cast={:?}", m_expr, m_cast);
  debug!("check_ptr_ptr_cast m_expr={:?} m_cast={:?}", m_src, m_dst);
  // ptr-ptr cast. vtables must match.
 
- let expr_kind = fcx.pointer_kind(m_expr.ty, self.span)?;
- let cast_kind = fcx.pointer_kind(m_cast.ty, self.span)?;
  let src_kind = fcx.tcx.erase_regions(fcx.pointer_kind(m_src.ty, self.span)?);
  let dst_kind = fcx.tcx.erase_regions(fcx.pointer_kind(m_dst.ty, self.span)?);
 
- let Some(cast_kind) = cast_kind else {
  match (src_kind, dst_kind) {
  // We can't cast if target pointer kind is unknown
- return Err(CastError::UnknownCastPtrKind);
- };
-
- // Cast to thin pointer is OK
- if cast_kind == PointerKind::Thin {
- return Ok(CastKind::PtrPtrCast);
- }
  (_, None) => Err(CastError::UnknownCastPtrKind),
  // Cast to thin pointer is OK
  (_, Some(PointerKind::Thin)) => Ok(CastKind::PtrPtrCast),
 
- let Some(expr_kind) = expr_kind else {
  // We can't cast to fat pointer if source pointer kind is unknown
- return Err(CastError::UnknownExprPtrKind);
- };
  (None, _) => Err(CastError::UnknownExprPtrKind),
 
  // thin -> fat? report invalid cast (don't complain about vtable kinds)
  (Some(PointerKind::Thin), _) => Err(CastError::SizedUnsizedCast),
 
  // trait object -> trait object? need to do additional checks
  (Some(PointerKind::VTable(src_tty)), Some(PointerKind::VTable(dst_tty))) => {
  match (src_tty.principal(), dst_tty.principal()) {
  // A<dyn Trait Auto> -> B<dyn Trait' Auto'>. need to make sure
  // - traits are the same & have the same generic arguments
  // - Auto' is a subset of Auto
  //
  // This is checked by checking `dyn Trait Auto 'erased: Unsize<dyn Trait' Auto' 'erased>`.
  (Some(_), Some(_)) => {
  let tcx = fcx.tcx;
 
  // We need to reconstruct trait object types.
  // `m_src` and `m_dst` won't work for us here because they will potentially
  // contain wrappers, which we do not care about.
  //
  // e.g. we want to allow `dyn T -> (dyn T,)`, etc.
  let src_obj = tcx.mk_ty_from_kind(ty::Dynamic(src_tty, tcx.lifetimes.re_erased, ty::Dyn));
  let dst_obj = tcx.mk_ty_from_kind(ty::Dynamic(dst_tty, tcx.lifetimes.re_erased, ty::Dyn));
 
  // `dyn Src: Unsize<dyn Dst>`
  let cause = fcx.misc(self.span);
  let obligation = Obligation::new(
  tcx,
  cause,
  fcx.param_env,
  ty::TraitRef::from_lang_item(
  tcx,
  LangItem::Unsize,
  self.span,
  [src_obj, dst_obj]
  )
  );
 
- // thin -> fat? report invalid cast (don't complain about vtable kinds)
- if expr_kind == PointerKind::Thin {
- return Err(CastError::SizedUnsizedCast);
- }
  fcx.register_predicate(obligation);
 
- // vtable kinds must match
- if fcx.tcx.erase_regions(cast_kind) == fcx.tcx.erase_regions(expr_kind) {
- Ok(CastKind::PtrPtrCast)
- } else {
- Err(CastError::DifferingKinds)
  // FIXME: ideally we'd maybe add a flag here, so that borrowck knows that
  // it needs to borrowck this ptr cast. this is made annoying by the
  // fact that `thir` does not have `CastKind` and mir restores it
  // from types.
  Ok(CastKind::PtrPtrCast)
  }
 
  // dyn Auto -> dyn Auto'? ok.
  (None, None)
  // dyn Trait -> dyn Auto? ok.
  | (Some(_), None)=> Ok(CastKind::PtrPtrCast),
 
  // dyn Auto -> dyn Trait? not ok.
  (None, Some(_)) => Err(CastError::DifferingKinds),
  }
  }
 
  // fat -> fat? metadata kinds must match
  (Some(src_kind), Some(dst_kind)) if src_kind == dst_kind => Ok(CastKind::PtrPtrCast),
 
  (_, _) => Err(CastError::DifferingKinds),
  }
  }
 

diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs
@@ -2220,7 2220,7 @@ impl dyn Error Send {
  let err: Box<dyn Error> = self;
  <dyn Error>::downcast(err).map_err(|s| unsafe {
  // Reapply the `Send` marker.
- Box::from_raw(Box::into_raw(s) as *mut (dyn Error Send))
  mem::transmute::<Box<dyn Error>, Box<dyn Error Send>>(s)
  })
  }
 }
@@ -2233,8 2233,8 @@ impl dyn Error Send Sync {
  pub fn downcast<T: Error 'static>(self: Box<Self>) -> Result<Box<T>, Box<Self>> {
  let err: Box<dyn Error> = self;
  <dyn Error>::downcast(err).map_err(|s| unsafe {
- // Reapply the `Send Sync` marker.
- Box::from_raw(Box::into_raw(s) as *mut (dyn Error Send Sync))
  // Reapply the `Send Sync` markers.
  mem::transmute::<Box<dyn Error>, Box<dyn Error Send Sync>>(s)
  })
  }
 }

diff --git a/tests/ui/cast/cast-rfc0401-vtable-kinds.rs b/tests/ui/cast/cast-rfc0401-vtable-kinds.rs
@@ -4,25 4,12 @@
 
 #![feature(unsized_tuple_coercion)]
 
-trait Foo<T> {
- fn foo(&self, _: T) -> u32 { 42 }
-}
-
 trait Bar { //~ WARN trait `Bar` is never used
  fn bar(&self) { println!("Bar!"); }
 }
 
-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 25,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;

diff --git a/tests/ui/cast/cast-rfc0401-vtable-kinds.stderr b/tests/ui/cast/cast-rfc0401-vtable-kinds.stderr
@@ -1,5 1,5 @@
 warning: trait `Bar` is never used
- --> $DIR/cast-rfc0401-vtable-kinds.rs:11:7
  --> $DIR/cast-rfc0401-vtable-kinds.rs:7:7
  |
 LL | trait Bar {
  | ^^^

diff --git a/tests/ui/cast/ptr-to-trait-obj-add-auto.rs b/tests/ui/cast/ptr-to-trait-obj-add-auto.rs
@@ -0,0 1,9 @@
 // check-fail
 
 trait Trait<'a> {}
 
 fn add_auto<'a>(x: *mut dyn Trait<'a>) -> *mut (dyn Trait<'a> Send) {
  x as _ //~ error: the trait bound `dyn Trait<'_>: Unsize<dyn Trait<'_> Send>` is not satisfied
 }
 
 fn main() {}
diff --git a/tests/ui/cast/ptr-to-trait-obj-add-auto.stderr b/tests/ui/cast/ptr-to-trait-obj-add-auto.stderr
@@ -0,0 1,11 @@
 error[E0277]: the trait bound `dyn Trait<'_>: Unsize<dyn Trait<'_> Send>` is not satisfied
  --> $DIR/ptr-to-trait-obj-add-auto.rs:6:5
  |
 LL | x as _
  | ^^^^^^ the trait `Unsize<dyn Trait<'_> Send>` is not implemented for `dyn Trait<'_>`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 
 error: aborting due to 1 previous error
 
 For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/cast/ptr-to-trait-obj-different-args.rs b/tests/ui/cast/ptr-to-trait-obj-different-args.rs
@@ -0,0 1,38 @@
 // 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: the trait bound `dyn A: Unsize<dyn B>` is not satisfied
 
  let x: *const dyn Trait<X> = &();
  let y: *const dyn Trait<Y> = x as _; //~ error: the trait bound `dyn Trait<X>: Unsize<dyn Trait<Y>>` is not satisfied
 
  _ = (b, y);
 }
 
 fn generic<T>(x: *const dyn Trait<X>, t: *const dyn Trait<T>) {
  let _: *const dyn Trait<T> = x as _; //~ error: the trait bound `dyn Trait<X>: Unsize<dyn Trait<T>>` is not satisfied
  let _: *const dyn Trait<X> = t as _; //~ error: the trait bound `dyn Trait<T>: Unsize<dyn Trait<X>>` is not satisfied
 }
 
 trait Assocked {
  type Assoc: ?Sized;
 }
 
 fn change_assoc(x: *mut dyn Assocked<Assoc = u8>) -> *mut dyn Assocked<Assoc = u32> {
  x as _ //~ error: the trait bound `dyn Assocked<Assoc = u8>: Unsize<dyn Assocked<Assoc = u32>>` is not satisfied
 }
diff --git a/tests/ui/cast/ptr-to-trait-obj-different-args.stderr b/tests/ui/cast/ptr-to-trait-obj-different-args.stderr
@@ -0,0 1,51 @@
 error[E0277]: the trait bound `dyn A: Unsize<dyn B>` is not satisfied
  --> $DIR/ptr-to-trait-obj-different-args.rs:19:27
  |
 LL | let b: *const dyn B = a as _;
  | ^^^^^^ the trait `Unsize<dyn B>` is not implemented for `dyn A`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 
 error[E0277]: the trait bound `dyn Trait<X>: Unsize<dyn Trait<Y>>` is not satisfied
  --> $DIR/ptr-to-trait-obj-different-args.rs:22:34
  |
 LL | let y: *const dyn Trait<Y> = x as _;
  | ^^^^^^ the trait `Unsize<dyn Trait<Y>>` is not implemented for `dyn Trait<X>`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 
 error[E0277]: the trait bound `dyn Trait<X>: Unsize<dyn Trait<T>>` is not satisfied
  --> $DIR/ptr-to-trait-obj-different-args.rs:28:34
  |
 LL | let _: *const dyn Trait<T> = x as _;
  | ^^^^^^ the trait `Unsize<dyn Trait<T>>` is not implemented for `dyn Trait<X>`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
  |
 LL | fn generic<T>(x: *const dyn Trait<X>, t: *const dyn Trait<T>) where dyn Trait<X>: Unsize<dyn Trait<T>> {
  | 
 
 error[E0277]: the trait bound `dyn Trait<T>: Unsize<dyn Trait<X>>` is not satisfied
  --> $DIR/ptr-to-trait-obj-different-args.rs:29:34
  |
 LL | let _: *const dyn Trait<X> = t as _;
  | ^^^^^^ the trait `Unsize<dyn Trait<X>>` is not implemented for `dyn Trait<T>`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
  |
 LL | fn generic<T>(x: *const dyn Trait<X>, t: *const dyn Trait<T>) where dyn Trait<T>: Unsize<dyn Trait<X>> {
  | 
 
 error[E0277]: the trait bound `dyn Assocked<Assoc = u8>: Unsize<dyn Assocked<Assoc = u32>>` is not satisfied
  --> $DIR/ptr-to-trait-obj-different-args.rs:37:5
  |
 LL | x as _
  | ^^^^^^ the trait `Unsize<dyn Assocked<Assoc = u32>>` is not implemented for `dyn Assocked<Assoc = u8>`
  |
  = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
 
 error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0277`.