Auto merge of rust-lang#74238 - RalfJung:offset_from, r=oli-obk

stabilize ptr_offset_from This stabilizes ptr::offset_from, and closes rust-lang#41079. It also removes the deprecated `wrapping_offset_from`. This function was deprecated 19 days ago and was never stable; given an FCP of 10 days and some waiting time until FCP starts, that leaves at least a month between deprecation and removal which I think is fine for a nightly-only API. Regarding the open questions in rust-lang#41079: * Should offset_from abort instead of panic on ZSTs? -- As far as I know, there is no precedent for such aborts. We could, however, declare this UB. Given that the size is always known statically and the check thus rather cheap, UB seems excessive. * Should there be more methods like this with different restrictions (to allow nuw/nsw, perhaps) or that return usize (like how isize-taking offset is more conveniently done with usize-taking add these days)? -- No reason to block stabilization on that, we can always add such methods later. Also nominating the lang team because this exposes an intrinsic. The stabilized method is best described [by its doc-comment](https://github.com/RalfJung/rust/blob/56d4b2d69abb93e4f0ca79471deca7aaaaeca214/src/libcore/ptr/const_ptr.rs#L227). The documentation forgot to mention the requirement that both pointers must "have the same provenance", aka "be derived from pointers to the same allocation", which I am adding in this PR. This is a precondition that [Miri already implements](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=a3b9d0a07a01321f5202cd99e9613480) and that, should LLVM ever obtain a `psub` operation to subtract pointers, will likely be required for that operation (following the semantics in [this paper](https://people.mpi-sws.org/~jung/twinsem/twinsem.pdf)).
m-ou-se · Aug 23, 2020 · 9d606d9 · 9d606d9
2 parents b88434e + 4129e07
commit 9d606d9
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 141 deletions.
diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs
@@ -113,7 +113,6 @@
 #![feature(or_patterns)]
 #![feature(pattern)]
 #![feature(ptr_internals)]
-#![feature(ptr_offset_from)]
 #![feature(raw_ref_op)]
 #![feature(rustc_attrs)]
 #![feature(receiver_trait)]

diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs
@@ -240,8 +240,8 @@ impl<T: ?Sized> *const T {
     /// different allocated object. Note that in Rust,
     /// every (stack-allocated) variable is considered a separate allocated object.
     ///
-    /// In other words, `x.wrapping_offset(y.wrapping_offset_from(x))` is
-    /// *not* the same as `y`, and dereferencing it is undefined behavior
+    /// In other words, `x.wrapping_offset((y as usize).wrapping_sub(x as usize) / size_of::<T>())`
+    /// is *not* the same as `y`, and dereferencing it is undefined behavior
     /// unless `x` and `y` point into the same allocated object.
     ///
     /// Compared to [`offset`], this method basically delays the requirement of staying
@@ -292,7 +292,6 @@ impl<T: ?Sized> *const T {
     /// This function is the inverse of [`offset`].
     ///
     /// [`offset`]: #method.offset
-    /// [`wrapping_offset_from`]: #method.wrapping_offset_from
     ///
     /// # Safety
     ///
@@ -303,6 +302,9 @@ impl<T: ?Sized> *const T {
     ///   byte past the end of the same allocated object. Note that in Rust,
     ///   every (stack-allocated) variable is considered a separate allocated object.
     ///
+    /// * Both pointers must be *derived from* a pointer to the same object.
+    ///   (See below for an example.)
+    ///
     /// * The distance between the pointers, **in bytes**, cannot overflow an `isize`.
     ///
     /// * The distance between the pointers, in bytes, must be an exact multiple
@@ -323,10 +325,6 @@ impl<T: ?Sized> *const T {
     /// Extension. As such, memory acquired directly from allocators or memory
     /// mapped files *may* be too large to handle with this function.
     ///
-    /// Consider using [`wrapping_offset_from`] instead if these constraints are
-    /// difficult to satisfy. The only advantage of this method is that it
-    /// enables more aggressive compiler optimizations.
-    ///
     /// # Panics
     ///
     /// This function panics if `T` is a Zero-Sized Type ("ZST").
@@ -336,8 +334,6 @@ impl<T: ?Sized> *const T {
     /// Basic usage:
     ///
     /// ```
-    /// #![feature(ptr_offset_from)]
-    ///
     /// let a = [0; 5];
     /// let ptr1: *const i32 = &a[1];
     /// let ptr2: *const i32 = &a[3];
@@ -348,7 +344,24 @@ impl<T: ?Sized> *const T {
     ///     assert_eq!(ptr2.offset(-2), ptr1);
     /// }
     /// ```
-    #[unstable(feature = "ptr_offset_from", issue = "41079")]
+    ///
+    /// *Incorrect* usage:
+    ///
+    /// ```rust,no_run
+    /// 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);
+    /// 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!
+    /// unsafe {
+    ///     let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior
+    /// }
+    /// ```
+    #[stable(feature = "ptr_offset_from", since = "1.47.0")]
     #[rustc_const_unstable(feature = "const_ptr_offset_from", issue = "41079")]
     #[inline]
     pub const unsafe fn offset_from(self, origin: *const T) -> isize
@@ -423,59 +436,6 @@ impl<T: ?Sized> *const T {
         intrinsics::ptr_guaranteed_ne(self, other)
     }
 
-    /// Calculates the distance between two pointers. The returned value is in
-    /// units of T: the distance in bytes is divided by `mem::size_of::<T>()`.
-    ///
-    /// If the address different between the two pointers is not a multiple of
-    /// `mem::size_of::<T>()` then the result of the division is rounded towards
-    /// zero.
-    ///
-    /// Though this method is safe for any two pointers, note that its result
-    /// will be mostly useless if the two pointers aren't into the same allocated
-    /// object, for example if they point to two different local variables.
-    ///
-    /// # Panics
-    ///
-    /// This function panics if `T` is a zero-sized type.
-    ///
-    /// # Examples
-    ///
-    /// Basic usage:
-    ///
-    /// ```
-    /// #![feature(ptr_wrapping_offset_from)]
-    ///
-    /// let a = [0; 5];
-    /// let ptr1: *const i32 = &a[1];
-    /// let ptr2: *const i32 = &a[3];
-    /// assert_eq!(ptr2.wrapping_offset_from(ptr1), 2);
-    /// assert_eq!(ptr1.wrapping_offset_from(ptr2), -2);
-    /// assert_eq!(ptr1.wrapping_offset(2), ptr2);
-    /// assert_eq!(ptr2.wrapping_offset(-2), ptr1);
-    ///
-    /// let ptr1: *const i32 = 3 as _;
-    /// let ptr2: *const i32 = 13 as _;
-    /// assert_eq!(ptr2.wrapping_offset_from(ptr1), 2);
-    /// ```
-    #[unstable(feature = "ptr_wrapping_offset_from", issue = "41079")]
-    #[rustc_deprecated(
-        since = "1.46.0",
-        reason = "Pointer distances across allocation \
-        boundaries are not typically meaningful. \
-        Use integer subtraction if you really need this."
-    )]
-    #[inline]
-    pub fn wrapping_offset_from(self, origin: *const T) -> isize
-    where
-        T: Sized,
-    {
-        let pointee_size = mem::size_of::<T>();
-        assert!(0 < pointee_size && pointee_size <= isize::MAX as usize);
-
-        let d = isize::wrapping_sub(self as _, origin as _);
-        d.wrapping_div(pointee_size as _)
-    }
-
     /// Calculates the offset from a pointer (convenience for `.offset(count as isize)`).
     ///
     /// `count` is in units of T; e.g., a `count` of 3 represents a pointer

diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs
@@ -246,8 +246,8 @@ impl<T: ?Sized> *mut T {
     /// different allocated object. Note that in Rust,
     /// every (stack-allocated) variable is considered a separate allocated object.
     ///
-    /// In other words, `x.wrapping_offset(y.wrapping_offset_from(x))` is
-    /// *not* the same as `y`, and dereferencing it is undefined behavior
+    /// In other words, `x.wrapping_offset((y as usize).wrapping_sub(x as usize) / size_of::<T>())`
+    /// is *not* the same as `y`, and dereferencing it is undefined behavior
     /// unless `x` and `y` point into the same allocated object.
     ///
     /// Compared to [`offset`], this method basically delays the requirement of staying
@@ -463,7 +463,6 @@ impl<T: ?Sized> *mut T {
     /// This function is the inverse of [`offset`].
     ///
     /// [`offset`]: #method.offset-1
-    /// [`wrapping_offset_from`]: #method.wrapping_offset_from-1
     ///
     /// # Safety
     ///
@@ -474,6 +473,9 @@ impl<T: ?Sized> *mut T {
     ///   byte past the end of the same allocated object. Note that in Rust,
     ///   every (stack-allocated) variable is considered a separate allocated object.
     ///
+    /// * Both pointers must be *derived from* a pointer to the same object.
+    ///   (See below for an example.)
+    ///
     /// * The distance between the pointers, **in bytes**, cannot overflow an `isize`.
     ///
     /// * The distance between the pointers, in bytes, must be an exact multiple
@@ -494,10 +496,6 @@ impl<T: ?Sized> *mut T {
     /// Extension. As such, memory acquired directly from allocators or memory
     /// mapped files *may* be too large to handle with this function.
     ///
-    /// Consider using [`wrapping_offset_from`] instead if these constraints are
-    /// difficult to satisfy. The only advantage of this method is that it
-    /// enables more aggressive compiler optimizations.
-    ///
     /// # Panics
     ///
     /// This function panics if `T` is a Zero-Sized Type ("ZST").
@@ -507,8 +505,6 @@ impl<T: ?Sized> *mut T {
     /// Basic usage:
     ///
     /// ```
-    /// #![feature(ptr_offset_from)]
-    ///
     /// let mut a = [0; 5];
     /// let ptr1: *mut i32 = &mut a[1];
     /// let ptr2: *mut i32 = &mut a[3];
@@ -519,7 +515,24 @@ impl<T: ?Sized> *mut T {
     ///     assert_eq!(ptr2.offset(-2), ptr1);
     /// }
     /// ```
-    #[unstable(feature = "ptr_offset_from", issue = "41079")]
+    ///
+    /// *Incorrect* usage:
+    ///
+    /// ```rust,no_run
+    /// 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);
+    /// 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!
+    /// unsafe {
+    ///     let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior
+    /// }
+    /// ```
+    #[stable(feature = "ptr_offset_from", since = "1.47.0")]
     #[rustc_const_unstable(feature = "const_ptr_offset_from", issue = "41079")]
     #[inline]
     pub const unsafe fn offset_from(self, origin: *const T) -> isize
@@ -530,56 +543,6 @@ impl<T: ?Sized> *mut T {
         unsafe { (self as *const T).offset_from(origin) }
     }
 
-    /// Calculates the distance between two pointers. The returned value is in
-    /// units of T: the distance in bytes is divided by `mem::size_of::<T>()`.
-    ///
-    /// If the address different between the two pointers is not a multiple of
-    /// `mem::size_of::<T>()` then the result of the division is rounded towards
-    /// zero.
-    ///
-    /// Though this method is safe for any two pointers, note that its result
-    /// will be mostly useless if the two pointers aren't into the same allocated
-    /// object, for example if they point to two different local variables.
-    ///
-    /// # Panics
-    ///
-    /// This function panics if `T` is a zero-sized type.
-    ///
-    /// # Examples
-    ///
-    /// Basic usage:
-    ///
-    /// ```
-    /// #![feature(ptr_wrapping_offset_from)]
-    ///
-    /// let mut a = [0; 5];
-    /// let ptr1: *mut i32 = &mut a[1];
-    /// let ptr2: *mut i32 = &mut a[3];
-    /// assert_eq!(ptr2.wrapping_offset_from(ptr1), 2);
-    /// assert_eq!(ptr1.wrapping_offset_from(ptr2), -2);
-    /// assert_eq!(ptr1.wrapping_offset(2), ptr2);
-    /// assert_eq!(ptr2.wrapping_offset(-2), ptr1);
-    ///
-    /// let ptr1: *mut i32 = 3 as _;
-    /// let ptr2: *mut i32 = 13 as _;
-    /// assert_eq!(ptr2.wrapping_offset_from(ptr1), 2);
-    /// ```
-    #[unstable(feature = "ptr_wrapping_offset_from", issue = "41079")]
-    #[rustc_deprecated(
-        since = "1.46.0",
-        reason = "Pointer distances across allocation \
-        boundaries are not typically meaningful. \
-        Use integer subtraction if you really need this."
-    )]
-    #[inline]
-    pub fn wrapping_offset_from(self, origin: *const T) -> isize
-    where
-        T: Sized,
-    {
-        #[allow(deprecated_in_future, deprecated)]
-        (self as *const T).wrapping_offset_from(origin)
-    }
-
     /// Calculates the offset from a pointer (convenience for `.offset(count as isize)`).
     ///
     /// `count` is in units of T; e.g., a `count` of 3 represents a pointer

diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs
@@ -208,7 +208,7 @@
 #![cfg_attr(test, feature(print_internals, set_stdio, update_panic_count))]
 #![cfg_attr(
     all(target_vendor = "fortanix", target_env = "sgx"),
-    feature(slice_index_methods, coerce_unsized, sgx_platform, ptr_wrapping_offset_from)
+    feature(slice_index_methods, coerce_unsized, sgx_platform)
 )]
 #![cfg_attr(all(test, target_vendor = "fortanix", target_env = "sgx"), feature(fixed_size_array))]
 // std is implemented with unstable features, many of which are internal

diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
@@ -9,7 +9,6 @@
 #![feature(nll)]
 #![feature(or_patterns)]
 #![feature(test)]
-#![feature(ptr_offset_from)]
 #![feature(crate_visibility_modifier)]
 #![feature(never_type)]
 #![feature(once_cell)]

diff --git a/src/test/ui/consts/offset.rs b/src/test/ui/consts/offset.rs
@@ -1,7 +1,6 @@
 // run-pass
 #![feature(const_ptr_offset)]
 #![feature(const_ptr_offset_from)]
-#![feature(ptr_offset_from)]
 use std::ptr;
 
 #[repr(C)]

diff --git a/src/test/ui/consts/offset_from.rs b/src/test/ui/consts/offset_from.rs
@@ -2,7 +2,6 @@
 
 #![feature(const_raw_ptr_deref)]
 #![feature(const_ptr_offset_from)]
-#![feature(ptr_offset_from)]
 
 struct Struct {
     field: (),

diff --git a/src/test/ui/consts/offset_from_ub.rs b/src/test/ui/consts/offset_from_ub.rs
@@ -1,6 +1,5 @@
 #![feature(const_raw_ptr_deref)]
 #![feature(const_ptr_offset_from)]
-#![feature(ptr_offset_from)]
 
 #[repr(C)]
 struct Struct {

diff --git a/src/test/ui/consts/offset_from_ub.stderr b/src/test/ui/consts/offset_from_ub.stderr
@@ -6,9 +6,9 @@ LL |           unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                    |
    |                    ptr_offset_from cannot compute offset of pointers into different allocations.
    |                    inside `std::ptr::const_ptr::<impl *const Struct>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   |                    inside `DIFFERENT_ALLOC` at $DIR/offset_from_ub.rs:17:27
+   |                    inside `DIFFERENT_ALLOC` at $DIR/offset_from_ub.rs:16:27
    | 
-  ::: $DIR/offset_from_ub.rs:11:1
+  ::: $DIR/offset_from_ub.rs:10:1
    |
 LL | / pub const DIFFERENT_ALLOC: usize = {
 LL | |
@@ -29,9 +29,9 @@ LL |           unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                    |
    |                    unable to turn bytes into a pointer
    |                    inside `std::ptr::const_ptr::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   |                    inside `NOT_PTR` at $DIR/offset_from_ub.rs:23:14
+   |                    inside `NOT_PTR` at $DIR/offset_from_ub.rs:22:14
    | 
-  ::: $DIR/offset_from_ub.rs:21:1
+  ::: $DIR/offset_from_ub.rs:20:1
    |
 LL | / pub const NOT_PTR: usize = {
 LL | |
@@ -47,9 +47,9 @@ LL |           unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                    |
    |                    exact_div: 1_isize cannot be divided by 2_isize without remainder
    |                    inside `std::ptr::const_ptr::<impl *const u16>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   |                    inside `NOT_MULTIPLE_OF_SIZE` at $DIR/offset_from_ub.rs:31:14
+   |                    inside `NOT_MULTIPLE_OF_SIZE` at $DIR/offset_from_ub.rs:30:14
    | 
-  ::: $DIR/offset_from_ub.rs:26:1
+  ::: $DIR/offset_from_ub.rs:25:1
    |
 LL | / pub const NOT_MULTIPLE_OF_SIZE: isize = {
 LL | |
@@ -68,9 +68,9 @@ LL |           unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                    |
    |                    inbounds test failed: 0x0 is not a valid pointer
    |                    inside `std::ptr::const_ptr::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   |                    inside `OFFSET_FROM_NULL` at $DIR/offset_from_ub.rs:37:14
+   |                    inside `OFFSET_FROM_NULL` at $DIR/offset_from_ub.rs:36:14
    | 
-  ::: $DIR/offset_from_ub.rs:34:1
+  ::: $DIR/offset_from_ub.rs:33:1
    |
 LL | / pub const OFFSET_FROM_NULL: isize = {
 LL | |
@@ -87,9 +87,9 @@ LL |           unsafe { intrinsics::ptr_offset_from(self, origin) }
    |                    |
    |                    unable to turn bytes into a pointer
    |                    inside `std::ptr::const_ptr::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   |                    inside `DIFFERENT_INT` at $DIR/offset_from_ub.rs:44:14
+   |                    inside `DIFFERENT_INT` at $DIR/offset_from_ub.rs:43:14
    | 
-  ::: $DIR/offset_from_ub.rs:40:1
+  ::: $DIR/offset_from_ub.rs:39:1
    |
 LL | / pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC
 LL | |

diff --git a/src/test/ui/offset_from.rs b/src/test/ui/offset_from.rs
@@ -1,7 +1,5 @@
 // run-pass
 
-#![feature(ptr_offset_from)]
-
 fn main() {
     let mut a = [0; 5];
     let ptr1: *mut i32 = &mut a[1];