Merge pull request #286 from RalfJung/mir-validate

Update MIR validation and test it

22 files changed, 93 insertions, 63 deletions

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index e25fb145e75..38917ad1f2a 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<DynamicLifetime, Vec<ValidationQuery<'tcx>>>,
 
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 9c436fb76cc..b200ece4ccf 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<M> Allocation<M> {
             .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<Item=(&'a MemoryRange, &'a mut LockInfo)> + '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<CodeExtent>, 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 6ecaba5ccb8..56195998b9e 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 23ac6bbfcd8..f77c7d65ff7 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<CodeExtent>,
-    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::|\
-<std::heap::Heap as std::heap::Alloc>::|\
-<std::mem::ManuallyDrop<T>><std::heap::AllocErr>::new$|\
-<std::mem::ManuallyDrop<T> as std::ops::DerefMut><std::heap::AllocErr>::deref_mut$|\
-std::sync::atomic::AtomicBool::get_mut$|\
-<std::boxed::Box<T>><[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$|\
-<alloc::raw_vec::RawVec<T, std::heap::Heap>><[a-zA-Z0-9_\\[\\]]+>::into_box$|\
-<std::vec::Vec<T>><[a-zA-Z0-9_\\[\\]]+>::into_boxed_slice$\
-)").unwrap();
+                    std::mem::uninitialized::|\
+                    std::mem::forget::|\
+                    <(std|alloc)::heap::Heap as (std::heap|alloc::allocator)::Alloc>::|\
+                    <std::mem::ManuallyDrop<T>><.*>::new$|\
+                    <std::mem::ManuallyDrop<T> as std::ops::DerefMut><.*>::deref_mut$|\
+                    std::ptr::read::|\
+                    \
+                    <std::sync::Arc<T>><.*>::inner$|\
+                    <std::sync::Arc<T>><.*>::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 163643be01c..88ffc57a8f0 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 96469814be2..912b1bd7d91 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 28d56a2cb62..23f85dbaf3e 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::<usize, fn(i32)>(42)
diff --git a/tests/compile-fail/execute_memory.rs b/tests/compile-fail/execute_memory.rs
index 8d3c9df0320..87d975e1f9d 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 2d240b6a55a..3e4c5d6ad39 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 bde78200b3c..9ce6d44ca46 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 b2bc6722afb..ee245daa310 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 0d594f9bd4c..dbe80fecd00 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 89d69cf7d7f..f030a9c281d 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 dd826c2fd74..0c46acb8ade 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 53c42995b8a..a6d7fdd3552 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 c386e6a528c..7d1829adb5a 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 7d9d31b0dda..6eeb7cd5117 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 c6de3675abe..ba1ef6d7043 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 77f2902917a..d259ca6361c 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 4ac1b5436f5..19ff4b30db1 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<I, F>(iter: I, f: F)
diff --git a/tests/run-pass/std.rs b/tests/run-pass/std.rs
index e0e23812d27..b15307bb48d 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 e744abaadfd..d4b0d06024a 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