Skip to content

Commit

Permalink
Auto merge of #74238 - RalfJung:offset_from, r=oli-obk
Browse files Browse the repository at this point in the history
stabilize ptr_offset_from

This stabilizes ptr::offset_from, and closes #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 #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)).
  • Loading branch information
bors committed Aug 23, 2020
2 parents b88434e + 4129e07 commit 9d606d9
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 141 deletions.
1 change: 0 additions & 1 deletion library/alloc/src/lib.rs
Expand Up @@ -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)]
Expand Down
86 changes: 23 additions & 63 deletions library/core/src/ptr/const_ptr.rs
Expand Up @@ -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
Expand Down Expand Up @@ -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
///
Expand All @@ -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
Expand All @@ -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").
Expand All @@ -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];
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
83 changes: 23 additions & 60 deletions library/core/src/ptr/mut_ptr.rs
Expand Up @@ -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
Expand Down Expand Up @@ -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
///
Expand All @@ -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
Expand All @@ -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").
Expand All @@ -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];
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/lib.rs
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/librustdoc/lib.rs
Expand Up @@ -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)]
Expand Down
1 change: 0 additions & 1 deletion 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)]
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/consts/offset_from.rs
Expand Up @@ -2,7 +2,6 @@

#![feature(const_raw_ptr_deref)]
#![feature(const_ptr_offset_from)]
#![feature(ptr_offset_from)]

struct Struct {
field: (),
Expand Down
1 change: 0 additions & 1 deletion 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 {
Expand Down
20 changes: 10 additions & 10 deletions src/test/ui/consts/offset_from_ub.stderr
Expand Up @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 | |
Expand Down
2 changes: 0 additions & 2 deletions 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];
Expand Down

0 comments on commit 9d606d9

Please sign in to comment.