Merge pull request #453 from RalfJung/zst

ZST pointers and pointer equality

Author: RalfJung

rust-toolchain

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

src/memory.rs

@@ -46,7 +46,7 @@ pub enum MemoryKind {
     C,
     /// Part of env var emulation
     Env,
-    // mutable statics
+    /// mutable statics
     MutStatic,
 }
 

src/operator.rs

@@ -152,18 +152,50 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             (Scalar::Ptr(ptr), Scalar::Bits { bits, size }) |
             (Scalar::Bits { bits, size }, Scalar::Ptr(ptr)) => {
                 assert_eq!(size as u64, self.pointer_size().bytes());
+                let bits = bits as u64;
+                let (alloc_size, alloc_align) = self.memory.get_size_and_align(ptr.alloc_id)?;
 
+                // Case I: Comparing with NULL
                 if bits == 0 {
-                    // Nothing equals 0, not even dangling pointers. Ideally we would
-                    // require them to be in-bounds of their (possilby dead) allocation,
-                    // but with the allocation gonew e cannot check that.
-                    false
-                } else {
-                    // Live pointers cannot equal an integer, but again do not
-                    // allow comparing dead pointers.
-                    self.memory.check_bounds(ptr, false)?;
-                    false
+                    // Test if the ptr is in-bounds. Then it cannot be NULL.
+                    if ptr.offset <= alloc_size {
+                        return Ok(false);
+                    }
+                }
+                // Case II: Alignment gives it away
+                if ptr.offset.bytes() % alloc_align.abi() == 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.abi() != 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 -- IOW, 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);
             }
         })
     }

tests/compile-fail/maybe_null_pointer_deref_zst.rs

@@ -0,0 +1,5 @@
+fn main() {
+    // This pointer *could* be NULL so we cannot load from it, not even at ZST
+    let ptr = (&0u8 as *const u8).wrapping_sub(0x800) as *const ();
+    let _x: () = unsafe { *ptr }; //~ ERROR outside bounds
+}

tests/compile-fail/maybe_null_pointer_write_zst.rs

@@ -0,0 +1,8 @@
+fn main() {
+    // This pointer *could* be NULL so we cannot load from it, not even at ZST.
+    // 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];
+    let ptr = (&0u8 as *const u8).wrapping_sub(0x800) as *mut [u8; 0];
+    unsafe { *ptr = zst_val; } //~ ERROR outside bounds
+}

tests/compile-fail/null_pointer_deref_zst.rs

@@ -0,0 +1,4 @@
+fn main() {
+    let x: () = unsafe { *std::ptr::null() }; //~ ERROR constant evaluation error: invalid use of NULL pointer
+    panic!("this should never print: {:?}", x);
+}

tests/compile-fail/null_pointer_write.rs

@@ -0,0 +1,3 @@
+fn main() {
+    unsafe { *std::ptr::null_mut() = 0i32 }; //~ ERROR constant evaluation error: invalid use of NULL pointer
+}

tests/compile-fail/null_pointer_write_zst.rs

@@ -0,0 +1,6 @@
+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];
+    unsafe { *std::ptr::null_mut() = zst_val }; //~ ERROR constant evaluation error: invalid use of NULL pointer
+}

tests/compile-fail/ptr_eq_dangling.rs

@@ -0,0 +1,10 @@
+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 dangling pointer
+}

tests/compile-fail/ptr_eq_integer.rs

@@ -0,0 +1,8 @@
+use std::mem;
+
+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 != mem::align_of::<i32>() as *const i32); //~ ERROR invalid arithmetic on pointers
+}

tests/compile-fail/ptr_eq_out_of_bounds.rs

@@ -0,0 +1,9 @@
+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 outside bounds
+}

tests/compile-fail/ptr_eq_out_of_bounds_null.rs

@@ -0,0 +1,6 @@
+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
+}

tests/run-pass-fullmir/loop-break-value.rs

@@ -61,7 +61,9 @@ pub fn main() {
             break Default::default()
         };
     };
-    assert_eq!(trait_unified_2, [""]);
+    // compare lengths; ptr comparison is not deterministic
+    assert_eq!(trait_unified_2.len(), 1);
+    assert_eq!(trait_unified_2[0].len(), 0);
 
     let trait_unified_3 = loop {
         break if false {

tests/run-pass/cast-rfc0401-vtable-kinds.rs

@@ -8,10 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-
-// FIXME: remove the next line when https://github.com/rust-lang/rust/issues/43358 is resolved
-// compile-flags: -Zmir-opt-level=0
-
 // Check that you can cast between different pointers to trait objects
 // whose vtable have the same kind (both lengths, or both trait pointers).
 

tests/run-pass/const-vec-of-fns.rs

@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// pretty-expanded FIXME #23616
-
 /*!
  * Try to double-check that static fns have the right size (with or
  * without dummy env ptr, as appropriate) by iterating a size-2 array.

tests/run-pass/issue-20575.rs

@@ -10,8 +10,6 @@
 
 // Test that overloaded calls work with zero arity closures
 
-// pretty-expanded FIXME #23616
-
 fn main() {
     let functions: [Box<Fn() -> Option<()>>; 1] = [Box::new(|| None)];
 

tests/run-pass/pointers.rs

@@ -1,3 +1,5 @@
+use std::usize;
+
 fn one_line_ref() -> i16 {
     *&1
 }
@@ -44,8 +46,8 @@ fn match_ref_mut() -> i8 {
 }
 
 fn dangling_pointer() -> *const i32 {
-    let b = Box::new(42);
-    &*b as *const i32
+    let b = Box::new((42, 42)); // make it bigger than the alignment, so that there is some "room" after this pointer
+    &b.0 as *const i32
 }
 
 fn main() {
@@ -55,7 +57,30 @@ fn main() {
     assert_eq!(basic_ref_mut_var(), 3);
     assert_eq!(tuple_ref_mut(), (10, 22));
     assert_eq!(match_ref_mut(), 42);
-    // FIXME: improve this test... how?
+
+    // Compare even dangling pointers with NULL, and with others in the same allocation, including
+    // out-of-bounds.
     assert!(dangling_pointer() != std::ptr::null());
     assert!(match dangling_pointer() as usize { 0 => false, _ => true });
+    let dangling = dangling_pointer();
+    assert!(dangling == dangling);
+    assert!(dangling.wrapping_add(1) != dangling);
+    assert!(dangling.wrapping_sub(1) != dangling);
+
+    // Compare pointer with BIG integers
+    let dangling = dangling as usize;
+    assert!(dangling != usize::MAX);
+    assert!(dangling != usize::MAX - 1);
+    assert!(dangling != usize::MAX - 2);
+    assert!(dangling != usize::MAX - 3); // this is even 4-aligned, but it still cannot be equal because of the extra "room" after this pointer
+    assert_eq!((usize::MAX - 3) % 4, 0); // just to be sure we got this right
+
+    // Compare pointer with unaligned integers
+    assert!(dangling != 1usize);
+    assert!(dangling != 2usize);
+    assert!(dangling != 3usize);
+    // 4 is a possible choice! So we cannot compare with that.
+    assert!(dangling != 5usize);
+    assert!(dangling != 6usize);
+    assert!(dangling != 7usize);
 }

tests/run-pass/regions-lifetime-nonfree-late-bound.rs

@@ -22,8 +22,6 @@
 // doing region-folding, when really all clients of the region-folding
 // case only want to see FREE lifetime variables, not bound ones.
 
-// pretty-expanded FIXME #23616
-
 #![allow(unused_features)]
 #![feature(box_syntax)]
 

tests/run-pass/sendable-class.rs

@@ -10,8 +10,6 @@
 
 // Test that a class with only sendable fields can be sent
 
-// pretty-expanded FIXME #23616
-
 use std::sync::mpsc::channel;
 
 #[allow(dead_code)]

tests/run-pass/zst.rs

@@ -11,8 +11,23 @@ fn use_zst() -> A {
 }
 
 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];
+
     assert_eq!(zst_ret(), A);
     assert_eq!(use_zst(), A);
-    let x = 42 as *mut ();
-    unsafe { *x = (); }
+    let x = 42 as *mut [u8; 0];
+    // reading and writing is okay
+    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 okay
+    unsafe { *x = zst_val; }
+    unsafe { let _y = *x; }
 }