Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite docs for pointer methods #53783

Merged
merged 34 commits into from
Sep 24, 2018
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
911d35f
Rewrite docs for `std::ptr`
ecstatic-morse Apr 6, 2018
da58beb
Mention alignment in top-level docs
May 24, 2018
9f5a3cc
Fix failing doctests
ecstatic-morse May 24, 2018
04a08c6
Fix unused variable warning in doctest
ecstatic-morse May 24, 2018
7b2ef6b
Update docs for `swap_nonoverlapping`
ecstatic-morse May 29, 2018
6f7338b
Reword module level docs re: alignment
ecstatic-morse Jun 5, 2018
30122e9
Fix off-by-one error when specifying a valid range
ecstatic-morse Jun 5, 2018
e40585f
Remove definiton of valid pointer
ecstatic-morse Jun 15, 2018
ea5570c
Redefine range validity
ecstatic-morse Jun 16, 2018
3a55c85
Incorporate RalfJung's suggestions
ecstatic-morse Jun 17, 2018
95a9088
You can't make an omlette without breaking a few links
ecstatic-morse Jun 17, 2018
7e165d9
Add a list of known facts re: validity
ecstatic-morse Jul 4, 2018
c8da321
Resolve null/ZST conflict correctly (whoops)
ecstatic-morse Jul 4, 2018
b0c5dc2
edit docs a little
RalfJung Aug 29, 2018
098bec8
clarify that these are preliminary guarantees
RalfJung Aug 29, 2018
fc63113
clarify ZST comment
RalfJung Aug 30, 2018
1ec66fb
apply comments
RalfJung Aug 30, 2018
e869b81
address remaining remarks and add example for dropping unaligned data
RalfJung Aug 30, 2018
d97f61f
avoid shadowing; fix examples
RalfJung Aug 30, 2018
c06f551
improve volatile comments
RalfJung Aug 30, 2018
2741224
fix example
RalfJung Aug 30, 2018
18a7bdb
fix example
RalfJung Aug 30, 2018
4ed469c
turn ptr type method docs into links to docs of free functions, to av…
RalfJung Aug 31, 2018
dc2237c
apply feedback
RalfJung Aug 31, 2018
b463871
(un)aligned
RalfJung Aug 31, 2018
408a6a0
fix doctests
RalfJung Aug 31, 2018
755de3c
Valid raw pointers
RalfJung Sep 1, 2018
bc809e0
remark on concurrency in validity section
RalfJung Sep 1, 2018
78f5b68
fix typos
ubsan Sep 11, 2018
c44e88c
Merge pull request #1 from ubsan/ptr-docs
RalfJung Sep 11, 2018
2713d36
tweaks
RalfJung Sep 17, 2018
0ec87d0
rearrange for clarity
RalfJung Sep 17, 2018
adcc0d2
clarify swap
RalfJung Sep 18, 2018
c197dc4
clarify write_bytes a bit
RalfJung Sep 21, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 147 additions & 36 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -962,59 +962,126 @@ extern "rust-intrinsic" {
/// value is not necessarily valid to be used to actually access memory.
pub fn arith_offset<T>(dst: *const T, offset: isize) -> *const T;

/// Copies `count * size_of<T>` bytes from `src` to `dst`. The source
/// and destination may *not* overlap.
/// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
/// and destination must *not* overlap.
///
/// `copy_nonoverlapping` is semantically equivalent to C's `memcpy`.
/// For regions of memory which might overlap, use [`copy`] instead.
///
/// `copy_nonoverlapping` is semantically equivalent to C's [`memcpy`].
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`copy`]: ./fn.copy.html
/// [`memcpy`]: https://www.gnu.org/software/libc/manual/html_node/Copying-Strings-and-Arrays.html#index-memcpy
///
/// # Safety
///
/// Beyond requiring that the program must be allowed to access both regions
/// of memory, it is Undefined Behavior for source and destination to
/// overlap. Care must also be taken with the ownership of `src` and
/// `dst`. This method semantically moves the values of `src` into `dst`.
/// However it does not drop the contents of `dst`, or prevent the contents
/// of `src` from being dropped or used.
/// Behavior is undefined if any of the following conditions are violated:
///
/// * `src` must be [valid] for reads of `count * size_of::<T>()` bytes.
///
/// * `dst` must be [valid] for writes of `count * size_of::<T>()` bytes.
///
/// * Both `src` and `dst` must be properly aligned.
///
/// * The region of memory beginning at `src` with a size of `count *
/// size_of::<T>()` bytes must *not* overlap with the region of memory
/// beginning at `dst` with the same size.
///
/// Like [`read`], `copy_nonoverlapping` creates a bitwise copy of `T`, regardless of
/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using *both* the values
/// in the region beginning at `*src` and the region beginning at `*dst` can
/// [violate memory safety][read-ownership].
///
/// Note that even if the effectively copied size (`count * size_of::<T>()`) is
/// `0`, the pointers must be non-NULL and properly aligned.
///
/// [`Copy`]: ../marker/trait.Copy.html
/// [`read`]: ../ptr/fn.read.html
/// [read-ownership]: ../ptr/fn.read.html#ownership-of-the-returned-value
/// [valid]: ../ptr/index.html#safety
///
/// # Examples
///
/// A safe swap function:
/// Manually implement [`Vec::append`]:
///
/// ```
/// use std::mem;
/// use std::ptr;
///
/// # #[allow(dead_code)]
/// fn swap<T>(x: &mut T, y: &mut T) {
/// unsafe {
/// // Give ourselves some scratch space to work with
/// let mut t: T = mem::uninitialized();
/// /// Moves all the elements of `src` into `dst`, leaving `src` empty.
/// fn append<T>(dst: &mut Vec<T>, src: &mut Vec<T>) {
/// let src_len = src.len();
/// let dst_len = dst.len();
///
/// // Perform the swap, `&mut` pointers never alias
/// ptr::copy_nonoverlapping(x, &mut t, 1);
/// ptr::copy_nonoverlapping(y, x, 1);
/// ptr::copy_nonoverlapping(&t, y, 1);
/// // Ensure that `dst` has enough capacity to hold all of `src`.
/// dst.reserve(src_len);
///
/// // y and t now point to the same thing, but we need to completely forget `t`
/// // because it's no longer relevant.
/// mem::forget(t);
/// unsafe {
/// // The call to offset is always safe because `Vec` will never
/// // allocate more than `isize::MAX` bytes.
/// let dst_ptr = dst.as_mut_ptr().offset(dst_len as isize);
/// let src_ptr = src.as_ptr();
///
/// // The two regions cannot overlap becuase mutable references do
/// // not alias, and two different vectors cannot own the same
/// // memory.
/// ptr::copy_nonoverlapping(src_ptr, dst_ptr, src_len);
///
/// // Truncate `src` without dropping its contents. This cannot panic,
/// // so double-drops cannot happen.
/// src.set_len(0);
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// // Notify `dst` that it now holds the contents of `src`.
/// dst.set_len(dst_len + src_len);
/// }
/// }
///
/// let mut a = vec!['r'];
/// let mut b = vec!['u', 's', 't'];
///
/// append(&mut a, &mut b);
///
/// assert_eq!(a, &['r', 'u', 's', 't']);
/// assert!(b.is_empty());
/// ```
///
/// [`Vec::append`]: ../../std/vec/struct.Vec.html#method.append
#[stable(feature = "rust1", since = "1.0.0")]
pub fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);

/// Copies `count * size_of<T>` bytes from `src` to `dst`. The source
/// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
/// and destination may overlap.
///
/// `copy` is semantically equivalent to C's `memmove`.
/// If the source and destination will *never* overlap,
/// [`copy_nonoverlapping`] can be used instead.
///
/// `copy` is semantically equivalent to C's [`memmove`]. Copying takes place as
/// if the bytes were copied from `src` to a temporary array and then copied from
/// the array to `dst`-
///
/// [`copy_nonoverlapping`]: ./fn.copy_nonoverlapping.html
/// [`memmove`]: https://www.gnu.org/software/libc/manual/html_node/Copying-Strings-and-Arrays.html#index-memmove
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Safety
///
/// Care must be taken with the ownership of `src` and `dst`.
/// This method semantically moves the values of `src` into `dst`.
/// However it does not drop the contents of `dst`, or prevent the contents of `src`
/// from being dropped or used.
/// Behavior is undefined if any of the following conditions are violated:
///
/// * `src` must be [valid] for reads of `count * size_of::<T>()` bytes.
///
/// * `dst` must be [valid] for writes of `count * size_of::<T>()` bytes.
///
/// * Both `src` and `dst` must be properly aligned.
///
/// Like [`read`], `copy` creates a bitwise copy of `T`, regardless of
/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the values
/// in the region beginning at `*src` and the region beginning at `*dst` can
/// [violate memory safety][read-ownership].
///
/// Note that even if the effectively copied size (`count * size_of::<T>()`) is
/// `0`, the pointers must be non-NULL and properly aligned.
///
/// [`Copy`]: ../marker/trait.Copy.html
/// [`read`]: ../ptr/fn.read.html
/// [read-ownership]: ../ptr/fn.read.html#ownership-of-the-returned-value
/// [valid]: ../ptr/index.html#safety
///
/// # Examples
///
Expand All @@ -1031,24 +1098,68 @@ extern "rust-intrinsic" {
/// dst
/// }
/// ```
///
#[stable(feature = "rust1", since = "1.0.0")]
pub fn copy<T>(src: *const T, dst: *mut T, count: usize);

/// Invokes memset on the specified pointer, setting `count * size_of::<T>()`
/// bytes of memory starting at `dst` to `val`.
/// Sets `count * size_of::<T>()` bytes of memory starting at `dst` to
/// `val`.
///
/// `write_bytes` is similar to C's [`memset`], but sets `count *
/// size_of::<T>()` bytes to `val`.
///
/// [`memset`]: https://www.gnu.org/software/libc/manual/html_node/Copying-Strings-and-Arrays.html#index-memset
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Safety
///
/// Behavior is undefined if any of the following conditions are violated:
///
/// * `dst` must be [valid] for writes of `count * size_of::<T>()` bytes.
///
/// * `dst` must be properly aligned.
///
/// Additionally, the caller should ensure that writing `count *
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
/// size_of::<T>()` bytes to the given region of memory results in a valid
/// value of `T`. Using a region of memory typed as a `T` that contains an
/// invalid value of `T` is undefined behavior.
///
/// Note that even if the effectively copied size (`count * size_of::<T>()`) is
/// `0`, the pointer must be non-NULL and properly aligned.
///
/// [valid]: ../ptr/index.html#safety
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::ptr;
///
/// let mut vec = vec![0; 4];
/// let mut vec = vec![0u32; 4];
/// unsafe {
/// let vec_ptr = vec.as_mut_ptr();
/// ptr::write_bytes(vec_ptr, b'a', 2);
/// ptr::write_bytes(vec_ptr, 0xfe, 2);
/// }
/// assert_eq!(vec, [b'a', b'a', 0, 0]);
/// assert_eq!(vec, [0xfefefefe, 0xfefefefe, 0, 0]);
/// ```
///
/// Creating an invalid value:
///
/// ```
/// use std::{mem, ptr};
///
/// let mut v = Box::new(0i32);
///
/// unsafe {
/// // Leaks the previously held value by overwriting the `Box<T>` with
/// // a null pointer.
/// ptr::write_bytes(&mut v, 0, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure what's going on here? Is it overwriting the pointer inside the Box itself? This seems like a situation where Deref can make things confusing if you're not careful - i.e. changing &mut v to &mut *v (adding the asterisk to force Deref) now means you're overwriting the heap contents rather than the pointer itself.

Making this &mut v as *mut Box<i32> would make this much more obvious that you're overwriting a heap pointer with null. (Though it's an extremely heavy-handed way to do it, since null_mut() exists...)

Copy link
Contributor

@ecstatic-morse ecstatic-morse Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention here was to demonstrate that creating an invalid value can result in UB later in the program, outside of the unsafe block which caused the issue.

Rereading it now, it does seem a bit convoluted. Perhaps we could write an arbitrary value (instead of 0), or-- since NonZeroU32 is now stable--rewrite this example to invalidate one of those instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the as *mut Box<i32>. However, TBH I am not sure how useful this example really is.

/// }
///
/// // At this point, using or dropping `v` results in undefined behavior.
/// // drop(v); // ERROR
///
/// // Leaking it does not invoke drop and is fine:
/// mem::forget(v)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am... very suspect of saying that this is "fine". Specifically, passing an invalid value seems incredibly suspect - fn foo(&T); foo(&*std::ptr::null()) should, in my opinion, be UB, and this is equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would ManuallyDrop::new(v) be any better ? We are just moving an invalid value into a memory location such that drop won't be called.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yeah, this violates the validity invariant. It is UB to touch this in any way.

I will change the example to instead overwrite it with a valid value again:

    /// ```
    /// use std::{mem, ptr};
    ///
    /// let mut v = Box::new(0i32);
    ///
    /// unsafe {
    ///     // Leaks the previously held value by overwriting the `Box<T>` with
    ///     // a null pointer.
    ///     ptr::write_bytes(&mut v, 0, 1);
    /// }
    ///
    /// // At this point, using or dropping `v` results in undefined behavior.
    /// // drop(v); // ERROR
    ///
    /// // Even leaking `v` "uses" it, and henc eis undefined behavior.
    /// // mem::forget(v); // ERROR
    ///
    /// // Let us instead put in a valid value
    /// ptr::write(&mut v, Box::new(42i32);
    ///
    /// // Now the box is fine
    /// assert_eq!(*v, 42);
    /// ```

/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn write_bytes<T>(dst: *mut T, val: u8, count: usize);
Expand All @@ -1066,7 +1177,7 @@ extern "rust-intrinsic" {
/// `min_align_of::<T>()`
///
/// The volatile parameter is set to `true`, so it will not be optimized out
/// unless size is equal to zero..
/// unless size is equal to zero.
pub fn volatile_copy_memory<T>(dst: *mut T, src: *const T, count: usize);
/// Equivalent to the appropriate `llvm.memset.p0i8.*` intrinsic, with a
/// size of `count` * `size_of::<T>()` and an alignment of
Expand Down
Loading