diff --git a/rust-version b/rust-version index 5422121beb..1733872825 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -305930cffeac1da0fd73a08d9f5680e4a49bfb9f +7e08576e4276a97b523c25bfd196d419c39c7b87 diff --git a/src/fn_call.rs b/src/fn_call.rs index eca1d9144e..b053d8c51e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -654,7 +654,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Hook pthread calls that go to the thread-local storage memory subsystem. "pthread_key_create" => { - let key_ptr = this.read_scalar(args[0])?.to_ptr()?; + 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()? { @@ -681,7 +681,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx return err!(OutOfTls); } - this.memory().check_align(key_ptr.into(), key_layout.align.abi)?; + let key_ptr = this.memory().check_ptr_access(key_ptr, key_layout.size, key_layout.align.abi)? + .expect("cannot be a ZST"); this.memory_mut().get_mut(key_ptr.alloc_id)?.write_scalar( tcx, key_ptr, diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 33bab4642a..3f9c4e53f0 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -517,13 +517,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let val_byte = this.read_scalar(args[1])?.to_u8()?; let ptr = this.read_scalar(args[0])?.not_undef()?; let count = this.read_scalar(args[2])?.to_usize(this)?; - this.memory().check_align(ptr, ty_layout.align.abi)?; let byte_count = ty_layout.size * count; - if byte_count.bytes() != 0 { - let ptr = ptr.to_ptr()?; - this.memory_mut() - .get_mut(ptr.alloc_id)? - .write_repeat(tcx, ptr, val_byte, byte_count)?; + match this.memory().check_ptr_access(ptr, byte_count, ty_layout.align.abi)? { + Some(ptr) => { + this.memory_mut() + .get_mut(ptr.alloc_id)? + .write_repeat(tcx, ptr, val_byte, byte_count)?; + } + None => { + // Size is 0, nothing to do. + } } } diff --git a/src/operator.rs b/src/operator.rs index b05d435ef6..cb258cf150 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -4,6 +4,11 @@ use rustc::mir; use crate::*; pub trait EvalContextExt<'tcx> { + fn pointer_inbounds( + &self, + ptr: Pointer + ) -> InterpResult<'tcx>; + fn ptr_op( &self, bin_op: mir::BinOp, @@ -34,6 +39,13 @@ pub trait EvalContextExt<'tcx> { } impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { + /// Test if the pointer is in-bounds of a live allocation. + #[inline] + fn pointer_inbounds(&self, ptr: Pointer) -> InterpResult<'tcx> { + let (size, _align) = self.memory().get_size_and_align(ptr.alloc_id, AllocCheck::Live)?; + ptr.check_in_alloc(size, CheckInAllocMsg::InboundsTest) + } + fn ptr_op( &self, bin_op: mir::BinOp, @@ -114,8 +126,8 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { let left = left.to_ptr().expect("we checked is_ptr"); let right = right.to_bits(self.memory().pointer_size()).expect("we checked is_bits"); let (_alloc_size, alloc_align) = self.memory() - .get_size_and_align(left.alloc_id, InboundsCheck::MaybeDead) - .expect("determining size+align of dead ptr cannot fail"); + .get_size_and_align(left.alloc_id, AllocCheck::MaybeDead) + .expect("alloc info with MaybeDead cannot fail"); let min_ptr_val = u128::from(alloc_align.bytes()) + u128::from(left.offset.bytes()); let result = match bin_op { Gt => min_ptr_val > right, @@ -170,6 +182,7 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { 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 @@ -179,10 +192,12 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { // 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. - self.memory().check_bounds_ptr(left, InboundsCheck::Live, CheckInAllocMsg::InboundsTest)?; - self.memory().check_bounds_ptr(right, InboundsCheck::Live, CheckInAllocMsg::InboundsTest)?; - // Two in-bounds pointers, we can compare across allocations. - left == right + if self.pointer_inbounds(left).is_ok() && self.pointer_inbounds(right).is_ok() { + // Two in-bounds pointers in different allocations are different. + false + } else { + return err!(InvalidPointerMath); + } } } // Comparing ptr and integer. @@ -202,16 +217,16 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { // 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 ptr is in-bounds. Then it cannot be NULL. - // Even dangling pointers cannot be NULL. - if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead, CheckInAllocMsg::NullPointerTest).is_ok() { + // 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, InboundsCheck::MaybeDead) - .expect("determining size+align of dead ptr cannot fail"); + .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 { @@ -359,9 +374,9 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> { if let Scalar::Ptr(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.memory().check_bounds_ptr(ptr, InboundsCheck::Live, CheckInAllocMsg::InboundsTest)?; + self.pointer_inbounds(ptr)?; let ptr = ptr.signed_offset(offset, self)?; - self.memory().check_bounds_ptr(ptr, InboundsCheck::Live, CheckInAllocMsg::InboundsTest)?; + self.pointer_inbounds(ptr)?; Ok(Scalar::Ptr(ptr)) } else { // An integer pointer. They can only be offset by 0, and we pretend there diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index f761767670..3a0c257428 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -10,7 +10,7 @@ use rustc::mir::RetagKind; use crate::{ InterpResult, InterpError, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor, - MemoryKind, MiriMemoryKind, RangeMap, Allocation, AllocationExtra, AllocId, CheckInAllocMsg, + MemoryKind, MiriMemoryKind, RangeMap, Allocation, AllocationExtra, AllocId, Pointer, Immediate, ImmTy, PlaceTy, MPlaceTy, }; @@ -531,13 +531,14 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let protector = if protect { Some(this.frame().extra) } else { None }; - let ptr = place.ptr.to_ptr()?; + let ptr = this.memory().check_ptr_access(place.ptr, size, place.align) + .expect("validity checks should have excluded dangling/unaligned pointer") + .expect("we shouldn't get here for ZST"); trace!("reborrow: {} reference {:?} derived from {:?} (pointee {}): {:?}, size {}", kind, new_tag, ptr.tag, place.layout.ty, ptr.erase_tag(), size.bytes()); // Get the allocation. It might not be mutable, so we cannot use `get_mut`. let alloc = this.memory().get(ptr.alloc_id)?; - alloc.check_bounds(this, ptr, size, CheckInAllocMsg::InboundsTest)?; // 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! diff --git a/tests/compile-fail/ptr_eq_dangling.rs b/tests/compile-fail/ptr_eq_dangling.rs index d05996a13d..5badf099e4 100644 --- a/tests/compile-fail/ptr_eq_dangling.rs +++ b/tests/compile-fail/ptr_eq_dangling.rs @@ -6,5 +6,5 @@ fn main() { 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 dangling pointer + assert!(x != y); //~ 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 index af4eed8d4e..7efa446d7f 100644 --- a/tests/compile-fail/ptr_eq_out_of_bounds.rs +++ b/tests/compile-fail/ptr_eq_out_of_bounds.rs @@ -5,5 +5,5 @@ fn main() { 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 outside bounds + assert!(x != y); //~ ERROR invalid arithmetic on pointers } diff --git a/tests/compile-fail/rc_as_raw.rs b/tests/compile-fail/rc_as_raw.rs index 3e6e96456f..086467eea3 100644 --- a/tests/compile-fail/rc_as_raw.rs +++ b/tests/compile-fail/rc_as_raw.rs @@ -1,3 +1,5 @@ +// This should fail even without validation +// compile-flags: -Zmiri-disable-validation #![feature(weak_into_raw)] use std::rc::{Rc, Weak}; diff --git a/tests/compile-fail/unaligned_ptr1.rs b/tests/compile-fail/unaligned_ptr1.rs new file mode 100644 index 0000000000..bcc4192d7d --- /dev/null +++ b/tests/compile-fail/unaligned_ptr1.rs @@ -0,0 +1,9 @@ +// This should fail even without validation +// compile-flags: -Zmiri-disable-validation + +fn main() { + let x = [2u16, 3, 4]; // Make it big enough so we don't get an out-of-bounds error. + let x = &x[0] as *const _ as *const u32; + // This must fail because alignment is violated: the allocation's base is not sufficiently aligned. + let _x = unsafe { *x }; //~ ERROR tried to access memory with alignment 2, but alignment 4 is required +} diff --git a/tests/compile-fail/unaligned_ptr2.rs b/tests/compile-fail/unaligned_ptr2.rs new file mode 100644 index 0000000000..225bd14ade --- /dev/null +++ b/tests/compile-fail/unaligned_ptr2.rs @@ -0,0 +1,10 @@ +// This should fail even without validation. +// compile-flags: -Zmiri-disable-validation + +fn main() { + let x = [2u32, 3]; // Make it big enough so we don't get an out-of-bounds error. + let x = (x.as_ptr() as *const u8).wrapping_offset(3) as *const u32; + // This must fail because alignment is violated: the offset is not sufficiently aligned. + // Also make the offset not a power of 2, that used to ICE. + let _x = unsafe { *x }; //~ ERROR tried to access memory with alignment 1, but alignment 4 is required +} diff --git a/tests/compile-fail/unaligned_ptr_cast2.rs b/tests/compile-fail/unaligned_ptr3.rs similarity index 64% rename from tests/compile-fail/unaligned_ptr_cast2.rs rename to tests/compile-fail/unaligned_ptr3.rs index 9fb138e353..f33a80d458 100644 --- a/tests/compile-fail/unaligned_ptr_cast2.rs +++ b/tests/compile-fail/unaligned_ptr3.rs @@ -2,8 +2,8 @@ // compile-flags: -Zmiri-disable-validation fn main() { - let x = &2u16; - let x = x as *const _ as *const *const u8; + let x = [2u16, 3, 4, 5]; // Make it big enough so we don't get an out-of-bounds error. + let x = &x[0] as *const _ as *const *const u8; // cast to ptr-to-ptr, so that we load a ptr // This must fail because alignment is violated. Test specifically for loading pointers, // which have special code in miri's memory. let _x = unsafe { *x }; diff --git a/tests/compile-fail/unaligned_ptr_cast1.rs b/tests/compile-fail/unaligned_ptr_cast1.rs deleted file mode 100644 index e64594d3e7..0000000000 --- a/tests/compile-fail/unaligned_ptr_cast1.rs +++ /dev/null @@ -1,9 +0,0 @@ -// This should fail even without validation -// compile-flags: -Zmiri-disable-validation - -fn main() { - let x = &2u16; - let x = x as *const _ as *const u32; - // This must fail because alignment is violated - let _x = unsafe { *x }; //~ ERROR tried to access memory with alignment 2, but alignment 4 is required -} diff --git a/tests/compile-fail/unaligned_ptr_cast_zst.rs b/tests/compile-fail/unaligned_ptr_zst.rs similarity index 75% rename from tests/compile-fail/unaligned_ptr_cast_zst.rs rename to tests/compile-fail/unaligned_ptr_zst.rs index d52b569175..127ec04027 100644 --- a/tests/compile-fail/unaligned_ptr_cast_zst.rs +++ b/tests/compile-fail/unaligned_ptr_zst.rs @@ -1,3 +1,6 @@ +// This should fail even without validation +// compile-flags: -Zmiri-disable-validation + fn main() { let x = &2u16; let x = x as *const _ as *const [u32; 0]; diff --git a/tests/compile-fail/validity/dangling_ref1.rs b/tests/compile-fail/validity/dangling_ref1.rs index c5845cb693..4318c7c902 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 tried to interpret some bytes as a pointer + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR integer pointer in non-ZST reference } diff --git a/tests/compile-fail/validity/dangling_ref2.rs b/tests/compile-fail/validity/dangling_ref2.rs index 21650ebf95..ef76b93d11 100644 --- a/tests/compile-fail/validity/dangling_ref2.rs +++ b/tests/compile-fail/validity/dangling_ref2.rs @@ -3,5 +3,5 @@ use std::mem; fn main() { let val = 14; let ptr = (&val as *const i32).wrapping_offset(1); - let _x: &i32 = unsafe { mem::transmute(ptr) }; //~ ERROR outside bounds of allocation + let _x: &i32 = unsafe { mem::transmute(ptr) }; //~ ERROR encountered dangling (not entirely in bounds) reference } diff --git a/tests/compile-fail/zst.rs b/tests/compile-fail/zst.rs deleted file mode 100644 index 0488926870..0000000000 --- a/tests/compile-fail/zst.rs +++ /dev/null @@ -1,4 +0,0 @@ -fn main() { - let x = &() as *const () as *const i32; - let _val = unsafe { *x }; //~ ERROR access memory with alignment 1, but alignment 4 is required -} diff --git a/tests/compile-fail/zst1.rs b/tests/compile-fail/zst1.rs new file mode 100644 index 0000000000..e4ce7b8ecd --- /dev/null +++ b/tests/compile-fail/zst1.rs @@ -0,0 +1,5 @@ +fn main() { + // make sure ZST locals cannot be accessed + let x = &() as *const () as *const i8; + let _val = unsafe { *x }; //~ ERROR pointer must be in-bounds +} diff --git a/tests/compile-fail/zst2.rs b/tests/compile-fail/zst2.rs new file mode 100644 index 0000000000..950f7b3d60 --- /dev/null +++ b/tests/compile-fail/zst2.rs @@ -0,0 +1,12 @@ +fn main() { + // Not using the () type here, as writes of that type do not even have MIR generated. + // Also not assigning directly as that's array initialization, not assignment. + let zst_val = [1u8; 0]; + + // make sure ZST accesses are checked against being "truly" dangling pointers + // (into deallocated allocations). + let mut x_box = Box::new(1u8); + let x = &mut *x_box as *mut _ as *mut [u8; 0]; + drop(x_box); + unsafe { *x = zst_val; } //~ ERROR dangling pointer was dereferenced +} diff --git a/tests/compile-fail/zst3.rs b/tests/compile-fail/zst3.rs new file mode 100644 index 0000000000..c94591174e --- /dev/null +++ b/tests/compile-fail/zst3.rs @@ -0,0 +1,15 @@ +fn main() { + // Not using the () type here, as writes of that type do not even have MIR generated. + // Also not assigning directly as that's array initialization, not assignment. + let zst_val = [1u8; 0]; + + // make sure ZST accesses are checked against being "truly" dangling pointers + // (that are out-of-bounds). + let mut x_box = Box::new(1u8); + let x = (&mut *x_box as *mut u8).wrapping_offset(1); + // This one is just "at the edge", but still okay + unsafe { *(x as *mut [u8; 0]) = zst_val; } + // One byte further is OOB. + let x = x.wrapping_offset(1); + unsafe { *(x as *mut [u8; 0]) = zst_val; } //~ ERROR pointer must be in-bounds +} diff --git a/tests/run-pass/zst.rs b/tests/run-pass/zst.rs index 9d97210b73..9884735709 100644 --- a/tests/run-pass/zst.rs +++ b/tests/run-pass/zst.rs @@ -21,13 +21,4 @@ fn main() { // Reading and writing is ok. unsafe { *x = zst_val; } unsafe { let _y = *x; } - - // We should even be able to use "true" pointers for ZST when the allocation has been - // removed already. The box is for a non-ZST to make sure there actually is an allocation. - let mut x_box = Box::new(((), 1u8)); - let x = &mut x_box.0 as *mut _ as *mut [u8; 0]; - drop(x_box); - // Reading and writing is ok. - unsafe { *x = zst_val; } - unsafe { let _y = *x; } }