Add additional checks for isize overflow

We now perform the correct checks even if the pointer size differs between the host and target. Signed-off-by: Joe Richey <[email protected]>
RalfJung · May 28, 2020 · 7d5415b · 7d5415b
1 parent 71ef841
commit 7d5415b
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 7 deletions.
diff --git a/src/librustc_middle/mir/interpret/pointer.rs b/src/librustc_middle/mir/interpret/pointer.rs
@@ -24,6 +24,12 @@ pub trait PointerArithmetic: HasDataLayout {
         u64::try_from(max_usize_plus_1 - 1).unwrap()
     }
 
+    #[inline]
+    fn machine_isize_min(&self) -> i64 {
+        let max_isize_plus_1 = 1i128 << (self.pointer_size().bits() - 1);
+        i64::try_from(-max_isize_plus_1).unwrap()
+    }
+
     #[inline]
     fn machine_isize_max(&self) -> i64 {
         let max_isize_plus_1 = 1u128 << (self.pointer_size().bits() - 1);
@@ -42,18 +48,23 @@ pub trait PointerArithmetic: HasDataLayout {
 
     #[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 = uabs(i);
         if i >= 0 {
-            self.overflowing_offset(val, n)
+            let (val, over) = self.overflowing_offset(val, n);
+            (val, over || i > self.machine_isize_max())
         } else {
             let res = val.overflowing_sub(n);
-            self.truncate_to_ptr(res)
+            let (val, over) = self.truncate_to_ptr(res);
+            (val, over || i < self.machine_isize_min())
         }
     }
 

diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
@@ -441,9 +441,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // We cannot overflow i64 as a type's size must be <= isize::MAX.
         let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap();
         // The computed offset, in bytes, cannot overflow an isize.
-        let offset_bytes = offset_count
-            .checked_mul(pointee_size)
-            .ok_or(err_ub_format!("inbounds pointer arithmetic: overflow computing offset"))?;
+        let offset_bytes =
+            offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?;
         // The offset being in bounds cannot rely on "wrapping around" the address space.
         // So, first rule out overflows in the pointer arithmetic.
         let offset_ptr = ptr.ptr_signed_offset(offset_bytes, self)?;

diff --git a/src/test/ui/consts/offset_ub.stderr b/src/test/ui/consts/offset_ub.stderr
@@ -51,7 +51,7 @@ error: any use of this value will cause an error
 LL |         intrinsics::offset(self, count)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
-   |         inbounds pointer arithmetic: overflow computing offset
+   |         overflowing in-bounds pointer arithmetic
    |         inside `std::ptr::const_ptr::<impl *const u16>::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL
    |         inside `OVERFLOW` at $DIR/offset_ub.rs:11:43
    | 
@@ -66,7 +66,7 @@ error: any use of this value will cause an error
 LL |         intrinsics::offset(self, count)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
-   |         inbounds pointer arithmetic: overflow computing offset
+   |         overflowing in-bounds pointer arithmetic
    |         inside `std::ptr::const_ptr::<impl *const u16>::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL
    |         inside `UNDERFLOW` at $DIR/offset_ub.rs:12:44
    |