Skip to content

Commit

Permalink
Rollup merge of rust-lang#49767 - ecstatic-morse:ptr-docs, r=stevekla…
Browse files Browse the repository at this point in the history
…bnik

Rewrite docs for `std::ptr`

This PR attempts to resolve rust-lang#29371.

This is a fairly major rewrite of the `std::ptr` docs, and deserves a fair bit of scrutiny. It adds links to the GNU libc docs for various instrinsics, adds internal links to types and functions referenced in the docs, adds new, more complex examples for many functions, and introduces a common template for discussing unsafety of functions in `std::ptr`.

All functions in `std::ptr` (with the exception of `ptr::eq`) are unsafe because they either read from or write to a raw pointer. The "Safety" section now informs that the function is unsafe because it dereferences a raw pointer and requires that any pointer to be read by the function points to "a valid vaue of type `T`".

Additionally, each function imposes some subset of the following conditions on its arguments.

* The pointer points to valid memory.
* The pointer points to initialized memory.
* The pointer is properly aligned.

These requirements are discussed in the "Undefined Behavior" section along with the  consequences of using functions that perform bitwise copies without requiring `T: Copy`. I don't love my new descriptions of the consequences of making such copies. Perhaps the old ones were good enough?

Some issues which still need to be addressed before this is merged:
- [ ] The new docs assert that `drop_in_place` is equivalent to calling `read` and discarding the value. Is this correct?
- [ ] Do `write_bytes` and `swap_nonoverlapping` require properly aligned pointers?
- [ ] The new example for `drop_in_place` is a lackluster.
- [ ] Should these docs rigorously define what `valid` memory is? Or should is that the job of the reference? Should we link to the reference?
- [ ] Is it correct to require that pointers that will be read from refer to "valid values of type `T`"?
- [x] I can't imagine ever using `{read,write}_volatile` with non-`Copy` types.  Should I just link to {read,write} and say that the same issues with non-`Copy` types apply?
- [x] `write_volatile` doesn't link back to `read_volatile`.
- [ ] Update docs for the unstable [`swap_nonoverlapping`](rust-lang#42818)
- [ ] Update docs for the unstable [unsafe pointer methods RFC](rust-lang/rfcs#1966)

Looking forward to your feedback.

r? @steveklabnik
  • Loading branch information
GuillaumeGomez authored May 15, 2018
2 parents 0b17da2 + 827251e commit 7a9eb83
Show file tree
Hide file tree
Showing 2 changed files with 430 additions and 101 deletions.
161 changes: 130 additions & 31 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -962,59 +962,122 @@ 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`].
///
/// [`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:
///
/// * The region of memory which begins at `src` and has a length of
/// `count * size_of::<T>()` bytes must be *both* valid and initialized.
///
/// * The region of memory which begins at `dst` and has a length of
/// `count * size_of::<T>()` bytes must be valid (but may or may not be
/// initialized).
///
/// * The two regions of memory must *not* overlap.
///
/// * `src` must be properly aligned.
///
/// * `dst` must be properly aligned.
///
/// Additionally, if `T` is not [`Copy`], only the region at `src` *or* the
/// region at `dst` can be used or dropped after calling
/// `copy_nonoverlapping`. `copy_nonoverlapping` creates bitwise copies of
/// `T`, regardless of whether `T: Copy`, which can result in undefined
/// behavior if both copies are used.
///
/// [`Copy`]: ../marker/trait.Copy.html
///
/// # 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) {
/// /// 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();
///
/// // Ensure that `dst` has enough capacity to hold all of `src`.
/// dst.reserve(src_len);
///
/// unsafe {
/// // Give ourselves some scratch space to work with
/// let mut t: T = mem::uninitialized();
/// // The call to offset is always safe because `Vec` will never
/// // allocate more than `isize::MAX` bytes.
/// let dst = dst.as_mut_ptr().offset(dst_len as isize);
/// let src = 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, dst, src_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);
/// unsafe {
/// // Truncate `src` without dropping its contents.
/// src.set_len(0);
///
/// // 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);
/// // 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`].
///
/// [`copy_nonoverlapping`]: ./fn.copy_nonoverlapping.html
/// [`memmove`]: https://www.gnu.org/software/libc/manual/html_node/Copying-Strings-and-Arrays.html#index-memmove
///
/// # 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:
///
/// * The region of memory which begins at `src` and has a length of
/// `count * size_of::<T>()` bytes must be *both* valid and initialized.
///
/// * The region of memory which begins at `dst` and has a length of
/// `count * size_of::<T>()` bytes must be valid (but may or may not be
/// initialized).
///
/// * `src` must be properly aligned.
///
/// * `dst` must be properly aligned.
///
/// Additionally, if `T` is not [`Copy`], only the region at `src` *or* the
/// region at `dst` can be used or dropped after calling `copy`. `copy`
/// creates bitwise copies of `T`, regardless of whether `T: Copy`, which
/// can result in undefined behavior if both copies are used.
///
/// [`Copy`]: ../marker/trait.Copy.html
///
/// # Examples
///
Expand All @@ -1031,15 +1094,34 @@ 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 semantically equivalent to C's [`memset`].
///
/// [`memset`]: https://www.gnu.org/software/libc/manual/html_node/Copying-Strings-and-Arrays.html#index-memset
///
/// # Safety
///
/// Behavior is undefined if any of the following conditions are violated:
///
/// * The region of memory which begins at `dst` and has a length of
/// `count` bytes must be valid.
///
/// * `dst` must be properly aligned.
///
/// Additionally, the caller must ensure that writing `count` bytes to the
/// given region of memory results in a valid value of `T`. Creating an
/// invalid value of `T` can result in undefined behavior. An example is
/// provided below.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::ptr;
///
Expand All @@ -1050,6 +1132,23 @@ extern "rust-intrinsic" {
/// }
/// assert_eq!(vec, [b'a', b'a', 0, 0]);
/// ```
///
/// Creating an invalid value:
///
/// ```no_run
/// 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, mem::size_of::<Box<i32>>());
/// }
///
/// // At this point, using or dropping `v` results in undefined behavior.
/// // v = Box::new(0i32); // ERROR
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn write_bytes<T>(dst: *mut T, val: u8, count: usize);

Expand Down
Loading

0 comments on commit 7a9eb83

Please sign in to comment.