From ce5b183e8bbaa7a2fa7d20077808027e0cc5bd78 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 9 Oct 2018 22:03:14 +0200 Subject: [PATCH 01/12] update for new return place handling --- src/fn_call.rs | 3 +-- src/lib.rs | 13 ++++++------- src/tls.rs | 11 +++++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 3e795f16533..9b6a9c67b16 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -252,12 +252,11 @@ fn emulate_foreign_item( // 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); self.push_stack_frame( f_instance, mir.span, mir, - closure_dest, + None, StackPopCleanup::Goto(Some(ret)), // directly return to caller )?; let mut args = self.frame().mir.args_iter(); diff --git a/src/lib.rs b/src/lib.rs index 81a2ceead0a..04405f383d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,7 +67,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 +88,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 +124,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 }, )?; @@ -286,7 +285,7 @@ fn box_alloc( 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. diff --git a/src/tls.rs b/src/tls.rs index a1ddaf64cc0..c04f7a9c350 100644 --- a/src/tls.rs +++ b/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 @@ fn run_tls_dtors(&mut self) -> EvalResult<'tcx> { // 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( From 791f464ea02d017f6499627223a1981b6531ca48 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 9 Oct 2018 22:35:14 +0200 Subject: [PATCH 02/12] update for size_and_align considering extern types --- src/intrinsic.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/intrinsic.rs b/src/intrinsic.rs index b7338d4d521..2128ff5d7ac 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -238,7 +238,10 @@ fn call_intrinsic( "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 @@ fn call_intrinsic( "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 @@ fn call_intrinsic( "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 @@ fn call_intrinsic( "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(..) => { From a090edbc03b95eb21f75675c026e674d646cae74 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 10 Oct 2018 09:06:44 +0200 Subject: [PATCH 03/12] explain a test --- tests/run-pass/ref-invalid-ptr.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/run-pass/ref-invalid-ptr.rs b/tests/run-pass/ref-invalid-ptr.rs index ebbbb77748d..8edc944c7c8 100644 --- a/tests/run-pass/ref-invalid-ptr.rs +++ b/tests/run-pass/ref-invalid-ptr.rs @@ -1,7 +1,9 @@ fn main() { let x = 2usize as *const u32; + // this is not aligned, but we immediately cast it to a raw ptr so that must be okay let _y = unsafe { &*x as *const u32 }; let x = 0usize as *const u32; + // this is NULL, but we immediately cast it to a raw ptr so that must be okay let _y = unsafe { &*x as *const u32 }; } From d94d32e937c672317527cb440788138f688bf3c4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 11 Oct 2018 09:02:28 +0200 Subject: [PATCH 04/12] enforce_validity became a function --- src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 04405f383d1..965810d0e40 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,7 +239,11 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { type MemoryMap = MonoHashMap, Allocation<()>)>; const STATIC_KIND: Option = Some(MiriMemoryKind::MutStatic); - const ENFORCE_VALIDITY: bool = false; // this is still WIP + + #[inline(always)] + fn enforce_validity(_ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool { + false // this is still WIP + } /// Returns Ok() when the function was handled, fail otherwise fn find_fn( From e4dfb7013b71b9c6f573aed24e76eab5fdf088e0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 11 Oct 2018 11:24:22 +0200 Subject: [PATCH 05/12] enable validation per default, but add a flag to disable it and use that for some run-pass tests compile-fail does not do validation yet --- src/bin/miri.rs | 56 ++++++++++++------- src/lib.rs | 25 +++++++-- tests/compiletest.rs | 1 + tests/run-pass/btreemap.rs | 6 +- .../run-pass/call_drop_through_owned_slice.rs | 3 + tests/run-pass/issue-29746.rs | 3 + tests/run-pass/rc.rs | 3 + tests/run-pass/ref-invalid-ptr.rs | 3 + tests/run-pass/sendable-class.rs | 3 + tests/run-pass/unique-send.rs | 3 + 10 files changed, 80 insertions(+), 26 deletions(-) diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 57d49b2e6bc..d7207da0b3c 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -25,10 +25,14 @@ struct MiriCompilerCalls { default: Box, - /// 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 @@ fn build_controller( 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,16 +107,21 @@ 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 { @@ -118,13 +129,13 @@ fn visit_item(&mut self, i: &'hir hir::Item) { 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(); } } } @@ -132,7 +143,7 @@ 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 visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {} } 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); diff --git a/src/lib.rs b/src/lib.rs index 965810d0e40..13b42d8e3c2 100644 --- a/src/lib.rs +++ b/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, + 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(), ); @@ -145,8 +146,9 @@ pub fn eval_main<'a, 'tcx: 'a>( tcx: TyCtxt<'a, 'tcx, 'tcx>, main_id: DefId, start_wrapper: Option, + 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()?; @@ -221,7 +223,7 @@ fn into(self) -> MemoryKind { } -#[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 @@ -229,6 +231,19 @@ pub struct Evaluator<'tcx> { /// 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> { @@ -241,8 +256,8 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { const STATIC_KIND: Option = Some(MiriMemoryKind::MutStatic); #[inline(always)] - fn enforce_validity(_ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool { - false // this is still WIP + fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool { + ecx.machine.validate } /// Returns Ok() when the function was handled, fail otherwise diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 82a2144a337..151aa89be3f 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -64,6 +64,7 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, need_fullm flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs config.src_base = PathBuf::from(path.to_string()); flags.push("-Zmir-emit-validate=1".to_owned()); + flags.push("-Zmiri-disable-validation".to_owned()); config.target_rustcflags = Some(flags.join(" ")); config.target = target.to_owned(); config.host = host.to_owned(); diff --git a/tests/run-pass/btreemap.rs b/tests/run-pass/btreemap.rs index 0fd28d6f1e8..dc0fa0987ae 100644 --- a/tests/run-pass/btreemap.rs +++ b/tests/run-pass/btreemap.rs @@ -1,5 +1,5 @@ -// mir validation can't cope with `mem::uninitialized()`, so this test fails with validation & full-MIR. -// compile-flags: -Zmir-emit-validate=0 +// FIXME: Validation disabled due to https://github.com/rust-lang/rust/issues/54957 +// compile-flags: -Zmiri-disable-validation #[derive(PartialEq, Eq, PartialOrd, Ord)] pub enum Foo { @@ -14,4 +14,6 @@ pub fn main() { b.insert(Foo::A("/=")); b.insert(Foo::A("#")); b.insert(Foo::A("0o")); + assert!(b.remove(&Foo::A("/="))); + assert!(!b.remove(&Foo::A("/="))); } diff --git a/tests/run-pass/call_drop_through_owned_slice.rs b/tests/run-pass/call_drop_through_owned_slice.rs index 3ec6be65ed8..b0e336c0480 100644 --- a/tests/run-pass/call_drop_through_owned_slice.rs +++ b/tests/run-pass/call_drop_through_owned_slice.rs @@ -1,3 +1,6 @@ +// FIXME validation disabled because ptr::read uses mem::uninitialized +// compile-flags: -Zmiri-disable-validation + struct Bar; static mut DROP_COUNT: usize = 0; diff --git a/tests/run-pass/issue-29746.rs b/tests/run-pass/issue-29746.rs index 61c601ac6a9..94ca146db1c 100644 --- a/tests/run-pass/issue-29746.rs +++ b/tests/run-pass/issue-29746.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// FIXME validation disabled because ptr::read uses mem::uninitialized +// compile-flags: -Zmiri-disable-validation + // zip!(a1,a2,a3,a4) is equivalent to: // a1.zip(a2).zip(a3).zip(a4).map(|(((x1,x2),x3),x4)| (x1,x2,x3,x4)) macro_rules! zip { diff --git a/tests/run-pass/rc.rs b/tests/run-pass/rc.rs index 0bf70750311..4d89066035c 100644 --- a/tests/run-pass/rc.rs +++ b/tests/run-pass/rc.rs @@ -1,3 +1,6 @@ +// FIXME: Validation disabled due to https://github.com/rust-lang/rust/issues/54908 +// compile-flags: -Zmiri-disable-validation + use std::cell::RefCell; use std::rc::Rc; diff --git a/tests/run-pass/ref-invalid-ptr.rs b/tests/run-pass/ref-invalid-ptr.rs index 8edc944c7c8..627c821a9f3 100644 --- a/tests/run-pass/ref-invalid-ptr.rs +++ b/tests/run-pass/ref-invalid-ptr.rs @@ -1,3 +1,6 @@ +// FIXME validation disabled because it checks these references too eagerly +// compile-flags: -Zmiri-disable-validation + fn main() { let x = 2usize as *const u32; // this is not aligned, but we immediately cast it to a raw ptr so that must be okay diff --git a/tests/run-pass/sendable-class.rs b/tests/run-pass/sendable-class.rs index 66f0c84e23c..7ca4e1a9084 100644 --- a/tests/run-pass/sendable-class.rs +++ b/tests/run-pass/sendable-class.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// FIXME validation disabled because ptr::read uses mem::uninitialized +// compile-flags: -Zmiri-disable-validation + // Test that a class with only sendable fields can be sent use std::sync::mpsc::channel; diff --git a/tests/run-pass/unique-send.rs b/tests/run-pass/unique-send.rs index 7644da08e4a..8a48d331f45 100644 --- a/tests/run-pass/unique-send.rs +++ b/tests/run-pass/unique-send.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// FIXME validation disabled because ptr::read uses mem::uninitialized +// compile-flags: -Zmiri-disable-validation + #![feature(box_syntax)] use std::sync::mpsc::channel; From 1846f111c987e160f880c6893b834c24ff5b816c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 11 Oct 2018 15:28:21 +0200 Subject: [PATCH 06/12] fix return place for __rust_maybe_catch_panic --- src/fn_call.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 9b6a9c67b16..812df49b0b8 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -252,11 +252,12 @@ fn emulate_foreign_item( // 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 ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into(); self.push_stack_frame( f_instance, mir.span, mir, - None, + Some(ret_place), StackPopCleanup::Goto(Some(ret)), // directly return to caller )?; let mut args = self.frame().mir.args_iter(); From 26f9d617c347185433b77c481a5c50c55d9b72ce Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 11 Oct 2018 16:10:04 +0200 Subject: [PATCH 07/12] do not validate start-fn code --- tests/compiletest.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 151aa89be3f..43989a49b30 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -103,6 +103,8 @@ fn miri_pass(sysroot: &Path, path: &str, target: &str, host: &str, need_fullmir: flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs if have_fullmir() { flags.push("-Zmiri-start-fn".to_owned()); + // start-fn uses ptr::read, and so fails validation + flags.push("-Zmiri-disable-validation".to_owned()); } if opt { flags.push("-Zmir-opt-level=3".to_owned()); From 62b819ba18a4a32833bb823afeb32778fd28bbde Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 12 Oct 2018 10:40:44 +0200 Subject: [PATCH 08/12] whitelist std::ptr::read --- src/lib.rs | 21 +++++++++++++++++-- tests/compiletest.rs | 2 -- .../run-pass/call_drop_through_owned_slice.rs | 3 --- tests/run-pass/issue-29746.rs | 3 --- tests/run-pass/sendable-class.rs | 3 --- tests/run-pass/unique-send.rs | 3 --- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 13b42d8e3c2..d52fd002803 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -255,9 +255,26 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { const STATIC_KIND: Option = Some(MiriMemoryKind::MutStatic); - #[inline(always)] fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool { - ecx.machine.validate + 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", + ]; + 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 diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 43989a49b30..151aa89be3f 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -103,8 +103,6 @@ fn miri_pass(sysroot: &Path, path: &str, target: &str, host: &str, need_fullmir: flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs if have_fullmir() { flags.push("-Zmiri-start-fn".to_owned()); - // start-fn uses ptr::read, and so fails validation - flags.push("-Zmiri-disable-validation".to_owned()); } if opt { flags.push("-Zmir-opt-level=3".to_owned()); diff --git a/tests/run-pass/call_drop_through_owned_slice.rs b/tests/run-pass/call_drop_through_owned_slice.rs index b0e336c0480..3ec6be65ed8 100644 --- a/tests/run-pass/call_drop_through_owned_slice.rs +++ b/tests/run-pass/call_drop_through_owned_slice.rs @@ -1,6 +1,3 @@ -// FIXME validation disabled because ptr::read uses mem::uninitialized -// compile-flags: -Zmiri-disable-validation - struct Bar; static mut DROP_COUNT: usize = 0; diff --git a/tests/run-pass/issue-29746.rs b/tests/run-pass/issue-29746.rs index 94ca146db1c..61c601ac6a9 100644 --- a/tests/run-pass/issue-29746.rs +++ b/tests/run-pass/issue-29746.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME validation disabled because ptr::read uses mem::uninitialized -// compile-flags: -Zmiri-disable-validation - // zip!(a1,a2,a3,a4) is equivalent to: // a1.zip(a2).zip(a3).zip(a4).map(|(((x1,x2),x3),x4)| (x1,x2,x3,x4)) macro_rules! zip { diff --git a/tests/run-pass/sendable-class.rs b/tests/run-pass/sendable-class.rs index 7ca4e1a9084..66f0c84e23c 100644 --- a/tests/run-pass/sendable-class.rs +++ b/tests/run-pass/sendable-class.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME validation disabled because ptr::read uses mem::uninitialized -// compile-flags: -Zmiri-disable-validation - // Test that a class with only sendable fields can be sent use std::sync::mpsc::channel; diff --git a/tests/run-pass/unique-send.rs b/tests/run-pass/unique-send.rs index 8a48d331f45..7644da08e4a 100644 --- a/tests/run-pass/unique-send.rs +++ b/tests/run-pass/unique-send.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME validation disabled because ptr::read uses mem::uninitialized -// compile-flags: -Zmiri-disable-validation - #![feature(box_syntax)] use std::sync::mpsc::channel; From c9cf0344eed9a5a475c86a7a1e8be426d1f899ef Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 12 Oct 2018 10:54:37 +0200 Subject: [PATCH 09/12] enable validation for compile-fail tests, and add some new ones --- tests/compile-fail/cast_box_int_to_fn_ptr.rs | 2 +- tests/compile-fail/cast_int_to_fn_ptr.rs | 2 +- tests/compile-fail/execute_memory.rs | 2 +- tests/compile-fail/fn_ptr_offset.rs | 2 +- tests/compile-fail/invalid_bool.rs | 8 +------- tests/compile-fail/invalid_bool2.rs | 3 +++ tests/compile-fail/invalid_char.rs | 8 ++++++++ .../{match_char2.rs => invalid_char2.rs} | 3 +++ tests/compile-fail/invalid_enum_discriminant.rs | 11 +---------- tests/compile-fail/invalid_enum_discriminant2.rs | 2 +- tests/compile-fail/match_char.rs | 13 ------------- tests/compile-fail/never_say_never.rs | 2 +- tests/compile-fail/never_transmute_humans.rs | 2 +- tests/compile-fail/never_transmute_void.rs | 2 +- tests/compile-fail/reference_to_packed.rs | 2 +- tests/compile-fail/storage_dead_dangling.rs | 3 +++ tests/compile-fail/validation_cast_fn_ptr1.rs | 10 ++++++++++ tests/compile-fail/validation_cast_fn_ptr2.rs | 10 ++++++++++ tests/compiletest.rs | 1 - 19 files changed, 48 insertions(+), 40 deletions(-) create mode 100644 tests/compile-fail/invalid_char.rs rename tests/compile-fail/{match_char2.rs => invalid_char2.rs} (65%) delete mode 100644 tests/compile-fail/match_char.rs create mode 100644 tests/compile-fail/validation_cast_fn_ptr1.rs create mode 100644 tests/compile-fail/validation_cast_fn_ptr2.rs 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 2a317f579f5..cbf370e0236 100644 --- a/tests/compile-fail/cast_box_int_to_fn_ptr.rs +++ b/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); diff --git a/tests/compile-fail/cast_int_to_fn_ptr.rs b/tests/compile-fail/cast_int_to_fn_ptr.rs index 29d16e9a425..2a08d9f1f9f 100644 --- a/tests/compile-fail/cast_int_to_fn_ptr.rs +++ b/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 { diff --git a/tests/compile-fail/execute_memory.rs b/tests/compile-fail/execute_memory.rs index bcde13d13ee..2f8fea38d8f 100644 --- a/tests/compile-fail/execute_memory.rs +++ b/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)] diff --git a/tests/compile-fail/fn_ptr_offset.rs b/tests/compile-fail/fn_ptr_offset.rs index 0c0590e375b..e6d1da1e073 100644 --- a/tests/compile-fail/fn_ptr_offset.rs +++ b/tests/compile-fail/fn_ptr_offset.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 use std::mem; diff --git a/tests/compile-fail/invalid_bool.rs b/tests/compile-fail/invalid_bool.rs index 49328ef5d74..af4ad67a4f0 100644 --- a/tests/compile-fail/invalid_bool.rs +++ b/tests/compile-fail/invalid_bool.rs @@ -1,9 +1,3 @@ -//ignore-test FIXME: do some basic validation of invariants for all values in flight -//This does currently not get caught becuase it compiles to SwitchInt, which -//has no knowledge about data invariants. - fn main() { - let b = unsafe { std::mem::transmute::(2) }; - if b { unreachable!() } else { unreachable!() } //~ ERROR constant evaluation error - //~^ NOTE invalid boolean value read + let _b = unsafe { std::mem::transmute::(2) }; //~ ERROR encountered 2, but expected something in the range 0..=1 } diff --git a/tests/compile-fail/invalid_bool2.rs b/tests/compile-fail/invalid_bool2.rs index 47c4e8b410e..2348c62559b 100644 --- a/tests/compile-fail/invalid_bool2.rs +++ b/tests/compile-fail/invalid_bool2.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation + fn main() { let b = unsafe { std::mem::transmute::(2) }; let _x = b == true; //~ ERROR invalid boolean value read diff --git a/tests/compile-fail/invalid_char.rs b/tests/compile-fail/invalid_char.rs new file mode 100644 index 00000000000..3ff0ed60f66 --- /dev/null +++ b/tests/compile-fail/invalid_char.rs @@ -0,0 +1,8 @@ +fn main() { + assert!(std::char::from_u32(-1_i32 as u32).is_none()); + let _ = match unsafe { std::mem::transmute::(-1) } { //~ ERROR encountered 4294967295, but expected something in the range 0..=1114111 + 'a' => {true}, + 'b' => {false}, + _ => {true}, + }; +} diff --git a/tests/compile-fail/match_char2.rs b/tests/compile-fail/invalid_char2.rs similarity index 65% rename from tests/compile-fail/match_char2.rs rename to tests/compile-fail/invalid_char2.rs index 786dd813a1e..5de2d073f32 100644 --- a/tests/compile-fail/match_char2.rs +++ b/tests/compile-fail/invalid_char2.rs @@ -1,3 +1,6 @@ +// Validation makes this fail in the wrong place +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation + fn main() { assert!(std::char::from_u32(-1_i32 as u32).is_none()); let c = unsafe { std::mem::transmute::(-1) }; diff --git a/tests/compile-fail/invalid_enum_discriminant.rs b/tests/compile-fail/invalid_enum_discriminant.rs index 94c100b9ef5..543a797d44f 100644 --- a/tests/compile-fail/invalid_enum_discriminant.rs +++ b/tests/compile-fail/invalid_enum_discriminant.rs @@ -1,17 +1,8 @@ -// Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 - #[repr(C)] pub enum Foo { A, B, C, D } fn main() { - let f = unsafe { std::mem::transmute::(42) }; - match f { - Foo::A => {}, //~ ERROR invalid enum discriminant - Foo::B => {}, - Foo::C => {}, - Foo::D => {}, - } + let _f = unsafe { std::mem::transmute::(42) }; //~ ERROR encountered invalid enum discriminant 42 } diff --git a/tests/compile-fail/invalid_enum_discriminant2.rs b/tests/compile-fail/invalid_enum_discriminant2.rs index 5a5a20c4869..ea94081693e 100644 --- a/tests/compile-fail/invalid_enum_discriminant2.rs +++ b/tests/compile-fail/invalid_enum_discriminant2.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 // error-pattern: invalid enum discriminant diff --git a/tests/compile-fail/match_char.rs b/tests/compile-fail/match_char.rs deleted file mode 100644 index e7fee1e3e36..00000000000 --- a/tests/compile-fail/match_char.rs +++ /dev/null @@ -1,13 +0,0 @@ -//ignore-test FIXME: do some basic validation of invariants for all values in flight -//This does currently not get caught becuase it compiles to SwitchInt, which -//has no knowledge about data invariants. - -fn main() { - assert!(std::char::from_u32(-1_i32 as u32).is_none()); - let _ = match unsafe { std::mem::transmute::(-1) } { //~ ERROR constant evaluation error - //~^ NOTE tried to interpret an invalid 32-bit value as a char: 4294967295 - 'a' => {true}, - 'b' => {false}, - _ => {true}, - }; -} diff --git a/tests/compile-fail/never_say_never.rs b/tests/compile-fail/never_say_never.rs index fd76ecbd150..9821723deb3 100644 --- a/tests/compile-fail/never_say_never.rs +++ b/tests/compile-fail/never_say_never.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/never_transmute_humans.rs b/tests/compile-fail/never_transmute_humans.rs index 7652cdfdd3d..c5c53d4231c 100644 --- a/tests/compile-fail/never_transmute_humans.rs +++ b/tests/compile-fail/never_transmute_humans.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/never_transmute_void.rs b/tests/compile-fail/never_transmute_void.rs index 9329cd36599..11fc0f068de 100644 --- a/tests/compile-fail/never_transmute_void.rs +++ b/tests/compile-fail/never_transmute_void.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/reference_to_packed.rs b/tests/compile-fail/reference_to_packed.rs index 946a6b89a77..d18f314c8aa 100644 --- a/tests/compile-fail/reference_to_packed.rs +++ b/tests/compile-fail/reference_to_packed.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation #![allow(dead_code, unused_variables)] diff --git a/tests/compile-fail/storage_dead_dangling.rs b/tests/compile-fail/storage_dead_dangling.rs index 6abae2069fc..69917dce859 100644 --- a/tests/compile-fail/storage_dead_dangling.rs +++ b/tests/compile-fail/storage_dead_dangling.rs @@ -1,3 +1,6 @@ +// This should fail even without validation +// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation + static mut LEAK: usize = 0; fn fill(v: &mut i32) { diff --git a/tests/compile-fail/validation_cast_fn_ptr1.rs b/tests/compile-fail/validation_cast_fn_ptr1.rs new file mode 100644 index 00000000000..82f2d10ee4b --- /dev/null +++ b/tests/compile-fail/validation_cast_fn_ptr1.rs @@ -0,0 +1,10 @@ +fn main() { + // Cast a function pointer such that on a call, the argument gets transmuted + // from raw ptr to reference. This is ABI-compatible, so it's not the call that + // should fail, but validation should. + fn f(_x: &i32) { } + + let g: fn(*const i32) = unsafe { std::mem::transmute(f as fn(&i32)) }; + + g(0usize as *const i32) //~ ERROR encountered 0, but expected something greater or equal to 1 +} diff --git a/tests/compile-fail/validation_cast_fn_ptr2.rs b/tests/compile-fail/validation_cast_fn_ptr2.rs new file mode 100644 index 00000000000..2f3b91a53e6 --- /dev/null +++ b/tests/compile-fail/validation_cast_fn_ptr2.rs @@ -0,0 +1,10 @@ +fn main() { + // Cast a function pointer such that when returning, the return value gets transmuted + // from raw ptr to reference. This is ABI-compatible, so it's not the call that + // should fail, but validation should. + fn f() -> *const i32 { 0usize as *const i32 } + + let g: fn() -> &'static i32 = unsafe { std::mem::transmute(f as fn() -> *const i32) }; + + let _x = g(); //~ ERROR encountered 0, but expected something greater or equal to 1 +} diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 151aa89be3f..82a2144a337 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -64,7 +64,6 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, need_fullm flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs config.src_base = PathBuf::from(path.to_string()); flags.push("-Zmir-emit-validate=1".to_owned()); - flags.push("-Zmiri-disable-validation".to_owned()); config.target_rustcflags = Some(flags.join(" ")); config.target = target.to_owned(); config.host = host.to_owned(); From 99ca3820e7097f654d4ad49330534a8e49e2165a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Oct 2018 10:35:48 +0200 Subject: [PATCH 10/12] bump toolchain --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index 721790eb1a8..abbc6c90452 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -nightly-2018-10-11 +nightly-2018-10-14 From b24e1b789dbf765b34c82924b1d8494fc8d84397 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Oct 2018 10:50:35 +0200 Subject: [PATCH 11/12] fix rustc_test compilation --- rustc_tests/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rustc_tests/src/main.rs b/rustc_tests/src/main.rs index 969888ec991..73a8d19c309 100644 --- a/rustc_tests/src/main.rs +++ b/rustc_tests/src/main.rs @@ -95,7 +95,7 @@ fn visit_item(&mut self, i: &'hir hir::Item) { 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 visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {} 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 { From 9a1dd865c1d91d6ad8561ba16209998388d98309 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Oct 2018 11:06:36 +0200 Subject: [PATCH 12/12] whitelist Windows Mutex --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index d52fd002803..0bebe40d529 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -265,6 +265,7 @@ fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool { const WHITELIST: &[&str] = &[ // Uses mem::uninitialized "std::ptr::read", + "std::sys::windows::mutex::Mutex::", ]; for frame in ecx.stack().iter() .rev().take(3)