Rollup merge of rust-lang#124921 - RalfJung:offset-from-same-addr, r=…

…oli-obk

offset_from: always allow pointers to point to the same address

This PR implements the last remaining part of the t-opsem consensus in rust-lang/unsafe-code-guidelines#472: always permits offset_from when both pointers have the same address, no matter how they are computed. This is required to achieve *provenance monotonicity*.

Tracking issue: rust-lang#117945

### What is provenance monotonicity and why does it matter?

Provenance monotonicity is the property that adding arbitrary provenance to any no-provenance pointer must never make the program UB. More specifically, in the program state, data in memory is stored as a sequence of [abstract bytes](https://rust-lang.github.io/unsafe-code-guidelines/glossary.html#abstract-byte), where each byte can optionally carry provenance. When a pointer is stored in memory, all of the bytes it is stored in carry that provenance. Provenance monotonicity means: if we take some byte that does not have provenance, and give it some arbitrary provenance, then that cannot change program behavior or introduce UB into a UB-free program.

We care about provenance monotonicity because we want to allow the optimizer to remove provenance-stripping operations. Removing a provenance-stripping operation effectively means the program after the optimization has provenance where the program before the optimization did not -- since the provenance removal does not happen in the optimized program. IOW, the compiler transformation added provenance to previously provenance-free bytes. This is exactly what provenance monotonicity lets us do.

We care about removing provenance-stripping operations because `*ptr = *ptr` is, in general, (likely) a provenance-stripping operation. Specifically, consider `ptr: *mut usize` (or any integer type), and imagine the data at `*ptr` is actually a pointer (i.e., we are type-punning between pointers and integers). Then `*ptr` on the right-hand side evaluates to the data in memory *without* any provenance (because [integers do not have provenance](https://rust-lang.github.io/rfcs/3559-rust-has-provenance.html#integers-do-not-have-provenance)). Storing that back to `*ptr` means that the abstract bytes `ptr` points to are the same as before, except their provenance is now gone. This makes  `*ptr = *ptr`  a provenance-stripping operation  (Here we assume `*ptr` is fully initialized. If it is not initialized, evaluating `*ptr` to a value is UB, so removing `*ptr = *ptr` is trivially correct.)

### What does `offset_from` have to do with provenance monotonicity?

With `ptr = without_provenance(N)`, `ptr.offset_from(ptr)` is always well-defined and returns 0. By provenance monotonicity, I can now add provenance to the two arguments of `offset_from` and it must still be well-defined. Crucially, I can add *different* provenance to the two arguments, and it must still be well-defined. In other words, this must always be allowed: `ptr1.with_addr(N).offset_from(ptr2.with_addr(N))` (and it returns 0). But the current spec for `offset_from` says that the two pointers must either both be derived from an integer or both be derived from the same allocation, which is not in general true for arbitrary `ptr1`, `ptr2`.

To obtain provenance monotonicity, this PR hence changes the spec for offset_from to say that if both pointers have the same address, the function is always well-defined.

### What further consequences does this have?

It means the compiler can no longer transform `end2 = begin.offset(end.offset_from(begin))` into `end2 = end`. However, it can still be transformed into `end2 = begin.with_addr(end.addr())`, which later parts of the backend (when provenance has been erased) can trivially turn into `end2 = end`.

The only alternative I am aware of is a fundamentally different handling of zero-sized accesses, where a "no provenance" pointer is not allowed to do zero-sized accesses and instead we have a special provenance that indicates "may be used for zero-sized accesses (and nothing else)". `offset` and `offset_from` would then always be UB on a "no provenance" pointer, and permit zero-sized offsets on a "zero-sized provenance" pointer. This achieves provenance monotonicity. That is, however, a breaking change as it contradicts what we landed in rust-lang#117329. It's also a whole bunch of extra UB, which doesn't seem worth it just to achieve that transformation.

### What about the backend?

LLVM currently doesn't have an intrinsic for pointer difference, so we anyway cast to integer and subtract there. That's never UB so it is compatible with any relaxation we may want to apply.

If LLVM gets a `ptrsub` in the future, then plausibly it will be consistent with `ptradd` and [consider two equal pointers to be inbounds](rust-lang#124921 (comment)).

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -20,7 +20,7 @@ use super::{
     err_inval, err_ub_custom, err_unsup_format, memory::MemoryKind, throw_inval, throw_ub_custom,
     throw_ub_format, util::ensure_monomorphic_enough, Allocation, CheckInAllocMsg, ConstAllocation,
     GlobalId, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, Pointer, PointerArithmetic,
-    Scalar,
+    Provenance, Scalar,
 };
 
 use crate::fluent_generated as fluent;
@@ -259,25 +259,28 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                             // This will always return 0.
                             (a, b)
                         }
-                        (Err(_), _) | (_, Err(_)) => {
-                            // 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.
+                        _ if M::Provenance::OFFSET_IS_ADDR && a.addr() == b.addr() => {
+                            // At least one of the pointers has provenance, but they also point to
+                            // the same address so it doesn't matter; this is fine. `(0, 0)` means
+                            // we pass all the checks below and return 0.
+                            (0, 0)
+                        }
+                        // From here onwards, the pointers are definitely for different addresses
+                        // (or we can't determine their absolute address).
+                        (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _)))
+                            if a_alloc_id == b_alloc_id =>
+                        {
+                            // Found allocation for both, and it's the same.
+                            // Use these offsets for distance calculation.
+                            (a_offset.bytes(), b_offset.bytes())
+                        }
+                        _ => {
+                            // Not into the same allocation -- this is UB.
                             throw_ub_custom!(
                                 fluent::const_eval_offset_from_different_allocations,
                                 name = 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_custom!(
-                                    fluent::const_eval_offset_from_different_allocations,
-                                    name = intrinsic_name,
-                                );
-                            }
-                            // Use these offsets for distance calculation.
-                            (a_offset.bytes(), b_offset.bytes())
-                        }
                     };
 
                 // Compute distance.

library/core/src/ptr/const_ptr.rs

@@ -604,9 +604,9 @@ impl<T: ?Sized> *const T {
     ///
     /// * `self` and `origin` must either
     ///
+    ///   * point to the same address, or
     ///   * both be *derived from* a pointer to the same [allocated object], and the memory range between
-    ///     the two pointers must be either empty or in bounds of that object. (See below for an example.)
-    ///   * or both be derived from an integer literal/constant, and point to the same address.
+    ///     the two pointers must be in bounds of that object. (See below for an example.)
     ///
     /// * The distance between the pointers, in bytes, must be an exact multiple
     ///   of the size of `T`.
@@ -653,14 +653,14 @@ impl<T: ?Sized> *const T {
     /// let ptr1 = Box::into_raw(Box::new(0u8)) as *const u8;
     /// let ptr2 = Box::into_raw(Box::new(1u8)) as *const u8;
     /// let diff = (ptr2 as isize).wrapping_sub(ptr1 as isize);
-    /// // Make ptr2_other an "alias" of ptr2, but derived from ptr1.
-    /// let ptr2_other = (ptr1 as *const u8).wrapping_offset(diff);
+    /// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1.
+    /// let ptr2_other = (ptr1 as *const u8).wrapping_offset(diff).wrapping_offset(1);
     /// assert_eq!(ptr2 as usize, ptr2_other as usize);
     /// // Since ptr2_other and ptr2 are derived from pointers to different objects,
     /// // computing their offset is undefined behavior, even though
-    /// // they point to the same address!
+    /// // they point to addresses that are in-bounds of the same object!
     /// unsafe {
-    ///     let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior
+    ///     let one = ptr2_other.offset_from(ptr2); // Undefined Behavior! ⚠️
     /// }
     /// ```
     #[stable(feature = "ptr_offset_from", since = "1.47.0")]

library/core/src/ptr/mut_ptr.rs

@@ -829,9 +829,9 @@ impl<T: ?Sized> *mut T {
     ///
     /// * `self` and `origin` must either
     ///
+    ///   * point to the same address, or
     ///   * both be *derived from* a pointer to the same [allocated object], and the memory range between
-    ///     the two pointers must be either empty or in bounds of that object. (See below for an example.)
-    ///   * or both be derived from an integer literal/constant, and point to the same address.
+    ///     the two pointers must be in bounds of that object. (See below for an example.)
     ///
     /// * The distance between the pointers, in bytes, must be an exact multiple
     ///   of the size of `T`.
@@ -878,14 +878,14 @@ impl<T: ?Sized> *mut T {
     /// let ptr1 = Box::into_raw(Box::new(0u8));
     /// let ptr2 = Box::into_raw(Box::new(1u8));
     /// let diff = (ptr2 as isize).wrapping_sub(ptr1 as isize);
-    /// // Make ptr2_other an "alias" of ptr2, but derived from ptr1.
-    /// let ptr2_other = (ptr1 as *mut u8).wrapping_offset(diff);
+    /// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1.
+    /// let ptr2_other = (ptr1 as *mut u8).wrapping_offset(diff).wrapping_offset(1);
     /// assert_eq!(ptr2 as usize, ptr2_other as usize);
     /// // Since ptr2_other and ptr2 are derived from pointers to different objects,
     /// // computing their offset is undefined behavior, even though
-    /// // they point to the same address!
+    /// // they point to addresses that are in-bounds of the same object!
     /// unsafe {
-    ///     let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior
+    ///     let one = ptr2_other.offset_from(ptr2); // Undefined Behavior! ⚠️
     /// }
     /// ```
     #[stable(feature = "ptr_offset_from", since = "1.47.0")]

library/core/src/ptr/non_null.rs

@@ -735,9 +735,9 @@ impl<T: ?Sized> NonNull<T> {
     ///
     /// * `self` and `origin` must either
     ///
+    ///   * point to the same address, or
     ///   * both be *derived from* a pointer to the same [allocated object], and the memory range between
-    ///     the two pointers must be either empty or in bounds of that object. (See below for an example.)
-    ///   * or both be derived from an integer literal/constant, and point to the same address.
+    ///     the two pointers must be in bounds of that object. (See below for an example.)
     ///
     /// * The distance between the pointers, in bytes, must be an exact multiple
     ///   of the size of `T`.
@@ -789,14 +789,15 @@ impl<T: ?Sized> NonNull<T> {
     /// let ptr1 = NonNull::new(Box::into_raw(Box::new(0u8))).unwrap();
     /// let ptr2 = NonNull::new(Box::into_raw(Box::new(1u8))).unwrap();
     /// let diff = (ptr2.addr().get() as isize).wrapping_sub(ptr1.addr().get() as isize);
-    /// // Make ptr2_other an "alias" of ptr2, but derived from ptr1.
-    /// let ptr2_other = NonNull::new(ptr1.as_ptr().wrapping_byte_offset(diff)).unwrap();
+    /// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1.
+    /// let diff_plus_1 = diff.wrapping_add(1);
+    /// let ptr2_other = NonNull::new(ptr1.as_ptr().wrapping_byte_offset(diff_plus_1)).unwrap();
     /// assert_eq!(ptr2.addr(), ptr2_other.addr());
     /// // Since ptr2_other and ptr2 are derived from pointers to different objects,
     /// // computing their offset is undefined behavior, even though
-    /// // they point to the same address!
+    /// // they point to addresses that are in-bounds of the same object!
     ///
-    /// let zero = unsafe { ptr2_other.offset_from(ptr2) }; // Undefined Behavior
+    /// let one = unsafe { ptr2_other.offset_from(ptr2) }; // Undefined Behavior! ⚠️
     /// ```
     #[inline]
     #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces

src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs

@@ -39,8 +39,6 @@ fn test_ptr(ptr: *mut ()) {
         // Distance.
         let ptr = ptr.cast::<i32>();
         ptr.offset_from(ptr);
-        /*
-        FIXME: this is disabled for now as these cases are not yet allowed.
         // Distance from other "bad" pointers that have the same address, but different provenance. Some
         // of this is library UB, but we don't want it to be language UB since that would violate
         // provenance monotonicity: if we allow computing the distance between two ptrs with no
@@ -54,6 +52,5 @@ fn test_ptr(ptr: *mut ()) {
         // - Distance from use-after-free pointer
         drop(b);
         ptr.offset_from(other_ptr.with_addr(ptr.addr()));
-        */
     }
 }

tests/ui/consts/offset_from_ub.rs

@@ -32,12 +32,6 @@ pub const NOT_MULTIPLE_OF_SIZE: isize = {
     //~| 1_isize cannot be divided by 2_isize without remainder
 };
 
-pub const OFFSET_FROM_NULL: isize = {
-    let ptr = 0 as *const u8;
-    // Null isn't special for zero-sized "accesses" (i.e., the range between the two pointers)
-    unsafe { ptr_offset_from(ptr, ptr) }
-};
-
 pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC
     let ptr1 = 8 as *const u8;
     let ptr2 = 16 as *const u8;
@@ -63,14 +57,6 @@ const OUT_OF_BOUNDS_2: isize = {
     //~| pointer to 10 bytes starting at offset 0 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);
-    // Out-of-bounds is fine as long as the range between the pointers is empty.
-    unsafe { ptr_offset_from(end_ptr, end_ptr) }
-};
-
 pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
     let uninit = std::mem::MaybeUninit::<Struct>::uninit();
     let base_ptr: *const Struct = &uninit as *const _ as *const Struct;
@@ -130,4 +116,23 @@ pub const OFFSET_VERY_FAR2: isize = {
     //~^ inside
 };
 
+// If the pointers are the same, OOB/null/UAF is fine.
+pub const OFFSET_FROM_NULL_SAME: isize = {
+    let ptr = 0 as *const u8;
+    unsafe { ptr_offset_from(ptr, ptr) }
+};
+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) }
+};
+const UAF_SAME: isize = {
+    let uaf_ptr = {
+        let x = 0;
+        &x as *const i32
+    };
+    unsafe { ptr_offset_from(uaf_ptr, uaf_ptr) }
+};
+
 fn main() {}

tests/ui/consts/offset_from_ub.stderr

@@ -24,55 +24,55 @@ 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:44:14
+  --> $DIR/offset_from_ub.rs:38:14
    |
 LL |     unsafe { ptr_offset_from(ptr2, ptr1) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation)
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:53:14
+  --> $DIR/offset_from_ub.rs:47:14
    |
 LL |     unsafe { ptr_offset_from(end_ptr, start_ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: ALLOC0 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:62:14
+  --> $DIR/offset_from_ub.rs:56:14
    |
 LL |     unsafe { ptr_offset_from(start_ptr, end_ptr) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: ALLOC1 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:79:14
+  --> $DIR/offset_from_ub.rs:65:14
    |
 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
+  --> $DIR/offset_from_ub.rs:72: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
+  --> $DIR/offset_from_ub.rs:78: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:100:14
+  --> $DIR/offset_from_ub.rs:86: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:107:14
+  --> $DIR/offset_from_ub.rs:93: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
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/offset_from_ub.rs:114:14
+  --> $DIR/offset_from_ub.rs:100:14
    |
 LL |     unsafe { ptr_offset_from_unsigned(ptr2, ptr1) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer is too far ahead of second
@@ -85,7 +85,7 @@ error[E0080]: evaluation of constant value failed
 note: inside `std::ptr::const_ptr::<impl *const u8>::offset_from`
   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
 note: inside `OFFSET_VERY_FAR1`
-  --> $DIR/offset_from_ub.rs:123:14
+  --> $DIR/offset_from_ub.rs:109:14
    |
 LL |     unsafe { ptr2.offset_from(ptr1) }
    |              ^^^^^^^^^^^^^^^^^^^^^^
@@ -98,7 +98,7 @@ error[E0080]: evaluation of constant value failed
 note: inside `std::ptr::const_ptr::<impl *const u8>::offset_from`
   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
 note: inside `OFFSET_VERY_FAR2`
-  --> $DIR/offset_from_ub.rs:129:14
+  --> $DIR/offset_from_ub.rs:115:14
    |
 LL |     unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^