interpret: simplify pointer arithmetic logic

Author: RalfJung

compiler/rustc_abi/src/lib.rs

@@ -516,7 +516,7 @@ impl Size {
     /// Truncates `value` to `self` bits and then sign-extends it to 128 bits
     /// (i.e., if it is negative, fill with 1's on the left).
     #[inline]
-    pub fn sign_extend(self, value: u128) -> u128 {
+    pub fn sign_extend(self, value: u128) -> i128 {
         let size = self.bits();
         if size == 0 {
             // Truncated until nothing is left.
@@ -526,7 +526,7 @@ impl Size {
         let shift = 128 - size;
         // Shift the unsigned value to the left, then shift back to the right as signed
         // (essentially fills with sign bit on the left).
-        (((value << shift) as i128) >> shift) as u128
+        ((value << shift) as i128) >> shift
     }
 
     /// Truncates `value` to `self` bits.
@@ -544,7 +544,7 @@ impl Size {
 
     #[inline]
     pub fn signed_int_min(&self) -> i128 {
-        self.sign_extend(1_u128 << (self.bits() - 1)) as i128
+        self.sign_extend(1_u128 << (self.bits() - 1))
     }
 
     #[inline]

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -560,17 +560,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         self.frame().body
     }
 
-    #[inline(always)]
-    pub fn sign_extend(&self, value: u128, ty: TyAndLayout<'_>) -> u128 {
-        assert!(ty.abi.is_signed());
-        ty.size.sign_extend(value)
-    }
-
-    #[inline(always)]
-    pub fn truncate(&self, value: u128, ty: TyAndLayout<'_>) -> u128 {
-        ty.size.truncate(value)
-    }
-
     #[inline]
     pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool {
         ty.is_freeze(*self.tcx, self.param_env)

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -206,7 +206,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 } else {
                     (val_bits >> shift_bits) | (val_bits << inv_shift_bits)
                 };
-                let truncated_bits = self.truncate(result_bits, layout_val);
+                let truncated_bits = layout_val.size.truncate(result_bits);
                 let result = Scalar::from_uint(truncated_bits, layout_val.size);
                 self.write_scalar(result, dest)?;
             }
@@ -580,13 +580,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         ptr: Pointer<Option<M::Provenance>>,
         offset_bytes: i64,
     ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
-        // We first compute the pointer with overflow checks, to get a specific error for when it
-        // overflows (though technically this is redundant with the following inbounds check).
-        let result = ptr.signed_offset(offset_bytes, self)?;
         // The offset must be in bounds starting from `ptr`.
         self.check_ptr_access_signed(ptr, offset_bytes, CheckInAllocMsg::PointerArithmeticTest)?;
-        // Done.
-        Ok(result)
+        // This also implies that there is no overflow, so we are done.
+        Ok(ptr.wrapping_signed_offset(offset_bytes, self))
     }
 
     /// Copy `count*size_of::<T>()` many bytes from `*src` to `*dst`.

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -473,7 +473,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                     throw_ub!(PointerOutOfBounds {
                         alloc_id,
                         alloc_size,
-                        ptr_offset: self.target_usize_to_isize(offset),
+                        ptr_offset: self.sign_extend_to_target_isize(offset),
                         inbounds_size: size,
                         msg,
                     })

compiler/rustc_const_eval/src/interpret/place.rs

@@ -285,10 +285,8 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for PlaceTy<'tcx, Prov> {
                 // projections are type-checked and bounds-checked.
                 assert!(offset + layout.size <= self.layout.size);
 
-                let new_offset = Size::from_bytes(
-                    ecx.data_layout()
-                        .offset(old_offset.unwrap_or(Size::ZERO).bytes(), offset.bytes())?,
-                );
+                // Size `+`, ensures no overflow.
+                let new_offset = old_offset.unwrap_or(Size::ZERO) + offset;
 
                 PlaceTy {
                     place: Place::Local { local, offset: Some(new_offset), locals_addr },

compiler/rustc_const_eval/src/interpret/step.rs

@@ -362,7 +362,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             // of the first element.
             let elem_size = first.layout.size;
             let first_ptr = first.ptr();
-            let rest_ptr = first_ptr.offset(elem_size, self)?;
+            let rest_ptr = first_ptr.wrapping_offset(elem_size, self);
             // No alignment requirement since `copy_op` above already checked it.
             self.mem_copy_repeatedly(
                 first_ptr,

compiler/rustc_lint/src/types.rs

@@ -309,11 +309,7 @@ fn report_bin_hex_error(
 ) {
     let (t, actually) = match ty {
         attr::IntType::SignedInt(t) => {
-            let actually = if negative {
-                -(size.sign_extend(val) as i128)
-            } else {
-                size.sign_extend(val) as i128
-            };
+            let actually = if negative { -(size.sign_extend(val)) } else { size.sign_extend(val) };
             (t.name_str(), actually.to_string())
         }
         attr::IntType::UnsignedInt(t) => {

compiler/rustc_middle/src/mir/interpret/pointer.rs

@@ -5,7 +5,7 @@ use rustc_data_structures::static_assert_size;
 use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_target::abi::{HasDataLayout, Size};
 
-use super::{AllocId, InterpResult};
+use super::AllocId;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Pointer arithmetic
@@ -40,62 +40,13 @@ pub trait PointerArithmetic: HasDataLayout {
     }
 
     #[inline]
-    fn target_usize_to_isize(&self, val: u64) -> i64 {
-        let val = val as i64;
-        // Now wrap-around into the machine_isize range.
-        if val > self.target_isize_max() {
-            // This can only happen if the ptr size is < 64, so we know max_usize_plus_1 fits into
-            // i64.
-            debug_assert!(self.pointer_size().bits() < 64);
-            let max_usize_plus_1 = 1u128 << self.pointer_size().bits();
-            val - i64::try_from(max_usize_plus_1).unwrap()
-        } else {
-            val
-        }
-    }
-
-    /// Helper function: truncate given value-"overflowed flag" pair to pointer size and
-    /// update "overflowed flag" if there was an overflow.
-    /// This should be called by all the other methods before returning!
-    #[inline]
-    fn truncate_to_ptr(&self, (val, over): (u64, bool)) -> (u64, bool) {
-        let val = u128::from(val);
-        let max_ptr_plus_1 = 1u128 << self.pointer_size().bits();
-        (u64::try_from(val % max_ptr_plus_1).unwrap(), over || val >= max_ptr_plus_1)
-    }
-
-    #[inline]
-    fn overflowing_offset(&self, val: u64, i: u64) -> (u64, bool) {
-        // We do not need to check if i fits in a machine usize. If it doesn't,
-        // either the wrapping_add will wrap or res will not fit in a pointer.
-        let res = val.overflowing_add(i);
-        self.truncate_to_ptr(res)
-    }
-
-    #[inline]
-    fn overflowing_signed_offset(&self, val: u64, i: i64) -> (u64, bool) {
-        // We need to make sure that i fits in a machine isize.
-        let n = i.unsigned_abs();
-        if i >= 0 {
-            let (val, over) = self.overflowing_offset(val, n);
-            (val, over || i > self.target_isize_max())
-        } else {
-            let res = val.overflowing_sub(n);
-            let (val, over) = self.truncate_to_ptr(res);
-            (val, over || i < self.target_isize_min())
-        }
-    }
-
-    #[inline]
-    fn offset<'tcx>(&self, val: u64, i: u64) -> InterpResult<'tcx, u64> {
-        let (res, over) = self.overflowing_offset(val, i);
-        if over { throw_ub!(PointerArithOverflow) } else { Ok(res) }
+    fn truncate_to_target_usize(&self, val: u64) -> u64 {
+        self.pointer_size().truncate(val.into()).try_into().unwrap()
     }
 
     #[inline]
-    fn signed_offset<'tcx>(&self, val: u64, i: i64) -> InterpResult<'tcx, u64> {
-        let (res, over) = self.overflowing_signed_offset(val, i);
-        if over { throw_ub!(PointerArithOverflow) } else { Ok(res) }
+    fn sign_extend_to_target_isize(&self, val: u64) -> i64 {
+        self.pointer_size().sign_extend(val.into()).try_into().unwrap()
     }
 }
 
@@ -331,7 +282,7 @@ impl<Prov> Pointer<Option<Prov>> {
     }
 }
 
-impl<'tcx, Prov> Pointer<Prov> {
+impl<Prov> Pointer<Prov> {
     #[inline(always)]
     pub fn new(provenance: Prov, offset: Size) -> Self {
         Pointer { provenance, offset }
@@ -349,43 +300,16 @@ impl<'tcx, Prov> Pointer<Prov> {
         Pointer { provenance: f(self.provenance), ..self }
     }
 
-    #[inline]
-    pub fn offset(self, i: Size, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> {
-        Ok(Pointer {
-            offset: Size::from_bytes(cx.data_layout().offset(self.offset.bytes(), i.bytes())?),
-            ..self
-        })
-    }
-
-    #[inline]
-    pub fn overflowing_offset(self, i: Size, cx: &impl HasDataLayout) -> (Self, bool) {
-        let (res, over) = cx.data_layout().overflowing_offset(self.offset.bytes(), i.bytes());
-        let ptr = Pointer { offset: Size::from_bytes(res), ..self };
-        (ptr, over)
-    }
-
     #[inline(always)]
     pub fn wrapping_offset(self, i: Size, cx: &impl HasDataLayout) -> Self {
-        self.overflowing_offset(i, cx).0
-    }
-
-    #[inline]
-    pub fn signed_offset(self, i: i64, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> {
-        Ok(Pointer {
-            offset: Size::from_bytes(cx.data_layout().signed_offset(self.offset.bytes(), i)?),
-            ..self
-        })
-    }
-
-    #[inline]
-    pub fn overflowing_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> (Self, bool) {
-        let (res, over) = cx.data_layout().overflowing_signed_offset(self.offset.bytes(), i);
-        let ptr = Pointer { offset: Size::from_bytes(res), ..self };
-        (ptr, over)
+        let res =
+            cx.data_layout().truncate_to_target_usize(self.offset.bytes().wrapping_add(i.bytes()));
+        Pointer { offset: Size::from_bytes(res), ..self }
     }
 
     #[inline(always)]
     pub fn wrapping_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> Self {
-        self.overflowing_signed_offset(i, cx).0
+        // It's wrapping anyway, so we can just cast to `u64`.
+        self.wrapping_offset(Size::from_bytes(i as u64), cx)
     }
 }

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -393,7 +393,7 @@ impl<'tcx, Prov: Provenance> Scalar<Prov> {
     #[inline]
     pub fn to_int(self, size: Size) -> InterpResult<'tcx, i128> {
         let b = self.to_bits(size)?;
-        Ok(size.sign_extend(b) as i128)
+        Ok(size.sign_extend(b))
     }
 
     /// Converts the scalar to produce an `i8`. Fails if the scalar is a pointer.

compiler/rustc_middle/src/ty/consts/int.rs

@@ -234,7 +234,7 @@ impl ScalarInt {
         let data = i.into();
         // `into` performed sign extension, we have to truncate
         let r = Self::raw(size.truncate(data as u128), size);
-        (r, size.sign_extend(r.data) as i128 != data)
+        (r, size.sign_extend(r.data) != data)
     }
 
     #[inline]
@@ -335,7 +335,7 @@ impl ScalarInt {
     #[inline]
     pub fn to_int(self, size: Size) -> i128 {
         let b = self.to_bits(size);
-        size.sign_extend(b) as i128
+        size.sign_extend(b)
     }
 
     /// Converts the `ScalarInt` to i8.