Rollup merge of rust-lang#120248 - WaffleLapkin:bonk-ptr-object-casts…

…, r=compiler-errors,oli-obk,lnicola

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.

Before this PR the rules were as follows:

> When casting `*const X<dyn A>` -> `*const Y<dyn B>`, principal traits in `A` and `B` must refer to the same trait definition (or no trait).

With this PR the rules are changed to

> When casting `*const X<dyn Src>` -> `*const Y<dyn Dst>`
> - if `Dst` has a principal trait `DstP`,
>   - `Src` must have a principal trait `SrcP`
>   - `dyn SrcP` and `dyn DstP` must be the same type (modulo the trait object lifetime, `dyn T+'a` -> `dyn T+'b` is allowed)
>   - Auto traits in `Dst` must be a subset of auto traits in `Src`
>     - Not adhering to this is currently a FCW (warn-by-default + `FutureReleaseErrorReportInDeps`), instead of an error
> - if `Src` has a principal trait `Dst` must as well
>   - this restriction will be removed in a follow up PR

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 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`

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -38,6 +38,7 @@ use rustc_span::def_id::CRATE_DEF_ID;
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::sym;
 use rustc_span::Span;
+use rustc_span::DUMMY_SP;
 use rustc_target::abi::{FieldIdx, FIRST_VARIANT};
 use rustc_trait_selection::traits::query::type_op::custom::scrape_region_constraints;
 use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
@@ -49,6 +50,7 @@ use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
 use rustc_mir_dataflow::move_paths::MoveData;
 use rustc_mir_dataflow::ResultsCursor;
 
+use crate::renumber::RegionCtxt;
 use crate::session_diagnostics::{MoveUnsized, SimdIntrinsicArgConst};
 use crate::{
     borrow_set::BorrowSet,
@@ -2333,7 +2335,57 @@ 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 mut normalize = |t| self.normalize(t, location);
+                                let src_tail =
+                                    tcx.struct_tail_with_normalize(src.ty, &mut normalize, || ());
+                                let dst_tail =
+                                    tcx.struct_tail_with_normalize(dst.ty, &mut normalize, || ());
+
+                                // This checks (lifetime part of) vtable validity for pointer casts,
+                                // which is irrelevant when there are aren't principal traits on both sides (aka only auto traits).
+                                //
+                                // Note that other checks (such as denying `dyn Send` -> `dyn Debug`) are in `rustc_hir_typeck`.
+                                if let ty::Dynamic(src_tty, ..) = src_tail.kind()
+                                    && let ty::Dynamic(dst_tty, ..) = dst_tail.kind()
+                                    && src_tty.principal().is_some()
+                                    && dst_tty.principal().is_some()
+                                {
+                                    // Remove auto traits.
+                                    // Auto trait checks are handled in `rustc_hir_typeck` as FCW.
+                                    let src_obj = tcx.mk_ty_from_kind(ty::Dynamic(
+                                        tcx.mk_poly_existential_predicates(
+                                            &src_tty.without_auto_traits().collect::<Vec<_>>(),
+                                        ),
+                                        tcx.lifetimes.re_static,
+                                        ty::Dyn,
+                                    ));
+                                    let dst_obj = tcx.mk_ty_from_kind(ty::Dynamic(
+                                        tcx.mk_poly_existential_predicates(
+                                            &dst_tty.without_auto_traits().collect::<Vec<_>>(),
+                                        ),
+                                        tcx.lifetimes.re_static,
+                                        ty::Dyn,
+                                    ));
+
+                                    // Replace trait object lifetimes with fresh vars, to allow casts like
+                                    // `*mut dyn FnOnce() + 'a` -> `*mut dyn FnOnce() + 'static`,
+                                    let src_obj =
+                                        freshen_single_trait_object_lifetime(self.infcx, src_obj);
+                                    let dst_obj =
+                                        freshen_single_trait_object_lifetime(self.infcx, dst_obj);
+
+                                    debug!(?src_tty, ?dst_tty, ?src_obj, ?dst_obj);
+
+                                    self.eq_types(
+                                        src_obj,
+                                        dst_obj,
+                                        location.to_locations(),
+                                        ConstraintCategory::Cast { unsize_to: None },
+                                    )
+                                    .unwrap();
+                                }
+                            }
                             _ => {
                                 span_mirbug!(
                                     self,
@@ -2856,3 +2908,16 @@ impl<'tcx> TypeOp<'tcx> for InstantiateOpaqueType<'tcx> {
         Ok(output)
     }
 }
+
+fn freshen_single_trait_object_lifetime<'tcx>(
+    infcx: &BorrowckInferCtxt<'tcx>,
+    ty: Ty<'tcx>,
+) -> Ty<'tcx> {
+    let &ty::Dynamic(tty, _, dyn_kind @ ty::Dyn) = ty.kind() else { bug!("expected trait object") };
+
+    let fresh = infcx
+        .next_region_var(rustc_infer::infer::RegionVariableOrigin::MiscVariable(DUMMY_SP), || {
+            RegionCtxt::Unknown
+        });
+    infcx.tcx.mk_ty_from_kind(ty::Dynamic(tty, fresh, dyn_kind))
+}

compiler/rustc_errors/src/diagnostic_impls.rs

@@ -298,15 +298,21 @@ impl IntoDiagArg for hir::def::Namespace {
 }
 
 #[derive(Clone)]
-pub struct DiagSymbolList(Vec<Symbol>);
+pub struct DiagSymbolList<S = Symbol>(Vec<S>);
 
-impl From<Vec<Symbol>> for DiagSymbolList {
-    fn from(v: Vec<Symbol>) -> Self {
+impl<S> From<Vec<S>> for DiagSymbolList<S> {
+    fn from(v: Vec<S>) -> Self {
         DiagSymbolList(v)
     }
 }
 
-impl IntoDiagArg for DiagSymbolList {
+impl<S> FromIterator<S> for DiagSymbolList<S> {
+    fn from_iter<T: IntoIterator<Item = S>>(iter: T) -> Self {
+        iter.into_iter().collect::<Vec<_>>().into()
+    }
+}
+
+impl<S: std::fmt::Display> IntoDiagArg for DiagSymbolList<S> {
     fn into_diag_arg(self) -> DiagArgValue {
         DiagArgValue::StrListSepByAnd(
             self.0.into_iter().map(|sym| Cow::Owned(format!("`{sym}`"))).collect(),

compiler/rustc_hir_typeck/messages.ftl

@@ -138,6 +138,11 @@ hir_typeck_option_result_asref = use `{$def_path}::as_ref` to convert `{$expecte
 hir_typeck_option_result_cloned = use `{$def_path}::cloned` to clone the value inside the `{$def_path}`
 hir_typeck_option_result_copied = use `{$def_path}::copied` to copy the value inside the `{$def_path}`
 
+hir_typeck_ptr_cast_add_auto_to_object = adding {$traits_len ->
+    [1] an auto trait {$traits}
+    *[other] auto traits {$traits}
+} to a trait object in a pointer cast may cause UB later on
+
 hir_typeck_remove_semi_for_coerce = you might have meant to return the `match` expression
 hir_typeck_remove_semi_for_coerce_expr = this could be implicitly returned but it is a statement, not a tail expression
 hir_typeck_remove_semi_for_coerce_ret = the `match` arms can conform to this return type

compiler/rustc_hir_typeck/src/cast.rs

@@ -32,9 +32,9 @@ use super::FnCtxt;
 
 use crate::errors;
 use crate::type_error_struct;
-use hir::ExprKind;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{codes::*, Applicability, Diag, ErrorGuaranteed};
-use rustc_hir as hir;
+use rustc_hir::{self as hir, ExprKind};
 use rustc_macros::{TypeFoldable, TypeVisitable};
 use rustc_middle::bug;
 use rustc_middle::mir::Mutability;
@@ -44,7 +44,7 @@ use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::TyCtxt;
 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_span::DUMMY_SP;
@@ -73,7 +73,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
@@ -101,7 +101,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) => {
@@ -755,7 +755,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
@@ -799,40 +799,126 @@ 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_src={m_src:?} m_dst={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);
-        };
+            (_, None) => Err(CastError::UnknownCastPtrKind),
+            // Cast to thin pointer is OK
+            (_, Some(PointerKind::Thin)) => Ok(CastKind::PtrPtrCast),
 
-        // Cast to thin pointer is OK
-        if cast_kind == PointerKind::Thin {
-            return 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 Src<...> + SrcAuto> -> B<dyn Dst<...> + DstAuto>. need to make sure
+                    // - `Src` and `Dst` traits are the same
+                    // - traits have the same generic arguments
+                    // - `SrcAuto` is a superset of `DstAuto`
+                    (Some(src_principal), Some(dst_principal)) => {
+                        let tcx = fcx.tcx;
+
+                        // Check that the traits are actually the same.
+                        // The `dyn Src = dyn Dst` check below would suffice,
+                        // but this may produce a better diagnostic.
+                        //
+                        // Note that trait upcasting goes through a different mechanism (`coerce_unsized`)
+                        // and is unaffected by this check.
+                        if src_principal.def_id() != dst_principal.def_id() {
+                            return Err(CastError::DifferingKinds);
+                        }
 
-        // thin -> fat? report invalid cast (don't complain about vtable kinds)
-        if expr_kind == PointerKind::Thin {
-            return Err(CastError::SizedUnsizedCast);
-        }
+                        // 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.
+                        //
+                        // We also need to skip auto traits to emit an FCW and not an error.
+                        let src_obj = tcx.mk_ty_from_kind(ty::Dynamic(
+                            tcx.mk_poly_existential_predicates(
+                                &src_tty.without_auto_traits().collect::<Vec<_>>(),
+                            ),
+                            tcx.lifetimes.re_erased,
+                            ty::Dyn,
+                        ));
+                        let dst_obj = tcx.mk_ty_from_kind(ty::Dynamic(
+                            tcx.mk_poly_existential_predicates(
+                                &dst_tty.without_auto_traits().collect::<Vec<_>>(),
+                            ),
+                            tcx.lifetimes.re_erased,
+                            ty::Dyn,
+                        ));
 
-        // vtable kinds must match
-        if fcx.tcx.erase_regions(cast_kind) == fcx.tcx.erase_regions(expr_kind) {
-            Ok(CastKind::PtrPtrCast)
-        } else {
-            Err(CastError::DifferingKinds)
+                        // `dyn Src = dyn Dst`, this checks for matching traits/generics
+                        fcx.demand_eqtype(self.span, src_obj, dst_obj);
+
+                        // Check that `SrcAuto` (+auto traits implied by `Src`) is a superset of `DstAuto`.
+                        // Emit an FCW otherwise.
+                        let src_auto: FxHashSet<_> = src_tty
+                            .auto_traits()
+                            .chain(
+                                tcx.supertrait_def_ids(src_principal.def_id())
+                                    .filter(|def_id| tcx.trait_is_auto(*def_id)),
+                            )
+                            .collect();
+
+                        let added = dst_tty
+                            .auto_traits()
+                            .filter(|trait_did| !src_auto.contains(trait_did))
+                            .collect::<Vec<_>>();
+
+                        if !added.is_empty() {
+                            tcx.emit_node_span_lint(
+                                lint::builtin::PTR_CAST_ADD_AUTO_TO_OBJECT,
+                                self.expr.hir_id,
+                                self.span,
+                                errors::PtrCastAddAutoToObject {
+                                    traits_len: added.len(),
+                                    traits: {
+                                        let mut traits: Vec<_> = added
+                                            .into_iter()
+                                            .map(|trait_did| tcx.def_path_str(trait_did))
+                                            .collect();
+
+                                        traits.sort();
+                                        traits.into()
+                                    },
+                                },
+                            )
+                        }
+
+                        Ok(CastKind::PtrPtrCast)
+                    }
+
+                    // dyn Auto -> dyn Auto'? ok.
+                    (None, None) => Ok(CastKind::PtrPtrCast),
+
+                    // dyn Trait -> dyn Auto? should be ok, but we used to not allow it.
+                    // FIXME: allow this
+                    (Some(_), None) => Err(CastError::DifferingKinds),
+
+                    // 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),
         }
     }
 

compiler/rustc_hir_typeck/src/errors.rs

@@ -4,8 +4,8 @@ use std::borrow::Cow;
 
 use crate::fluent_generated as fluent;
 use rustc_errors::{
-    codes::*, Applicability, Diag, DiagArgValue, EmissionGuarantee, IntoDiagArg, MultiSpan,
-    SubdiagMessageOp, Subdiagnostic,
+    codes::*, Applicability, Diag, DiagArgValue, DiagSymbolList, EmissionGuarantee, IntoDiagArg,
+    MultiSpan, SubdiagMessageOp, Subdiagnostic,
 };
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::ty::{self, Ty};
@@ -253,6 +253,13 @@ pub struct LossyProvenanceInt2Ptr<'tcx> {
     pub sugg: LossyProvenanceInt2PtrSuggestion,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(hir_typeck_ptr_cast_add_auto_to_object)]
+pub struct PtrCastAddAutoToObject {
+    pub traits_len: usize,
+    pub traits: DiagSymbolList<String>,
+}
+
 #[derive(Subdiagnostic)]
 #[multipart_suggestion(hir_typeck_suggestion, applicability = "has-placeholders")]
 pub struct LossyProvenanceInt2PtrSuggestion {