From 4135441137ca02168522c24c0f5a3fbbf3af8741 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 00:12:45 +0200 Subject: [PATCH 01/30] don't call Memory::get without checking the pointer first; avoid Memory::get if we just need to know align/size --- src/operator.rs | 10 +++++++--- src/shims/foreign_items.rs | 32 ++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index 0e25de7da5a..d3af1f0db0e 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -206,7 +206,7 @@ fn ptr_eq( // on read hardware this can easily happen. Thus for comparisons we require // both pointers to be live. if self.pointer_inbounds(left).is_ok() && self.pointer_inbounds(right).is_ok() { - // Two in-bounds pointers in different allocations are different. + // Two in-bounds (and hence live) pointers in different allocations are different. false } else { return err!(InvalidPointerMath); @@ -303,7 +303,9 @@ fn map_to_primval((res, over): (Pointer, bool)) -> (Scalar, bool) { map_to_primval(left.overflowing_offset(Size::from_bytes(right as u64), self)), BitAnd if !signed => { - let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes(); + let ptr_base_align = self.memory().get_size_and_align(left.alloc_id, AllocCheck::MaybeDead) + .expect("alloc info with MaybeDead cannot fail") + .1.bytes(); let base_mask = { // FIXME: use `interpret::truncate`, once that takes a `Size` instead of a `Layout`. let shift = 128 - self.memory().pointer_size().bits(); @@ -337,7 +339,9 @@ fn map_to_primval((res, over): (Pointer, bool)) -> (Scalar, bool) { Rem if !signed => { // Doing modulo a divisor of the alignment is allowed. // (Intuition: modulo a divisor leaks less information.) - let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes(); + let ptr_base_align = self.memory().get_size_and_align(left.alloc_id, AllocCheck::MaybeDead) + .expect("alloc info with MaybeDead cannot fail") + .1.bytes(); let right = right as u64; let ptr_size = self.memory().pointer_size(); if right == 1 { diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 9c9e77abfed..0fa857eff75 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -252,9 +252,10 @@ fn emulate_foreign_item( Align::from_bytes(align).unwrap(), MiriMemoryKind::Rust.into() ); + // We just allocated this, the access cannot fail this.memory_mut() - .get_mut(ptr.alloc_id)? - .write_repeat(tcx, ptr, 0, Size::from_bytes(size))?; + .get_mut(ptr.alloc_id).unwrap() + .write_repeat(tcx, ptr, 0, Size::from_bytes(size)).unwrap(); this.write_scalar(Scalar::Ptr(ptr), dest)?; } "__rust_dealloc" => { @@ -494,15 +495,15 @@ fn emulate_foreign_item( Align::from_bytes(1).unwrap(), MiriMemoryKind::Env.into(), ); - { - let alloc = this.memory_mut().get_mut(value_copy.alloc_id)?; - alloc.write_bytes(tcx, value_copy, &value)?; - let trailing_zero_ptr = value_copy.offset( - Size::from_bytes(value.len() as u64), - tcx, - )?; - alloc.write_bytes(tcx, trailing_zero_ptr, &[0])?; - } + // We just allocated these, so the write cannot fail. + let alloc = this.memory_mut().get_mut(value_copy.alloc_id).unwrap(); + alloc.write_bytes(tcx, value_copy, &value).unwrap(); + let trailing_zero_ptr = value_copy.offset( + Size::from_bytes(value.len() as u64), + tcx, + ).unwrap(); + alloc.write_bytes(tcx, trailing_zero_ptr, &[0]).unwrap(); + if let Some(var) = this.machine.env_vars.insert( name.to_owned(), value_copy, @@ -839,7 +840,14 @@ fn emulate_foreign_item( }, "GetSystemInfo" => { let system_info = this.deref_operand(args[0])?; - let system_info_ptr = system_info.ptr.to_ptr()?; + let (system_info_ptr, align) = system_info.to_scalar_ptr_align(); + let system_info_ptr = this.memory() + .check_ptr_access( + system_info_ptr, + system_info.layout.size, + align, + )? + .expect("cannot be a ZST"); // Initialize with `0`. this.memory_mut().get_mut(system_info_ptr.alloc_id)? .write_repeat(tcx, system_info_ptr, 0, system_info.layout.size)?; From 7b702b92588316adda4992e0ef247176b91a2f07 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 23:24:08 +0200 Subject: [PATCH 02/30] move find_fn (which is not specific to foreign items) out of foreign_items --- src/lib.rs | 1 + src/shims/foreign_items.rs | 41 --------------------------------- src/shims/mod.rs | 47 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 41 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6b2de4ac08b..50cf32f8520 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ // Resolve ambiguity. pub use rustc_mir::interpret::{self, AllocMap, PlaceTy}; +pub use crate::shims::{EvalContextExt as ShimsEvalContextExt}; pub use crate::shims::foreign_items::EvalContextExt as ForeignItemsEvalContextExt; pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 1a39df9cce1..aece4d3e1ae 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -1,4 +1,3 @@ -use rustc::ty; use rustc::ty::layout::{Align, LayoutOf, Size}; use rustc::hir::def_id::DefId; use rustc::mir; @@ -11,46 +10,6 @@ impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { - fn find_fn( - &mut self, - instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx, Tag>], - dest: Option>, - ret: Option, - ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { - let this = self.eval_context_mut(); - trace!("eval_fn_call: {:#?}, {:?}", instance, dest.map(|place| *place)); - - // First, run the common hooks also supported by CTFE. - if this.hook_fn(instance, args, dest)? { - this.goto_block(ret)?; - return Ok(None); - } - // There are some more lang items we want to hook that CTFE does not hook (yet). - if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { - // FIXME: return a real value in case the target allocation has an - // alignment bigger than the one requested. - let n = u128::max_value(); - let dest = dest.unwrap(); - let n = this.truncate(n, dest.layout); - this.write_scalar(Scalar::from_uint(n, dest.layout.size), dest)?; - this.goto_block(ret)?; - return Ok(None); - } - - // Try to see if we can do something about foreign items. - if this.tcx.is_foreign_item(instance.def_id()) { - // An external function that we cannot find MIR for, but we can still run enough - // of them to make miri viable. - this.emulate_foreign_item(instance.def_id(), args, dest, ret)?; - // `goto_block` already handled. - return Ok(None); - } - - // Otherwise, load the MIR. - Ok(Some(this.load_mir(instance.def)?)) - } - /// Returns the minimum alignment for the target architecture. fn min_align(&self) -> Align { let this = self.eval_context_ref(); diff --git a/src/shims/mod.rs b/src/shims/mod.rs index cadfc05681d..0fc23e81193 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -1,2 +1,49 @@ pub mod foreign_items; pub mod intrinsics; + +use rustc::{ty, mir}; + +use crate::*; + +impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { + fn find_fn( + &mut self, + instance: ty::Instance<'tcx>, + args: &[OpTy<'tcx, Tag>], + dest: Option>, + ret: Option, + ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { + let this = self.eval_context_mut(); + trace!("eval_fn_call: {:#?}, {:?}", instance, dest.map(|place| *place)); + + // First, run the common hooks also supported by CTFE. + if this.hook_fn(instance, args, dest)? { + this.goto_block(ret)?; + return Ok(None); + } + // There are some more lang items we want to hook that CTFE does not hook (yet). + if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { + // FIXME: return a real value in case the target allocation has an + // alignment bigger than the one requested. + let n = u128::max_value(); + let dest = dest.unwrap(); + let n = this.truncate(n, dest.layout); + this.write_scalar(Scalar::from_uint(n, dest.layout.size), dest)?; + this.goto_block(ret)?; + return Ok(None); + } + + // Try to see if we can do something about foreign items. + if this.tcx.is_foreign_item(instance.def_id()) { + // An external function that we cannot find MIR for, but we can still run enough + // of them to make miri viable. + this.emulate_foreign_item(instance.def_id(), args, dest, ret)?; + // `goto_block` already handled. + return Ok(None); + } + + // Otherwise, load the MIR. + Ok(Some(this.load_mir(instance.def)?)) + } +} From b04452223a8b4256e32306eab2f1bcb578c09c16 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 11:05:57 +0200 Subject: [PATCH 03/30] be explicit about our line endings --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000000..6313b56c578 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text=auto eol=lf From e8e42ab5ecead88c23dec9eab9a23019e45410fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 09:51:16 +0200 Subject: [PATCH 04/30] add another bug we found to the list --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index be104ed0b2b..8c0c87ab9eb 100644 --- a/README.md +++ b/README.md @@ -327,6 +327,7 @@ Miri has already found a number of bugs in the Rust standard library and beyond, Definite bugs found: * [`Debug for vec_deque::Iter` accessing uninitialized memory](https://github.com/rust-lang/rust/issues/53566) +* [`Vec::into_iter` doing an unaligned ZST read](https://github.com/rust-lang/rust/pull/53804) * [`From<&[T]> for Rc` creating a not sufficiently aligned reference](https://github.com/rust-lang/rust/issues/54908) * [`BTreeMap` creating a shared reference pointing to a too small allocation](https://github.com/rust-lang/rust/issues/54957) * [`Vec::append` creating a dangling reference](https://github.com/rust-lang/rust/pull/61082) From 8d8481fed5c0417e01bfa4726e16e0181213e93f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 13:02:48 +0200 Subject: [PATCH 05/30] fix outdated test name: overalign -> align --- tests/run-pass/heap_allocator.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index 2f3a48f535d..e7a609a7d98 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -41,7 +41,7 @@ fn check_alloc(mut allocator: T) { unsafe { } } } -fn check_overalign_requests(mut allocator: T) { +fn check_align_requests(mut allocator: T) { for &size in &[2, 8, 64] { // size less than and bigger than alignment for &align in &[4, 8, 16, 32] { // Be sure to cover less than and bigger than `MIN_ALIGN` for all architectures let iterations = 32; @@ -88,8 +88,8 @@ fn box_to_global() { fn main() { check_alloc(System); check_alloc(Global); - check_overalign_requests(System); - check_overalign_requests(Global); + check_align_requests(System); + check_align_requests(Global); global_to_box(); box_to_global(); } From ccbc035f6a57dd6315114e4de3ee89513c355bb2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 22:20:37 +0200 Subject: [PATCH 06/30] run all run-pass tests with intrptrcast. makes many of them fail! --- tests/compiletest.rs | 3 +++ tests/run-pass/intptrcast_format.rs | 6 ------ tests/run-pass/intptrcast_format.stdout | 2 -- 3 files changed, 3 insertions(+), 8 deletions(-) delete mode 100644 tests/run-pass/intptrcast_format.rs delete mode 100644 tests/run-pass/intptrcast_format.stdout diff --git a/tests/compiletest.rs b/tests/compiletest.rs index d59be08c8e0..cb61c1c01b3 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -81,6 +81,9 @@ fn miri_pass(path: &str, target: &str, opt: bool) { let mut flags = Vec::new(); if opt { flags.push("-Zmir-opt-level=3".to_owned()); + } else { + // Run with intptrcast. Avoid test matrix explosion by doing either this or opt-level=3. + flags.push("-Zmiri-seed=".to_owned()); } run_tests("ui", path, target, flags); diff --git a/tests/run-pass/intptrcast_format.rs b/tests/run-pass/intptrcast_format.rs deleted file mode 100644 index c0d3e9398dc..00000000000 --- a/tests/run-pass/intptrcast_format.rs +++ /dev/null @@ -1,6 +0,0 @@ -// compile-flags: -Zmiri-seed= - -fn main() { - println!("Hello {}", 13); - println!("{:0 Date: Wed, 3 Jul 2019 09:06:35 +0200 Subject: [PATCH 07/30] dont add the -Zmiri-seed flag twice --- tests/compiletest.rs | 7 ++++--- tests/{run-pass => run-pass-noseed}/hashmap.rs | 0 tests/{run-pass => run-pass-noseed}/heap_allocator.rs | 0 tests/{run-pass => run-pass-noseed}/intptrcast.rs | 0 4 files changed, 4 insertions(+), 3 deletions(-) rename tests/{run-pass => run-pass-noseed}/hashmap.rs (100%) rename tests/{run-pass => run-pass-noseed}/heap_allocator.rs (100%) rename tests/{run-pass => run-pass-noseed}/intptrcast.rs (100%) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index cb61c1c01b3..1d32d3a7324 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -69,7 +69,7 @@ fn compile_fail(path: &str, target: &str, opt: bool) { run_tests("compile-fail", path, target, flags); } -fn miri_pass(path: &str, target: &str, opt: bool) { +fn miri_pass(path: &str, target: &str, opt: bool, noseed: bool) { let opt_str = if opt { " with optimizations" } else { "" }; eprintln!("{}", format!( "## Running run-pass tests in {} against miri for target {}{}", @@ -81,7 +81,7 @@ fn miri_pass(path: &str, target: &str, opt: bool) { let mut flags = Vec::new(); if opt { flags.push("-Zmir-opt-level=3".to_owned()); - } else { + } else if !noseed { // Run with intptrcast. Avoid test matrix explosion by doing either this or opt-level=3. flags.push("-Zmiri-seed=".to_owned()); } @@ -107,7 +107,8 @@ fn get_target() -> String { } fn run_pass_miri(opt: bool) { - miri_pass("tests/run-pass", &get_target(), opt); + miri_pass("tests/run-pass", &get_target(), opt, false); + miri_pass("tests/run-pass-noseed", &get_target(), opt, true); } fn compile_fail_miri(opt: bool) { diff --git a/tests/run-pass/hashmap.rs b/tests/run-pass-noseed/hashmap.rs similarity index 100% rename from tests/run-pass/hashmap.rs rename to tests/run-pass-noseed/hashmap.rs diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass-noseed/heap_allocator.rs similarity index 100% rename from tests/run-pass/heap_allocator.rs rename to tests/run-pass-noseed/heap_allocator.rs diff --git a/tests/run-pass/intptrcast.rs b/tests/run-pass-noseed/intptrcast.rs similarity index 100% rename from tests/run-pass/intptrcast.rs rename to tests/run-pass-noseed/intptrcast.rs From 457c8237652e123010555b5ecc795a5a4fd6111f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 09:06:48 +0200 Subject: [PATCH 08/30] only treat integer operations as such --- src/operator.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index d3af1f0db0e..31def2fec0e 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -56,10 +56,10 @@ fn ptr_op( trace!("ptr_op: {:?} {:?} {:?}", *left, bin_op, *right); - // If intptrcast is enabled and the operation is not an offset - // we can force the cast from pointers to integer addresses and - // then dispatch to rustc binary operation method - if self.memory().extra.rng.is_some() && bin_op != Offset { + // If intptrcast is enabled, treat everything of integer *type* at integer *value*. + if self.memory().extra.rng.is_some() && left.layout.ty.is_integral() { + // This is actually an integer operation, so dispatch back to the core engine. + assert!(right.layout.ty.is_integral()); let l_bits = self.force_bits(left.imm.to_scalar()?, left.layout.size)?; let r_bits = self.force_bits(right.imm.to_scalar()?, right.layout.size)?; From c6e4f760a21f0d25215a9b5b6b01798e5d08c5ca Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 09:23:31 +0200 Subject: [PATCH 09/30] allow dangling ptr-to-int casts; use force_bits for ptr comparison --- src/intptrcast.rs | 4 +++- src/operator.rs | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 54807000050..805cc2ad2a5 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -75,7 +75,9 @@ pub fn ptr_to_int( let mut global_state = memory.extra.intptrcast.borrow_mut(); let global_state = &mut *global_state; - let (size, align) = memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live)?; + // There is nothing wrong with a raw pointer being cast to an integer only after + // it became dangling. Hence `MaybeDead`. + let (size, align) = memory.get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead)?; let base_addr = match global_state.base_addr.entry(ptr.alloc_id) { Entry::Occupied(entry) => *entry.get(), diff --git a/src/operator.rs b/src/operator.rs index 31def2fec0e..21944843168 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -59,6 +59,8 @@ fn ptr_op( // If intptrcast is enabled, treat everything of integer *type* at integer *value*. if self.memory().extra.rng.is_some() && left.layout.ty.is_integral() { // This is actually an integer operation, so dispatch back to the core engine. + // TODO: Once intptrcast is the default, librustc_mir should never even call us + // for integer types. assert!(right.layout.ty.is_integral()); let l_bits = self.force_bits(left.imm.to_scalar()?, left.layout.size)?; let r_bits = self.force_bits(right.imm.to_scalar()?, right.layout.size)?; @@ -186,6 +188,13 @@ fn ptr_eq( right: Scalar, ) -> InterpResult<'tcx, bool> { let size = self.pointer_size(); + if self.memory().extra.rng.is_some() { + // Just compare the integers. + // TODO: Do we really want to *always* do that, even when comparing two live in-bounds pointers? + let left = self.force_bits(left, size)?; + let right = self.force_bits(right, size)?; + return Ok(left == right); + } Ok(match (left, right) { (Scalar::Raw { .. }, Scalar::Raw { .. }) => left.to_bits(size)? == right.to_bits(size)?, From 12b8d4366cc48ed93441f4b039bc18e30b0f0baa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 09:32:21 +0200 Subject: [PATCH 10/30] avoid integer overflow in ptr-to-int cast --- src/intptrcast.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 805cc2ad2a5..5797895c54f 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -4,7 +4,8 @@ use rand::Rng; -use rustc_mir::interpret::{AllocId, Pointer, InterpResult, Memory, AllocCheck}; +use rustc::ty::layout::HasDataLayout; +use rustc_mir::interpret::{AllocId, Pointer, InterpResult, Memory, AllocCheck, PointerArithmetic}; use rustc_target::abi::Size; use crate::{Evaluator, Tag, STACK_ADDR}; @@ -109,7 +110,9 @@ pub fn ptr_to_int( }; debug_assert_eq!(base_addr % align.bytes(), 0); // sanity check - Ok(base_addr + ptr.offset.bytes()) + // Add offset with the right kind of pointer-overflowing arithmetic. + let dl = memory.data_layout(); + Ok(dl.overflowing_offset(base_addr, ptr.offset.bytes()).0) } /// Shifts `addr` to make it aligned with `align` by rounding `addr` to the smallest multiple From c3da843ca02dab7b5f6563ffeb0d654bbe70e1f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 09:42:07 +0200 Subject: [PATCH 11/30] we don't need zero-sized freeze-sensitive visiting --- src/helpers.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/helpers.rs b/src/helpers.rs index f65eef557c9..16451fb8726 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -58,6 +58,7 @@ fn visit_freeze_sensitive( .map(|(size, _)| size) .unwrap_or_else(|| place.layout.size) ); + assert!(size.bytes() > 0); // Store how far we proceeded into the place so far. Everything to the left of // this offset has already been handled, in the sense that the frozen parts // have had `action` called on them. From c8450bda4fc3ed70aa019f83c58ff8ee8f9fcb09 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 09:56:35 +0200 Subject: [PATCH 12/30] support integers that can be cast to pointers in in-bounds offset operation --- src/operator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index 21944843168..2e9b8238479 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -397,7 +397,7 @@ fn pointer_offset_inbounds( .checked_mul(pointee_size) .ok_or_else(|| InterpError::Overflow(mir::BinOp::Mul))?; // Now let's see what kind of pointer this is. - if let Scalar::Ptr(ptr) = ptr { + if let Ok(ptr) = self.force_ptr(ptr) { // Both old and new pointer must be in-bounds of a *live* allocation. // (Of the same allocation, but that part is trivial with our representation.) self.pointer_inbounds(ptr)?; @@ -405,7 +405,7 @@ fn pointer_offset_inbounds( self.pointer_inbounds(ptr)?; Ok(Scalar::Ptr(ptr)) } else { - // An integer pointer. They can only be offset by 0, and we pretend there + // A "true" integer pointer. They can only be offset by 0, and we pretend there // is a little zero-sized allocation here. if offset == 0 { Ok(ptr) From eb4128fb4203f926370fdf17d6961940986728cd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 10:19:55 +0200 Subject: [PATCH 13/30] don't call Stacked Borrows hooks at all when validation is disabled --- README.md | 2 + src/eval.rs | 4 +- src/machine.rs | 78 ++++++++++++++++++++++----------- src/stacked_borrows.rs | 5 ++- tests/run-pass/transmute_fat.rs | 3 ++ 5 files changed, 62 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 8c0c87ab9eb..69c66db5214 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,8 @@ Several `-Z` flags are relevant for Miri: * `-Zalways-encode-mir` makes rustc dump MIR even for completely monomorphic functions. This is needed so that Miri can execute such functions, so Miri sets this flag per default. +* `-Zmir-emit-retag` controls whether `Retag` statements are emitted. Miri + enables this per default because it is needed for validation. Moreover, Miri recognizes some environment variables: diff --git a/src/eval.rs b/src/eval.rs index ec97a77357c..a1157af0e39 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -32,11 +32,11 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( let mut ecx = InterpretCx::new( tcx.at(syntax::source_map::DUMMY_SP), ty::ParamEnv::reveal_all(), - Evaluator::new(config.validate), + Evaluator::new(), ); // FIXME: InterpretCx::new should take an initial MemoryExtra - ecx.memory_mut().extra = MemoryExtra::with_rng(config.seed.map(StdRng::seed_from_u64)); + ecx.memory_mut().extra = MemoryExtra::new(config.seed.map(StdRng::seed_from_u64), config.validate); let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); let main_mir = ecx.load_mir(main_instance.def)?; diff --git a/src/machine.rs b/src/machine.rs index 20fe2e055fb..0ddb2d40b8e 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -41,7 +41,8 @@ fn into(self) -> MemoryKind { /// Extra per-allocation data #[derive(Debug, Clone)] pub struct AllocExtra { - pub stacked_borrows: stacked_borrows::AllocExtra, + /// Stacked Borrows state is only added if validation is enabled. + pub stacked_borrows: Option, } /// Extra global memory data @@ -49,17 +50,22 @@ pub struct AllocExtra { pub struct MemoryExtra { pub stacked_borrows: stacked_borrows::MemoryExtra, pub intptrcast: intptrcast::MemoryExtra, + /// The random number generator to use if Miri is running in non-deterministic mode and to /// enable intptrcast - pub(crate) rng: Option> + pub(crate) rng: Option>, + + /// Whether to enforce the validity invariant. + pub(crate) validate: bool, } impl MemoryExtra { - pub fn with_rng(rng: Option) -> Self { + pub fn new(rng: Option, validate: bool) -> Self { MemoryExtra { stacked_borrows: Default::default(), intptrcast: Default::default(), rng: rng.map(RefCell::new), + validate, } } } @@ -82,13 +88,10 @@ 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> { - pub(crate) fn new(validate: bool) -> Self { + pub(crate) fn new() -> Self { Evaluator { env_vars: HashMap::default(), argc: None, @@ -96,7 +99,6 @@ pub(crate) fn new(validate: bool) -> Self { cmd_line: None, last_error: 0, tls: TlsData::default(), - validate, } } } @@ -135,7 +137,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { #[inline(always)] fn enforce_validity(ecx: &InterpretCx<'mir, 'tcx, Self>) -> bool { - ecx.machine.validate + ecx.memory().extra.validate } /// Returns `Ok()` when the function was handled; fail otherwise. @@ -251,12 +253,17 @@ fn tag_allocation<'b>( ) -> (Cow<'b, Allocation>, Self::PointerTag) { let kind = kind.expect("we set our STATIC_KIND so this cannot be None"); let alloc = alloc.into_owned(); - let (stacks, base_tag) = Stacks::new_allocation( - id, - Size::from_bytes(alloc.bytes.len() as u64), - Rc::clone(&memory.extra.stacked_borrows), - kind, - ); + let (stacks, base_tag) = if !memory.extra.validate { + (None, Tag::Untagged) + } else { + let (stacks, base_tag) = Stacks::new_allocation( + id, + Size::from_bytes(alloc.bytes.len() as u64), + Rc::clone(&memory.extra.stacked_borrows), + kind, + ); + (Some(stacks), base_tag) + }; if kind != MiriMemoryKind::Static.into() { assert!(alloc.relocations.is_empty(), "Only statics can come initialized with inner pointers"); // Now we can rely on the inner pointers being static, too. @@ -268,7 +275,14 @@ fn tag_allocation<'b>( alloc.relocations.iter() // The allocations in the relocations (pointers stored *inside* this allocation) // all get the base pointer tag. - .map(|&(offset, ((), alloc))| (offset, (memory_extra.static_base_ptr(alloc), alloc))) + .map(|&(offset, ((), alloc))| { + let tag = if !memory.extra.validate { + Tag::Untagged + } else { + memory_extra.static_base_ptr(alloc) + }; + (offset, (tag, alloc)) + }) .collect() ), undef_mask: alloc.undef_mask, @@ -286,7 +300,11 @@ fn tag_static_base_pointer( id: AllocId, memory: &Memory<'mir, 'tcx, Self>, ) -> Self::PointerTag { - memory.extra.stacked_borrows.borrow_mut().static_base_ptr(id) + if !memory.extra.validate { + Tag::Untagged + } else { + memory.extra.stacked_borrows.borrow_mut().static_base_ptr(id) + } } #[inline(always)] @@ -295,12 +313,8 @@ fn retag( kind: mir::RetagKind, place: PlaceTy<'tcx, Tag>, ) -> InterpResult<'tcx> { - if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !Self::enforce_validity(ecx) { - // No tracking, or no retagging. The latter is possible because a dependency of ours - // might be called with different flags than we are, so there are `Retag` - // statements but we do not want to execute them. - // Also, honor the whitelist in `enforce_validity` because otherwise we might retag - // uninitialized data. + if !Self::enforce_validity(ecx) { + // No tracking. Ok(()) } else { ecx.retag(kind, place) @@ -354,7 +368,11 @@ fn memory_read<'tcx>( ptr: Pointer, size: Size, ) -> InterpResult<'tcx> { - alloc.extra.stacked_borrows.memory_read(ptr, size) + if let Some(ref stacked_borrows) = alloc.extra.stacked_borrows { + stacked_borrows.memory_read(ptr, size) + } else { + Ok(()) + } } #[inline(always)] @@ -363,7 +381,11 @@ fn memory_written<'tcx>( ptr: Pointer, size: Size, ) -> InterpResult<'tcx> { - alloc.extra.stacked_borrows.memory_written(ptr, size) + if let Some(ref mut stacked_borrows) = alloc.extra.stacked_borrows { + stacked_borrows.memory_written(ptr, size) + } else { + Ok(()) + } } #[inline(always)] @@ -372,7 +394,11 @@ fn memory_deallocated<'tcx>( ptr: Pointer, size: Size, ) -> InterpResult<'tcx> { - alloc.extra.stacked_borrows.memory_deallocated(ptr, size) + if let Some(ref mut stacked_borrows) = alloc.extra.stacked_borrows { + stacked_borrows.memory_deallocated(ptr, size) + } else { + Ok(()) + } } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 524ad2b47af..5c59066c475 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -538,6 +538,7 @@ fn reborrow( // Get the allocation. It might not be mutable, so we cannot use `get_mut`. let alloc = this.memory().get(ptr.alloc_id)?; + let stacked_borrows = alloc.extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data"); // Update the stacks. // Make sure that raw pointers and mutable shared references are reborrowed "weak": // There could be existing unique pointers reborrowed from them that should remain valid! @@ -553,14 +554,14 @@ fn reborrow( // We are only ever `SharedReadOnly` inside the frozen bits. let perm = if frozen { Permission::SharedReadOnly } else { Permission::SharedReadWrite }; let item = Item { perm, tag: new_tag, protector }; - alloc.extra.stacked_borrows.for_each(cur_ptr, size, |stack, global| { + stacked_borrows.for_each(cur_ptr, size, |stack, global| { stack.grant(cur_ptr.tag, item, global) }) }); } }; let item = Item { perm, tag: new_tag, protector }; - alloc.extra.stacked_borrows.for_each(ptr, size, |stack, global| { + stacked_borrows.for_each(ptr, size, |stack, global| { stack.grant(ptr.tag, item, global) }) } diff --git a/tests/run-pass/transmute_fat.rs b/tests/run-pass/transmute_fat.rs index ec887902d05..27da44935b1 100644 --- a/tests/run-pass/transmute_fat.rs +++ b/tests/run-pass/transmute_fat.rs @@ -1,3 +1,6 @@ +// Validation disallows this becuase the reference is never cast to a raw pointer. +// compile-flags: -Zmiri-disable-validation + fn main() { // If we are careful, we can exploit data layout... let raw = unsafe { From 8ec25066e741fd0e4a87f2eed53a777dbc64b802 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 10:46:51 +0200 Subject: [PATCH 14/30] make a test noseed for now that does not work with intptrcast yet --- tests/{run-pass => run-pass-noseed}/ptr_int_casts.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{run-pass => run-pass-noseed}/ptr_int_casts.rs (100%) diff --git a/tests/run-pass/ptr_int_casts.rs b/tests/run-pass-noseed/ptr_int_casts.rs similarity index 100% rename from tests/run-pass/ptr_int_casts.rs rename to tests/run-pass-noseed/ptr_int_casts.rs From b29cb7d551cece20ec0fe0b378edaeb1318d702d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 10:54:16 +0200 Subject: [PATCH 15/30] avoid catching errors --- src/operator.rs | 33 ++++++++++--------- tests/compile-fail/ptr_offset_int_plus_int.rs | 2 +- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index 2e9b8238479..df60acc661e 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -397,21 +397,24 @@ fn pointer_offset_inbounds( .checked_mul(pointee_size) .ok_or_else(|| InterpError::Overflow(mir::BinOp::Mul))?; // Now let's see what kind of pointer this is. - if let Ok(ptr) = self.force_ptr(ptr) { - // Both old and new pointer must be in-bounds of a *live* allocation. - // (Of the same allocation, but that part is trivial with our representation.) - self.pointer_inbounds(ptr)?; - let ptr = ptr.signed_offset(offset, self)?; - self.pointer_inbounds(ptr)?; - Ok(Scalar::Ptr(ptr)) - } else { - // A "true" integer pointer. They can only be offset by 0, and we pretend there - // is a little zero-sized allocation here. - if offset == 0 { - Ok(ptr) - } else { - err!(InvalidPointerMath) + let ptr = if offset == 0 { + match ptr { + Scalar::Ptr(ptr) => ptr, + Scalar::Raw { .. } => { + // Offset 0 on an integer. We accept that, pretending there is + // a little zero-sized allocation here. + return Ok(ptr); + } } - } + } else { + // Offset > 0. We *require* a pointer. + self.force_ptr(ptr)? + }; + // Both old and new pointer must be in-bounds of a *live* allocation. + // (Of the same allocation, but that part is trivial with our representation.) + self.pointer_inbounds(ptr)?; + let ptr = ptr.signed_offset(offset, self)?; + self.pointer_inbounds(ptr)?; + Ok(Scalar::Ptr(ptr)) } } diff --git a/tests/compile-fail/ptr_offset_int_plus_int.rs b/tests/compile-fail/ptr_offset_int_plus_int.rs index d0273961081..2d3282a0a97 100644 --- a/tests/compile-fail/ptr_offset_int_plus_int.rs +++ b/tests/compile-fail/ptr_offset_int_plus_int.rs @@ -1,4 +1,4 @@ -// error-pattern: invalid arithmetic on pointers +// error-pattern: tried to interpret some bytes as a pointer fn main() { // Can't offset an integer pointer by non-zero offset. From 074e20eb7b3f36846c6c3a182060a5b82da46498 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 29 Jun 2019 13:17:15 -0500 Subject: [PATCH 16/30] Add intptrcast test for explicit casts --- tests/run-pass-noseed/intptrcast.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/run-pass-noseed/intptrcast.rs b/tests/run-pass-noseed/intptrcast.rs index 40b21f9a472..919083f98d3 100644 --- a/tests/run-pass-noseed/intptrcast.rs +++ b/tests/run-pass-noseed/intptrcast.rs @@ -1,4 +1,7 @@ // compile-flags: -Zmiri-seed=0000000000000000 +fn transmute_ptr_to_int(x: *const T) -> usize { + unsafe { std::mem::transmute::<*const T, usize>(x) * 1 } +} fn main() { // Some casting-to-int with arithmetic. @@ -11,4 +14,11 @@ fn main() { // Pointer string formatting! We can't check the output as it changes when libstd changes, // but we can make sure Miri does not error. format!("{:?}", &mut 13 as *mut _); + + // Check that intptrcast is triggered for explicit casts and that it is consistent with + // transmuting. + let a: *const i32 = &42; + let b = transmute_ptr_to_int(a) as u8; + let c = a as usize as u8; + assert_eq!(b, c); } From 39d383d9e729965b406009033977cb942dc899b3 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 3 Jul 2019 13:28:30 -0500 Subject: [PATCH 17/30] Update rust-version --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 17338728254..9e4ddf2acaa 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -7e08576e4276a97b523c25bfd196d419c39c7b87 +088b987307b91612ab164026e1dcdd0129fdb62b From 8dfb278ac5ad47f7fac3d161ebb30d9efb9246a3 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 3 Jul 2019 13:42:01 -0500 Subject: [PATCH 18/30] Fix explicit cast test --- tests/run-pass-noseed/intptrcast.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/run-pass-noseed/intptrcast.rs b/tests/run-pass-noseed/intptrcast.rs index 919083f98d3..1b5251c9111 100644 --- a/tests/run-pass-noseed/intptrcast.rs +++ b/tests/run-pass-noseed/intptrcast.rs @@ -1,11 +1,13 @@ // compile-flags: -Zmiri-seed=0000000000000000 -fn transmute_ptr_to_int(x: *const T) -> usize { - unsafe { std::mem::transmute::<*const T, usize>(x) * 1 } + +// This returns a miri pointer at type usize, if the argument is a proper pointer +fn transmute_ptr_to_int(x: *const T) -> usize { + unsafe { std::mem::transmute(x) } } fn main() { // Some casting-to-int with arithmetic. - let x = &42 as *const i32 as usize; + let x = &42 as *const i32 as usize; let y = x * 2; assert_eq!(y, x + x); let z = y as u8 as usize; From 93c62a4912c7350924ec942aed37ff1cc363352f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 23:12:44 +0200 Subject: [PATCH 19/30] move tls.rs into shims module --- src/lib.rs | 3 +-- src/shims/mod.rs | 1 + src/{ => shims}/tls.rs | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{ => shims}/tls.rs (100%) diff --git a/src/lib.rs b/src/lib.rs index 50cf32f8520..31e70707776 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,6 @@ mod shims; mod operator; mod helpers; -mod tls; mod range_map; mod mono_hash_map; mod stacked_borrows; @@ -31,8 +30,8 @@ pub use crate::shims::{EvalContextExt as ShimsEvalContextExt}; pub use crate::shims::foreign_items::EvalContextExt as ForeignItemsEvalContextExt; pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt; +pub use crate::shims::tls::{EvalContextExt as TlsEvalContextExt, TlsData}; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; -pub use crate::tls::{EvalContextExt as TlsEvalContextExt, TlsData}; pub use crate::range_map::RangeMap; pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt}; pub use crate::mono_hash_map::MonoHashMap; diff --git a/src/shims/mod.rs b/src/shims/mod.rs index 0fc23e81193..3258cf3d9c1 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -1,5 +1,6 @@ pub mod foreign_items; pub mod intrinsics; +pub mod tls; use rustc::{ty, mir}; diff --git a/src/tls.rs b/src/shims/tls.rs similarity index 100% rename from src/tls.rs rename to src/shims/tls.rs From 802dcb7f890cf1e05a98fd21466f74d9c9b5b748 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 00:06:41 +0200 Subject: [PATCH 20/30] temporarily disable ptr_offset, maybe that helps --- tests/run-pass-noseed/ptr_int_casts.rs | 1 + tests/{run-pass => run-pass-noseed}/ptr_offset.rs | 2 ++ 2 files changed, 3 insertions(+) rename tests/{run-pass => run-pass-noseed}/ptr_offset.rs (85%) diff --git a/tests/run-pass-noseed/ptr_int_casts.rs b/tests/run-pass-noseed/ptr_int_casts.rs index c279024f35e..ebf65ac3fe2 100644 --- a/tests/run-pass-noseed/ptr_int_casts.rs +++ b/tests/run-pass-noseed/ptr_int_casts.rs @@ -1,3 +1,4 @@ +// FIXME move this to run-pass, it should work with intptrcast. use std::mem; use std::ptr; diff --git a/tests/run-pass/ptr_offset.rs b/tests/run-pass-noseed/ptr_offset.rs similarity index 85% rename from tests/run-pass/ptr_offset.rs rename to tests/run-pass-noseed/ptr_offset.rs index 1c7f0eb7179..a836e02812d 100644 --- a/tests/run-pass/ptr_offset.rs +++ b/tests/run-pass-noseed/ptr_offset.rs @@ -1,3 +1,5 @@ +// FIXME move this to run-pass, it should work with intptrcast. + fn f() -> i32 { 42 } fn main() { From 07d5e9917cb17d8c8aa2c132919e985841ed24bc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 09:56:42 +0200 Subject: [PATCH 21/30] avoid Scalar::is_null_ptr, it is going away --- src/helpers.rs | 22 ++++++++++++++++++++++ src/shims/foreign_items.rs | 35 +++++++++++++---------------------- src/shims/tls.rs | 20 +++++++++++++------- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 16451fb8726..3503af43690 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -43,6 +43,28 @@ fn resolve_path(&self, path: &[&str]) -> InterpResult<'tcx, ty::Instance<'tcx>> }) } + /// Write a 0 of the appropriate size to `dest`. + fn write_null(&mut self, dest: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> { + self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest) + } + + /// Test if this immediate equals 0. + fn is_null(&self, val: Scalar) -> InterpResult<'tcx, bool> { + let this = self.eval_context_ref(); + let null = Scalar::from_int(0, this.memory().pointer_size()); + this.ptr_eq(val, null) + } + + /// Turn a Scalar into an Option + fn test_null(&self, val: Scalar) -> InterpResult<'tcx, Option>> { + let this = self.eval_context_ref(); + Ok(if this.is_null(val)? { + None + } else { + Some(val) + }) + } + /// Visits the memory covered by `place`, sensitive to freezing: the 3rd parameter /// will be true if this is frozen, false if this is in an `UnsafeCell`. fn visit_freeze_sensitive( diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index c9b10e02c97..d43374f1bcf 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -49,7 +49,7 @@ fn free( ptr: Scalar, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - if !ptr.is_null_ptr(this) { + if !this.is_null(ptr)? { this.memory_mut().deallocate( ptr.to_ptr()?, None, @@ -66,7 +66,7 @@ fn realloc( ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let align = this.min_align(); - if old_ptr.is_null_ptr(this) { + if this.is_null(old_ptr)? { if new_size == 0 { Ok(Scalar::from_int(0, this.pointer_size())) } else { @@ -427,7 +427,7 @@ fn emulate_foreign_item( let mut success = None; { let name_ptr = this.read_scalar(args[0])?.not_undef()?; - if !name_ptr.is_null_ptr(this) { + if !this.is_null(name_ptr)? { let name_ptr = name_ptr.to_ptr()?; let name = this .memory() @@ -455,7 +455,7 @@ fn emulate_foreign_item( let name_ptr = this.read_scalar(args[0])?.not_undef()?; let value_ptr = this.read_scalar(args[1])?.to_ptr()?; let value = this.memory().get(value_ptr.alloc_id)?.read_c_str(tcx, value_ptr)?; - if !name_ptr.is_null_ptr(this) { + if !this.is_null(name_ptr)? { let name_ptr = name_ptr.to_ptr()?; let name = this.memory().get(name_ptr.alloc_id)?.read_c_str(tcx, name_ptr)?; if !name.is_empty() && !name.contains(&b'=') { @@ -638,14 +638,9 @@ fn emulate_foreign_item( let key_ptr = this.read_scalar(args[0])?.not_undef()?; // Extract the function type out of the signature (that seems easier than constructing it ourselves). - let dtor = match this.read_scalar(args[1])?.not_undef()? { - Scalar::Ptr(dtor_ptr) => Some(this.memory().get_fn(dtor_ptr)?), - Scalar::Raw { data: 0, size } => { - // NULL pointer - assert_eq!(size as u64, this.memory().pointer_size().bytes()); - None - }, - Scalar::Raw { .. } => return err!(ReadBytesAsPointer), + let dtor = match this.test_null(this.read_scalar(args[1])?.not_undef()?)? { + Some(dtor_ptr) => Some(this.memory().get_fn(dtor_ptr.to_ptr()?)?), + None => None, }; // Figure out how large a pthread TLS key actually is. @@ -657,7 +652,7 @@ fn emulate_foreign_item( let key_layout = this.layout_of(key_type)?; // Create key and write it into the memory where `key_ptr` wants it. - let key = this.machine.tls.create_tls_key(dtor, tcx) as u128; + let key = this.machine.tls.create_tls_key(dtor) as u128; if key_layout.size.bits() < 128 && key >= (1u128 << key_layout.size.bits() as u128) { return err!(OutOfTls); } @@ -682,13 +677,13 @@ fn emulate_foreign_item( } "pthread_getspecific" => { let key = this.read_scalar(args[0])?.to_bits(args[0].layout.size)?; - let ptr = this.machine.tls.load_tls(key)?; + let ptr = this.machine.tls.load_tls(key, tcx)?; this.write_scalar(ptr, dest)?; } "pthread_setspecific" => { let key = this.read_scalar(args[0])?.to_bits(args[0].layout.size)?; let new_ptr = this.read_scalar(args[1])?.not_undef()?; - this.machine.tls.store_tls(key, new_ptr)?; + this.machine.tls.store_tls(key, this.test_null(new_ptr)?)?; // Return success (`0`). this.write_null(dest)?; @@ -842,7 +837,7 @@ fn emulate_foreign_item( // This just creates a key; Windows does not natively support TLS destructors. // Create key and return it. - let key = this.machine.tls.create_tls_key(None, tcx) as u128; + let key = this.machine.tls.create_tls_key(None) as u128; // Figure out how large a TLS key actually is. This is `c::DWORD`. if dest.layout.size.bits() < 128 @@ -853,13 +848,13 @@ fn emulate_foreign_item( } "TlsGetValue" => { let key = this.read_scalar(args[0])?.to_u32()? as u128; - let ptr = this.machine.tls.load_tls(key)?; + let ptr = this.machine.tls.load_tls(key, tcx)?; this.write_scalar(ptr, dest)?; } "TlsSetValue" => { let key = this.read_scalar(args[0])?.to_u32()? as u128; let new_ptr = this.read_scalar(args[1])?.not_undef()?; - this.machine.tls.store_tls(key, new_ptr)?; + this.machine.tls.store_tls(key, this.test_null(new_ptr)?)?; // Return success (`1`). this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?; @@ -936,10 +931,6 @@ fn emulate_foreign_item( Ok(()) } - fn write_null(&mut self, dest: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> { - self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest) - } - /// Evaluates the scalar at the specified path. Returns Some(val) /// if the path could be resolved, and None otherwise fn eval_path_scalar(&mut self, path: &[&str]) -> InterpResult<'tcx, Option>> { diff --git a/src/shims/tls.rs b/src/shims/tls.rs index ddc301447c7..6d6a71b8eb2 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -12,7 +12,10 @@ #[derive(Copy, Clone, Debug)] pub struct TlsEntry<'tcx> { - pub(crate) data: Scalar, // Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. + /// The data for this key. None is used to represent NULL. + /// (We normalize this early to avoid having to do a NULL-ptr-test each time we access the data.) + /// Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. + pub(crate) data: Option>, pub(crate) dtor: Option>, } @@ -38,14 +41,13 @@ impl<'tcx> TlsData<'tcx> { pub fn create_tls_key( &mut self, dtor: Option>, - cx: &impl HasDataLayout, ) -> TlsKey { let new_key = self.next_key; self.next_key += 1; self.keys.insert( new_key, TlsEntry { - data: Scalar::ptr_null(cx).into(), + data: None, dtor, }, ); @@ -63,17 +65,21 @@ pub fn delete_tls_key(&mut self, key: TlsKey) -> InterpResult<'tcx> { } } - pub fn load_tls(&mut self, key: TlsKey) -> InterpResult<'tcx, Scalar> { + pub fn load_tls( + &mut self, + key: TlsKey, + cx: &impl HasDataLayout, + ) -> InterpResult<'tcx, Scalar> { match self.keys.get(&key) { Some(&TlsEntry { data, .. }) => { trace!("TLS key {} loaded: {:?}", key, data); - Ok(data) + Ok(data.unwrap_or_else(|| Scalar::ptr_null(cx).into())) } None => err!(TlsOutOfBounds), } } - pub fn store_tls(&mut self, key: TlsKey, new_data: Scalar) -> InterpResult<'tcx> { + pub fn store_tls(&mut self, key: TlsKey, new_data: Option>) -> InterpResult<'tcx> { match self.keys.get_mut(&key) { Some(&mut TlsEntry { ref mut data, .. }) => { trace!("TLS key {} stored: {:?}", key, new_data); @@ -117,7 +123,7 @@ fn fetch_tls_dtor( for (&key, &mut TlsEntry { ref mut data, dtor }) in thread_local.range_mut((start, Unbounded)) { - if !data.is_null_ptr(cx) { + if let Some(ref mut data) = *data { if let Some(dtor) = dtor { let ret = Some((dtor, *data, key)); *data = Scalar::ptr_null(cx); From 698b311a596b5d141b0c42f1b5400adc1f11150f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 10:08:57 +0200 Subject: [PATCH 22/30] fix NULL in TLS dtors --- src/shims/tls.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 6d6a71b8eb2..9a22c03bf2f 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -6,6 +6,7 @@ use crate::{ InterpResult, InterpError, StackPopCleanup, MPlaceTy, Scalar, Tag, + HelpersEvalContextExt, }; pub type TlsKey = u128; @@ -111,7 +112,6 @@ pub fn store_tls(&mut self, key: TlsKey, new_data: Option>) -> Inter fn fetch_tls_dtor( &mut self, key: Option, - cx: &impl HasDataLayout, ) -> Option<(ty::Instance<'tcx>, Scalar, TlsKey)> { use std::collections::Bound::*; @@ -123,10 +123,10 @@ fn fetch_tls_dtor( for (&key, &mut TlsEntry { ref mut data, dtor }) in thread_local.range_mut((start, Unbounded)) { - if let Some(ref mut data) = *data { + if let Some(data_scalar) = *data { if let Some(dtor) = dtor { - let ret = Some((dtor, *data, key)); - *data = Scalar::ptr_null(cx); + let ret = Some((dtor, data_scalar, key)); + *data = None; return ret; } } @@ -139,10 +139,11 @@ impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tc pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { fn run_tls_dtors(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let mut dtor = this.machine.tls.fetch_tls_dtor(None, &*this.tcx); + let mut dtor = this.machine.tls.fetch_tls_dtor(None); // FIXME: replace loop by some structure that works with stepping while let Some((instance, ptr, key)) = dtor { trace!("Running TLS dtor {:?} on {:?}", instance, ptr); + assert!(!this.is_null(ptr).unwrap(), "Data can't be NULL when dtor is called!"); // TODO: Potentially, this has to support all the other possible instances? // See eval_fn_call in interpret/terminator/mod.rs let mir = this.load_mir(instance.def)?; @@ -163,9 +164,9 @@ fn run_tls_dtors(&mut self) -> InterpResult<'tcx> { // step until out of stackframes this.run()?; - dtor = match this.machine.tls.fetch_tls_dtor(Some(key), &*this.tcx) { + dtor = match this.machine.tls.fetch_tls_dtor(Some(key)) { dtor @ Some(_) => dtor, - None => this.machine.tls.fetch_tls_dtor(None, &*this.tcx), + None => this.machine.tls.fetch_tls_dtor(None), }; } // FIXME: On a windows target, call `unsafe extern "system" fn on_tls_callback`. From aad5fde70316f87f023b918001f02db21d7ff0e1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 19:21:21 +0200 Subject: [PATCH 23/30] fix deallocating/reallocating with integer pointers --- src/shims/foreign_items.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index c9b10e02c97..540e3fc9640 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -50,8 +50,9 @@ fn free( ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if !ptr.is_null_ptr(this) { + let ptr = this.force_ptr(ptr)?; this.memory_mut().deallocate( - ptr.to_ptr()?, + ptr, None, MiriMemoryKind::C.into(), )?; @@ -78,7 +79,7 @@ fn realloc( Ok(Scalar::Ptr(new_ptr)) } } else { - let old_ptr = old_ptr.to_ptr()?; + let old_ptr = this.force_ptr(old_ptr)?; let memory = this.memory_mut(); let old_size = Size::from_bytes(memory.get(old_ptr.alloc_id)?.bytes.len() as u64); if new_size == 0 { @@ -234,7 +235,7 @@ fn emulate_foreign_item( this.write_scalar(Scalar::Ptr(ptr), dest)?; } "__rust_dealloc" => { - let ptr = this.read_scalar(args[0])?.to_ptr()?; + let ptr = this.read_scalar(args[0])?.not_undef()?; let old_size = this.read_scalar(args[1])?.to_usize(this)?; let align = this.read_scalar(args[2])?.to_usize(this)?; if old_size == 0 { @@ -243,6 +244,7 @@ fn emulate_foreign_item( if !align.is_power_of_two() { return err!(HeapAllocNonPowerOfTwoAlignment(align)); } + let ptr = this.force_ptr(ptr)?; this.memory_mut().deallocate( ptr, Some((Size::from_bytes(old_size), Align::from_bytes(align).unwrap())), From 6c58d40a8d42650332a4c68b44ea3827263c395f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 19:21:42 +0200 Subject: [PATCH 24/30] temporarily disable validation for 'cargo miri test' testing --- test-cargo-miri/run-test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 73515c74e40..a9aba008e9a 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -52,8 +52,9 @@ def test_cargo_miri_run(): ) def test_cargo_miri_test(): + # FIXME: enable validation again, once that no longer conflicts with intptrcast test("cargo miri test", - cargo_miri("test") + ["--", "-Zmiri-seed=feed"], + cargo_miri("test") + ["--", "-Zmiri-seed=feed", "-Zmiri-disable-validation"], "test.stdout.ref", "test.stderr.ref" ) test("cargo miri test (with filter)", From 9b58492df179be10cc2004ad0fe0616cdfd9b505 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 19:22:22 +0200 Subject: [PATCH 25/30] temporarily disable intptrcast advanced testing on Windows --- tests/compiletest.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 1d32d3a7324..b31be0a4f32 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -83,6 +83,7 @@ fn miri_pass(path: &str, target: &str, opt: bool, noseed: bool) { flags.push("-Zmir-opt-level=3".to_owned()); } else if !noseed { // Run with intptrcast. Avoid test matrix explosion by doing either this or opt-level=3. + #[cfg(not(windows))] // FIXME re-enable on Windows flags.push("-Zmiri-seed=".to_owned()); } From 4d76dd1f09db1b5a69435d3aa85c4c2f0a5887c1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 21:26:58 +0200 Subject: [PATCH 26/30] temporarily disable validation on Windows --- src/eval.rs | 10 +++++++++- tests/compiletest.rs | 5 +++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index a1157af0e39..91a563fa56b 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -35,8 +35,16 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( Evaluator::new(), ); + // FIXME(https://github.com/rust-lang/miri/pull/803): no validation on Windows. + let target_os = ecx.tcx.tcx.sess.target.target.target_os.to_lowercase(); + let validate = if target_os == "windows" { + false + } else { + config.validate + }; + // FIXME: InterpretCx::new should take an initial MemoryExtra - ecx.memory_mut().extra = MemoryExtra::new(config.seed.map(StdRng::seed_from_u64), config.validate); + ecx.memory_mut().extra = MemoryExtra::new(config.seed.map(StdRng::seed_from_u64), validate); let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); let main_mir = ecx.load_mir(main_instance.def)?; diff --git a/tests/compiletest.rs b/tests/compiletest.rs index b31be0a4f32..076deca6a31 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -83,7 +83,6 @@ fn miri_pass(path: &str, target: &str, opt: bool, noseed: bool) { flags.push("-Zmir-opt-level=3".to_owned()); } else if !noseed { // Run with intptrcast. Avoid test matrix explosion by doing either this or opt-level=3. - #[cfg(not(windows))] // FIXME re-enable on Windows flags.push("-Zmiri-seed=".to_owned()); } @@ -113,7 +112,9 @@ fn run_pass_miri(opt: bool) { } fn compile_fail_miri(opt: bool) { - compile_fail("tests/compile-fail", &get_target(), opt); + if !cfg!(windows) { // FIXME re-enable on Windows + compile_fail("tests/compile-fail", &get_target(), opt); + } } fn test_runner(_tests: &[&()]) { From f23b782101c03331413870a11a787d4b605b84df Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 09:03:45 +0200 Subject: [PATCH 27/30] align small malloc-allocations even less, and test that we do --- src/machine.rs | 4 +- src/shims/foreign_items.rs | 70 +++++++++++++++--------- tests/run-pass/{realloc.rs => malloc.rs} | 11 ++++ 3 files changed, 58 insertions(+), 27 deletions(-) rename tests/run-pass/{realloc.rs => malloc.rs} (70%) diff --git a/src/machine.rs b/src/machine.rs index 0ddb2d40b8e..a58903f6d51 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -25,6 +25,8 @@ pub enum MiriMemoryKind { Rust, /// `malloc` memory. C, + /// Windows `HeapAlloc` memory. + WinHeap, /// Part of env var emulation. Env, /// Statics. @@ -407,7 +409,7 @@ impl MayLeak for MiriMemoryKind { fn may_leak(self) -> bool { use self::MiriMemoryKind::*; match self { - Rust | C => false, + Rust | C | WinHeap => false, Env | Static => true, } } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 2a3644e45cb..38fc36609e6 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -10,8 +10,8 @@ impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { - /// Returns the minimum alignment for the target architecture. - fn min_align(&self) -> Align { + /// Returns the minimum alignment for the target architecture for allocations of the given size. + fn min_align(&self, size: u64, kind: MiriMemoryKind) -> Align { let this = self.eval_context_ref(); // List taken from `libstd/sys_common/alloc.rs`. let min_align = match this.tcx.tcx.sess.target.target.arch.as_str() { @@ -19,21 +19,39 @@ fn min_align(&self) -> Align { "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" => 16, arch => bug!("Unsupported target architecture: {}", arch), }; - Align::from_bytes(min_align).unwrap() + // Windows always aligns, even small allocations. + // Source: + // But jemalloc does not, so for the C heap we only align if the allocation is sufficiently big. + if kind == MiriMemoryKind::WinHeap || size >= min_align { + return Align::from_bytes(min_align).unwrap(); + } + // We have `size < min_align`. Round `size` *down* to the next power of two and use that. + fn prev_power_of_two(x: u64) -> u64 { + let next_pow2 = x.next_power_of_two(); + if next_pow2 == x { + // x *is* a power of two, just use that. + x + } else { + // x is between two powers, so next = 2*prev. + next_pow2 / 2 + } + } + Align::from_bytes(prev_power_of_two(size)).unwrap() } fn malloc( &mut self, size: u64, zero_init: bool, + kind: MiriMemoryKind, ) -> Scalar { let this = self.eval_context_mut(); let tcx = &{this.tcx.tcx}; if size == 0 { Scalar::from_int(0, this.pointer_size()) } else { - let align = this.min_align(); - let ptr = this.memory_mut().allocate(Size::from_bytes(size), align, MiriMemoryKind::C.into()); + let align = this.min_align(size, kind); + let ptr = this.memory_mut().allocate(Size::from_bytes(size), align, kind.into()); if zero_init { // We just allocated this, the access cannot fail this.memory_mut() @@ -47,6 +65,7 @@ fn malloc( fn free( &mut self, ptr: Scalar, + kind: MiriMemoryKind, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if !this.is_null(ptr)? { @@ -54,7 +73,7 @@ fn free( this.memory_mut().deallocate( ptr, None, - MiriMemoryKind::C.into(), + kind.into(), )?; } Ok(()) @@ -64,39 +83,38 @@ fn realloc( &mut self, old_ptr: Scalar, new_size: u64, + kind: MiriMemoryKind, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let align = this.min_align(); + let new_align = this.min_align(new_size, kind); if this.is_null(old_ptr)? { if new_size == 0 { Ok(Scalar::from_int(0, this.pointer_size())) } else { let new_ptr = this.memory_mut().allocate( Size::from_bytes(new_size), - align, - MiriMemoryKind::C.into() + new_align, + kind.into() ); Ok(Scalar::Ptr(new_ptr)) } } else { let old_ptr = this.force_ptr(old_ptr)?; let memory = this.memory_mut(); - let old_size = Size::from_bytes(memory.get(old_ptr.alloc_id)?.bytes.len() as u64); if new_size == 0 { memory.deallocate( old_ptr, - Some((old_size, align)), - MiriMemoryKind::C.into(), + None, + kind.into(), )?; Ok(Scalar::from_int(0, this.pointer_size())) } else { let new_ptr = memory.reallocate( old_ptr, - old_size, - align, + None, Size::from_bytes(new_size), - align, - MiriMemoryKind::C.into(), + new_align, + kind.into(), )?; Ok(Scalar::Ptr(new_ptr)) } @@ -145,14 +163,14 @@ fn emulate_foreign_item( match link_name { "malloc" => { let size = this.read_scalar(args[0])?.to_usize(this)?; - let res = this.malloc(size, /*zero_init:*/ false); + let res = this.malloc(size, /*zero_init:*/ false, MiriMemoryKind::C); this.write_scalar(res, dest)?; } "calloc" => { let items = this.read_scalar(args[0])?.to_usize(this)?; let len = this.read_scalar(args[1])?.to_usize(this)?; let size = items.checked_mul(len).ok_or_else(|| InterpError::Overflow(mir::BinOp::Mul))?; - let res = this.malloc(size, /*zero_init:*/ true); + let res = this.malloc(size, /*zero_init:*/ true, MiriMemoryKind::C); this.write_scalar(res, dest)?; } "posix_memalign" => { @@ -187,12 +205,12 @@ fn emulate_foreign_item( } "free" => { let ptr = this.read_scalar(args[0])?.not_undef()?; - this.free(ptr)?; + this.free(ptr, MiriMemoryKind::C)?; } "realloc" => { let old_ptr = this.read_scalar(args[0])?.not_undef()?; let new_size = this.read_scalar(args[1])?.to_usize(this)?; - let res = this.realloc(old_ptr, new_size)?; + let res = this.realloc(old_ptr, new_size, MiriMemoryKind::C)?; this.write_scalar(res, dest)?; } @@ -262,12 +280,12 @@ fn emulate_foreign_item( if !align.is_power_of_two() { return err!(HeapAllocNonPowerOfTwoAlignment(align)); } + let align = Align::from_bytes(align).unwrap(); let new_ptr = this.memory_mut().reallocate( ptr, - Size::from_bytes(old_size), - Align::from_bytes(align).unwrap(), + Some((Size::from_bytes(old_size), align)), Size::from_bytes(new_size), - Align::from_bytes(align).unwrap(), + align, MiriMemoryKind::Rust.into(), )?; this.write_scalar(Scalar::Ptr(new_ptr), dest)?; @@ -765,14 +783,14 @@ fn emulate_foreign_item( let flags = this.read_scalar(args[1])?.to_u32()?; let size = this.read_scalar(args[2])?.to_usize(this)?; let zero_init = (flags & 0x00000008) != 0; // HEAP_ZERO_MEMORY - let res = this.malloc(size, zero_init); + let res = this.malloc(size, zero_init, MiriMemoryKind::WinHeap); this.write_scalar(res, dest)?; } "HeapFree" => { let _handle = this.read_scalar(args[0])?.to_isize(this)?; let _flags = this.read_scalar(args[1])?.to_u32()?; let ptr = this.read_scalar(args[2])?.not_undef()?; - this.free(ptr)?; + this.free(ptr, MiriMemoryKind::WinHeap)?; this.write_scalar(Scalar::from_int(1, Size::from_bytes(4)), dest)?; } "HeapReAlloc" => { @@ -780,7 +798,7 @@ fn emulate_foreign_item( let _flags = this.read_scalar(args[1])?.to_u32()?; let ptr = this.read_scalar(args[2])?.not_undef()?; let size = this.read_scalar(args[3])?.to_usize(this)?; - let res = this.realloc(ptr, size)?; + let res = this.realloc(ptr, size, MiriMemoryKind::WinHeap)?; this.write_scalar(res, dest)?; } diff --git a/tests/run-pass/realloc.rs b/tests/run-pass/malloc.rs similarity index 70% rename from tests/run-pass/realloc.rs rename to tests/run-pass/malloc.rs index c23b3e645c7..a50b3f3606d 100644 --- a/tests/run-pass/realloc.rs +++ b/tests/run-pass/malloc.rs @@ -1,4 +1,5 @@ //ignore-windows: Uses POSIX APIs +//compile-flags: -Zmiri-seed= #![feature(rustc_private)] @@ -7,6 +8,16 @@ extern crate libc; fn main() { + // Test that small allocations sometimes *are* not very aligned. + let saw_unaligned = (0..64).any(|_| unsafe { + let p = libc::malloc(3); + let addr = p as usize; + let unaligned = addr % 4 != 0; // test that this is not 4-aligned + libc::free(p); // FIXME have to free *after* test; should allow ptr-to-int of dangling ptr. + unaligned + }); + assert!(saw_unaligned); + unsafe { // Use calloc for initialized memory let p1 = libc::calloc(20, 1); From 1729965808dec239aa46f2eafaa39261f491a0b4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 23:47:10 +0200 Subject: [PATCH 28/30] rename InterpretCx -> InterpCx --- src/eval.rs | 8 ++++---- src/machine.rs | 22 +++++++++++----------- src/shims/foreign_items.rs | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 91a563fa56b..b29ce353770 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -8,7 +8,7 @@ use rustc::mir; use crate::{ - InterpResult, InterpError, InterpretCx, StackPopCleanup, struct_error, + InterpResult, InterpError, InterpCx, StackPopCleanup, struct_error, Scalar, Tag, Pointer, MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, }; @@ -28,8 +28,8 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig, -) -> InterpResult<'tcx, InterpretCx<'mir, 'tcx, Evaluator<'tcx>>> { - let mut ecx = InterpretCx::new( +) -> InterpResult<'tcx, InterpCx<'mir, 'tcx, Evaluator<'tcx>>> { + let mut ecx = InterpCx::new( tcx.at(syntax::source_map::DUMMY_SP), ty::ParamEnv::reveal_all(), Evaluator::new(), @@ -43,7 +43,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( config.validate }; - // FIXME: InterpretCx::new should take an initial MemoryExtra + // FIXME: InterpCx::new should take an initial MemoryExtra ecx.memory_mut().extra = MemoryExtra::new(config.seed.map(StdRng::seed_from_u64), validate); let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); diff --git a/src/machine.rs b/src/machine.rs index a58903f6d51..930eeee96b5 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -105,8 +105,8 @@ pub(crate) fn new() -> Self { } } -/// A rustc InterpretCx for Miri. -pub type MiriEvalContext<'mir, 'tcx> = InterpretCx<'mir, 'tcx, Evaluator<'tcx>>; +/// A rustc InterpCx for Miri. +pub type MiriEvalContext<'mir, 'tcx> = InterpCx<'mir, 'tcx, Evaluator<'tcx>>; /// A little trait that's useful to be inherited by extension traits. pub trait MiriEvalContextExt<'mir, 'tcx> { @@ -138,14 +138,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { const STATIC_KIND: Option = Some(MiriMemoryKind::Static); #[inline(always)] - fn enforce_validity(ecx: &InterpretCx<'mir, 'tcx, Self>) -> bool { + fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { ecx.memory().extra.validate } /// Returns `Ok()` when the function was handled; fail otherwise. #[inline(always)] fn find_fn( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Tag>], dest: Option>, @@ -156,7 +156,7 @@ fn find_fn( #[inline(always)] fn call_intrinsic( - ecx: &mut rustc_mir::interpret::InterpretCx<'mir, 'tcx, Self>, + ecx: &mut rustc_mir::interpret::InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Tag>], dest: PlaceTy<'tcx, Tag>, @@ -166,7 +166,7 @@ fn call_intrinsic( #[inline(always)] fn ptr_op( - ecx: &rustc_mir::interpret::InterpretCx<'mir, 'tcx, Self>, + ecx: &rustc_mir::interpret::InterpCx<'mir, 'tcx, Self>, bin_op: mir::BinOp, left: ImmTy<'tcx, Tag>, right: ImmTy<'tcx, Tag>, @@ -175,7 +175,7 @@ fn ptr_op( } fn box_alloc( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, dest: PlaceTy<'tcx, Tag>, ) -> InterpResult<'tcx> { trace!("box_alloc for {:?}", dest.layout.ty); @@ -241,7 +241,7 @@ fn find_foreign_static( } #[inline(always)] - fn before_terminator(_ecx: &mut InterpretCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> + fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { // We are not interested in detecting loops. Ok(()) @@ -311,7 +311,7 @@ fn tag_static_base_pointer( #[inline(always)] fn retag( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, kind: mir::RetagKind, place: PlaceTy<'tcx, Tag>, ) -> InterpResult<'tcx> { @@ -325,14 +325,14 @@ fn retag( #[inline(always)] fn stack_push( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, ) -> InterpResult<'tcx, stacked_borrows::CallId> { Ok(ecx.memory().extra.stacked_borrows.borrow_mut().new_call()) } #[inline(always)] fn stack_pop( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, extra: stacked_borrows::CallId, ) -> InterpResult<'tcx> { Ok(ecx.memory().extra.stacked_borrows.borrow_mut().end_call(extra)) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 38fc36609e6..2fe2ecc1958 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -345,7 +345,7 @@ fn emulate_foreign_item( trace!("__rust_maybe_catch_panic: {:?}", f_instance); // Now we make a function call. - // TODO: consider making this reusable? `InterpretCx::step` does something similar + // TODO: consider making this reusable? `InterpCx::step` does something similar // for the TLS destructors, and of course `eval_main`. let mir = this.load_mir(f_instance.def)?; let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into(); From b75e9179bf21254a198e5b7359589cd0502d135c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 23:49:30 +0200 Subject: [PATCH 29/30] bump rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 9e4ddf2acaa..cbd8e335771 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -088b987307b91612ab164026e1dcdd0129fdb62b +24a9bcbb7cb0d8bdc11b8252a9c13f7562c7e4ca From 029a29407acd30454fb41f340a8d108a47bef349 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 23:51:11 +0200 Subject: [PATCH 30/30] dangling-ptr-to-int should work now; move to noseed --- tests/{run-pass => run-pass-noseed}/malloc.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) rename tests/{run-pass => run-pass-noseed}/malloc.rs (84%) diff --git a/tests/run-pass/malloc.rs b/tests/run-pass-noseed/malloc.rs similarity index 84% rename from tests/run-pass/malloc.rs rename to tests/run-pass-noseed/malloc.rs index a50b3f3606d..bf51baacd35 100644 --- a/tests/run-pass/malloc.rs +++ b/tests/run-pass-noseed/malloc.rs @@ -11,10 +11,8 @@ fn main() { // Test that small allocations sometimes *are* not very aligned. let saw_unaligned = (0..64).any(|_| unsafe { let p = libc::malloc(3); - let addr = p as usize; - let unaligned = addr % 4 != 0; // test that this is not 4-aligned - libc::free(p); // FIXME have to free *after* test; should allow ptr-to-int of dangling ptr. - unaligned + libc::free(p); + (p as usize) % 4 != 0 // find any that this is *not* 4-aligned }); assert!(saw_unaligned);