diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index e25fb145e753e..38917ad1f2af5 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -37,8 +37,6 @@ pub struct EvalContext<'a, 'tcx: 'a, M: Machine<'tcx>> { /// The virtual memory system. pub memory: Memory<'a, 'tcx, M>, - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch /// Lvalues that were suspended by the validation subsystem, and will be recovered later pub(crate) suspended: HashMap>>, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 9c436fb76cccd..b200ece4ccf9c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -35,8 +35,6 @@ mod range { } impl MemoryRange { - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch pub fn new(offset: u64, len: u64) -> MemoryRange { assert!(len > 0); MemoryRange { @@ -61,8 +59,6 @@ mod range { left..right } - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch pub fn contained_in(&self, offset: u64, len: u64) -> bool { assert!(len > 0); offset <= self.start && self.end <= (offset + len) @@ -143,8 +139,6 @@ impl Allocation { .filter(move |&(range, _)| range.overlaps(offset, len)) } - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch fn iter_locks_mut<'a>(&'a mut self, offset: u64, len: u64) -> impl Iterator + 'a { self.locks.range_mut(MemoryRange::range(offset, len)) .filter(move |&(range, _)| range.overlaps(offset, len)) @@ -474,8 +468,6 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { .map_err(|lock| EvalErrorKind::MemoryLockViolation { ptr, len, frame, access, lock }.into()) } - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch /// Acquire the lock for the given lifetime pub(crate) fn acquire_lock(&mut self, ptr: MemoryPointer, len: u64, region: Option, kind: AccessKind) -> EvalResult<'tcx> { use std::collections::btree_map::Entry::*; @@ -504,8 +496,6 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { Ok(()) } - #[allow(dead_code)] - // FIXME(@RalfJung): validation branch /// Release a write lock prematurely. If there's a read lock or someone else's lock, fail. pub(crate) fn release_write_lock(&mut self, ptr: MemoryPointer, len: u64) -> EvalResult<'tcx> { assert!(len > 0); diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 6ecaba5ccb857..56195998b9e7a 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -133,8 +133,15 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { self.deallocate_local(old_val)?; } - // NOPs for now. - EndRegion(_ce) => {} + // Validity checks. + Validate(op, ref lvalues) => { + for operand in lvalues { + self.validation_op(op, operand)?; + } + } + EndRegion(ce) => { + self.end_region(ce)?; + } // Defined to do nothing. These are added by optimization passes, to avoid changing the // size of MIR constantly. diff --git a/src/librustc_mir/interpret/validation.rs b/src/librustc_mir/interpret/validation.rs index 23ac6bbfcd841..f77c7d65ff7de 100644 --- a/src/librustc_mir/interpret/validation.rs +++ b/src/librustc_mir/interpret/validation.rs @@ -1,9 +1,6 @@ -// code for @RalfJung's validation branch is dead for now -#![allow(dead_code)] - use rustc::hir::Mutability; use rustc::hir::Mutability::*; -use rustc::mir; +use rustc::mir::{self, ValidationOp, ValidationOperand}; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::ty::subst::Subst; use rustc::traits::Reveal; @@ -14,28 +11,11 @@ use super::{ EvalError, EvalResult, EvalErrorKind, EvalContext, DynamicLifetime, AccessKind, LockInfo, - PrimVal, Value, + Value, Lvalue, LvalueExtra, Machine, }; -// FIXME remove this once it lands in rustc -#[derive(Copy, Clone, PartialEq, Eq)] -pub enum ValidationOp { - Acquire, - Release, - Suspend(CodeExtent), -} - -#[derive(Clone, Debug)] -pub struct ValidationOperand<'tcx, T> { - pub lval: T, - pub ty: Ty<'tcx>, - pub re: Option, - pub mutbl: Mutability, -} -// FIXME end - pub type ValidationQuery<'tcx> = ValidationOperand<'tcx, Lvalue<'tcx>>; #[derive(Copy, Clone, Debug)] @@ -59,26 +39,35 @@ impl ValidationMode { // Validity checks impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { pub(crate) fn validation_op(&mut self, op: ValidationOp, operand: &ValidationOperand<'tcx, mir::Lvalue<'tcx>>) -> EvalResult<'tcx> { + // If mir-emit-validate is set to 0 (i.e., disabled), we may still see validation commands + // because other crates may have been compiled with mir-emit-validate > 0. Ignore those + // commands. This makes mir-emit-validate also a flag to control whether miri will do + // validation or not. + if self.tcx.sess.opts.debugging_opts.mir_emit_validate == 0 { + return Ok(()); + } + // HACK: Determine if this method is whitelisted and hence we do not perform any validation. + // We currently insta-UB on anything passing around uninitialized memory, so we have to whitelist + // the places that are allowed to do that. + // The second group is stuff libstd does that is forbidden even under relaxed validation. { // The regexp we use for filtering use regex::Regex; lazy_static! { static ref RE: Regex = Regex::new("^(\ -std::mem::swap::|\ -std::mem::uninitialized::|\ -std::ptr::read::|\ -std::panicking::try::do_call::|\ -std::slice::from_raw_parts_mut::|\ -::|\ ->::new$|\ - as std::ops::DerefMut>::deref_mut$|\ -std::sync::atomic::AtomicBool::get_mut$|\ -><[a-zA-Z0-9_\\[\\]]+>::from_raw|\ -<[a-zA-Z0-9_:<>]+ as std::slice::SliceIndex<[a-zA-Z0-9_\\[\\]]+>><[a-zA-Z0-9_\\[\\]]+>::get_unchecked_mut$|\ -><[a-zA-Z0-9_\\[\\]]+>::into_box$|\ -><[a-zA-Z0-9_\\[\\]]+>::into_boxed_slice$\ -)").unwrap(); + std::mem::uninitialized::|\ + std::mem::forget::|\ + <(std|alloc)::heap::Heap as (std::heap|alloc::allocator)::Alloc>::|\ + ><.*>::new$|\ + as std::ops::DerefMut><.*>::deref_mut$|\ + std::ptr::read::|\ + \ + ><.*>::inner$|\ + ><.*>::drop_slow$|\ + (std::heap|alloc::allocator)::Layout::for_value::|\ + std::mem::(size|align)_of_val::\ + )").unwrap(); } // Now test let name = self.stack[self.cur_frame()].instance.to_string(); @@ -167,10 +156,11 @@ std::sync::atomic::AtomicBool::get_mut$|\ fn validate(&mut self, query: ValidationQuery<'tcx>, mode: ValidationMode) -> EvalResult<'tcx> { match self.try_validate(query, mode) { - // HACK: If, during releasing, we hit memory we cannot use, we just ignore that. - // This can happen because releases are added before drop elaboration. - // TODO: Fix the MIR so that these releases do not happen. - res @ Err(EvalError{ kind: EvalErrorKind::DanglingPointerDeref, ..}) | + // Releasing an uninitalized variable is a NOP. This is needed because + // we have to release the return value of a function; due to destination-passing-style + // the callee may directly write there. + // TODO: Ideally we would know whether the destination is already initialized, and only + // release if it is. res @ Err(EvalError{ kind: EvalErrorKind::ReadUndefBytes, ..}) => { if let ValidationMode::Release = mode { return Ok(()); @@ -199,18 +189,13 @@ std::sync::atomic::AtomicBool::get_mut$|\ } // HACK: For now, bail out if we hit a dead local during recovery (can happen because sometimes we have - // StorageDead before EndRegion). + // StorageDead before EndRegion due to https://github.com/rust-lang/rust/issues/43481). // TODO: We should rather fix the MIR. - // HACK: Releasing on dead/undef local variables is a NOP. This can happen because of releases being added - // before drop elaboration. - // TODO: Fix the MIR so that these releases do not happen. match query.lval { Lvalue::Local { frame, local } => { let res = self.stack[frame].get_local(local); match (res, mode) { - (Err(EvalError{ kind: EvalErrorKind::DeadLocal, ..}), ValidationMode::Recover(_)) | - (Err(EvalError{ kind: EvalErrorKind::DeadLocal, ..}), ValidationMode::Release) | - (Ok(Value::ByVal(PrimVal::Undef)), ValidationMode::Release) => { + (Err(EvalError{ kind: EvalErrorKind::DeadLocal, ..}), ValidationMode::Recover(_)) => { return Ok(()); } _ => {}, diff --git a/src/librustc_mir/interpret/value.rs b/src/librustc_mir/interpret/value.rs index 163643be01c32..88ffc57a8f068 100644 --- a/src/librustc_mir/interpret/value.rs +++ b/src/librustc_mir/interpret/value.rs @@ -197,6 +197,7 @@ impl<'a, 'tcx: 'a> Value { ByValPair(ptr, vtable) => Ok((ptr.into(), vtable.to_ptr()?)), + ByVal(PrimVal::Undef) => err!(ReadUndefBytes), _ => bug!("expected ptr and vtable, got {:?}", self), } } @@ -216,6 +217,7 @@ impl<'a, 'tcx: 'a> Value { assert_eq!(len as u64 as u128, len); Ok((ptr.into(), len as u64)) }, + ByVal(PrimVal::Undef) => err!(ReadUndefBytes), ByVal(_) => bug!("expected ptr and length, got {:?}", self), } } diff --git a/tests/compile-fail/cast_box_int_to_fn_ptr.rs b/tests/compile-fail/cast_box_int_to_fn_ptr.rs index 96469814be29b..912b1bd7d91f2 100644 --- a/tests/compile-fail/cast_box_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_box_int_to_fn_ptr.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 + fn main() { let b = Box::new(42); let g = unsafe { diff --git a/tests/compile-fail/cast_int_to_fn_ptr.rs b/tests/compile-fail/cast_int_to_fn_ptr.rs index 28d56a2cb6271..23f85dbaf3ecb 100644 --- a/tests/compile-fail/cast_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_int_to_fn_ptr.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 + fn main() { let g = unsafe { std::mem::transmute::(42) diff --git a/tests/compile-fail/execute_memory.rs b/tests/compile-fail/execute_memory.rs index 8d3c9df0320ba..87d975e1f9d48 100644 --- a/tests/compile-fail/execute_memory.rs +++ b/tests/compile-fail/execute_memory.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 + #![feature(box_syntax)] fn main() { diff --git a/tests/compile-fail/fn_ptr_offset.rs b/tests/compile-fail/fn_ptr_offset.rs index 2d240b6a55ade..3e4c5d6ad3916 100644 --- a/tests/compile-fail/fn_ptr_offset.rs +++ b/tests/compile-fail/fn_ptr_offset.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 + use std::mem; fn f() {} diff --git a/tests/compile-fail/invalid_enum_discriminant.rs b/tests/compile-fail/invalid_enum_discriminant.rs index bde78200b3c47..9ce6d44ca460f 100644 --- a/tests/compile-fail/invalid_enum_discriminant.rs +++ b/tests/compile-fail/invalid_enum_discriminant.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 + #[repr(C)] pub enum Foo { A, B, C, D diff --git a/tests/compile-fail/memleak_rc.rs b/tests/compile-fail/memleak_rc.rs index b2bc6722afb04..ee245daa31069 100644 --- a/tests/compile-fail/memleak_rc.rs +++ b/tests/compile-fail/memleak_rc.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + //error-pattern: the evaluated program leaked memory use std::rc::Rc; diff --git a/tests/compile-fail/panic.rs b/tests/compile-fail/panic.rs index 0d594f9bd4c3b..dbe80fecd00f1 100644 --- a/tests/compile-fail/panic.rs +++ b/tests/compile-fail/panic.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + //error-pattern: the evaluated program panicked fn main() { diff --git a/tests/compile-fail/static_memory_modification2.rs b/tests/compile-fail/static_memory_modification2.rs index 89d69cf7d7f4b..f030a9c281de7 100644 --- a/tests/compile-fail/static_memory_modification2.rs +++ b/tests/compile-fail/static_memory_modification2.rs @@ -1,3 +1,6 @@ +// Validation detects that we are casting & to &mut and so it changes why we fail +// compile-flags: -Zmir-emit-validate=0 + use std::mem::transmute; #[allow(mutable_transmutes)] diff --git a/tests/compile-fail/zst2.rs b/tests/compile-fail/zst2.rs index dd826c2fd74eb..0c46acb8ade40 100644 --- a/tests/compile-fail/zst2.rs +++ b/tests/compile-fail/zst2.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + // error-pattern: the evaluated program panicked #[derive(Debug)] diff --git a/tests/compile-fail/zst3.rs b/tests/compile-fail/zst3.rs index 53c42995b8a12..a6d7fdd35521d 100644 --- a/tests/compile-fail/zst3.rs +++ b/tests/compile-fail/zst3.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + // error-pattern: the evaluated program panicked #[derive(Debug)] diff --git a/tests/compiletest.rs b/tests/compiletest.rs index c386e6a528c77..7d1829adb5a4d 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -20,6 +20,7 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, fullmir: b let mut config = compiletest::default_config(); config.mode = "compile-fail".parse().expect("Invalid mode"); config.rustc_path = MIRI_PATH.into(); + let mut flags = Vec::new(); if fullmir { if host != target { // skip fullmir on nonhost @@ -32,6 +33,8 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, fullmir: b config.target_rustcflags = Some(format!("--sysroot {}", sysroot.to_str().unwrap())); config.src_base = PathBuf::from(path.to_string()); } + flags.push("-Zmir-emit-validate=1".to_owned()); + config.target_rustcflags = Some(flags.join(" ")); config.target = target.to_owned(); compiletest::run_tests(&config); } @@ -72,6 +75,8 @@ fn miri_pass(path: &str, target: &str, host: &str, fullmir: bool, opt: bool) { flags.push("-Zmir-opt-level=3".to_owned()); } else { flags.push("-Zmir-opt-level=0".to_owned()); + // For now, only validate without optimizations. Inlining breaks validation. + flags.push("-Zmir-emit-validate=1".to_owned()); } config.target_rustcflags = Some(flags.join(" ")); // don't actually execute the final binary, it might be for other targets and we only care diff --git a/tests/run-pass-fullmir/regions-mock-trans.rs b/tests/run-pass-fullmir/regions-mock-trans.rs index 7d9d31b0dda19..6eeb7cd5117e9 100644 --- a/tests/run-pass-fullmir/regions-mock-trans.rs +++ b/tests/run-pass-fullmir/regions-mock-trans.rs @@ -8,7 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// pretty-expanded FIXME #23616 +// FIXME: We handle uninitialzied storage here, which currently makes validation fail. +// compile-flags: -Zmir-emit-validate=0 #![feature(libc)] diff --git a/tests/run-pass/rc.rs b/tests/run-pass/rc.rs index c6de3675abe8c..ba1ef6d704314 100644 --- a/tests/run-pass/rc.rs +++ b/tests/run-pass/rc.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + use std::cell::RefCell; use std::rc::Rc; diff --git a/tests/run-pass/recursive_static.rs b/tests/run-pass/recursive_static.rs index 77f2902917a1c..d259ca6361c97 100644 --- a/tests/run-pass/recursive_static.rs +++ b/tests/run-pass/recursive_static.rs @@ -1,3 +1,6 @@ +// FIXME: Disable validation until we figure out how to handle recursive statics. +// compile-flags: -Zmir-emit-validate=0 + struct S(&'static S); static S1: S = S(&S2); static S2: S = S(&S1); diff --git a/tests/run-pass/send-is-not-static-par-for.rs b/tests/run-pass/send-is-not-static-par-for.rs index 4ac1b5436f522..19ff4b30db1d7 100644 --- a/tests/run-pass/send-is-not-static-par-for.rs +++ b/tests/run-pass/send-is-not-static-par-for.rs @@ -10,6 +10,9 @@ //ignore-windows +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + use std::sync::Mutex; fn par_for(iter: I, f: F) diff --git a/tests/run-pass/std.rs b/tests/run-pass/std.rs index e0e23812d275e..b15307bb48d83 100644 --- a/tests/run-pass/std.rs +++ b/tests/run-pass/std.rs @@ -1,3 +1,6 @@ +// FIXME: Due to https://github.com/rust-lang/rust/issues/43457 we have to disable validation +// compile-flags: -Zmir-emit-validate=0 + use std::cell::{Cell, RefCell}; use std::rc::Rc; use std::sync::Arc; diff --git a/xargo/build.sh b/xargo/build.sh index e744abaadfdfe..d4b0d06024afd 100755 --- a/xargo/build.sh +++ b/xargo/build.sh @@ -1,3 +1,3 @@ #!/bin/bash cd "$(readlink -e "$(dirname "$0")")" -RUSTFLAGS='-Zalways-encode-mir' xargo build +RUSTFLAGS='-Zalways-encode-mir -Zmir-emit-validate=1' xargo build