Merge pull request #473 from solson/rustup

rustup: Update for rustc validation fixes

Author: RalfJung

rust-toolchain

@@ -1 +1 @@
-nightly-2018-10-11
+nightly-2018-10-14

rustc_tests/src/main.rs

@@ -95,7 +95,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) {
                     if i.attrs.iter().any(|attr| attr.name() == "test") {
                         let did = self.0.hir.body_owner_def_id(body_id);
                         println!("running test: {}", self.0.def_path_debug_str(did));
-                        miri::eval_main(self.0, did, None);
+                        miri::eval_main(self.0, did, None, /*validate*/true);
                         self.1.session.abort_if_errors();
                     }
                 }
@@ -106,7 +106,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) {
         state.hir_crate.unwrap().visit_all_item_likes(&mut Visitor(tcx, state));
     } else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() {
         let entry_def_id = tcx.hir.local_def_id(entry_node_id);
-        miri::eval_main(tcx, entry_def_id, None);
+        miri::eval_main(tcx, entry_def_id, None, /*validate*/true);
 
         state.session.abort_if_errors();
     } else {

src/bin/miri.rs

@@ -25,10 +25,14 @@ use std::path::PathBuf;
 
 struct MiriCompilerCalls {
     default: Box<RustcDefaultCalls>,
-    /// Whether to begin interpretation at the start_fn lang item or not
+
+    /// Whether to begin interpretation at the start_fn lang item or not.
     ///
-    /// If false, the interpretation begins at the `main` function
+    /// If false, the interpretation begins at the `main` function.
     start_fn: bool,
+
+    /// Whether to enforce the validity invariant.
+    validate: bool,
 }
 
 impl<'a> CompilerCalls<'a> for MiriCompilerCalls {
@@ -87,7 +91,9 @@ impl<'a> CompilerCalls<'a> for MiriCompilerCalls {
         let mut control = this.default.build_controller(sess, matches);
         control.after_hir_lowering.callback = Box::new(after_hir_lowering);
         let start_fn = this.start_fn;
-        control.after_analysis.callback = Box::new(move |state| after_analysis(state, start_fn));
+        let validate = this.validate;
+        control.after_analysis.callback =
+            Box::new(move |state| after_analysis(state, start_fn, validate));
         control.after_analysis.stop = Compilation::Stop;
         control
     }
@@ -101,38 +107,43 @@ fn after_hir_lowering(state: &mut CompileState) {
     state.session.plugin_attributes.borrow_mut().push(attr);
 }
 
-fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>, use_start_fn: bool) {
+fn after_analysis<'a, 'tcx>(
+    state: &mut CompileState<'a, 'tcx>,
+    use_start_fn: bool,
+    validate: bool,
+) {
     state.session.abort_if_errors();
 
     let tcx = state.tcx.unwrap();
 
     if std::env::args().any(|arg| arg == "--test") {
-        struct Visitor<'a, 'tcx: 'a>(
-            TyCtxt<'a, 'tcx, 'tcx>,
-            &'a CompileState<'a, 'tcx>
-        );
+        struct Visitor<'a, 'tcx: 'a> {
+            tcx: TyCtxt<'a, 'tcx, 'tcx>,
+            state: &'a CompileState<'a, 'tcx>,
+            validate: bool,
+        };
         impl<'a, 'tcx: 'a, 'hir> itemlikevisit::ItemLikeVisitor<'hir> for Visitor<'a, 'tcx> {
             fn visit_item(&mut self, i: &'hir hir::Item) {
                 if let hir::ItemKind::Fn(.., body_id) = i.node {
                     if i.attrs.iter().any(|attr| {
                         attr.name() == "test"
                     })
                     {
-                        let did = self.0.hir.body_owner_def_id(body_id);
+                        let did = self.tcx.hir.body_owner_def_id(body_id);
                         println!(
                             "running test: {}",
-                            self.0.def_path_debug_str(did),
+                            self.tcx.def_path_debug_str(did),
                         );
-                        miri::eval_main(self.0, did, None);
-                        self.1.session.abort_if_errors();
+                        miri::eval_main(self.tcx, did, None, self.validate);
+                        self.state.session.abort_if_errors();
                     }
                 }
             }
             fn visit_trait_item(&mut self, _trait_item: &'hir hir::TraitItem) {}
             fn visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {}
         }
         state.hir_crate.unwrap().visit_all_item_likes(
-            &mut Visitor(tcx, state),
+            &mut Visitor { tcx, state, validate }
         );
     } else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() {
         let entry_def_id = tcx.hir.local_def_id(entry_node_id);
@@ -142,7 +153,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>, use_start_fn: bo
         } else {
             None
         };
-        miri::eval_main(tcx, entry_def_id, start_wrapper);
+        miri::eval_main(tcx, entry_def_id, start_wrapper, validate);
 
         state.session.abort_if_errors();
     } else {
@@ -221,12 +232,18 @@ fn main() {
     }
 
     let mut start_fn = false;
+    let mut validate = true;
     args.retain(|arg| {
-        if arg == "-Zmiri-start-fn" {
-            start_fn = true;
-            false
-        } else {
-            true
+        match arg.as_str() {
+            "-Zmiri-start-fn" => {
+                start_fn = true;
+                false
+            },
+            "-Zmiri-disable-validation" => {
+                validate = false;
+                false
+            },
+            _ => true
         }
     });
 
@@ -235,6 +252,7 @@ fn main() {
         rustc_driver::run_compiler(&args, Box::new(MiriCompilerCalls {
             default: Box::new(RustcDefaultCalls),
             start_fn,
+            validate,
         }), None, None)
     });
     std::process::exit(result as i32);

src/fn_call.rs

@@ -252,12 +252,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, '
                 // Now we make a function call.  TODO: Consider making this re-usable?  EvalContext::step does sth. similar for the TLS dtors,
                 // and of course eval_main.
                 let mir = self.load_mir(f_instance.def)?;
-                let closure_dest = Place::null(&self);
+                let ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into();
                 self.push_stack_frame(
                     f_instance,
                     mir.span,
                     mir,
-                    closure_dest,
+                    Some(ret_place),
                     StackPopCleanup::Goto(Some(ret)), // directly return to caller
                 )?;
                 let mut args = self.frame().mir.args_iter();

src/intrinsic.rs

@@ -238,7 +238,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "init" => {
                 // Check fast path: we don't want to force an allocation in case the destination is a simple value,
                 // but we also do not want to create a new allocation with 0s and then copy that over.
-                if !dest.layout.is_zst() { // notzhing to do for ZST
+                // FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
+                // However, this only affects direct calls of the intrinsic; calls to the stable
+                // functions wrapping them do get their validation.
+                if !dest.layout.is_zst() { // nothing to do for ZST
                     match dest.layout.abi {
                         layout::Abi::Scalar(ref s) => {
                             let x = Scalar::from_int(0, s.value.size(&self));
@@ -338,7 +341,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
 
             "size_of_val" => {
                 let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
-                let (size, _) = self.size_and_align_of_mplace(mplace)?;
+                let (size, _) = self.size_and_align_of_mplace(mplace)?
+                    .expect("size_of_val called on extern type");
                 let ptr_size = self.pointer_size();
                 self.write_scalar(
                     Scalar::from_uint(size.bytes() as u128, ptr_size),
@@ -349,7 +353,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "min_align_of_val" |
             "align_of_val" => {
                 let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
-                let (_, align) = self.size_and_align_of_mplace(mplace)?;
+                let (_, align) = self.size_and_align_of_mplace(mplace)?
+                    .expect("size_of_val called on extern type");
                 let ptr_size = self.pointer_size();
                 self.write_scalar(
                     Scalar::from_uint(align.abi(), ptr_size),
@@ -397,6 +402,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "uninit" => {
                 // Check fast path: we don't want to force an allocation in case the destination is a simple value,
                 // but we also do not want to create a new allocation with 0s and then copy that over.
+                // FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
+                // However, this only affects direct calls of the intrinsic; calls to the stable
+                // functions wrapping them do get their validation.
                 if !dest.layout.is_zst() { // nothing to do for ZST
                     match dest.layout.abi {
                         layout::Abi::Scalar(..) => {

src/lib.rs

@@ -50,11 +50,12 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     main_id: DefId,
     start_wrapper: Option<DefId>,
+    validate: bool,
 ) -> EvalResult<'tcx, EvalContext<'a, 'mir, 'tcx, Evaluator<'tcx>>> {
     let mut ecx = EvalContext::new(
         tcx.at(syntax::source_map::DUMMY_SP),
         ty::ParamEnv::reveal_all(),
-        Default::default(),
+        Evaluator::new(validate),
         Default::default(),
     );
 
@@ -67,7 +68,6 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
                 .to_owned(),
         ));
     }
-    let ptr_size = ecx.memory.pointer_size();
 
     if let Some(start_id) = start_wrapper {
         let main_ret_ty = ecx.tcx.fn_sig(main_id).output();
@@ -89,16 +89,15 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
         }
 
         // Return value (in static memory so that it does not count as leak)
-        let size = ecx.tcx.data_layout.pointer_size;
-        let align = ecx.tcx.data_layout.pointer_align;
-        let ret_ptr = ecx.memory_mut().allocate(size, align, MiriMemoryKind::MutStatic.into())?;
+        let ret = ecx.layout_of(start_mir.return_ty())?;
+        let ret_ptr = ecx.allocate(ret, MiriMemoryKind::MutStatic.into())?;
 
         // Push our stack frame
         ecx.push_stack_frame(
             start_instance,
             start_mir.span,
             start_mir,
-            Place::from_ptr(ret_ptr, align),
+            Some(ret_ptr.into()),
             StackPopCleanup::None { cleanup: true },
         )?;
 
@@ -126,11 +125,12 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
 
         assert!(args.next().is_none(), "start lang item has more arguments than expected");
     } else {
+        let ret_place = MPlaceTy::dangling(ecx.layout_of(tcx.mk_unit())?, &ecx).into();
         ecx.push_stack_frame(
             main_instance,
             main_mir.span,
             main_mir,
-            Place::from_scalar_ptr(Scalar::from_int(1, ptr_size).into(), ty::layout::Align::from_bytes(1, 1).unwrap()),
+            Some(ret_place),
             StackPopCleanup::None { cleanup: true },
         )?;
 
@@ -146,8 +146,9 @@ pub fn eval_main<'a, 'tcx: 'a>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     main_id: DefId,
     start_wrapper: Option<DefId>,
+    validate: bool,
 ) {
-    let mut ecx = create_ecx(tcx, main_id, start_wrapper).expect("Couldn't create ecx");
+    let mut ecx = create_ecx(tcx, main_id, start_wrapper, validate).expect("Couldn't create ecx");
 
     let res: EvalResult = (|| {
         ecx.run()?;
@@ -222,14 +223,27 @@ impl Into<MemoryKind<MiriMemoryKind>> for MiriMemoryKind {
 }
 
 
-#[derive(Clone, Default, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 pub struct Evaluator<'tcx> {
     /// Environment variables set by `setenv`
     /// Miri does not expose env vars from the host to the emulated program
     pub(crate) env_vars: HashMap<Vec<u8>, Pointer>,
 
     /// TLS state
     pub(crate) tls: TlsData<'tcx>,
+
+    /// Whether to enforce the validity invariant
+    pub(crate) validate: bool,
+}
+
+impl<'tcx> Evaluator<'tcx> {
+    fn new(validate: bool) -> Self {
+        Evaluator {
+            env_vars: HashMap::default(),
+            tls: TlsData::default(),
+            validate,
+        }
+    }
 }
 
 impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
@@ -240,7 +254,29 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
     type MemoryMap = MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<()>)>;
 
     const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::MutStatic);
-    const ENFORCE_VALIDITY: bool = false; // this is still WIP
+
+    fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool {
+        if !ecx.machine.validate {
+            return false;
+        }
+
+        // Some functions are whitelisted until we figure out how to fix them.
+        // We walk up the stack a few frames to also cover their callees.
+        const WHITELIST: &[&str] = &[
+            // Uses mem::uninitialized
+            "std::ptr::read",
+            "std::sys::windows::mutex::Mutex::",
+        ];
+        for frame in ecx.stack().iter()
+            .rev().take(3)
+        {
+            let name = frame.instance.to_string();
+            if WHITELIST.iter().any(|white| name.starts_with(white)) {
+                return false;
+            }
+        }
+        true
+    }
 
     /// Returns Ok() when the function was handled, fail otherwise
     fn find_fn(
@@ -286,7 +322,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
             malloc,
             malloc_mir.span,
             malloc_mir,
-            *dest,
+            Some(dest),
             // Don't do anything when we are done.  The statement() function will increment
             // the old stack frame's stmt counter to the next statement, which means that when
             // exchange_malloc returns, we go on evaluating exactly where we want to be.

src/tls.rs

@@ -1,9 +1,12 @@
 use std::collections::BTreeMap;
 
+use rustc_target::abi::LayoutOf;
 use rustc::{ty, ty::layout::HasDataLayout, mir};
 
-use super::{EvalResult, EvalErrorKind, Scalar, Evaluator,
-            Place, StackPopCleanup, EvalContext};
+use super::{
+    EvalResult, EvalErrorKind, StackPopCleanup, EvalContext, Evaluator,
+    MPlaceTy, Scalar,
+};
 
 pub type TlsKey = u128;
 
@@ -139,12 +142,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for EvalContext<'a, 'mir, '
             // TODO: Potentially, this has to support all the other possible instances?
             // See eval_fn_call in interpret/terminator/mod.rs
             let mir = self.load_mir(instance.def)?;
-            let ret = Place::null(&self);
+            let ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into();
             self.push_stack_frame(
                 instance,
                 mir.span,
                 mir,
-                ret,
+                Some(ret_place),
                 StackPopCleanup::None { cleanup: true },
             )?;
             let arg_local = self.frame().mir.args_iter().next().ok_or_else(

tests/compile-fail/cast_box_int_to_fn_ptr.rs

@@ -1,5 +1,5 @@
 // Validation makes this fail in the wrong place
-// compile-flags: -Zmir-emit-validate=0
+// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
 
 fn main() {
     let b = Box::new(42);

tests/compile-fail/cast_int_to_fn_ptr.rs

@@ -1,5 +1,5 @@
 // Validation makes this fail in the wrong place
-// compile-flags: -Zmir-emit-validate=0
+// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
 
 fn main() {
     let g = unsafe {

tests/compile-fail/execute_memory.rs

@@ -1,5 +1,5 @@
 // Validation makes this fail in the wrong place
-// compile-flags: -Zmir-emit-validate=0
+// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
 
 #![feature(box_syntax)]