Merge pull request rust-lang#206 from RalfJung/ptrs

Pointer Arithmetic

Author: oli-obk

src/memory.rs

@@ -64,10 +64,20 @@ impl Pointer {
         Pointer::new(self.alloc_id, value::wrapping_signed_offset(self.offset, i, layout))
     }
 
+    pub fn overflowing_signed_offset<'tcx>(self, i: i128, layout: &TargetDataLayout) -> (Self, bool) {
+        let (res, over) = value::overflowing_signed_offset(self.offset, i, layout);
+        (Pointer::new(self.alloc_id, res), over)
+    }
+
     pub fn signed_offset<'tcx>(self, i: i64, layout: &TargetDataLayout) -> EvalResult<'tcx, Self> {
         Ok(Pointer::new(self.alloc_id, value::signed_offset(self.offset, i, layout)?))
     }
 
+    pub fn overflowing_offset<'tcx>(self, i: u64, layout: &TargetDataLayout) -> (Self, bool) {
+        let (res, over) = value::overflowing_offset(self.offset, i, layout);
+        (Pointer::new(self.alloc_id, res), over)
+    }
+
     pub fn offset<'tcx>(self, i: u64, layout: &TargetDataLayout) -> EvalResult<'tcx, Self> {
         Ok(Pointer::new(self.alloc_id, value::offset(self.offset, i, layout)?))
     }
@@ -470,11 +480,14 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         }
     }
 
-    pub fn get_fn(&self, id: AllocId) -> EvalResult<'tcx, ty::Instance<'tcx>> {
-        debug!("reading fn ptr: {}", id);
-        match self.functions.get(&id) {
+    pub fn get_fn(&self, ptr: Pointer) -> EvalResult<'tcx, ty::Instance<'tcx>> {
+        if ptr.offset != 0 {
+            return Err(EvalError::InvalidFunctionPointer);
+        }
+        debug!("reading fn ptr: {}", ptr.alloc_id);
+        match self.functions.get(&ptr.alloc_id) {
             Some(&fndef) => Ok(fndef),
-            None => match self.alloc_map.get(&id) {
+            None => match self.alloc_map.get(&ptr.alloc_id) {
                 Some(_) => Err(EvalError::ExecuteMemory),
                 None => Err(EvalError::InvalidFunctionPointer),
             }

src/operator.rs

@@ -3,6 +3,7 @@ use rustc::ty::{self, Ty};
 
 use error::{EvalError, EvalResult};
 use eval_context::EvalContext;
+use memory::Pointer;
 use lvalue::Lvalue;
 use value::{
     PrimVal,
@@ -145,46 +146,69 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
         let left_kind  = self.ty_to_primval_kind(left_ty)?;
         let right_kind = self.ty_to_primval_kind(right_ty)?;
-
-        // Offset is handled early, before we dispatch to unrelated_ptr_ops.  We have to also catch the case where both arguments *are* convertible to integers.
-        if bin_op == Offset {
-            if left_kind == Ptr && right_kind == PrimValKind::from_uint_size(self.memory.pointer_size()) {
-                let pointee_ty = left_ty.builtin_deref(true, ty::LvaluePreference::NoPreference).expect("Offset called on non-ptr type").ty;
-                let ptr = self.pointer_offset(left, pointee_ty, right.to_bytes()? as i64)?;
-                return Ok((ptr, false));
-            } else {
-                bug!("Offset used with wrong type");
-            }
-        }
-
-        // unrelated pointer ops
-        let op: Option<fn(&PrimVal, &PrimVal) -> bool> = match bin_op {
-            Eq => Some(PrimVal::eq),
-            Ne => Some(PrimVal::ne),
-            _ => None,
-        };
-        if let Some(op) = op {
-            // only floats can't be binary compared
-            let ok = left_kind != F32 && left_kind != F64;
-            let ok = ok && right_kind != F32 && right_kind != F64;
-            if ok {
-                return Ok((PrimVal::from_bool(op(&left, &right)), false));
-            }
-        }
-
-        
-        if let (Ok(left), Ok(right)) = (left.to_ptr(), right.to_ptr()) {
-            if left.alloc_id == right.alloc_id {
-                return self.ptr_ops(
-                    bin_op,
-                    left.offset,
-                    right.offset,
-                );
-            } else {
-                return Err(EvalError::InvalidPointerMath);
+        //trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})", bin_op, left, left_kind, right, right_kind);
+
+        // I: Handle operations that support pointers
+        let usize = PrimValKind::from_uint_size(self.memory.pointer_size());
+        let isize = PrimValKind::from_int_size(self.memory.pointer_size());
+        if !left_kind.is_float() && !right_kind.is_float() {
+            match bin_op {
+                Offset if left_kind == Ptr && right_kind == usize => {
+                    let pointee_ty = left_ty.builtin_deref(true, ty::LvaluePreference::NoPreference).expect("Offset called on non-ptr type").ty;
+                    let ptr = self.pointer_offset(left, pointee_ty, right.to_bytes()? as i64)?;
+                    return Ok((ptr, false));
+                },
+                // These work on anything
+                Eq if left_kind == right_kind => {
+                    return Ok((PrimVal::from_bool(left == right), false));
+                }
+                Ne if left_kind == right_kind => {
+                    return Ok((PrimVal::from_bool(left != right), false));
+                }
+                // These need both pointers to be in the same allocation
+                Lt | Le | Gt | Ge | Sub
+                if left_kind == right_kind
+                && (left_kind == Ptr || left_kind == usize || left_kind == isize)
+                && left.is_ptr() && right.is_ptr() => {
+                    let left = left.to_ptr()?;
+                    let right = right.to_ptr()?;
+                    if left.alloc_id == right.alloc_id {
+                        let res = match bin_op {
+                            Lt => left.offset < right.offset,
+                            Le => left.offset <= right.offset,
+                            Gt => left.offset > right.offset,
+                            Ge => left.offset >= right.offset,
+                            Sub => {
+                                return int_arithmetic!(left_kind, overflowing_sub, left.offset, right.offset);
+                            }
+                            _ => bug!("We already established it has to be one of these operators."),
+                        };
+                        return Ok((PrimVal::from_bool(res), false));
+                    } else {
+                        // Both are pointers, but from different allocations.
+                        return Err(EvalError::InvalidPointerMath);
+                    }
+                }
+                // These work if one operand is a pointer, the other an integer
+                Add | Sub
+                if left_kind == right_kind && (left_kind == usize || left_kind == isize)
+                && left.is_ptr() && right.is_bytes() => {
+                    // Cast to i128 is fine as we checked the kind to be ptr-sized
+                    let (res, over) = self.ptr_int_arithmetic(bin_op, left.to_ptr()?, right.to_bytes()? as i128, left_kind == isize)?;
+                    return Ok((PrimVal::Ptr(res), over));
+                }
+                Add
+                if left_kind == right_kind && (left_kind == usize || left_kind == isize)
+                && left.is_bytes() && right.is_ptr() => {
+                    // This is a commutative operation, just swap the operands
+                    let (res, over) = self.ptr_int_arithmetic(bin_op, right.to_ptr()?, left.to_bytes()? as i128, left_kind == isize)?;
+                    return Ok((PrimVal::Ptr(res), over));
+                }
+                _ => {}
             }
         }
 
+        // II: From now on, everything must be bytes, no pointers
         let l = left.to_bytes()?;
         let r = right.to_bytes()?;
 
@@ -229,8 +253,6 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             (Div, F64) => f64_arithmetic!(/, l, r),
             (Rem, F64) => f64_arithmetic!(%, l, r),
 
-            (Eq, _) => PrimVal::from_bool(l == r),
-            (Ne, _) => PrimVal::from_bool(l != r),
             (Lt, k) if k.is_signed_int() => PrimVal::from_bool((l as i128) < (r as i128)),
             (Lt, _) => PrimVal::from_bool(l <  r),
             (Le, k) if k.is_signed_int() => PrimVal::from_bool((l as i128) <= (r as i128)),
@@ -259,35 +281,25 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         Ok((val, false))
     }
 
-    fn ptr_ops(
+    fn ptr_int_arithmetic(
         &self,
         bin_op: mir::BinOp,
-        left: u64,
-        right: u64,
-    ) -> EvalResult<'tcx, (PrimVal, bool)> {
+        left: Pointer,
+        right: i128,
+        signed: bool,
+    ) -> EvalResult<'tcx, (Pointer, bool)> {
         use rustc::mir::BinOp::*;
 
-        let val = match bin_op {
-            Eq => PrimVal::from_bool(left == right),
-            Ne => PrimVal::from_bool(left != right),
-            Lt | Le | Gt | Ge => {
-                PrimVal::from_bool(match bin_op {
-                    Lt => left < right,
-                    Le => left <= right,
-                    Gt => left > right,
-                    Ge => left >= right,
-                    _ => bug!("We already established it has to be a comparison operator."),
-                })
-            }
-            Sub => {
-                let usize = PrimValKind::from_uint_size(self.memory.pointer_size());
-                return int_arithmetic!(usize, overflowing_sub, left, right);
-            }
-            _ => {
-                return Err(EvalError::ReadPointerAsBytes);
-            }
-        };
-        Ok((val, false))
+        Ok(match bin_op {
+            Sub =>
+                // The only way this can overflow is by underflowing, so signdeness of the right operands does not matter
+                left.overflowing_signed_offset(-right, self.memory.layout),
+            Add if signed =>
+                left.overflowing_signed_offset(right, self.memory.layout),
+            Add if !signed =>
+                left.overflowing_offset(right as u64, self.memory.layout),
+            _ => bug!("ptr_int_arithmetic called on unsupported operation")
+        })
     }
 }
 

src/terminator/intrinsic.rs

@@ -424,9 +424,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let ty_align = self.type_align(ty)?;
                 let val_byte = self.value_to_primval(arg_vals[1], u8)?.to_u128()? as u8;
                 let size = self.type_size(ty)?.expect("write_bytes() type must be sized");
-                let ptr = arg_vals[0].read_ptr(&self.memory)?.to_ptr()?;
+                let ptr = arg_vals[0].read_ptr(&self.memory)?;
                 let count = self.value_to_primval(arg_vals[2], usize)?.to_u64()?;
                 if count > 0 {
+                    let ptr = ptr.to_ptr()?;
                     self.memory.check_align(ptr, ty_align, size * count)?;
                     self.memory.write_repeat(ptr, val_byte, size * count)?;
                 }

src/terminator/mod.rs

@@ -65,7 +65,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let (fn_def, sig) = match func_ty.sty {
                     ty::TyFnPtr(sig) => {
                         let fn_ptr = self.eval_operand_to_primval(func)?.to_ptr()?;
-                        let instance = self.memory.get_fn(fn_ptr.alloc_id)?;
+                        let instance = self.memory.get_fn(fn_ptr)?;
                         let instance_ty = instance.def.def_ty(self.tcx);
                         let instance_ty = self.monomorphize(instance_ty, instance.substs);
                         match instance_ty.sty {
@@ -388,7 +388,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let ptr_size = self.memory.pointer_size();
                 let (_, vtable) = self.eval_operand(&arg_operands[0])?.expect_ptr_vtable_pair(&self.memory)?;
                 let fn_ptr = self.memory.read_ptr(vtable.offset(ptr_size * (idx as u64 + 3), self.memory.layout)?)?;
-                let instance = self.memory.get_fn(fn_ptr.to_ptr()?.alloc_id)?;
+                let instance = self.memory.get_fn(fn_ptr.to_ptr()?)?;
                 let mut arg_operands = arg_operands.to_vec();
                 let ty = self.operand_ty(&arg_operands[0]);
                 let ty = self.get_field_ty(ty, 0)?;
@@ -596,7 +596,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let u8_ptr_ty = self.tcx.mk_mut_ptr(self.tcx.types.u8);
                 let f = args[0].read_ptr(&self.memory)?.to_ptr()?;
                 let data = args[1].read_ptr(&self.memory)?;
-                let f_instance = self.memory.get_fn(f.alloc_id)?;
+                let f_instance = self.memory.get_fn(f)?;
                 self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?;
 
                 // Now we make a function call.  TODO: Consider making this re-usable?  EvalContext::step does sth. similar for the TLS dtors,
@@ -614,7 +614,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let arg_dest = self.eval_lvalue(&mir::Lvalue::Local(arg_local))?;
                 self.write_primval(arg_dest, data, u8_ptr_ty)?;
 
-                // We ourselbes return 0
+                // We ourselves return 0
                 self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?;
 
                 // Don't fall through
@@ -723,7 +723,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
                 // Extract the function type out of the signature (that seems easier than constructing it ourselves...)
                 let dtor = match args[1].read_ptr(&self.memory)? {
-                    PrimVal::Ptr(dtor_ptr) => Some(self.memory.get_fn(dtor_ptr.alloc_id)?),
+                    PrimVal::Ptr(dtor_ptr) => Some(self.memory.get_fn(dtor_ptr)?),
                     PrimVal::Bytes(0) => None,
                     PrimVal::Bytes(_) => return Err(EvalError::ReadBytesAsPointer),
                     PrimVal::Undef => return Err(EvalError::ReadUndefBytes),

src/traits.rs

@@ -78,7 +78,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         match self.read_ptr(vtable, self.tcx.mk_nil_ptr())? {
             // some values don't need to call a drop impl, so the value is null
             Value::ByVal(PrimVal::Bytes(0)) => Ok(None),
-            Value::ByVal(PrimVal::Ptr(drop_fn)) => self.memory.get_fn(drop_fn.alloc_id).map(Some),
+            Value::ByVal(PrimVal::Ptr(drop_fn)) => self.memory.get_fn(drop_fn).map(Some),
             _ => Err(EvalError::ReadBytesAsPointer),
         }
     }

src/value.rs

@@ -160,6 +160,27 @@ impl<'tcx> PrimVal {
         }
     }
 
+    pub fn is_bytes(self) -> bool {
+        match self {
+            PrimVal::Bytes(_) => true,
+            _ => false,
+        }
+    }
+
+    pub fn is_ptr(self) -> bool {
+        match self {
+            PrimVal::Ptr(_) => true,
+            _ => false,
+        }
+    }
+
+    pub fn is_undef(self) -> bool {
+        match self {
+            PrimVal::Undef => true,
+            _ => false,
+        }
+    }
+
     pub fn to_u128(self) -> EvalResult<'tcx, u128> {
         self.to_bytes()
     }
@@ -239,32 +260,45 @@ impl<'tcx> PrimVal {
     }
 }
 
-pub fn signed_offset<'tcx>(val: u64, i: i64, layout: &TargetDataLayout) -> EvalResult<'tcx, u64> {
+// Overflow checking only works properly on the range from -u64 to +u64.
+pub fn overflowing_signed_offset<'tcx>(val: u64, i: i128, layout: &TargetDataLayout) -> (u64, bool) {
     // FIXME: is it possible to over/underflow here?
     if i < 0 {
         // trickery to ensure that i64::min_value() works fine
         // this formula only works for true negative values, it panics for zero!
         let n = u64::max_value() - (i as u64) + 1;
-        val.checked_sub(n).ok_or(EvalError::OverflowingMath)
+        val.overflowing_sub(n)
     } else {
-        offset(val, i as u64, layout)
+        overflowing_offset(val, i as u64, layout)
     }
 }
 
-pub fn offset<'tcx>(val: u64, i: u64, layout: &TargetDataLayout) -> EvalResult<'tcx, u64> {
-    if let Some(res) = val.checked_add(i) {
-        if res as u128 >= (1u128 << layout.pointer_size.bits()) {
-            Err(EvalError::OverflowingMath)
-        } else {
-            Ok(res)
-        }
+pub fn overflowing_offset<'tcx>(val: u64, i: u64, layout: &TargetDataLayout) -> (u64, bool) {
+    let (res, over) = val.overflowing_add(i);
+    ((res as u128 % (1u128 << layout.pointer_size.bits())) as u64,
+     over || res as u128 >= (1u128 << layout.pointer_size.bits()))
+}
+
+pub fn signed_offset<'tcx>(val: u64, i: i64, layout: &TargetDataLayout) -> EvalResult<'tcx, u64> {
+    let (res, over) = overflowing_signed_offset(val, i as i128, layout);
+    if over {
+        Err(EvalError::OverflowingMath)
     } else {
+        Ok(res)
+    }
+}
+
+pub fn offset<'tcx>(val: u64, i: u64, layout: &TargetDataLayout) -> EvalResult<'tcx, u64> {
+    let (res, over) = overflowing_offset(val, i, layout);
+    if over {
         Err(EvalError::OverflowingMath)
+    } else {
+        Ok(res)
     }
 }
 
 pub fn wrapping_signed_offset<'tcx>(val: u64, i: i64, layout: &TargetDataLayout) -> u64 {
-    (val.wrapping_add(i as u64) as u128 % (1u128 << layout.pointer_size.bits())) as u64
+    overflowing_signed_offset(val, i as i128, layout).0
 }
 
 impl PrimValKind {
@@ -284,6 +318,14 @@ impl PrimValKind {
         }
     }
 
+     pub fn is_float(self) -> bool {
+        use self::PrimValKind::*;
+        match self {
+            F32 | F64 => true,
+            _ => false,
+        }
+    }
+
     pub fn from_uint_size(size: u64) -> Self {
         match size {
             1 => PrimValKind::U8,

tests/compile-fail/fn_ptr_offset.rs

@@ -0,0 +1,11 @@
+use std::mem;
+
+fn f() {}
+
+fn main() {
+    let x : fn() = f;
+    let y : *mut u8 = unsafe { mem::transmute(x) };
+    let y = y.wrapping_offset(1);
+    let x : fn() = unsafe { mem::transmute(y) };
+    x(); //~ ERROR: tried to use an integer pointer or a dangling pointer as a function pointer
+}