Rollup merge of rust-lang#99692 - RalfJung:too-far, r=oli-obk

interpret, ptr_offset_from: refactor and test too-far-apart check

We didn't have any tests for the "too far apart" message, and indeed that check mostly relied on the in-bounds check and was otherwise probably not entirely correct... so I rewrote that check, and it is before the in-bounds check so we can test it separately.

compiler/rustc_const_eval/src/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::{ConstValue, GlobalId, InterpResult, PointerArithmetic, Scalar},
     BinOp,
 };
 use rustc_middle::ty;
@@ -328,15 +328,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             // 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.
                             throw_ub_format!(
-                                "{} called on pointers into different allocations",
+                                "`{}` called on pointers into different allocations",
                                 intrinsic_name
                             );
                         }
                         (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
                             // Found allocation for both. They must be into the same allocation.
                             if a_alloc_id != b_alloc_id {
                                 throw_ub_format!(
-                                    "{} called on pointers into different allocations",
+                                    "`{}` called on pointers into different allocations",
                                     intrinsic_name
                                 );
                             }
@@ -346,47 +346,71 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     };
 
                 // Compute distance.
-                let distance = {
-                    // The subtraction is always done in `isize` to enforce
-                    // the "no more than `isize::MAX` apart" requirement.
-                    let a_offset = ImmTy::from_uint(a_offset, isize_layout);
-                    let b_offset = ImmTy::from_uint(b_offset, isize_layout);
-                    let (val, overflowed, _ty) =
-                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
+                let dist = {
+                    // Addresses are unsigned, so this is a `usize` computation. We have to do the
+                    // overflow check separately anyway.
+                    let (val, overflowed, _ty) = {
+                        let a_offset = ImmTy::from_uint(a_offset, usize_layout);
+                        let b_offset = ImmTy::from_uint(b_offset, usize_layout);
+                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?
+                    };
                     if overflowed {
-                        throw_ub_format!("pointers were too far apart for {}", intrinsic_name);
+                        // a < b
+                        if intrinsic_name == sym::ptr_offset_from_unsigned {
+                            throw_ub_format!(
+                                "`{}` called when first pointer has smaller offset than second: {} < {}",
+                                intrinsic_name,
+                                a_offset,
+                                b_offset,
+                            );
+                        }
+                        // The signed form of the intrinsic allows this. If we interpret the
+                        // difference as isize, we'll get the proper signed difference. If that
+                        // seems *positive*, they were more than isize::MAX apart.
+                        let dist = val.to_machine_isize(self)?;
+                        if dist >= 0 {
+                            throw_ub_format!(
+                                "`{}` called when first pointer is too far before second",
+                                intrinsic_name
+                            );
+                        }
+                        dist
+                    } else {
+                        // b >= a
+                        let dist = val.to_machine_isize(self)?;
+                        // If converting to isize produced a *negative* result, we had an overflow
+                        // because they were more than isize::MAX apart.
+                        if dist < 0 {
+                            throw_ub_format!(
+                                "`{}` called when first pointer is too far ahead of second",
+                                intrinsic_name
+                            );
+                        }
+                        dist
                     }
-                    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 };
+                let min_ptr = if dist >= 0 { b } else { a };
                 self.check_ptr_access_align(
                     min_ptr,
-                    Size::from_bytes(distance.unsigned_abs()),
+                    Size::from_bytes(dist.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 {
+                    assert!(0 <= dist && dist <= self.machine_isize_max());
                     usize_layout
                 } else {
+                    assert!(self.machine_isize_min() <= dist && dist <= self.machine_isize_max());
                     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 val = ImmTy::from_int(dist, ret_layout);
                 let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
                 self.exact_div(&val, &size, dest)?;
             }

library/core/src/ptr/const_ptr.rs

@@ -730,7 +730,7 @@ impl<T: ?Sized> *const T {
     /// }
     ///
     /// // This would be incorrect, as the pointers are not correctly ordered:
-    /// // ptr1.offset_from(ptr2)
+    /// // ptr1.sub_ptr(ptr2)
     /// ```
     #[unstable(feature = "ptr_sub_ptr", issue = "95892")]
     #[rustc_const_unstable(feature = "const_ptr_sub_ptr", issue = "95892")]

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

@@ -222,7 +222,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned called on 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
@@ -241,7 +241,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned called on 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

@@ -222,7 +222,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned called on 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
@@ -241,7 +241,7 @@ error[E0080]: could not evaluate static initializer
 LL |         unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from_unsigned called on 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

@@ -2,6 +2,7 @@
 #![feature(core_intrinsics)]
 
 use std::intrinsics::{ptr_offset_from, ptr_offset_from_unsigned};
+use std::ptr;
 
 #[repr(C)]
 struct Struct {
@@ -75,9 +76,21 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
     let base_ptr: *const Struct = &uninit as *const _ as *const Struct;
     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
+    unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) } //~ERROR evaluation of constant value failed
     //~| pointers into different allocations
-    offset as usize
+};
+
+pub const TOO_FAR_APART1: isize = {
+    let ptr1 = ptr::null::<u8>();
+    let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42);
+    unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed
+    //~| too far ahead
+};
+pub const TOO_FAR_APART2: isize = {
+    let ptr1 = ptr::null::<u8>();
+    let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42);
+    unsafe { ptr_offset_from(ptr1, ptr2) } //~ERROR evaluation of constant value failed
+    //~| too far before
 };
 
 const WRONG_ORDER_UNSIGNED: usize = {
@@ -86,5 +99,27 @@ const WRONG_ORDER_UNSIGNED: usize = {
     unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } //~ERROR evaluation of constant value failed
     //~| first pointer has smaller offset than second: 0 < 8
 };
+pub const TOO_FAR_APART_UNSIGNED: usize = {
+    let ptr1 = ptr::null::<u8>();
+    let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42);
+    // This would fit into a `usize` but we still don't allow it.
+    unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } //~ERROR evaluation of constant value failed
+    //~| too far ahead
+};
+
+// These do NOT complain that pointers are too far apart; they pass that check (to then fail the
+// next one).
+pub const OFFSET_VERY_FAR1: isize = {
+    let ptr1 = ptr::null::<u8>();
+    let ptr2 = ptr1.wrapping_offset(isize::MAX);
+    unsafe { ptr2.offset_from(ptr1) }
+    //~^ inside
+};
+pub const OFFSET_VERY_FAR2: isize = {
+    let ptr1 = ptr::null::<u8>();
+    let ptr2 = ptr1.wrapping_offset(isize::MAX);
+    unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
+    //~^ inside
+};
 
 fn main() {}

src/test/ui/consts/offset_from_ub.stderr

@@ -1,71 +1,117 @@
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:17:27
+  --> $DIR/offset_from_ub.rs:18:27
    |
 LL |     let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) };
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from called on 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
    |
 LL |         unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  ptr_offset_from called on pointers into different allocations
+   |                  `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
+  ::: $DIR/offset_from_ub.rs:24:14
    |
 LL |     unsafe { (42 as *const u8).offset_from(&5u8) as usize }
-   |              ----------------------------------- inside `NOT_PTR` at $DIR/offset_from_ub.rs:23:14
+   |              ----------------------------------- inside `NOT_PTR` at $DIR/offset_from_ub.rs:24:14
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:30:14
+  --> $DIR/offset_from_ub.rs:31:14
    |
 LL |     unsafe { ptr_offset_from(field_ptr, base_ptr as *const u16) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ exact_div: 1_isize cannot be divided by 2_isize without remainder
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:36:14
+  --> $DIR/offset_from_ub.rs:37:14
    |
 LL |     unsafe { ptr_offset_from(ptr, ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: null pointer is a dangling pointer (it has no provenance)
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:43:14
+  --> $DIR/offset_from_ub.rs:44:14
    |
 LL |     unsafe { ptr_offset_from(ptr2, ptr1) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x8[noalloc] is a dangling pointer (it has no provenance)
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:52:14
+  --> $DIR/offset_from_ub.rs:53:14
    |
 LL |     unsafe { ptr_offset_from(end_ptr, start_ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc18 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
+  --> $DIR/offset_from_ub.rs:62:14
    |
 LL |     unsafe { ptr_offset_from(start_ptr, end_ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc21 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
+  --> $DIR/offset_from_ub.rs:70:14
    |
 LL |     unsafe { ptr_offset_from(end_ptr, end_ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc24 has size 4, so pointer at offset 10 is out-of-bounds
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:78:27
+  --> $DIR/offset_from_ub.rs:79:14
    |
-LL |     let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) };
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called on pointers into different allocations
+LL |     unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `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(ptr2, ptr1) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far ahead of second
+
+error[E0080]: evaluation of constant value failed
+  --> $DIR/offset_from_ub.rs:92:14
+   |
+LL |     unsafe { ptr_offset_from(ptr1, ptr2) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far before second
+
+error[E0080]: evaluation of constant value failed
+  --> $DIR/offset_from_ub.rs:99:14
+   |
 LL |     unsafe { ptr_offset_from_unsigned(p, p.add(2) ) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called when first pointer has smaller offset than second: 0 < 8
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: 0 < 8
+
+error[E0080]: evaluation of constant value failed
+  --> $DIR/offset_from_ub.rs:106:14
+   |
+LL |     unsafe { ptr_offset_from_unsigned(ptr2, ptr1) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer is too far ahead of second
+
+error[E0080]: evaluation of constant value failed
+  --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
+   |
+LL |         unsafe { intrinsics::ptr_offset_from(self, origin) }
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                  |
+   |                  out-of-bounds offset_from: null pointer is a dangling pointer (it has no provenance)
+   |                  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:115:14
+   |
+LL |     unsafe { ptr2.offset_from(ptr1) }
+   |              ---------------------- inside `OFFSET_VERY_FAR1` at $DIR/offset_from_ub.rs:115:14
+
+error[E0080]: evaluation of constant value failed
+  --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
+   |
+LL |         unsafe { intrinsics::ptr_offset_from(self, origin) }
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                  |
+   |                  out-of-bounds offset_from: null pointer is a dangling pointer (it has no provenance)
+   |                  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:121:14
+   |
+LL |     unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
+   |              ----------------------------------------- inside `OFFSET_VERY_FAR2` at $DIR/offset_from_ub.rs:121:14
 
-error: aborting due to 10 previous errors
+error: aborting due to 15 previous errors
 
 For more information about this error, try `rustc --explain E0080`.