memory: make sure we check non-NULL/undef even fore 0-sized accesses

miri/intrinsic.rs

@@ -4,7 +4,7 @@
 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 @@ fn call_intrinsic(
                 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)?;
                 }
             }

src/librustc_mir/interpret/memory.rs

@@ -3,7 +3,7 @@
 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 @@ pub fn allocations<'x>(
         })
     }
 
-    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 @@ pub fn endianess(&self) -> layout::Endian {
     }
 
     /// 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 @@ fn get_mut_unchecked(
         }
     }
 
-    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 @@ pub fn get_mut(&mut self, id: AllocId) -> EvalResult<'tcx, &mut Allocation<M::Me
         }
     }
 
-    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 @@ fn get_bytes_unchecked(
         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 @@ fn get_bytes_unchecked_mut(
         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 @@ fn get_bytes_mut(
 /// 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 @@ pub fn mark_static_initalized(
         };
         // 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 @@ pub fn copy(
         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 @@ pub fn read_c_str(&self, ptr: MemoryPointer) -> EvalResult<'tcx, &[u8]> {
     }
 
     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 @@ pub fn write_bytes(&mut self, ptr: Pointer, src: &[u8]) -> EvalResult<'tcx> {
     }
 
     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)?;

src/librustc_mir/interpret/mod.rs

@@ -27,9 +27,9 @@
 
 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;
 

src/librustc_mir/interpret/validation.rs

@@ -275,7 +275,7 @@ fn validate_ptr(
         // 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)?;

tests/compile-fail/null_pointer_deref.rs

 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);
 }

tests/compile-fail/wild_pointer_deref.rs

 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);
 }