Merge pull request #446 from RalfJung/zero-sized-accesses

Strictly enforce pointer validity even for zero-sized accesses

Author: RalfJung

rust-toolchain

@@ -1 +1 @@
-nightly-2018-09-15
+nightly-2018-09-17

src/intrinsic.rs

@@ -145,21 +145,17 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
                 let elem_layout = self.layout_of(elem_ty)?;
                 let elem_size = elem_layout.size.bytes();
                 let count = self.read_scalar(args[2])?.to_usize(&self)?;
-                if count * elem_size != 0 {
-                    // TODO: We do not even validate alignment for the 0-bytes case.  libstd relies on this in vec::IntoIter::next.
-                    // Also see the write_bytes intrinsic.
-                    let elem_align = elem_layout.align;
-                    let src = self.read_scalar(args[0])?.not_undef()?;
-                    let dest = self.read_scalar(args[1])?.not_undef()?;
-                    self.memory.copy(
-                        src,
-                        elem_align,
-                        dest,
-                        elem_align,
-                        Size::from_bytes(count * elem_size),
-                        intrinsic_name.ends_with("_nonoverlapping"),
-                    )?;
-                }
+                let elem_align = elem_layout.align;
+                let src = self.read_scalar(args[0])?.not_undef()?;
+                let dest = self.read_scalar(args[1])?.not_undef()?;
+                self.memory.copy(
+                    src,
+                    elem_align,
+                    dest,
+                    elem_align,
+                    Size::from_bytes(count * elem_size),
+                    intrinsic_name.ends_with("_nonoverlapping"),
+                )?;
             }
 
             "discriminant_value" => {
@@ -436,12 +432,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
                 let val_byte = self.read_scalar(args[1])?.to_u8()?;
                 let ptr = self.read_scalar(args[0])?.not_undef()?;
                 let count = self.read_scalar(args[2])?.to_usize(&self)?;
-                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_layout.align)?;
-                    self.memory.write_repeat(ptr, val_byte, ty_layout.size * count)?;
-                }
+                self.memory.check_align(ptr, ty_layout.align)?;
+                self.memory.write_repeat(ptr, val_byte, ty_layout.size * count)?;
             }
 
             name => return err!(Unimplemented(format!("unimplemented intrinsic: {}", name))),

tests/compile-fail/copy_null.rs

@@ -0,0 +1,18 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//error-pattern: invalid use of NULL pointer
+
+fn main() {
+    let mut data = [0u16; 4];
+    let ptr = &mut data[0] as *mut u16;
+    // Even copying 0 elements from NULL should error
+    unsafe { ptr.copy_from(std::ptr::null(), 0); }
+}

tests/compile-fail/copy_unaligned.rs

@@ -0,0 +1,18 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//error-pattern: tried to access memory with alignment 1, but alignment 2 is required
+
+fn main() {
+    let mut data = [0u16; 8];
+    let ptr = (&mut data[0] as *mut u16 as *mut u8).wrapping_add(1) as *mut u16;
+    // Even copying 0 elements to something unaligned should error
+    unsafe { ptr.copy_from(&data[5], 0); }
+}