diff options
| -rw-r--r-- | miri/intrinsic.rs | 4 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/memory.rs | 83 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/mod.rs | 4 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/validation.rs | 2 | ||||
| -rw-r--r-- | tests/compile-fail/null_pointer_deref.rs | 2 | ||||
| -rw-r--r-- | tests/compile-fail/wild_pointer_deref.rs | 2 |
6 files changed, 47 insertions, 50 deletions
diff --git a/miri/intrinsic.rs b/miri/intrinsic.rs index 3e04f859871..8c722a46ae3 100644 --- a/miri/intrinsic.rs +++ b/miri/intrinsic.rs @@ -4,7 +4,7 @@ use rustc::ty::layout::Layout; use rustc::ty::{self, Ty}; use rustc_miri::interpret::{EvalResult, Lvalue, LvalueExtra, PrimVal, PrimValKind, Value, Pointer, - HasMemory, EvalContext, PtrAndAlign, ValTy}; + HasMemory, AccessKind, EvalContext, PtrAndAlign, ValTy}; use helpers::EvalContextExt as HelperEvalContextExt; @@ -624,7 +624,7 @@ impl<'a, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'tcx, super::Evaluator> if count > 0 { // HashMap relies on write_bytes on a NULL ptr with count == 0 to work // TODO: Should we, at least, validate the alignment? (Also see the copy intrinsic) - self.memory.check_align(ptr, ty_align)?; + self.memory.check_align(ptr, ty_align, Some(AccessKind::Write))?; self.memory.write_repeat(ptr, val_byte, size * count)?; } } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 9930555c199..8c7a36f866d 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -3,7 +3,7 @@ use std::collections::{btree_map, BTreeMap, HashMap, HashSet, VecDeque}; use std::{fmt, iter, ptr, mem, io}; use std::cell::Cell; -use rustc::ty; +use rustc::ty::Instance; use rustc::ty::layout::{self, TargetDataLayout, HasDataLayout}; use syntax::ast::Mutability; use rustc::middle::region::CodeExtent; @@ -250,10 +250,10 @@ pub struct Memory<'a, 'tcx, M: Machine<'tcx>> { /// Function "allocations". They exist solely so pointers have something to point to, and /// we can figure out what they point to. - functions: Vec<ty::Instance<'tcx>>, + functions: Vec<Instance<'tcx>>, /// Inverse map of `functions` so we don't allocate a new pointer every time we need one - function_alloc_cache: HashMap<ty::Instance<'tcx>, AllocId>, + function_alloc_cache: HashMap<Instance<'tcx>, AllocId>, /// Target machine data layout to emulate. pub layout: &'a TargetDataLayout, @@ -297,7 +297,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { }) } - pub fn create_fn_alloc(&mut self, instance: ty::Instance<'tcx>) -> MemoryPointer { + pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> MemoryPointer { if let Some(&alloc_id) = self.function_alloc_cache.get(&instance) { return MemoryPointer::new(alloc_id, 0); } @@ -476,27 +476,38 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } /// Check that the pointer is aligned AND non-NULL. - pub fn check_align(&self, ptr: Pointer, align: u64) -> EvalResult<'tcx> { - let offset = match ptr.into_inner_primval() { + pub fn check_align(&self, ptr: Pointer, align: u64, access: Option<AccessKind>) -> EvalResult<'tcx> { + // Check non-NULL/Undef, extract offset + let (offset, alloc_align) = match ptr.into_inner_primval() { PrimVal::Ptr(ptr) => { let alloc = self.get(ptr.alloc_id)?; - if alloc.align < align { - return err!(AlignmentCheckFailed { - has: alloc.align, - required: align, - }); - } - ptr.offset + (ptr.offset, alloc.align) } PrimVal::Bytes(bytes) => { let v = ((bytes as u128) % (1 << self.pointer_size())) as u64; if v == 0 { return err!(InvalidNullPointerUsage); } - v + (v, align) // the base address if the "integer allocation" is 0 and hence always aligned } PrimVal::Undef => return err!(ReadUndefBytes), }; + // See if alignment checking is disabled + let enforce_alignment = match access { + Some(AccessKind::Read) => self.reads_are_aligned.get(), + Some(AccessKind::Write) => self.writes_are_aligned.get(), + None => true, + }; + if !enforce_alignment { + return Ok(()); + } + // Check alignment + if alloc_align < align { + return err!(AlignmentCheckFailed { + has: alloc_align, + required: align, + }); + } if offset % align == 0 { Ok(()) } else { @@ -804,7 +815,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } } - pub fn get_mut(&mut self, id: AllocId) -> EvalResult<'tcx, &mut Allocation<M::MemoryKinds>> { + fn get_mut(&mut self, id: AllocId) -> EvalResult<'tcx, &mut Allocation<M::MemoryKinds>> { let alloc = self.get_mut_unchecked(id)?; if alloc.mutable == Mutability::Mutable { Ok(alloc) @@ -813,7 +824,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } } - pub fn get_fn(&self, ptr: MemoryPointer) -> EvalResult<'tcx, ty::Instance<'tcx>> { + pub fn get_fn(&self, ptr: MemoryPointer) -> EvalResult<'tcx, Instance<'tcx>> { if ptr.offset != 0 { return err!(InvalidFunctionPointer); } @@ -933,9 +944,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { align: u64, ) -> EvalResult<'tcx, &[u8]> { // Zero-sized accesses can use dangling pointers, but they still have to be aligned and non-NULL - if self.reads_are_aligned.get() { - self.check_align(ptr.into(), align)?; - } + self.check_align(ptr.into(), align, Some(AccessKind::Read))?; if size == 0 { return Ok(&[]); } @@ -955,9 +964,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { align: u64, ) -> EvalResult<'tcx, &mut [u8]> { // Zero-sized accesses can use dangling pointers, but they still have to be aligned and non-NULL - if self.writes_are_aligned.get() { - self.check_align(ptr.into(), align)?; - } + self.check_align(ptr.into(), align, Some(AccessKind::Write))?; if size == 0 { return Ok(&mut []); } @@ -995,7 +1002,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { /// Reading and writing impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { /// mark an allocation pointed to by a static as static and initialized - pub fn mark_inner_allocation( + fn mark_inner_allocation_initialized( &mut self, alloc: AllocId, mutability: Mutability, @@ -1056,7 +1063,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { }; // recurse into inner allocations for &alloc in relocations.values() { - self.mark_inner_allocation(alloc, mutability)?; + self.mark_inner_allocation_initialized(alloc, mutability)?; } // put back the relocations self.alloc_map @@ -1074,14 +1081,10 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { align: u64, nonoverlapping: bool, ) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be aligned + self.check_align(src, align, Some(AccessKind::Read))?; + self.check_align(dest, align, Some(AccessKind::Write))?; if size == 0 { - // Empty accesses don't need to be valid pointers, but they should still be aligned - if self.reads_are_aligned.get() { - self.check_align(src, align)?; - } - if self.writes_are_aligned.get() { - self.check_align(dest, align)?; - } return Ok(()); } let src = src.to_ptr()?; @@ -1136,22 +1139,18 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } pub fn read_bytes(&self, ptr: Pointer, size: u64) -> EvalResult<'tcx, &[u8]> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + self.check_align(ptr, 1, Some(AccessKind::Read))?; if size == 0 { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - if self.reads_are_aligned.get() { - self.check_align(ptr, 1)?; - } return Ok(&[]); } self.get_bytes(ptr.to_ptr()?, size, 1) } pub fn write_bytes(&mut self, ptr: Pointer, src: &[u8]) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + self.check_align(ptr, 1, Some(AccessKind::Write))?; if src.is_empty() { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - if self.writes_are_aligned.get() { - self.check_align(ptr, 1)?; - } return Ok(()); } let bytes = self.get_bytes_mut(ptr.to_ptr()?, src.len() as u64, 1)?; @@ -1160,11 +1159,9 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } pub fn write_repeat(&mut self, ptr: Pointer, val: u8, count: u64) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + self.check_align(ptr, 1, Some(AccessKind::Write))?; if count == 0 { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - if self.writes_are_aligned.get() { - self.check_align(ptr, 1)?; - } return Ok(()); } let bytes = self.get_bytes_mut(ptr.to_ptr()?, count, 1)?; diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 3a5fdf273a4..634b8a0eeb8 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -27,9 +27,9 @@ pub use self::eval_context::{EvalContext, Frame, ResourceLimits, StackPopCleanup pub use self::lvalue::{Lvalue, LvalueExtra, GlobalId}; -pub use self::memory::{AllocId, Memory, MemoryPointer, MemoryKind, HasMemory}; +pub use self::memory::{AllocId, Memory, MemoryPointer, MemoryKind, HasMemory, AccessKind}; -use self::memory::{PointerArithmetic, Lock, AccessKind}; +use self::memory::{PointerArithmetic, Lock}; use self::range_map::RangeMap; diff --git a/src/librustc_mir/interpret/validation.rs b/src/librustc_mir/interpret/validation.rs index 20b601b538c..6454e12e037 100644 --- a/src/librustc_mir/interpret/validation.rs +++ b/src/librustc_mir/interpret/validation.rs @@ -275,7 +275,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { // Check alignment and non-NULLness let (_, align) = self.size_and_align_of_dst(pointee_ty, val)?; let ptr = val.into_ptr(&self.memory)?; - self.memory.check_align(ptr, align)?; + self.memory.check_align(ptr, align, None)?; // Recurse let pointee_lvalue = self.val_to_lvalue(val, pointee_ty)?; diff --git a/tests/compile-fail/null_pointer_deref.rs b/tests/compile-fail/null_pointer_deref.rs index 20b93aab160..5a26856eba0 100644 --- a/tests/compile-fail/null_pointer_deref.rs +++ b/tests/compile-fail/null_pointer_deref.rs @@ -1,4 +1,4 @@ fn main() { - let x: i32 = unsafe { *std::ptr::null() }; //~ ERROR: a memory access tried to interpret some bytes as a pointer + 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/wild_pointer_deref.rs b/tests/compile-fail/wild_pointer_deref.rs index 373e308e1c0..57da8dfc01b 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 = 42 as *const i32; + let p = 44 as *const i32; let x = unsafe { *p }; //~ ERROR: a memory access tried to interpret some bytes as a pointer panic!("this should never print: {}", x); } |