Skip to content

Commit 31b8d65

Browse files
authored
Rollup merge of #70806 - RalfJung:miri-assignment-check, r=eddyb
fix Miri assignment sanity check Thanks @eddyb for pointing me to the right APIs! r? @eddyb Fixes #70804
2 parents c185c4f + e52a451 commit 31b8d65

File tree

4 files changed

+49
-19
lines changed

4 files changed

+49
-19
lines changed

src/librustc_mir/interpret/eval_context.rs

+36-16
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ use rustc_middle::mir::interpret::{
1414
sign_extend, truncate, AllocId, FrameInfo, GlobalId, InterpResult, Pointer, Scalar,
1515
};
1616
use rustc_middle::ty::layout::{self, TyAndLayout};
17-
use rustc_middle::ty::query::TyCtxtAt;
18-
use rustc_middle::ty::subst::SubstsRef;
19-
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
17+
use rustc_middle::ty::{
18+
self, fold::BottomUpFolder, query::TyCtxtAt, subst::SubstsRef, Ty, TyCtxt, TypeFoldable,
19+
};
2020
use rustc_span::source_map::DUMMY_SP;
21-
use rustc_target::abi::{Abi, Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
21+
use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
2222

2323
use super::{
2424
Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
@@ -213,30 +213,50 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
213213
/// Test if it is valid for a MIR assignment to assign `src`-typed place to `dest`-typed value.
214214
/// This test should be symmetric, as it is primarily about layout compatibility.
215215
pub(super) fn mir_assign_valid_types<'tcx>(
216+
tcx: TyCtxt<'tcx>,
216217
src: TyAndLayout<'tcx>,
217218
dest: TyAndLayout<'tcx>,
218219
) -> bool {
219220
if src.ty == dest.ty {
220221
// Equal types, all is good.
221222
return true;
222223
}
223-
// Type-changing assignments can happen for (at least) two reasons:
224-
// - `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
225-
// - Subtyping is used. While all normal lifetimes are erased, higher-ranked lifetime
226-
// bounds are still around and can lead to type differences.
227-
// There is no good way to check the latter, so we compare layouts instead -- but only
228-
// for values with `Scalar`/`ScalarPair` abi.
229-
// FIXME: Do something more accurate, type-based.
230-
match &src.abi {
231-
Abi::Scalar(..) | Abi::ScalarPair(..) => src.layout == dest.layout,
232-
_ => false,
224+
if src.layout != dest.layout {
225+
// Layout differs, definitely not equal.
226+
// We do this here because Miri would *do the wrong thing* if we allowed layout-changing
227+
// assignments.
228+
return false;
233229
}
230+
231+
// Type-changing assignments can happen for (at least) two reasons:
232+
// 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
233+
// 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types
234+
// with their late-bound lifetimes are still around and can lead to type differences.
235+
// Normalize both of them away.
236+
let normalize = |ty: Ty<'tcx>| {
237+
ty.fold_with(&mut BottomUpFolder {
238+
tcx,
239+
// Normalize all references to immutable.
240+
ty_op: |ty| match ty.kind {
241+
ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee),
242+
_ => ty,
243+
},
244+
// We just erase all late-bound lifetimes, but this is not fully correct (FIXME):
245+
// lifetimes in invariant positions could matter (e.g. through associated types).
246+
// We rely on the fact that layout was confirmed to be equal above.
247+
lt_op: |_| tcx.lifetimes.re_erased,
248+
// Leave consts unchanged.
249+
ct_op: |ct| ct,
250+
})
251+
};
252+
normalize(src.ty) == normalize(dest.ty)
234253
}
235254

236255
/// Use the already known layout if given (but sanity check in debug mode),
237256
/// or compute the layout.
238257
#[cfg_attr(not(debug_assertions), inline(always))]
239258
pub(super) fn from_known_layout<'tcx>(
259+
tcx: TyCtxt<'tcx>,
240260
known_layout: Option<TyAndLayout<'tcx>>,
241261
compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>,
242262
) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
@@ -246,7 +266,7 @@ pub(super) fn from_known_layout<'tcx>(
246266
if cfg!(debug_assertions) {
247267
let check_layout = compute()?;
248268
assert!(
249-
mir_assign_valid_types(check_layout, known_layout),
269+
mir_assign_valid_types(tcx, check_layout, known_layout),
250270
"expected type differs from actual type.\nexpected: {:?}\nactual: {:?}",
251271
known_layout.ty,
252272
check_layout.ty,
@@ -424,7 +444,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
424444
// have to support that case (mostly by skipping all caching).
425445
match frame.locals.get(local).and_then(|state| state.layout.get()) {
426446
None => {
427-
let layout = from_known_layout(layout, || {
447+
let layout = from_known_layout(self.tcx.tcx, layout, || {
428448
let local_ty = frame.body.local_decls[local].ty;
429449
let local_ty =
430450
self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty);

src/librustc_mir/interpret/operand.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
529529
ty::ConstKind::Value(val_val) => val_val,
530530
};
531531
// Other cases need layout.
532-
let layout = from_known_layout(layout, || self.layout_of(val.ty))?;
532+
let layout = from_known_layout(self.tcx.tcx, layout, || self.layout_of(val.ty))?;
533533
let op = match val_val {
534534
ConstValue::ByRef { alloc, offset } => {
535535
let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc);

src/librustc_mir/interpret/place.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ where
868868
// We do NOT compare the types for equality, because well-typed code can
869869
// actually "transmute" `&mut T` to `&T` in an assignment without a cast.
870870
assert!(
871-
mir_assign_valid_types(src.layout, dest.layout),
871+
mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout),
872872
"type mismatch when copying!\nsrc: {:?},\ndest: {:?}",
873873
src.layout.ty,
874874
dest.layout.ty,
@@ -922,7 +922,7 @@ where
922922
src: OpTy<'tcx, M::PointerTag>,
923923
dest: PlaceTy<'tcx, M::PointerTag>,
924924
) -> InterpResult<'tcx> {
925-
if mir_assign_valid_types(src.layout, dest.layout) {
925+
if mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout) {
926926
// Fast path: Just use normal `copy_op`
927927
return self.copy_op(src, dest);
928928
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// check-pass
2+
#![feature(const_fn)]
3+
4+
const fn nested(x: (for<'a> fn(&'a ()), String)) -> (fn(&'static ()), String) {
5+
x
6+
}
7+
8+
pub const TEST: (fn(&'static ()), String) = nested((|_x| (), String::new()));
9+
10+
fn main() {}

0 commit comments

Comments
 (0)