From 8fc8e03150ed69cc37a3e21b319a4d9bf247292f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 23 Sep 2024 03:56:20 -0400 Subject: [PATCH] Validate unsize coercion in MIR validation --- compiler/rustc_mir_transform/src/validate.rs | 50 +++++++++++++++++-- tests/crashes/129219.rs | 26 ---------- tests/ui/mir/validate/validate-unsize-cast.rs | 33 ++++++++++++ .../mir/validate/validate-unsize-cast.stderr | 20 ++++++++ 4 files changed, 100 insertions(+), 29 deletions(-) delete mode 100644 tests/crashes/129219.rs create mode 100644 tests/ui/mir/validate/validate-unsize-cast.rs create mode 100644 tests/ui/mir/validate/validate-unsize-cast.stderr diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs index eda0b8c75f3..c9a844e4734 100644 --- a/compiler/rustc_mir_transform/src/validate.rs +++ b/compiler/rustc_mir_transform/src/validate.rs @@ -4,7 +4,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::LangItem; use rustc_index::IndexVec; use rustc_index::bit_set::BitSet; -use rustc_infer::traits::Reveal; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::{Obligation, ObligationCause, Reveal}; use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; @@ -16,6 +17,8 @@ use rustc_middle::ty::{ use rustc_middle::{bug, span_bug}; use rustc_target::abi::{FIRST_VARIANT, Size}; use rustc_target::spec::abi::Abi; +use rustc_trait_selection::traits::ObligationCtxt; +use rustc_type_ir::Upcast; use crate::util::{is_within_packed, relate_types}; @@ -586,6 +589,22 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { crate::util::relate_types(self.tcx, self.param_env, variance, src, dest) } + + /// Check that the given predicate definitely holds in the param-env of this MIR body. + fn predicate_must_hold_modulo_regions( + &self, + pred: impl Upcast, ty::Predicate<'tcx>>, + ) -> bool { + let infcx = self.tcx.infer_ctxt().build(); + let ocx = ObligationCtxt::new(&infcx); + ocx.register_obligation(Obligation::new( + self.tcx, + ObligationCause::dummy(), + self.param_env, + pred, + )); + ocx.select_all_or_error().is_empty() + } } impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { @@ -1202,8 +1221,33 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } CastKind::PointerCoercion(PointerCoercion::Unsize, _) => { - // This is used for all `CoerceUnsized` types, - // not just pointers/references, so is hard to check. + // Pointers being unsize coerced should at least implement + // `CoerceUnsized`. + if !self.predicate_must_hold_modulo_regions(ty::TraitRef::new( + self.tcx, + self.tcx.require_lang_item( + LangItem::CoerceUnsized, + Some(self.body.source_info(location).span), + ), + [op_ty, *target_type], + )) { + self.fail(location, format!("Unsize coercion, but `{op_ty}` isn't coercible to `{target_type}`")); + } + + // FIXME: Codegen has an additional assumption, where if the + // principal trait def id of what's being casted doesn't change, + // then we don't need to adjust the vtable at all. This + // corresponds to the fact that `dyn Tr: Unsize>` + // requires that `A = B`; we don't allow *upcasting* objects + // between the same trait with different args. Nothing actually + // validates this, though. While it's true right now, if we for + // some reason were to relax the `Unsize` trait, it could become + // unsound. We should eventually validate that, but it would + // require peeling `&Box, ..>>` down to + // the trait object that's being unsized, and that's rather + // annoying, and also it would need to be opportunistic since + // this MIR is not yet fully monomorphized, so we may bottom + // out in an alias or a projection or something. } CastKind::PointerCoercion(PointerCoercion::DynStar, _) => { // FIXME(dyn-star): make sure nothing needs to be done here. diff --git a/tests/crashes/129219.rs b/tests/crashes/129219.rs deleted file mode 100644 index effbfcd8b8e..00000000000 --- a/tests/crashes/129219.rs +++ /dev/null @@ -1,26 +0,0 @@ -//@ known-bug: rust-lang/rust#129219 -//@ compile-flags: -Zmir-opt-level=5 -Zvalidate-mir --edition=2018 - -use core::marker::Unsize; - -pub trait CastTo: Unsize {} - -impl CastTo for U {} - -impl Cast for T {} -pub trait Cast { - fn cast(&self) -> &T - where - Self: CastTo, - { - self - } -} - -pub trait Foo {} -impl Foo for [i32; 0] {} - -fn main() { - let x: &dyn Foo = &[]; - let x = x.cast::<[i32]>(); -} diff --git a/tests/ui/mir/validate/validate-unsize-cast.rs b/tests/ui/mir/validate/validate-unsize-cast.rs new file mode 100644 index 00000000000..198af8d2e13 --- /dev/null +++ b/tests/ui/mir/validate/validate-unsize-cast.rs @@ -0,0 +1,33 @@ +//@ compile-flags: -Zmir-opt-level=0 -Zmir-enable-passes=+Inline,+GVN -Zvalidate-mir + +#![feature(unsize)] + +use std::marker::Unsize; + +pub trait CastTo: Unsize {} + +// Not well-formed! +impl CastTo for T {} +//~^ ERROR the trait bound `T: Unsize` is not satisfied + +pub trait Cast { + fn cast(&self) + where + Self: CastTo; +} +impl Cast for T { + #[inline(always)] + fn cast(&self) + where + Self: CastTo, + { + let x: &U = self; + } +} + +fn main() { + // When we inline this call, then we run GVN, then + // GVN tries to evaluate the `() -> [i32]` unsize. + // That's invalid! + ().cast::<[i32]>(); +} diff --git a/tests/ui/mir/validate/validate-unsize-cast.stderr b/tests/ui/mir/validate/validate-unsize-cast.stderr new file mode 100644 index 00000000000..cfb47b34e98 --- /dev/null +++ b/tests/ui/mir/validate/validate-unsize-cast.stderr @@ -0,0 +1,20 @@ +error[E0277]: the trait bound `T: Unsize` is not satisfied + --> $DIR/validate-unsize-cast.rs:10:42 + | +LL | impl CastTo for T {} + | ^ the trait `Unsize` is not implemented for `T` + | + = note: all implementations of `Unsize` are provided automatically by the compiler, see for more information +note: required by a bound in `CastTo` + --> $DIR/validate-unsize-cast.rs:7:30 + | +LL | pub trait CastTo: Unsize {} + | ^^^^^^^^^ required by this bound in `CastTo` +help: consider further restricting this bound + | +LL | impl, U: ?Sized> CastTo for T {} + | ++++++++++++++++++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`.