Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Miri assignment sanity check #70806

Merged
merged 3 commits into from
Apr 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {}