Add checks and tests for computing abs(offset_bytes)

The previous code paniced if offset_bytes == i64::MIN. This commit: - Properly computes the absoulte value to avoid this panic - Adds a test for this edge case Signed-off-by: Joe Richey <[email protected]>
RalfJung · May 26, 2020 · 71ef841 · 71ef841
1 parent 6367b54
commit 71ef841
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
@@ -7,7 +7,7 @@ use std::convert::TryFrom;
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::{
     self,
-    interpret::{ConstValue, GlobalId, InterpResult, Scalar},
+    interpret::{uabs, ConstValue, GlobalId, InterpResult, Scalar},
     BinOp,
 };
 use rustc_middle::ty;
@@ -438,6 +438,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         pointee_ty: Ty<'tcx>,
         offset_count: i64,
     ) -> InterpResult<'tcx, Scalar<M::PointerTag>> {
+        // 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
@@ -450,7 +451,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // memory between these pointers must be accessible. Note that we do not require the
         // pointers to be properly aligned (unlike a read/write operation).
         let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr };
-        let size = offset_bytes.checked_abs().unwrap();
+        let size: u64 = uabs(offset_bytes);
         // This call handles checking for integer/NULL pointers.
         self.memory.check_ptr_access_align(
             min_ptr,

diff --git a/src/test/ui/consts/offset_ub.rs b/src/test/ui/consts/offset_ub.rs
@@ -19,4 +19,7 @@ pub const DANGLING: *const u8 = unsafe { ptr::NonNull::<u8>::dangling().as_ptr()
 // Right now, a zero offset from null is UB
 pub const NULL_OFFSET_ZERO: *const u8 = unsafe { ptr::null::<u8>().offset(0) }; //~NOTE
 
+// Make sure that we don't panic when computing abs(offset*size_of::<T>())
+pub const UNDERFLOW_ABS: *const u8 = unsafe { (usize::MAX as *const u8).offset(isize::MIN) }; //~NOTE
+
 fn main() {}
diff --git a/src/test/ui/consts/offset_ub.stderr b/src/test/ui/consts/offset_ub.stderr
@@ -150,5 +150,20 @@ LL |         intrinsics::offset(self, count)
 LL | pub const NULL_OFFSET_ZERO: *const u8 = unsafe { ptr::null::<u8>().offset(0) };
    | -------------------------------------------------------------------------------
 
-error: aborting due to 10 previous errors
+error: any use of this value will cause an error
+  --> $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL
+   |
+LL |         intrinsics::offset(self, count)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         unable to turn bytes into a pointer
+   |         inside `std::ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL
+   |         inside `UNDERFLOW_ABS` at $DIR/offset_ub.rs:23:47
+   | 
+  ::: $DIR/offset_ub.rs:23:1
+   |
+LL | pub const UNDERFLOW_ABS: *const u8 = unsafe { (usize::MAX as *const u8).offset(isize::MIN) };
+   | ---------------------------------------------------------------------------------------------
+
+error: aborting due to 11 previous errors