more compile-fail ptr equality tests, to rule out any non-determinism; and fix ptr equality to fail all of them.

At least these are the cases I can think of right now.

Author: RalfJung

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/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/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() {
@@ -56,10 +58,29 @@ fn main() {
     assert_eq!(tuple_ref_mut(), (10, 22));
     assert_eq!(match_ref_mut(), 42);
 
-    // Compare even dangling pointers with NULL, and with others in the same allocation.
+    // 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);
 }