Rollup merge of rust-lang#94827 - RalfJung:offset-from-ub, r=oli-obk

CTFE/Miri: detect out-of-bounds pointers in offset_from

Also I became uneasy with aggressively doing `try_to_int` here -- this will always succeed on Miri, leading to the wrong codepath being taken. We should rather try to convert them both to pointers, and use the integer path as a fallback, so that's what I implemented now.

Hiding whitespaces helps with the diff.

Fixes rust-lang/miri#1950

r? `@oli-obk`

Author: Dylan-DPC

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -307,53 +307,57 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 self.write_pointer(offset_ptr, dest)?;
             }
             sym::ptr_offset_from => {
-                let a = self.read_immediate(&args[0])?.to_scalar()?;
-                let b = self.read_immediate(&args[1])?.to_scalar()?;
+                let a = self.read_pointer(&args[0])?;
+                let b = self.read_pointer(&args[1])?;
 
                 // Special case: if both scalars are *equal integers*
                 // and not null, we pretend there is an allocation of size 0 right there,
                 // and their offset is 0. (There's never a valid object at null, making it an
                 // exception from the exception.)
                 // This is the dual to the special exception for offset-by-0
-                // in the inbounds pointer offset operation (see the Miri code, `src/operator.rs`).
-                //
-                // Control flow is weird because we cannot early-return (to reach the
-                // `go_to_block` at the end).
-                let done = if let (Ok(a), Ok(b)) = (a.try_to_int(), b.try_to_int()) {
-                    let a = a.try_to_machine_usize(*self.tcx).unwrap();
-                    let b = b.try_to_machine_usize(*self.tcx).unwrap();
-                    if a == b && a != 0 {
+                // in the inbounds pointer offset operation (see `ptr_offset_inbounds` below).
+                match (self.memory.ptr_try_get_alloc(a), self.memory.ptr_try_get_alloc(b)) {
+                    (Err(a), Err(b)) if a == b && a != 0 => {
+                        // Both are the same non-null integer.
                         self.write_scalar(Scalar::from_machine_isize(0, self), dest)?;
-                        true
-                    } else {
-                        false
                     }
-                } else {
-                    false
-                };
-
-                if !done {
-                    // General case: we need two pointers.
-                    let a = self.scalar_to_ptr(a);
-                    let b = self.scalar_to_ptr(b);
-                    let (a_alloc_id, a_offset, _) = self.memory.ptr_get_alloc(a)?;
-                    let (b_alloc_id, b_offset, _) = self.memory.ptr_get_alloc(b)?;
-                    if a_alloc_id != b_alloc_id {
-                        throw_ub_format!(
-                            "ptr_offset_from cannot compute offset of pointers into different \
-                            allocations.",
-                        );
+                    (Err(offset), _) | (_, Err(offset)) => {
+                        throw_ub!(DanglingIntPointer(offset, CheckInAllocMsg::OffsetFromTest));
+                    }
+                    (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!(
+                                "ptr_offset_from cannot compute offset of pointers into different \
+                                allocations.",
+                            );
+                        }
+                        // And they must both be valid for zero-sized accesses ("in-bounds or one past the end").
+                        self.memory.check_ptr_access_align(
+                            a,
+                            Size::ZERO,
+                            Align::ONE,
+                            CheckInAllocMsg::OffsetFromTest,
+                        )?;
+                        self.memory.check_ptr_access_align(
+                            b,
+                            Size::ZERO,
+                            Align::ONE,
+                            CheckInAllocMsg::OffsetFromTest,
+                        )?;
+
+                        // Compute offset.
+                        let usize_layout = self.layout_of(self.tcx.types.usize)?;
+                        let isize_layout = self.layout_of(self.tcx.types.isize)?;
+                        let a_offset = ImmTy::from_uint(a_offset.bytes(), usize_layout);
+                        let b_offset = ImmTy::from_uint(b_offset.bytes(), usize_layout);
+                        let (val, _overflowed, _ty) =
+                            self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
+                        let pointee_layout = self.layout_of(substs.type_at(0))?;
+                        let val = ImmTy::from_scalar(val, isize_layout);
+                        let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout);
+                        self.exact_div(&val, &size, dest)?;
                     }
-                    let usize_layout = self.layout_of(self.tcx.types.usize)?;
-                    let isize_layout = self.layout_of(self.tcx.types.isize)?;
-                    let a_offset = ImmTy::from_uint(a_offset.bytes(), usize_layout);
-                    let b_offset = ImmTy::from_uint(b_offset.bytes(), usize_layout);
-                    let (val, _overflowed, _ty) =
-                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
-                    let pointee_layout = self.layout_of(substs.type_at(0))?;
-                    let val = ImmTy::from_scalar(val, isize_layout);
-                    let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout);
-                    self.exact_div(&val, &size, dest)?;
                 }
             }
 

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -388,9 +388,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                 CheckInAllocMsg::DerefTest | CheckInAllocMsg::MemoryAccessTest => {
                     AllocCheck::Dereferenceable
                 }
-                CheckInAllocMsg::PointerArithmeticTest | CheckInAllocMsg::InboundsTest => {
-                    AllocCheck::Live
-                }
+                CheckInAllocMsg::PointerArithmeticTest
+                | CheckInAllocMsg::OffsetFromTest
+                | CheckInAllocMsg::InboundsTest => AllocCheck::Live,
             };
             let (size, align) = self.get_size_and_align(alloc_id, check)?;
             Ok((size, align, ()))

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

@@ -184,6 +184,8 @@ pub enum CheckInAllocMsg {
     MemoryAccessTest,
     /// We are doing pointer arithmetic.
     PointerArithmeticTest,
+    /// We are doing pointer offset_from.
+    OffsetFromTest,
     /// None of the above -- generic/unspecific inbounds test.
     InboundsTest,
 }
@@ -199,6 +201,7 @@ impl fmt::Display for CheckInAllocMsg {
                 CheckInAllocMsg::DerefTest => "dereferencing pointer failed: ",
                 CheckInAllocMsg::MemoryAccessTest => "memory access failed: ",
                 CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic failed: ",
+                CheckInAllocMsg::OffsetFromTest => "out-of-bounds offset_from: ",
                 CheckInAllocMsg::InboundsTest => "",
             }
         )
@@ -358,6 +361,9 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
             DanglingIntPointer(0, CheckInAllocMsg::InboundsTest) => {
                 write!(f, "null pointer is not a valid pointer for this operation")
             }
+            DanglingIntPointer(0, msg) => {
+                write!(f, "{}null pointer is not a valid pointer", msg)
+            }
             DanglingIntPointer(i, msg) => {
                 write!(f, "{}0x{:x} is not a valid pointer", msg, i)
             }

src/test/ui/consts/offset_from_ub.rs

@@ -1,4 +1,4 @@
-#![feature(const_ptr_offset_from)]
+#![feature(const_ptr_offset_from, const_ptr_offset)]
 #![feature(core_intrinsics)]
 
 use std::intrinsics::ptr_offset_from;
@@ -44,4 +44,30 @@ pub const DIFFERENT_INT: isize = { // offset_from with two different integers: l
     //~| 0x10 is not a valid pointer
 };
 
+const OUT_OF_BOUNDS_1: isize = {
+    let start_ptr = &4 as *const _ as *const u8;
+    let length = 10;
+    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
+};
+
+const OUT_OF_BOUNDS_2: isize = {
+    let start_ptr = &4 as *const _ as *const u8;
+    let length = 10;
+    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
+};
+
+const OUT_OF_BOUNDS_SAME: isize = {
+    let start_ptr = &4 as *const _ as *const u8;
+    let length = 10;
+    let end_ptr = (start_ptr).wrapping_add(length);
+    unsafe { ptr_offset_from(end_ptr, end_ptr) } //~ERROR evaluation of constant value failed
+    //~| pointer at offset 10 is out-of-bounds
+};
+
 fn main() {}

src/test/ui/consts/offset_from_ub.stderr

@@ -10,7 +10,7 @@ error[E0080]: evaluation of constant value failed
 LL |         unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  0x2a is not a valid pointer
+   |                  out-of-bounds offset_from: 0x2a is not a valid pointer
    |                  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
@@ -28,14 +28,32 @@ error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:36:14
    |
 LL |     unsafe { ptr_offset_from(ptr, ptr) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^ null pointer is not a valid pointer for this operation
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: null pointer is not a valid pointer
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/offset_from_ub.rs:43:14
    |
 LL |     unsafe { ptr_offset_from(ptr2, ptr1) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 0x10 is not a valid pointer
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x10 is not a valid pointer
 
-error: aborting due to 5 previous errors
+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: alloc18 has size 4, so pointer at offset 10 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: alloc21 has size 4, so pointer at offset 10 is out-of-bounds
+
+error[E0080]: evaluation of constant value failed
+  --> $DIR/offset_from_ub.rs:69: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: aborting due to 8 previous errors
 
 For more information about this error, try `rustc --explain E0080`.

src/test/ui/consts/offset_ub.stderr

@@ -144,7 +144,7 @@ error[E0080]: evaluation of constant value failed
 LL |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
-   |                  pointer arithmetic failed: 0x0 is not a valid pointer
+   |                  pointer arithmetic failed: null pointer is not a valid pointer
    |                  inside `ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
   ::: $DIR/offset_ub.rs:22:50