Merge pull request #203 from RalfJung/offset

Allow any offset on integer and ZST pointers

3 files changed, 26 insertions, 17 deletions

diff --git a/src/error.rs b/src/error.rs
index e7e446e93ad..ee805695c51 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -21,6 +21,7 @@ pub enum EvalError<'tcx> {
         access: bool,
         allocation_size: u64,
     },
+    NullPointerOutOfBounds,
     ReadPointerAsBytes,
     ReadBytesAsPointer,
     InvalidPointerMath,
@@ -80,12 +81,14 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "invalid enum discriminant value read",
             EvalError::PointerOutOfBounds { .. } =>
                 "pointer offset outside bounds of allocation",
+            EvalError::NullPointerOutOfBounds =>
+                "invalid NULL pointer offset",
             EvalError::ReadPointerAsBytes =>
                 "a raw memory access tried to access part of a pointer value as raw bytes",
             EvalError::ReadBytesAsPointer =>
                 "a memory access tried to interpret some bytes as a pointer",
             EvalError::InvalidPointerMath =>
-                "attempted to do math or a comparison on pointers into different allocations",
+                "attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. compating pointers into different allocations",
             EvalError::ReadUndefBytes =>
                 "attempted to read undefined bytes",
             EvalError::DeadLocal =>
diff --git a/src/eval_context.rs b/src/eval_context.rs
index 8995a199f13..9300ae3dbc2 100644
--- a/src/eval_context.rs
+++ b/src/eval_context.rs
@@ -892,19 +892,27 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
     }
 
     pub(super) fn pointer_offset(&self, ptr: PrimVal, pointee_ty: Ty<'tcx>, offset: i64) -> EvalResult<'tcx, PrimVal> {
-        if offset == 0 {
-            // rustc relies on Offset-by-0 to be well-defined even for "bad" pointers like Unique::empty().
-            return Ok(ptr);
+        // This function raises an error if the offset moves the pointer outside of its allocation.  We consider
+        // ZSTs their own huge allocation that doesn't overlap with anything (and nothing moves in there because the size is 0).
+        // We also consider the NULL pointer its own separate allocation, and all the remaining integers pointers their own
+        // allocation.
+
+        if ptr.is_null()? { // NULL pointers must only be offset by 0
+            return if offset == 0 { Ok(ptr) } else { Err(EvalError::NullPointerOutOfBounds) };
         }
         // FIXME: assuming here that type size is < i64::max_value()
         let pointee_size = self.type_size(pointee_ty)?.expect("cannot offset a pointer to an unsized type") as i64;
-        if pointee_size == 0 {
-            // rustc relies on offsetting pointers to zsts to be a nop
-            return Ok(ptr);
-        }
         return if let Some(offset) = offset.checked_mul(pointee_size) {
             let ptr = ptr.signed_offset(offset, self.memory.layout)?;
-            self.memory.check_bounds(ptr.to_ptr()?, false)?;
+            // Do not do bounds-checking for integers or ZST; they can never alias a normal pointer anyway.
+            if let PrimVal::Ptr(ptr) = ptr {
+                if !(ptr.points_to_zst() && (offset == 0 || pointee_size == 0)) {
+                    self.memory.check_bounds(ptr, false)?;
+                }
+            } else if ptr.is_null()? {
+                // We moved *to* a NULL pointer.  That seems wrong, LLVM considers the NULL pointer its own small allocation.  Reject this, for now.
+                return Err(EvalError::NullPointerOutOfBounds);
+            }
             Ok(ptr)
         } else {
             Err(EvalError::OverflowingMath)
diff --git a/src/terminator/mod.rs b/src/terminator/mod.rs
index 7cb1a3cd629..90f0aed7f10 100644
--- a/src/terminator/mod.rs
+++ b/src/terminator/mod.rs
@@ -585,14 +585,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             }
 
             "dlsym" => {
-                let handle = args[0].read_ptr(&self.memory)?;
-                {
-                    let symbol = args[1].read_ptr(&self.memory)?.to_ptr()?;
-                    let symbol_name = self.memory.read_c_str(symbol)?;
-                    let err = format!("bad c unicode symbol: {:?}", symbol_name);
-                    let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err);
-                    return Err(EvalError::Unimplemented(format!("miri does not support dynamically loading libraries (requested symbol: {})", symbol_name)));
-                }
+                let _handle = args[0].read_ptr(&self.memory)?;
+                let symbol = args[1].read_ptr(&self.memory)?.to_ptr()?;
+                let symbol_name = self.memory.read_c_str(symbol)?;
+                let err = format!("bad c unicode symbol: {:?}", symbol_name);
+                let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err);
+                return Err(EvalError::Unimplemented(format!("miri does not support dynamically loading libraries (requested symbol: {})", symbol_name)));
             }
 
             "__rust_allocate" => {