interpret: unify offset_from check with offset check

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -313,78 +313,82 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let a = self.read_pointer(&args[0])?;
                 let b = self.read_pointer(&args[1])?;
 
-                // Special case: if both scalars are *equal integers*
+                let usize_layout = self.layout_of(self.tcx.types.usize)?;
-                // and not null, we pretend there is an allocation of size 0 right there,
+                let isize_layout = self.layout_of(self.tcx.types.isize)?;
-                // and their offset is 0. (There's never a valid object at null, making it an
+
-                // exception from the exception.)
+                // Get offsets for both that are at least relative to the same base.
-                // This is the dual to the special exception for offset-by-0
+                let (a_offset, b_offset) =
-                // in the inbounds pointer offset operation (see `ptr_offset_inbounds` below).
+                    match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
-                match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
+                        (Err(a), Err(b)) => {
-                    (Err(a), Err(b)) if a == b && a != 0 => {
+                            // Neither poiner points to an allocation.
-                        // Both are the same non-null integer.
+                            // If these are inequal or null, this *will* fail the deref check below.
-                        self.write_scalar(Scalar::from_machine_isize(0, self), dest)?;
+                            (a, b)
-                    }
+                        }
-                    (Err(offset), _) | (_, Err(offset)) => {
+                        (Err(_), _) | (_, Err(_)) => {
-                        throw_ub!(DanglingIntPointer(offset, CheckInAllocMsg::OffsetFromTest));
+                            // We managed to find a valid allocation for one pointer, but not the other.
-                    }
+                            // That means they are definitely not pointing to the same allocation.
-                    (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
-                        // Both are pointers. They must be into the same allocation.
-                        if a_alloc_id != b_alloc_id {
                             throw_ub_format!(
-                                "{} cannot compute offset of pointers into different allocations.",
+                                "{} called on pointers into different allocations",
-                                intrinsic_name,
+                                intrinsic_name
                             );
                         }
-                        // And they must both be valid for zero-sized accesses ("in-bounds or one past the end").
+                        (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
-                        self.check_ptr_access_align(
+                            // Found allocation for both. They must be into the same allocation.
-                            a,
+                            if a_alloc_id != b_alloc_id {
-                            Size::ZERO,
+                                throw_ub_format!(
-                            Align::ONE,
+                                    "{} called on pointers into different allocations",
-                            CheckInAllocMsg::OffsetFromTest,
+                                    intrinsic_name
-                        )?;
+                                );
-                        self.check_ptr_access_align(
+                            }
-                            b,
+                            // Use these offsets for distance calculation.
-                            Size::ZERO,
+                            (a_offset.bytes(), b_offset.bytes())
-                            Align::ONE,
-                            CheckInAllocMsg::OffsetFromTest,
-                        )?;
-
-                        if intrinsic_name == sym::ptr_offset_from_unsigned && a_offset < b_offset {
-                            throw_ub_format!(
-                                "{} cannot compute a negative offset, but {} < {}",
-                                intrinsic_name,
-                                a_offset.bytes(),
-                                b_offset.bytes(),
-                            );
                         }
+                    };
 
-                        // Compute offset.
+                // Compute distance.
-                        let usize_layout = self.layout_of(self.tcx.types.usize)?;
+                let distance = {
-                        let isize_layout = self.layout_of(self.tcx.types.isize)?;
+                    // The subtraction is always done in `isize` to enforce
-                        let ret_layout = if intrinsic_name == sym::ptr_offset_from {
+                    // the "no more than `isize::MAX` apart" requirement.
-                            isize_layout
+                    let a_offset = ImmTy::from_uint(a_offset, isize_layout);
-                        } else {
+                    let b_offset = ImmTy::from_uint(b_offset, isize_layout);
-                            usize_layout
+                    let (val, overflowed, _ty) =
-                        };
+                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
-
+                    if overflowed {
-                        // The subtraction is always done in `isize` to enforce
+                        throw_ub_format!("pointers were too far apart for {}", intrinsic_name);
-                        // the "no more than `isize::MAX` apart" requirement.
-                        let a_offset = ImmTy::from_uint(a_offset.bytes(), isize_layout);
-                        let b_offset = ImmTy::from_uint(b_offset.bytes(), isize_layout);
-                        let (val, overflowed, _ty) =
-                            self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
-                        if overflowed {
-                            throw_ub_format!("Pointers were too far apart for {}", intrinsic_name);
-                        }
-
-                        let pointee_layout = self.layout_of(substs.type_at(0))?;
-                        // This re-interprets an isize at ret_layout, but we already checked
-                        // that if ret_layout is usize, then the result must be non-negative.
-                        let val = ImmTy::from_scalar(val, ret_layout);
-                        let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
-                        self.exact_div(&val, &size, dest)?;
                     }
+                    val.to_machine_isize(self)?
+                };
+
+                // Check that the range between them is dereferenceable ("in-bounds or one past the
+                // end of the same allocation"). This is like the check in ptr_offset_inbounds.
+                let min_ptr = if distance >= 0 { b } else { a };
+                self.check_ptr_access_align(
+                    min_ptr,
+                    Size::from_bytes(distance.unsigned_abs()),
+                    Align::ONE,
+                    CheckInAllocMsg::OffsetFromTest,
+                )?;
+
+                if intrinsic_name == sym::ptr_offset_from_unsigned && distance < 0 {
+                    throw_ub_format!(
+                        "{} called when first pointer has smaller offset than second: {} < {}",
+                        intrinsic_name,
+                        a_offset,
+                        b_offset,
+                    );
                 }
+
+                // Perform division by size to compute return value.
+                let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned {
+                    usize_layout
+                } else {
+                    isize_layout
+                };
+                let pointee_layout = self.layout_of(substs.type_at(0))?;
+                // If ret_layout is unsigned, we checked that so is the distance, so we are good.
+                let val = ImmTy::from_int(distance, ret_layout);
+                let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
+                self.exact_div(&val, &size, dest)?;
             }
 
             sym::transmute => {
@@ -575,11 +579,10 @@ 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.unsigned_abs();
         // This call handles checking for integer/null pointers.
         self.check_ptr_access_align(
             min_ptr,
-            Size::from_bytes(size),
+            Size::from_bytes(offset_bytes.unsigned_abs()),
             Align::ONE,
             CheckInAllocMsg::PointerArithmeticTest,
         )?;

src/test/ui/const-ptr/forbidden_slices.32bit.stderr

@@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+   |                  ptr_offset_from_unsigned called on pointers into different allocations
    |                  inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
@@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+   |                  ptr_offset_from_unsigned called on pointers into different allocations
    |                  inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL

src/test/ui/const-ptr/forbidden_slices.64bit.stderr

@@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+   |                  ptr_offset_from_unsigned called on pointers into different allocations
    |                  inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
@@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+   |                  ptr_offset_from_unsigned called on pointers into different allocations
    |                  inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL

src/test/ui/consts/offset_from_ub.rs

@@ -15,7 +15,7 @@ pub const DIFFERENT_ALLOC: usize = {
     let uninit2 = std::mem::MaybeUninit::<Struct>::uninit();
     let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct;
     let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed
-    //~| ptr_offset_from cannot compute offset of pointers into different allocations.
+    //~| pointers into different allocations
     offset as usize
 };
 
@@ -41,7 +41,7 @@ pub const DIFFERENT_INT: isize = { // offset_from with two different integers: l
     let ptr1 = 8 as *const u8;
     let ptr2 = 16 as *const u8;
     unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed
-    //~| 0x10 is not a valid pointer
+    //~| 0x8 is not a valid pointer
 };
 
 const OUT_OF_BOUNDS_1: isize = {
@@ -50,7 +50,7 @@ const OUT_OF_BOUNDS_1: isize = {
     let end_ptr = (start_ptr).wrapping_add(length);
     // First ptr is out of bounds
     unsafe { ptr_offset_from(end_ptr, start_ptr) } //~ERROR evaluation of constant value failed
-    //~| pointer at offset 10 is out-of-bounds
+    //~| pointer to 10 bytes starting at offset 0 is out-of-bounds
 };
 
 const OUT_OF_BOUNDS_2: isize = {
@@ -59,7 +59,7 @@ const OUT_OF_BOUNDS_2: isize = {
     let end_ptr = (start_ptr).wrapping_add(length);
     // Second ptr is out of bounds
     unsafe { ptr_offset_from(start_ptr, end_ptr) } //~ERROR evaluation of constant value failed
-    //~| pointer at offset 10 is out-of-bounds
+    //~| pointer to 10 bytes starting at offset 0 is out-of-bounds
 };
 
 const OUT_OF_BOUNDS_SAME: isize = {
@@ -76,7 +76,7 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
     let uninit2 = std::mem::MaybeUninit::<Struct>::uninit();
     let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct;
     let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed
-    //~| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+    //~| pointers into different allocations
     offset as usize
 };
 
@@ -84,7 +84,7 @@ const WRONG_ORDER_UNSIGNED: usize = {
     let a = ['a', 'b', 'c'];
     let p = a.as_ptr();
     unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } //~ERROR evaluation of constant value failed
-    //~| cannot compute a negative offset, but 0 < 8
+    //~| first pointer has smaller offset than second: 0 < 8
 };
 
 fn main() {}

src/test/ui/consts/offset_from_ub.stderr

@@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:17:27
    |
 LL |     let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) };
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from cannot compute offset of pointers into different allocations.
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from called on pointers into different allocations
 
 error[E0080]: evaluation of constant value failed
   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
@@ -10,7 +10,7 @@ error[E0080]: evaluation of constant value failed
 LL |         unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  out-of-bounds offset_from: 0x2a is not a valid pointer
+   |                  ptr_offset_from called on pointers into different allocations
    |                  inside `ptr::const_ptr::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $DIR/offset_from_ub.rs:23:14
@@ -34,19 +34,19 @@ error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:43:14
    |
 LL |     unsafe { ptr_offset_from(ptr2, ptr1) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x10 is not a valid pointer
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x8 is not a valid pointer
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:52:14
    |
 LL |     unsafe { ptr_offset_from(end_ptr, start_ptr) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer at offset 10 is out-of-bounds
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:61:14
    |
 LL |     unsafe { ptr_offset_from(start_ptr, end_ptr) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer at offset 10 is out-of-bounds
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:69:14
@@ -58,13 +58,13 @@ error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:78:27
    |
 LL |     let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) };
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called on pointers into different allocations
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:86:14
    |
 LL |     unsafe { ptr_offset_from_unsigned(p, p.add(2) ) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute a negative offset, but 0 < 8
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called when first pointer has smaller offset than second: 0 < 8
 
 error: aborting due to 10 previous errors