Skip to content

Commit

Permalink
Rollup merge of #70806 - RalfJung:miri-assignment-check, r=eddyb
Browse files Browse the repository at this point in the history
fix Miri assignment sanity check

Thanks @eddyb for pointing me to the right APIs!

r? @eddyb
Fixes #70804
  • Loading branch information
Dylan-DPC authored Apr 5, 2020
2 parents c185c4f + e52a451 commit 31b8d65
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 19 deletions.
52 changes: 36 additions & 16 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ use rustc_middle::mir::interpret::{
sign_extend, truncate, AllocId, FrameInfo, GlobalId, InterpResult, Pointer, Scalar,
};
use rustc_middle::ty::layout::{self, TyAndLayout};
use rustc_middle::ty::query::TyCtxtAt;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{
self, fold::BottomUpFolder, query::TyCtxtAt, subst::SubstsRef, Ty, TyCtxt, TypeFoldable,
};
use rustc_span::source_map::DUMMY_SP;
use rustc_target::abi::{Abi, Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};

use super::{
Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
Expand Down Expand Up @@ -213,30 +213,50 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
/// Test if it is valid for a MIR assignment to assign `src`-typed place to `dest`-typed value.
/// This test should be symmetric, as it is primarily about layout compatibility.
pub(super) fn mir_assign_valid_types<'tcx>(
tcx: TyCtxt<'tcx>,
src: TyAndLayout<'tcx>,
dest: TyAndLayout<'tcx>,
) -> bool {
if src.ty == dest.ty {
// Equal types, all is good.
return true;
}
// Type-changing assignments can happen for (at least) two reasons:
// - `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
// - Subtyping is used. While all normal lifetimes are erased, higher-ranked lifetime
// bounds are still around and can lead to type differences.
// There is no good way to check the latter, so we compare layouts instead -- but only
// for values with `Scalar`/`ScalarPair` abi.
// FIXME: Do something more accurate, type-based.
match &src.abi {
Abi::Scalar(..) | Abi::ScalarPair(..) => src.layout == dest.layout,
_ => false,
if src.layout != dest.layout {
// Layout differs, definitely not equal.
// We do this here because Miri would *do the wrong thing* if we allowed layout-changing
// assignments.
return false;
}

// Type-changing assignments can happen for (at least) two reasons:
// 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
// 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types
// with their late-bound lifetimes are still around and can lead to type differences.
// Normalize both of them away.
let normalize = |ty: Ty<'tcx>| {
ty.fold_with(&mut BottomUpFolder {
tcx,
// Normalize all references to immutable.
ty_op: |ty| match ty.kind {
ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee),
_ => ty,
},
// We just erase all late-bound lifetimes, but this is not fully correct (FIXME):
// lifetimes in invariant positions could matter (e.g. through associated types).
// We rely on the fact that layout was confirmed to be equal above.
lt_op: |_| tcx.lifetimes.re_erased,
// Leave consts unchanged.
ct_op: |ct| ct,
})
};
normalize(src.ty) == normalize(dest.ty)
}

/// Use the already known layout if given (but sanity check in debug mode),
/// or compute the layout.
#[cfg_attr(not(debug_assertions), inline(always))]
pub(super) fn from_known_layout<'tcx>(
tcx: TyCtxt<'tcx>,
known_layout: Option<TyAndLayout<'tcx>>,
compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
Expand All @@ -246,7 +266,7 @@ pub(super) fn from_known_layout<'tcx>(
if cfg!(debug_assertions) {
let check_layout = compute()?;
assert!(
mir_assign_valid_types(check_layout, known_layout),
mir_assign_valid_types(tcx, check_layout, known_layout),
"expected type differs from actual type.\nexpected: {:?}\nactual: {:?}",
known_layout.ty,
check_layout.ty,
Expand Down Expand Up @@ -424,7 +444,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// have to support that case (mostly by skipping all caching).
match frame.locals.get(local).and_then(|state| state.layout.get()) {
None => {
let layout = from_known_layout(layout, || {
let layout = from_known_layout(self.tcx.tcx, layout, || {
let local_ty = frame.body.local_decls[local].ty;
let local_ty =
self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ty::ConstKind::Value(val_val) => val_val,
};
// Other cases need layout.
let layout = from_known_layout(layout, || self.layout_of(val.ty))?;
let layout = from_known_layout(self.tcx.tcx, layout, || self.layout_of(val.ty))?;
let op = match val_val {
ConstValue::ByRef { alloc, offset } => {
let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ where
// We do NOT compare the types for equality, because well-typed code can
// actually "transmute" `&mut T` to `&T` in an assignment without a cast.
assert!(
mir_assign_valid_types(src.layout, dest.layout),
mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout),
"type mismatch when copying!\nsrc: {:?},\ndest: {:?}",
src.layout.ty,
dest.layout.ty,
Expand Down Expand Up @@ -922,7 +922,7 @@ where
src: OpTy<'tcx, M::PointerTag>,
dest: PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx> {
if mir_assign_valid_types(src.layout, dest.layout) {
if mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout) {
// Fast path: Just use normal `copy_op`
return self.copy_op(src, dest);
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/consts/const-eval/issue-70804-fn-subtyping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// check-pass
#![feature(const_fn)]

const fn nested(x: (for<'a> fn(&'a ()), String)) -> (fn(&'static ()), String) {
x
}

pub const TEST: (fn(&'static ()), String) = nested((|_x| (), String::new()));

fn main() {}

0 comments on commit 31b8d65

Please sign in to comment.