From 068517ae66a41052dea2c86c88fc10794304622d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 21:38:53 +0200 Subject: [PATCH 1/7] make sure we always have an RNG --- README.md | 8 ++-- src/eval.rs | 2 +- src/helpers.rs | 23 +++------- src/intptrcast.rs | 6 ++- src/machine.rs | 25 ++++------- src/operator.rs | 107 +++------------------------------------------- 6 files changed, 30 insertions(+), 141 deletions(-) diff --git a/README.md b/README.md index 9baaae568a..fbd1f51c9e 100644 --- a/README.md +++ b/README.md @@ -262,10 +262,10 @@ With this, you should now have a working development setup! See Several `-Z` flags are relevant for Miri: -* `-Zmiri-seed=` is a custom `-Z` flag added by Miri. It enables the - interpreted program to seed an RNG with system entropy. Miri will keep an RNG - on its own that is seeded with the given seed, and use that to generate the - "system entropy" that seeds the RNG(s) in the interpreted program. +* `-Zmiri-seed=` is a custom `-Z` flag added by Miri. It configures the + seed of the RNG that Miri uses to resolve non-determinism. This RNG is used + to pick base addresses for allocations, and when the interpreted program + requests system entropy. The default seed is 0. **NOTE**: This entropy is not good enough for cryptographic use! Do not generate secret keys in Miri or perform other kinds of cryptographic operations that rely on proper random numbers. diff --git a/src/eval.rs b/src/eval.rs index cde4da833e..83307c99f9 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -34,7 +34,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( tcx.at(syntax::source_map::DUMMY_SP), ty::ParamEnv::reveal_all(), Evaluator::new(), - MemoryExtra::new(config.seed.map(StdRng::seed_from_u64), config.validate), + MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate), ); let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); diff --git a/src/helpers.rs b/src/helpers.rs index 19abbd6b81..3857020a71 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -88,25 +88,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx None => return Ok(()), // zero-sized access }; - let data = match &mut this.memory_mut().extra.rng { - Some(rng) => { - let mut rng = rng.borrow_mut(); - let mut data = vec![0; len]; - rng.fill_bytes(&mut data); - data - } - None => { - return err!(Unimplemented( - "miri does not support gathering system entropy in deterministic mode! - Use '-Zmiri-seed=' to enable random number generation. - WARNING: Miri does *not* generate cryptographically secure entropy - - do not use Miri to run any program that needs secure random number generation".to_owned(), - )); - } - }; + let rng = this.memory_mut().extra.rng.get_mut(); + let mut data = vec![0; len]; + rng.fill_bytes(&mut data); + let tcx = &{this.tcx.tcx}; - this.memory_mut().get_mut(ptr.alloc_id)? - .write_bytes(tcx, ptr, &data) + this.memory_mut().get_mut(ptr.alloc_id)?.write_bytes(tcx, ptr, &data) } /// Visits the memory covered by `place`, sensitive to freezing: the 3rd parameter diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 5797895c54..8fbf7ed764 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -42,6 +42,10 @@ impl<'mir, 'tcx> GlobalState { int: u64, memory: &Memory<'mir, 'tcx, Evaluator<'tcx>>, ) -> InterpResult<'tcx, Pointer> { + if int == 0 { + return err!(InvalidNullPointerUsage); + } + let global_state = memory.extra.intptrcast.borrow(); match global_state.int_to_ptr_map.binary_search_by_key(&int, |(addr, _)| *addr) { @@ -86,7 +90,7 @@ impl<'mir, 'tcx> GlobalState { // This allocation does not have a base address yet, pick one. // Leave some space to the previous allocation, to give it some chance to be less aligned. let slack = { - let mut rng = memory.extra.rng.as_ref().unwrap().borrow_mut(); + let mut rng = memory.extra.rng.borrow_mut(); // This means that `(global_state.next_base_addr + slack) % 16` is uniformly distributed. rng.gen_range(0, 16) }; diff --git a/src/machine.rs b/src/machine.rs index c2e7802410..d50afec253 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -57,20 +57,19 @@ 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>, + /// The random number generator used for resolving non-determinism. + pub(crate) rng: RefCell, /// Whether to enforce the validity invariant. pub(crate) validate: bool, } impl MemoryExtra { - pub fn new(rng: Option, validate: bool) -> Self { + pub fn new(rng: StdRng, validate: bool) -> Self { MemoryExtra { stacked_borrows: Default::default(), intptrcast: Default::default(), - rng: rng.map(RefCell::new), + rng: RefCell::new(rng), validate, } } @@ -353,28 +352,20 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { Ok(ecx.memory().extra.stacked_borrows.borrow_mut().end_call(extra)) } + #[inline(always)] fn int_to_ptr( memory: &Memory<'mir, 'tcx, Self>, int: u64, ) -> InterpResult<'tcx, Pointer> { - if int == 0 { - err!(InvalidNullPointerUsage) - } else if memory.extra.rng.is_none() { - err!(ReadBytesAsPointer) - } else { - intptrcast::GlobalState::int_to_ptr(int, memory) - } + intptrcast::GlobalState::int_to_ptr(int, memory) } + #[inline(always)] fn ptr_to_int( memory: &Memory<'mir, 'tcx, Self>, ptr: Pointer, ) -> InterpResult<'tcx, u64> { - if memory.extra.rng.is_none() { - err!(ReadPointerAsBytes) - } else { - intptrcast::GlobalState::ptr_to_int(ptr, memory) - } + intptrcast::GlobalState::ptr_to_int(ptr, memory) } } diff --git a/src/operator.rs b/src/operator.rs index df60acc661..5eeed5eac6 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -56,8 +56,8 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { trace!("ptr_op: {:?} {:?} {:?}", *left, bin_op, *right); - // If intptrcast is enabled, treat everything of integer *type* at integer *value*. - if self.memory().extra.rng.is_some() && left.layout.ty.is_integral() { + // Treat everything of integer *type* at integer *value*. + if 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. @@ -188,104 +188,11 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { 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)?, - (Scalar::Ptr(left), Scalar::Ptr(right)) => { - // Comparison illegal if one of them is out-of-bounds, *unless* they - // are in the same allocation. - if left.alloc_id == right.alloc_id { - left.offset == right.offset - } else { - // Make sure both pointers are in-bounds. - // This accepts one-past-the end. Thus, there is still technically - // some non-determinism that we do not fully rule out when two - // allocations sit right next to each other. The C/C++ standards are - // somewhat fuzzy about this case, so pragmatically speaking I think - // for now this check is "good enough". - // FIXME: Once we support intptrcast, we could try to fix these holes. - // Dead allocations in miri cannot overlap with live allocations, but - // 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 (and hence live) pointers in different allocations are different. - false - } else { - return err!(InvalidPointerMath); - } - } - } - // Comparing ptr and integer. - (Scalar::Ptr(ptr), Scalar::Raw { data, size }) | - (Scalar::Raw { data, size }, Scalar::Ptr(ptr)) => { - assert_eq!(size as u64, self.pointer_size().bytes()); - let bits = data as u64; - - // Case I: Comparing real pointers with "small" integers. - // Really we should only do this for NULL, but pragmatically speaking on non-bare-metal systems, - // an allocation will never be at the very bottom of the address space. - // Such comparisons can arise when comparing empty slices, which sometimes are "fake" - // integer pointers (okay because the slice is empty) and sometimes point into a - // real allocation. - // The most common source of such integer pointers is `NonNull::dangling()`, which - // equals the type's alignment. i128 might have an alignment of 16 bytes, but few types have - // alignment 32 or higher, hence the limit of 32. - // FIXME: Once we support intptrcast, we could try to fix these holes. - if bits < 32 { - // Test if the pointer can be different from NULL or not. - // We assume that pointers that are not NULL are also not "small". - if !self.memory().ptr_may_be_null(ptr) { - return Ok(false); - } - } - - let (alloc_size, alloc_align) = self.memory() - .get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead) - .expect("alloc info with MaybeDead cannot fail"); - - // Case II: Alignment gives it away - if ptr.offset.bytes() % alloc_align.bytes() == 0 { - // The offset maintains the allocation alignment, so we know `base+offset` - // is aligned by `alloc_align`. - // FIXME: We could be even more general, e.g., offset 2 into a 4-aligned - // allocation cannot equal 3. - if bits % alloc_align.bytes() != 0 { - // The integer is *not* aligned. So they cannot be equal. - return Ok(false); - } - } - // Case III: The integer is too big, and the allocation goes on a bit - // without wrapping around the address space. - { - // Compute the highest address at which this allocation could live. - // Substract one more, because it must be possible to add the size - // to the base address without overflowing; that is, the very last address - // of the address space is never dereferencable (but it can be in-bounds, i.e., - // one-past-the-end). - let max_base_addr = - ((1u128 << self.pointer_size().bits()) - - u128::from(alloc_size.bytes()) - - 1 - ) as u64; - if let Some(max_addr) = max_base_addr.checked_add(ptr.offset.bytes()) { - if bits > max_addr { - // The integer is too big, this cannot possibly be equal. - return Ok(false) - } - } - } - - // None of the supported cases. - return err!(InvalidPointerMath); - } - }) + // 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)?; + Ok(left == right) } fn ptr_int_arithmetic( From c094d42504c41ebd4e452542f3fa2cbc303fc975 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 21:53:47 +0200 Subject: [PATCH 2/7] update miri-seed handling for run-pass test suite --- tests/compiletest.rs | 8 ++------ tests/{run-pass-noseed => run-pass}/hashmap.rs | 2 -- tests/{run-pass-noseed => run-pass}/heap_allocator.rs | 1 - tests/{run-pass-noseed => run-pass}/intptrcast.rs | 2 -- tests/{run-pass-noseed => run-pass}/malloc.rs | 1 - 5 files changed, 2 insertions(+), 12 deletions(-) rename tests/{run-pass-noseed => run-pass}/hashmap.rs (95%) rename tests/{run-pass-noseed => run-pass}/heap_allocator.rs (99%) rename tests/{run-pass-noseed => run-pass}/intptrcast.rs (94%) rename tests/{run-pass-noseed => run-pass}/malloc.rs (97%) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 16311d0749..9394084ad3 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -68,7 +68,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, noseed: bool) { +fn miri_pass(path: &str, target: &str, opt: bool) { let opt_str = if opt { " with optimizations" } else { "" }; eprintln!("{}", format!( "## Running run-pass tests in {} against miri for target {}{}", @@ -80,9 +80,6 @@ fn miri_pass(path: &str, target: &str, opt: bool, noseed: bool) { let mut flags = Vec::new(); if opt { 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. - flags.push("-Zmiri-seed=".to_owned()); } run_tests("ui", path, target, flags); @@ -106,8 +103,7 @@ fn get_target() -> String { } fn run_pass_miri(opt: bool) { - miri_pass("tests/run-pass", &get_target(), opt, false); - miri_pass("tests/run-pass-noseed", &get_target(), opt, true); + miri_pass("tests/run-pass", &get_target(), opt); } fn compile_fail_miri(opt: bool) { diff --git a/tests/run-pass-noseed/hashmap.rs b/tests/run-pass/hashmap.rs similarity index 95% rename from tests/run-pass-noseed/hashmap.rs rename to tests/run-pass/hashmap.rs index e249238d48..1ff9c26ba1 100644 --- a/tests/run-pass-noseed/hashmap.rs +++ b/tests/run-pass/hashmap.rs @@ -1,5 +1,3 @@ -// compile-flags: -Zmiri-seed=0000000000000000 - use std::collections::{self, HashMap}; use std::hash::{BuildHasherDefault, BuildHasher}; diff --git a/tests/run-pass-noseed/heap_allocator.rs b/tests/run-pass/heap_allocator.rs similarity index 99% rename from tests/run-pass-noseed/heap_allocator.rs rename to tests/run-pass/heap_allocator.rs index e7a609a7d9..7bcb08058c 100644 --- a/tests/run-pass-noseed/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -1,4 +1,3 @@ -// compile-flags: -Zmiri-seed= #![feature(allocator_api)] use std::ptr::NonNull; diff --git a/tests/run-pass-noseed/intptrcast.rs b/tests/run-pass/intptrcast.rs similarity index 94% rename from tests/run-pass-noseed/intptrcast.rs rename to tests/run-pass/intptrcast.rs index 1b5251c911..c28958b872 100644 --- a/tests/run-pass-noseed/intptrcast.rs +++ b/tests/run-pass/intptrcast.rs @@ -1,5 +1,3 @@ -// compile-flags: -Zmiri-seed=0000000000000000 - // 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) } diff --git a/tests/run-pass-noseed/malloc.rs b/tests/run-pass/malloc.rs similarity index 97% rename from tests/run-pass-noseed/malloc.rs rename to tests/run-pass/malloc.rs index bf51baacd3..f66263425e 100644 --- a/tests/run-pass-noseed/malloc.rs +++ b/tests/run-pass/malloc.rs @@ -1,5 +1,4 @@ //ignore-windows: Uses POSIX APIs -//compile-flags: -Zmiri-seed= #![feature(rustc_private)] From 3c1ab7819698b374d833e8818e39ef011a48eb83 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 23:25:06 +0200 Subject: [PATCH 3/7] review failing compile-fail tests --- tests/compile-fail/cast_box_int_to_fn_ptr.rs | 4 +- tests/compile-fail/cast_int_to_fn_ptr.rs | 2 +- tests/compile-fail/getrandom.rs | 13 ---- ...er_byte_read_2.rs => pointer_byte_read.rs} | 0 tests/compile-fail/pointer_byte_read_1.rs | 7 -- tests/compile-fail/ptr_bitops1.rs | 7 -- tests/compile-fail/ptr_bitops2.rs | 5 -- tests/compile-fail/ptr_eq_dangling.rs | 10 --- tests/compile-fail/ptr_eq_integer.rs | 6 -- tests/compile-fail/ptr_eq_out_of_bounds.rs | 9 --- .../compile-fail/ptr_eq_out_of_bounds_null.rs | 6 -- tests/compile-fail/ptr_int_cast.rs | 8 --- tests/compile-fail/ptr_offset_int_plus_int.rs | 2 +- tests/compile-fail/ptr_offset_int_plus_ptr.rs | 2 +- tests/compile-fail/ptr_rem.rs | 5 -- .../ptr_wrapping_offset_int_plus_ptr.rs | 8 --- tests/compile-fail/validity/dangling_ref1.rs | 2 +- tests/compile-fail/wild_pointer_deref.rs | 2 +- .../bitop-beyond-alignment.rs | 2 +- tests/run-pass/intptrcast.rs | 67 ++++++++++++++++++- 20 files changed, 74 insertions(+), 93 deletions(-) delete mode 100644 tests/compile-fail/getrandom.rs rename tests/compile-fail/{pointer_byte_read_2.rs => pointer_byte_read.rs} (100%) delete mode 100644 tests/compile-fail/pointer_byte_read_1.rs delete mode 100644 tests/compile-fail/ptr_bitops1.rs delete mode 100644 tests/compile-fail/ptr_bitops2.rs delete mode 100644 tests/compile-fail/ptr_eq_dangling.rs delete mode 100644 tests/compile-fail/ptr_eq_integer.rs delete mode 100644 tests/compile-fail/ptr_eq_out_of_bounds.rs delete mode 100644 tests/compile-fail/ptr_eq_out_of_bounds_null.rs delete mode 100644 tests/compile-fail/ptr_int_cast.rs delete mode 100644 tests/compile-fail/ptr_rem.rs delete mode 100644 tests/compile-fail/ptr_wrapping_offset_int_plus_ptr.rs rename tests/{compile-fail => run-pass}/bitop-beyond-alignment.rs (87%) 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 7ee3bc6276..6dad2a4727 100644 --- a/tests/compile-fail/cast_box_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_box_int_to_fn_ptr.rs @@ -4,8 +4,8 @@ fn main() { let b = Box::new(42); let g = unsafe { - std::mem::transmute::<&usize, &fn(i32)>(&b) + std::mem::transmute::<&Box, &fn(i32)>(&b) }; - (*g)(42) //~ ERROR a memory access tried to interpret some bytes as a pointer + (*g)(42) //~ ERROR tried to treat a memory pointer as a function pointer } diff --git a/tests/compile-fail/cast_int_to_fn_ptr.rs b/tests/compile-fail/cast_int_to_fn_ptr.rs index 207af4ae2c..4fd14751a2 100644 --- a/tests/compile-fail/cast_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_int_to_fn_ptr.rs @@ -6,5 +6,5 @@ fn main() { std::mem::transmute::(42) }; - g(42) //~ ERROR a memory access tried to interpret some bytes as a pointer + g(42) //~ ERROR dangling pointer was dereferenced } diff --git a/tests/compile-fail/getrandom.rs b/tests/compile-fail/getrandom.rs deleted file mode 100644 index 246893a5c6..0000000000 --- a/tests/compile-fail/getrandom.rs +++ /dev/null @@ -1,13 +0,0 @@ -// ignore-macos: Uses Linux-only APIs -// ignore-windows: Uses Linux-only APIs - -#![feature(rustc_private)] -extern crate libc; - -fn main() { - let mut buf = [0u8; 5]; - unsafe { - libc::syscall(libc::SYS_getrandom, buf.as_mut_ptr() as *mut libc::c_void, 5 as libc::size_t, 0 as libc::c_uint); - //~^ ERROR miri does not support gathering system entropy in deterministic mode! - } -} diff --git a/tests/compile-fail/pointer_byte_read_2.rs b/tests/compile-fail/pointer_byte_read.rs similarity index 100% rename from tests/compile-fail/pointer_byte_read_2.rs rename to tests/compile-fail/pointer_byte_read.rs diff --git a/tests/compile-fail/pointer_byte_read_1.rs b/tests/compile-fail/pointer_byte_read_1.rs deleted file mode 100644 index a584863654..0000000000 --- a/tests/compile-fail/pointer_byte_read_1.rs +++ /dev/null @@ -1,7 +0,0 @@ -fn main() { - let x = 13; - let y = &x; - let z = &y as *const &i32 as *const usize; - let ptr_bytes = unsafe { *z }; // the actual deref is fine, because we read the entire pointer at once - let _val = ptr_bytes / 432; //~ ERROR invalid arithmetic on pointers that would leak base addresses -} diff --git a/tests/compile-fail/ptr_bitops1.rs b/tests/compile-fail/ptr_bitops1.rs deleted file mode 100644 index 2706b0970d..0000000000 --- a/tests/compile-fail/ptr_bitops1.rs +++ /dev/null @@ -1,7 +0,0 @@ -fn main() { - let bytes = [0i8, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - let one = bytes.as_ptr().wrapping_offset(1); - let three = bytes.as_ptr().wrapping_offset(3); - let res = (one as usize) | (three as usize); //~ ERROR invalid arithmetic on pointers that would leak base addresses - println!("{}", res); -} diff --git a/tests/compile-fail/ptr_bitops2.rs b/tests/compile-fail/ptr_bitops2.rs deleted file mode 100644 index 5d5eab1550..0000000000 --- a/tests/compile-fail/ptr_bitops2.rs +++ /dev/null @@ -1,5 +0,0 @@ -fn main() { - let val = 13usize; - let addr = &val as *const _ as usize; - let _val = addr & 13; //~ ERROR access part of a pointer value as raw bytes -} diff --git a/tests/compile-fail/ptr_eq_dangling.rs b/tests/compile-fail/ptr_eq_dangling.rs deleted file mode 100644 index 5badf099e4..0000000000 --- a/tests/compile-fail/ptr_eq_dangling.rs +++ /dev/null @@ -1,10 +0,0 @@ -fn main() { - let b = Box::new(0); - let x = &*b as *const i32; // soon-to-be dangling - drop(b); - let b = Box::new(0); - let y = &*b as *const i32; // different allocation - // We cannot compare these even though both are inbounds -- they *could* be - // equal if memory was reused. - assert!(x != y); //~ ERROR invalid arithmetic on pointers -} diff --git a/tests/compile-fail/ptr_eq_integer.rs b/tests/compile-fail/ptr_eq_integer.rs deleted file mode 100644 index 396abaf449..0000000000 --- a/tests/compile-fail/ptr_eq_integer.rs +++ /dev/null @@ -1,6 +0,0 @@ -fn main() { - let b = Box::new(0); - let x = &*b as *const i32; - // We cannot compare this with a non-NULL integer. After all, these *could* be equal (with the right base address). - assert!(x != 64 as *const i32); //~ ERROR invalid arithmetic on pointers -} diff --git a/tests/compile-fail/ptr_eq_out_of_bounds.rs b/tests/compile-fail/ptr_eq_out_of_bounds.rs deleted file mode 100644 index 7efa446d7f..0000000000 --- a/tests/compile-fail/ptr_eq_out_of_bounds.rs +++ /dev/null @@ -1,9 +0,0 @@ -fn main() { - let b = Box::new(0); - let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds - let b = Box::new(0); - let y = &*b as *const i32; // different allocation - // We cannot compare these even though both allocations are live -- they *could* be - // equal (with the right base addresses). - assert!(x != y); //~ ERROR invalid arithmetic on pointers -} diff --git a/tests/compile-fail/ptr_eq_out_of_bounds_null.rs b/tests/compile-fail/ptr_eq_out_of_bounds_null.rs deleted file mode 100644 index 3b7b51fc19..0000000000 --- a/tests/compile-fail/ptr_eq_out_of_bounds_null.rs +++ /dev/null @@ -1,6 +0,0 @@ -fn main() { - let b = Box::new(0); - let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds - // We cannot compare this with NULL. After all, this *could* be NULL (with the right base address). - assert!(x != std::ptr::null()); //~ ERROR invalid arithmetic on pointers -} diff --git a/tests/compile-fail/ptr_int_cast.rs b/tests/compile-fail/ptr_int_cast.rs deleted file mode 100644 index a823a0f49b..0000000000 --- a/tests/compile-fail/ptr_int_cast.rs +++ /dev/null @@ -1,8 +0,0 @@ -fn main() { - let x = &1; - // Casting down to u8 and back up to a pointer loses too much precision; this must not work. - let x = x as *const i32; - let x = x as u8; //~ ERROR a raw memory access tried to access part of a pointer value as raw bytes - let x = x as *const i32; - let _val = unsafe { *x }; -} diff --git a/tests/compile-fail/ptr_offset_int_plus_int.rs b/tests/compile-fail/ptr_offset_int_plus_int.rs index 2d3282a0a9..051874696b 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: tried to interpret some bytes as a pointer +// error-pattern: dangling pointer was dereferenced fn main() { // Can't offset an integer pointer by non-zero offset. diff --git a/tests/compile-fail/ptr_offset_int_plus_ptr.rs b/tests/compile-fail/ptr_offset_int_plus_ptr.rs index b49c758c72..bd90d06909 100644 --- a/tests/compile-fail/ptr_offset_int_plus_ptr.rs +++ b/tests/compile-fail/ptr_offset_int_plus_ptr.rs @@ -1,4 +1,4 @@ -// error-pattern: pointer value as raw bytes +// error-pattern: dangling pointer was dereferenced fn main() { let ptr = Box::into_raw(Box::new(0u32)); diff --git a/tests/compile-fail/ptr_rem.rs b/tests/compile-fail/ptr_rem.rs deleted file mode 100644 index dfc91e9dc1..0000000000 --- a/tests/compile-fail/ptr_rem.rs +++ /dev/null @@ -1,5 +0,0 @@ -fn main() { - let val = 13usize; - let addr = &val as *const _ as usize; - let _val = addr % 16; //~ ERROR access part of a pointer value as raw bytes -} diff --git a/tests/compile-fail/ptr_wrapping_offset_int_plus_ptr.rs b/tests/compile-fail/ptr_wrapping_offset_int_plus_ptr.rs deleted file mode 100644 index eacb9f07ff..0000000000 --- a/tests/compile-fail/ptr_wrapping_offset_int_plus_ptr.rs +++ /dev/null @@ -1,8 +0,0 @@ -// error-pattern: pointer value as raw bytes - -fn main() { - let ptr = Box::into_raw(Box::new(0u32)); - // Can't start with an integer pointer and get to something usable - let ptr = (1 as *mut u8).wrapping_offset(ptr as isize); - let _val = unsafe { *ptr }; -} diff --git a/tests/compile-fail/validity/dangling_ref1.rs b/tests/compile-fail/validity/dangling_ref1.rs index 2c1984b715..194445b1ad 100644 --- a/tests/compile-fail/validity/dangling_ref1.rs +++ b/tests/compile-fail/validity/dangling_ref1.rs @@ -1,5 +1,5 @@ use std::mem; fn main() { - let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR dangling reference (created from integer) + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR dangling reference (not entirely in bounds) } diff --git a/tests/compile-fail/wild_pointer_deref.rs b/tests/compile-fail/wild_pointer_deref.rs index 8eec973754..ae32f7d556 100644 --- a/tests/compile-fail/wild_pointer_deref.rs +++ b/tests/compile-fail/wild_pointer_deref.rs @@ -1,5 +1,5 @@ fn main() { let p = 44 as *const i32; - let x = unsafe { *p }; //~ ERROR a memory access tried to interpret some bytes as a pointer + let x = unsafe { *p }; //~ ERROR dangling pointer was dereferenced panic!("this should never print: {}", x); } diff --git a/tests/compile-fail/bitop-beyond-alignment.rs b/tests/run-pass/bitop-beyond-alignment.rs similarity index 87% rename from tests/compile-fail/bitop-beyond-alignment.rs rename to tests/run-pass/bitop-beyond-alignment.rs index a30c054ab5..f7bf7f9ff5 100644 --- a/tests/compile-fail/bitop-beyond-alignment.rs +++ b/tests/run-pass/bitop-beyond-alignment.rs @@ -28,7 +28,7 @@ fn mk_rec() -> Rec { fn is_u64_aligned(u: &Tag) -> bool { let p: usize = unsafe { mem::transmute(u) }; let u64_align = std::mem::align_of::(); - return (p & (u64_align + 1)) == 0; //~ ERROR a raw memory access tried to access part of a pointer value as raw bytes + return (p & (u64_align + 1)) == 0; } pub fn main() { diff --git a/tests/run-pass/intptrcast.rs b/tests/run-pass/intptrcast.rs index c28958b872..c2711f9845 100644 --- a/tests/run-pass/intptrcast.rs +++ b/tests/run-pass/intptrcast.rs @@ -3,18 +3,22 @@ fn transmute_ptr_to_int(x: *const T) -> usize { unsafe { std::mem::transmute(x) } } -fn main() { +fn cast() { // Some casting-to-int with arithmetic. let x = &42 as *const i32 as usize; let y = x * 2; assert_eq!(y, x + x); let z = y as u8 as usize; assert_eq!(z, y % 256); +} +fn format() { // 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 _); +} +fn transmute() { // Check that intptrcast is triggered for explicit casts and that it is consistent with // transmuting. let a: *const i32 = &42; @@ -22,3 +26,64 @@ fn main() { let c = a as usize as u8; assert_eq!(b, c); } + +fn ptr_bitops1() { + let bytes = [0i8, 1, 2, 3, 4, 5, 6, 7, 8, 9]; + let one = bytes.as_ptr().wrapping_offset(1); + let three = bytes.as_ptr().wrapping_offset(3); + let res = (one as usize) | (three as usize); + format!("{}", res); +} + +fn ptr_bitops2() { + let val = 13usize; + let addr = &val as *const _ as usize; + let _val = addr & 13; +} + +fn ptr_eq_dangling() { + let b = Box::new(0); + let x = &*b as *const i32; // soon-to-be dangling + drop(b); + let b = Box::new(0); + let y = &*b as *const i32; // different allocation + // We cannot compare these even though both are inbounds -- they *could* be + // equal if memory was reused. + assert!(x != y); +} + +fn ptr_eq_out_of_bounds() { + let b = Box::new(0); + let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds + let b = Box::new(0); + let y = &*b as *const i32; // different allocation + // We cannot compare these even though both allocations are live -- they *could* be + // equal (with the right base addresses). + assert!(x != y); +} + +fn ptr_eq_out_of_bounds_null() { + let b = Box::new(0); + let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds + // We cannot compare this with NULL. After all, this *could* be NULL (with the right base address). + assert!(x != std::ptr::null()); +} + +fn ptr_eq_integer() { + let b = Box::new(0); + let x = &*b as *const i32; + // We cannot compare this with a non-NULL integer. After all, these *could* be equal (with the right base address). + assert!(x != 64 as *const i32); +} + +fn main() { + cast(); + format(); + transmute(); + ptr_bitops1(); + ptr_bitops2(); + ptr_eq_dangling(); + ptr_eq_out_of_bounds(); + ptr_eq_out_of_bounds_null(); + ptr_eq_integer(); +} From d5ca345c36b037c4cdfe1a9f165464e7f815e2c1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 23:26:25 +0200 Subject: [PATCH 4/7] remove redundant tests / flags --- tests/compile-fail/intptrcast_alignment_check.rs | 2 +- tests/compile-fail/intptrcast_cast_int_to_fn_ptr.rs | 10 ---------- tests/compile-fail/intptrcast_null_pointer_deref.rs | 6 ------ tests/compile-fail/intptrcast_wild_pointer_deref.rs | 7 ------- 4 files changed, 1 insertion(+), 24 deletions(-) delete mode 100644 tests/compile-fail/intptrcast_cast_int_to_fn_ptr.rs delete mode 100644 tests/compile-fail/intptrcast_null_pointer_deref.rs delete mode 100644 tests/compile-fail/intptrcast_wild_pointer_deref.rs diff --git a/tests/compile-fail/intptrcast_alignment_check.rs b/tests/compile-fail/intptrcast_alignment_check.rs index 5a35844d12..4e12a8a9e2 100644 --- a/tests/compile-fail/intptrcast_alignment_check.rs +++ b/tests/compile-fail/intptrcast_alignment_check.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmiri-disable-validation -Zmiri-seed=0000000000000000 +// compile-flags: -Zmiri-disable-validation // Even with intptrcast and without validation, we want to be *sure* to catch bugs // that arise from pointers being insufficiently aligned. The only way to achieve diff --git a/tests/compile-fail/intptrcast_cast_int_to_fn_ptr.rs b/tests/compile-fail/intptrcast_cast_int_to_fn_ptr.rs deleted file mode 100644 index f4064cf92e..0000000000 --- a/tests/compile-fail/intptrcast_cast_int_to_fn_ptr.rs +++ /dev/null @@ -1,10 +0,0 @@ -// Validation makes this fail in the wrong place -// compile-flags: -Zmiri-disable-validation -Zmiri-seed=0000000000000000 - -fn main() { - let g = unsafe { - std::mem::transmute::(42) - }; - - g(42) //~ ERROR dangling pointer was dereferenced -} diff --git a/tests/compile-fail/intptrcast_null_pointer_deref.rs b/tests/compile-fail/intptrcast_null_pointer_deref.rs deleted file mode 100644 index 0c5d46609c..0000000000 --- a/tests/compile-fail/intptrcast_null_pointer_deref.rs +++ /dev/null @@ -1,6 +0,0 @@ -// compile-flags: -Zmiri-seed=0000000000000000 - -fn main() { - let x: i32 = unsafe { *std::ptr::null() }; //~ ERROR invalid use of NULL pointer - panic!("this should never print: {}", x); -} diff --git a/tests/compile-fail/intptrcast_wild_pointer_deref.rs b/tests/compile-fail/intptrcast_wild_pointer_deref.rs deleted file mode 100644 index 2ee664eb68..0000000000 --- a/tests/compile-fail/intptrcast_wild_pointer_deref.rs +++ /dev/null @@ -1,7 +0,0 @@ -// compile-flags: -Zmiri-seed=0000000000000000 - -fn main() { - let p = 44 as *const i32; - let x = unsafe { *p }; //~ ERROR dangling pointer was dereferenced - panic!("this should never print: {}", x); -} From 724cf41eb1a4fa529a5b1fe629d077dbb217019f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 23:43:37 +0200 Subject: [PATCH 5/7] use checked arithmetic in intrptrcast --- src/intptrcast.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 8fbf7ed764..1247150cc1 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -95,7 +95,8 @@ impl<'mir, 'tcx> GlobalState { rng.gen_range(0, 16) }; // From next_base_addr + slack, round up to adjust for alignment. - let base_addr = Self::align_addr(global_state.next_base_addr + slack, align.bytes()); + let base_addr = global_state.next_base_addr.checked_add(slack).unwrap(); + let base_addr = Self::align_addr(base_addr, align.bytes()); entry.insert(base_addr); trace!( "Assigning base address {:#x} to allocation {:?} (slack: {}, align: {})", @@ -104,7 +105,7 @@ impl<'mir, 'tcx> GlobalState { // Remember next base address. If this allocation is zero-sized, leave a gap // of at least 1 to avoid two allocations having the same base address. - global_state.next_base_addr = base_addr + max(size.bytes(), 1); + global_state.next_base_addr = base_addr.checked_add(max(size.bytes(), 1)).unwrap(); // Given that `next_base_addr` increases in each allocation, pushing the // corresponding tuple keeps `int_to_ptr_map` sorted global_state.int_to_ptr_map.push((base_addr, ptr.alloc_id)); @@ -124,7 +125,7 @@ impl<'mir, 'tcx> GlobalState { fn align_addr(addr: u64, align: u64) -> u64 { match addr % align { 0 => addr, - rem => addr + align - rem + rem => addr.checked_add(align).unwrap() - rem } } } From 85be8ab8ebaf5487ef7e852db809751d97a36605 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Jul 2019 23:48:28 +0200 Subject: [PATCH 6/7] fix non-deterministic test --- tests/run-pass/bitop-beyond-alignment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-pass/bitop-beyond-alignment.rs b/tests/run-pass/bitop-beyond-alignment.rs index f7bf7f9ff5..f609822464 100644 --- a/tests/run-pass/bitop-beyond-alignment.rs +++ b/tests/run-pass/bitop-beyond-alignment.rs @@ -33,5 +33,5 @@ fn is_u64_aligned(u: &Tag) -> bool { pub fn main() { let x = mk_rec(); - assert!(is_u64_aligned(&x.t)); + is_u64_aligned(&x.t); // the result of this is non-deterministic } From 758d88bbf96c726ca8c620068bb20f552b8bd35a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Jul 2019 08:57:05 +0200 Subject: [PATCH 7/7] explain better what is non-deterministic here --- tests/run-pass/bitop-beyond-alignment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-pass/bitop-beyond-alignment.rs b/tests/run-pass/bitop-beyond-alignment.rs index f609822464..a30f0fb613 100644 --- a/tests/run-pass/bitop-beyond-alignment.rs +++ b/tests/run-pass/bitop-beyond-alignment.rs @@ -33,5 +33,5 @@ fn is_u64_aligned(u: &Tag) -> bool { pub fn main() { let x = mk_rec(); - is_u64_aligned(&x.t); // the result of this is non-deterministic + is_u64_aligned(&x.t); // the result of this is non-deterministic (even with a fixed seed, results vary between platforms) }