Skip to content

Commit

Permalink
Auto merge of #115025 - ouz-a:ouz_testing, r=lcnr
Browse files Browse the repository at this point in the history
Make subtyping explicit in MIR

This adds new mir-opt that pushes new `ProjectionElem` called `ProjectionElem::Subtype(T)` to `Rvalue` of a subtyped assignment so we can unsoundness issues like #107205

Addresses #112651

r? `@lcnr`
  • Loading branch information
bors committed Oct 3, 2023
2 parents 9998f4a + 5d753ab commit eb0f3ed
Show file tree
Hide file tree
Showing 34 changed files with 282 additions and 65 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2828,6 +2828,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Subtype(_)
| ProjectionElem::Index(_) => kind,
},
place_ty.projection_ty(tcx, elem),
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ProjectionElem::Downcast(..) if opt.including_downcast => return None,
ProjectionElem::Downcast(..) => (),
ProjectionElem::OpaqueCast(..) => (),
ProjectionElem::Subtype(..) => (),
ProjectionElem::Field(field, _ty) => {
// FIXME(project-rfc_2229#36): print capture precisely here.
if let Some(field) = self.is_upvar_field_projection(PlaceRef {
Expand Down Expand Up @@ -322,7 +323,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
PlaceRef { local, projection: proj_base }.ty(self.body, self.infcx.tcx)
}
ProjectionElem::Downcast(..) => place.ty(self.body, self.infcx.tcx),
ProjectionElem::OpaqueCast(ty) => PlaceTy::from_ty(*ty),
ProjectionElem::Subtype(ty) | ProjectionElem::OpaqueCast(ty) => {
PlaceTy::from_ty(*ty)
}
ProjectionElem::Field(_, field_type) => PlaceTy::from_ty(*field_type),
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
[
..,
ProjectionElem::Index(_)
| ProjectionElem::Subtype(_)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::OpaqueCast { .. }
| ProjectionElem::Subslice { .. }
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
for (place_base, elem) in place.iter_projections().rev() {
match elem {
ProjectionElem::Index(_/*operand*/) |
ProjectionElem::Subtype(_) |
ProjectionElem::OpaqueCast(_) |
ProjectionElem::ConstantIndex { .. } |
// assigning to P[i] requires P to be valid.
Expand Down Expand Up @@ -2191,6 +2192,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Subtype(..)
| ProjectionElem::OpaqueCast { .. }
| ProjectionElem::Downcast(..) => {
let upvar_field_projection = self.is_upvar_field_projection(place);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/places_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ fn place_components_conflict<'tcx>(
| (ProjectionElem::ConstantIndex { .. }, _, _)
| (ProjectionElem::Subslice { .. }, _, _)
| (ProjectionElem::OpaqueCast { .. }, _, _)
| (ProjectionElem::Subtype(_), _, _)
| (ProjectionElem::Downcast { .. }, _, _) => {
// Recursive case. This can still be disjoint on a
// further iteration if this a shallow access and
Expand Down Expand Up @@ -508,6 +509,7 @@ fn place_projection_conflict<'tcx>(
| ProjectionElem::Field(..)
| ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subtype(_)
| ProjectionElem::OpaqueCast { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Downcast(..),
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/prefixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ impl<'cx, 'tcx> Iterator for Prefixes<'cx, 'tcx> {
cursor = cursor_base;
continue 'cursor;
}
ProjectionElem::Subtype(..) => {
panic!("Subtype projection is not allowed before borrow check")
}
ProjectionElem::Deref => {
// (handled below)
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,9 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
}
PlaceTy::from_ty(fty)
}
ProjectionElem::Subtype(_) => {
bug!("ProjectionElem::Subtype shouldn't exist in borrowck")
}
ProjectionElem::OpaqueCast(ty) => {
let ty = self.sanitize_type(place, ty);
let ty = self.cx.normalize(ty, location);
Expand Down Expand Up @@ -2563,6 +2566,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| ProjectionElem::Subslice { .. } => {
// other field access
}
ProjectionElem::Subtype(_) => {
bug!("ProjectionElem::Subtype shouldn't exist in borrowck")
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ pub(crate) fn codegen_place<'tcx>(
cplace = cplace.place_deref(fx);
}
PlaceElem::OpaqueCast(ty) => bug!("encountered OpaqueCast({ty}) in codegen"),
PlaceElem::Subtype(ty) => cplace = cplace.place_transmute_type(fx, fx.monomorphize(ty)),
PlaceElem::Field(field, _ty) => {
cplace = cplace.place_field(fx, field);
}
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_codegen_cranelift/src/value_and_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,16 @@ impl<'tcx> CPlace<'tcx> {
}
}

/// Used for `ProjectionElem::Subtype`, `ty` has to be monomorphized before
/// passed on.
pub(crate) fn place_transmute_type(
self,
fx: &mut FunctionCx<'_, '_, 'tcx>,
ty: Ty<'tcx>,
) -> CPlace<'tcx> {
CPlace { inner: self.inner, layout: fx.layout_of(ty) }
}

pub(crate) fn place_field(
self,
fx: &mut FunctionCx<'_, '_, 'tcx>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::ProjectionElem::OpaqueCast(ty) => {
bug!("encountered OpaqueCast({ty}) in codegen")
}
mir::ProjectionElem::Subtype(ty) => cg_base.project_type(bx, self.monomorphize(ty)),
mir::ProjectionElem::Index(index) => {
let index = &mir::Operand::Copy(mir::Place::from(index));
let index = self.codegen_operand(bx, index);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::ty::layout::{
self, FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf, LayoutOfHelpers,
TyAndLayout,
};
use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt, TypeFoldable, Variance};
use rustc_mir_dataflow::storage::always_storage_live_locals;
use rustc_session::Limit;
use rustc_span::Span;
Expand Down Expand Up @@ -384,7 +384,7 @@ pub(super) fn mir_assign_valid_types<'tcx>(
// all normal lifetimes are erased, higher-ranked types with their
// late-bound lifetimes are still around and can lead to type
// differences.
if util::is_subtype(tcx, param_env, src.ty, dest.ty) {
if util::relate_types(tcx, param_env, Variance::Covariant, src.ty, dest.ty) {
// Make sure the layout is equal, too -- just to be safe. Miri really
// needs layout equality. For performance reason we skip this check when
// the types are equal. Equal types *can* have different layouts when
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,8 @@ where
OpaqueCast(ty) => {
span_bug!(self.cur_span(), "OpaqueCast({ty}) encountered after borrowck")
}
// We don't want anything happening here, this is here as a dummy.
Subtype(_) => base.transmute(base.layout(), self)?,
Field(field, _) => self.project_field(base, field.index())?,
Downcast(_, variant) => self.project_downcast(base, variant)?,
Deref => self.deref_pointer(&base.to_op(self)?)?.into(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
| ProjectionElem::Downcast(..)
| ProjectionElem::OpaqueCast(..)
| ProjectionElem::Subslice { .. }
| ProjectionElem::Subtype(..)
| ProjectionElem::Field(..)
| ProjectionElem::Index(_) => {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ where
ProjectionElem::Index(index) if in_local(index) => return true,

ProjectionElem::Deref
| ProjectionElem::Subtype(_)
| ProjectionElem::Field(_, _)
| ProjectionElem::OpaqueCast(_)
| ProjectionElem::ConstantIndex { .. }
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,9 @@ impl<'tcx> Validator<'_, 'tcx> {
return Err(Unpromotable);
}

ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {}
ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subtype(_)
| ProjectionElem::Subslice { .. } => {}

ProjectionElem::Index(local) => {
let mut promotable = false;
Expand Down
32 changes: 30 additions & 2 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_infer::traits::Reveal;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitableExt, Variance};
use rustc_mir_dataflow::impls::MaybeStorageLive;
use rustc_mir_dataflow::storage::always_storage_live_locals;
use rustc_mir_dataflow::{Analysis, ResultsCursor};
Expand All @@ -16,6 +16,8 @@ use rustc_target::spec::abi::Abi;

use crate::util::is_within_packed;

use crate::util::relate_types;

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum EdgeKind {
Unwind,
Expand Down Expand Up @@ -602,7 +604,15 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
return true;
}

crate::util::is_subtype(self.tcx, self.param_env, src, dest)
// After borrowck subtyping should be fully explicit via
// `Subtype` projections.
let variance = if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
Variance::Invariant
} else {
Variance::Covariant
};

crate::util::relate_types(self.tcx, self.param_env, variance, src, dest)
}
}

Expand Down Expand Up @@ -753,6 +763,23 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
}
ProjectionElem::Subtype(ty) => {
if !relate_types(
self.tcx,
self.param_env,
Variance::Covariant,
ty,
place_ref.ty(&self.body.local_decls, self.tcx).ty,
) {
self.fail(
location,
format!(
"Failed subtyping {ty:#?} and {:#?}",
place_ref.ty(&self.body.local_decls, self.tcx).ty
),
)
}
}
_ => {}
}
self.super_projection_elem(place_ref, elem, context, location);
Expand Down Expand Up @@ -1088,6 +1115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
// LHS and RHS of the assignment must have the same type.
let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty;
let right_ty = rvalue.ty(&self.body.local_decls, self.tcx);

if !self.mir_assign_valid_types(right_ty, left_ty) {
self.fail(
location,
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_const_eval/src/util/compare_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::traits::{DefiningAnchor, ObligationCause};
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt, Variance};
use rustc_trait_selection::traits::ObligationCtxt;

/// Returns whether the two types are equal up to subtyping.
Expand All @@ -24,16 +24,22 @@ pub fn is_equal_up_to_subtyping<'tcx>(
}

// Check for subtyping in either direction.
is_subtype(tcx, param_env, src, dest) || is_subtype(tcx, param_env, dest, src)
relate_types(tcx, param_env, Variance::Covariant, src, dest)
|| relate_types(tcx, param_env, Variance::Covariant, dest, src)
}

/// Returns whether `src` is a subtype of `dest`, i.e. `src <: dest`.
///
/// When validating assignments, the variance should be `Covariant`. When checking
/// during `MirPhase` >= `MirPhase::Runtime(RuntimePhase::Initial)` variance should be `Invariant`
/// because we want to check for type equality.
///
/// This mostly ignores opaque types as it can be used in constraining contexts
/// while still computing the final underlying type.
pub fn is_subtype<'tcx>(
pub fn relate_types<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
variance: Variance,
src: Ty<'tcx>,
dest: Ty<'tcx>,
) -> bool {
Expand All @@ -48,7 +54,7 @@ pub fn is_subtype<'tcx>(
let cause = ObligationCause::dummy();
let src = ocx.normalize(&cause, param_env, src);
let dest = ocx.normalize(&cause, param_env, dest);
match ocx.sub(&cause, param_env, src, dest) {
match ocx.relate(&cause, param_env, variance, src, dest) {
Ok(()) => {}
Err(_) => return false,
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod type_name;

pub use self::alignment::{is_disaligned, is_within_packed};
pub use self::check_validity_requirement::check_validity_requirement;
pub use self::compare_types::{is_equal_up_to_subtyping, is_subtype};
pub use self::compare_types::{is_equal_up_to_subtyping, relate_types};
pub use self::type_name::type_name;

/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,7 @@ fn pre_fmt_projection(projection: &[PlaceElem<'_>], fmt: &mut Formatter<'_>) ->
for &elem in projection.iter().rev() {
match elem {
ProjectionElem::OpaqueCast(_)
| ProjectionElem::Subtype(_)
| ProjectionElem::Downcast(_, _)
| ProjectionElem::Field(_, _) => {
write!(fmt, "(").unwrap();
Expand All @@ -1125,6 +1126,9 @@ fn post_fmt_projection(projection: &[PlaceElem<'_>], fmt: &mut Formatter<'_>) ->
ProjectionElem::OpaqueCast(ty) => {
write!(fmt, " as {ty})")?;
}
ProjectionElem::Subtype(ty) => {
write!(fmt, " as subtype {ty})")?;
}
ProjectionElem::Downcast(Some(name), _index) => {
write!(fmt, " as {name})")?;
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl<V, T> ProjectionElem<V, T> {
Self::Field(_, _)
| Self::Index(_)
| Self::OpaqueCast(_)
| Self::Subtype(_)
| Self::ConstantIndex { .. }
| Self::Subslice { .. }
| Self::Downcast(_, _) => false,
Expand All @@ -70,6 +71,7 @@ impl<V, T> ProjectionElem<V, T> {
Self::Deref | Self::Index(_) => false,
Self::Field(_, _)
| Self::OpaqueCast(_)
| Self::Subtype(_)
| Self::ConstantIndex { .. }
| Self::Subslice { .. }
| Self::Downcast(_, _) => true,
Expand All @@ -95,6 +97,7 @@ impl<V, T> ProjectionElem<V, T> {
| Self::Field(_, _) => true,
Self::ConstantIndex { from_end: true, .. }
| Self::Index(_)
| Self::Subtype(_)
| Self::OpaqueCast(_)
| Self::Subslice { .. } => false,
}
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,18 @@ pub enum ProjectionElem<V, T> {
/// Like an explicit cast from an opaque type to a concrete type, but without
/// requiring an intermediate variable.
OpaqueCast(T),

/// A `Subtype(T)` projection is applied to any `StatementKind::Assign` where
/// type of lvalue doesn't match the type of rvalue, the primary goal is making subtyping
/// explicit during optimizations and codegen.
///
/// This projection doesn't impact the runtime behavior of the program except for potentially changing
/// some type metadata of the interpreter or codegen backend.
///
/// This goal is achieved with mir_transform pass `Subtyper`, which runs right after
/// borrowchecker, as we only care about subtyping that can affect trait selection and
/// `TypeId`.
Subtype(T),
}

/// Alias for projections as they appear in places, where the base is a place
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl<'tcx> PlaceTy<'tcx> {
param_env: ty::ParamEnv<'tcx>,
elem: &ProjectionElem<V, T>,
mut handle_field: impl FnMut(&Self, FieldIdx, T) -> Ty<'tcx>,
mut handle_opaque_cast: impl FnMut(&Self, T) -> Ty<'tcx>,
mut handle_opaque_cast_and_subtype: impl FnMut(&Self, T) -> Ty<'tcx>,
) -> PlaceTy<'tcx>
where
V: ::std::fmt::Debug,
Expand Down Expand Up @@ -110,7 +110,12 @@ impl<'tcx> PlaceTy<'tcx> {
PlaceTy { ty: self.ty, variant_index: Some(index) }
}
ProjectionElem::Field(f, fty) => PlaceTy::from_ty(handle_field(&self, f, fty)),
ProjectionElem::OpaqueCast(ty) => PlaceTy::from_ty(handle_opaque_cast(&self, ty)),
ProjectionElem::OpaqueCast(ty) => {
PlaceTy::from_ty(handle_opaque_cast_and_subtype(&self, ty))
}
ProjectionElem::Subtype(ty) => {
PlaceTy::from_ty(handle_opaque_cast_and_subtype(&self, ty))
}
};
debug!("projection_ty self: {:?} elem: {:?} yields: {:?}", self, elem, answer);
answer
Expand Down
Loading

0 comments on commit eb0f3ed

Please sign in to comment.