From 85f97e558068f6020afdcb7595b2b9bcd5025e6c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 6 Apr 2018 00:34:45 -0700 Subject: [PATCH 01/13] Rewrite docs for `std::ptr` - Add links to the GNU libc docs for `memmove`, `memcpy`, and `memset`, as well as internally linking to other functions in `std::ptr` - List invariants which, when violated, cause UB for all functions - Add example to `ptr::drop_in_place` and compares it to `ptr::read`. - Add examples which more closely mirror real world uses for the functions in `std::ptr`. Also, move the reimplementation of `mem::swap` to the examples of `ptr::read` and use a more interesting example for `copy_nonoverlapping`. - Change module level description - Define what constitutes a "valid" pointer. - Centralize discussion of ownership of bitwise copies in `ptr::read` and provide an example. --- src/libcore/intrinsics.rs | 180 ++++++++++++---- src/libcore/ptr.rs | 419 +++++++++++++++++++++++++++++++------- 2 files changed, 497 insertions(+), 102 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 95f13daf841ee..c7307f2f7f346 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -962,59 +962,129 @@ extern "rust-intrinsic" { /// value is not necessarily valid to be used to actually access memory. pub fn arith_offset(dst: *const T, offset: isize) -> *const T; - /// Copies `count * size_of` bytes from `src` to `dst`. The source - /// and destination may *not* overlap. + /// Copies `count * size_of::()` 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: + /// + /// * Both `src` and `dst` must be [valid]. + /// + /// * Both `src` and `dst` must be properly aligned. + /// + /// * `src.offset(count)` must be [valid]. In other words, the region of + /// memory which begins at `src` and has a length of `count * + /// size_of::()` bytes must belong to a single, live allocation. + /// + /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// memory which begins at `dst` and has a length of `count * + /// size_of::()` bytes must belong to a single, live allocation. + /// + /// * The two regions of memory must *not* overlap. + /// + /// 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]. + /// + /// [`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(x: &mut T, y: &mut T) { + /// /// Moves all the elements of `src` into `dst`, leaving `src` empty. + /// fn append(dst: &mut Vec, src: &mut Vec) { + /// 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(src: *const T, dst: *mut T, count: usize); - /// Copies `count * size_of` bytes from `src` to `dst`. The source + /// Copies `count * size_of::()` 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: + /// + /// * Both `src` and `dst` must be [valid]. + /// + /// * Both `src` and `dst` must be properly aligned. + /// + /// * `src.offset(count)` must be [valid]. In other words, the region of + /// memory which begins at `src` and has a length of `count * + /// size_of::()` bytes must belong to a single, live allocation. + /// + /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// memory which begins at `dst` and has a length of `count * + /// size_of::()` bytes must belong to a single, live allocation. + /// + /// 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]. + /// + /// [`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 /// @@ -1031,24 +1101,66 @@ extern "rust-intrinsic" { /// dst /// } /// ``` - /// #[stable(feature = "rust1", since = "1.0.0")] pub fn copy(src: *const T, dst: *mut T, count: usize); - /// Invokes memset on the specified pointer, setting `count * size_of::()` - /// bytes of memory starting at `dst` to `val`. + /// Sets `count * size_of::()` bytes of memory starting at `dst` to + /// `val`. + /// + /// `write_bytes` is similar to C's [`memset`], but sets `count * + /// size_of::()` bytes to `val`. + /// + /// [`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: + /// + /// * `dst` must be [valid]. + /// + /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// memory which begins at `dst` and has a length of `count * + /// size_of::()` bytes must belong to a single, live allocation. + /// + /// * `dst` must be properly aligned. + /// + /// Additionally, the caller must ensure that writing `count * + /// size_of::()` 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. + /// + /// [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: + /// + /// ```no_run + /// use std::{mem, ptr}; + /// + /// let mut v = Box::new(0i32); + /// + /// unsafe { + /// // Leaks the previously held value by overwriting the `Box` with + /// // a null pointer. + /// ptr::write_bytes(&mut v, 0, 1); + /// } + /// + /// // 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(dst: *mut T, val: u8, count: usize); diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 39315d8f0c8f8..ac3fd4492bab9 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -10,9 +10,24 @@ // FIXME: talk about offset, copy_memory, copy_nonoverlapping_memory -//! Raw, unsafe pointers, `*const T`, and `*mut T`. +//! Manually manage memory through raw pointers. //! //! *[See also the pointer primitive types](../../std/primitive.pointer.html).* +//! +//! # Safety +//! +//! Most functions in this module [dereference raw pointers]. +//! +//! In order for a pointer dereference to be safe, the pointer must be "valid". +//! A valid pointer is one that satisfies **all** of the following conditions: +//! +//! * The pointer is not null. +//! * The pointer is not dangling (it does not point to memory which has been +//! freed). +//! * The pointer satisfies [LLVM's pointer aliasing rules]. +//! +//! [dereference raw pointers]: https://doc.rust-lang.org/book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer +//! [LLVM's pointer aliasing rules]: https://llvm.org/docs/LangRef.html#pointer-aliasing-rules #![stable(feature = "rust1", since = "1.0.0")] @@ -38,21 +53,63 @@ pub use intrinsics::write_bytes; /// Executes the destructor (if any) of the pointed-to value. /// -/// This has two use cases: +/// This is semantically equivalent to calling [`ptr::read`] and discarding +/// the result, but has the following advantages: /// /// * It is *required* to use `drop_in_place` to drop unsized types like /// trait objects, because they can't be read out onto the stack and /// dropped normally. /// -/// * It is friendlier to the optimizer to do this over `ptr::read` when +/// * It is friendlier to the optimizer to do this over [`ptr::read`] when /// dropping manually allocated memory (e.g. when writing Box/Rc/Vec), /// as the compiler doesn't need to prove that it's sound to elide the /// copy. /// +/// [`ptr::read`]: ../ptr/fn.read.html +/// /// # Safety /// -/// This has all the same safety problems as `ptr::read` with respect to -/// invalid pointers, types, and double drops. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `to_drop` must be [valid]. +/// +/// * `to_drop` must be properly aligned. +/// +/// Additionally, if `T` is not [`Copy`], using the pointed-to value after +/// calling `drop_in_place` can cause undefined behavior. Note that `*to_drop = +/// foo` counts as a use because it will cause the the value to be dropped +/// again. [`write`] can be used to overwrite data without causing it to be +/// dropped. +/// +/// [valid]: ../ptr/index.html#safety +/// [`Copy`]: ../marker/trait.Copy.html +/// [`write`]: ../ptr/fn.write.html +/// +/// # Examples +/// +/// Manually remove the last item from a vector: +/// +/// ``` +/// use std::ptr; +/// use std::rc::Rc; +/// +/// let last = Rc::new(1); +/// let weak = Rc::downgrade(&last); +/// +/// let mut v = vec![Rc::new(0), last]; +/// +/// unsafe { +/// // Without a call `drop_in_place`, the last item would never be dropped, +/// // and the memory it manages would be leaked. +/// ptr::drop_in_place(&mut v[1]); +/// v.set_len(1); +/// } +/// +/// assert_eq!(v, &[0.into()]); +/// +/// // Ensure that the last item was dropped. +/// assert!(weak.upgrade().is_none()); +/// ``` #[stable(feature = "drop_in_place", since = "1.8.0")] #[lang = "drop_in_place"] #[allow(unconditional_recursion)] @@ -93,17 +150,27 @@ pub const fn null_mut() -> *mut T { 0 as *mut T } /// Swaps the values at two mutable locations of the same type, without /// deinitializing either. /// -/// The values pointed at by `x` and `y` may overlap, unlike `mem::swap` which -/// is otherwise equivalent. If the values do overlap, then the overlapping -/// region of memory from `x` will be used. This is demonstrated in the -/// examples section below. +/// But for the following two exceptions, this function is semantically +/// equivalent to [`mem::swap`]: +/// +/// * It operates on raw pointers instead of references. When references are +/// available, [`mem::swap`] should be preferred. +/// +/// * The two pointed-to values may overlap. If the values do overlap, then the +/// overlapping region of memory from `x` will be used. This is demonstrated +/// in the examples below. +/// +/// [`mem::swap`]: ../mem/fn.swap.html /// /// # Safety /// -/// This function copies the memory through the raw pointers passed to it -/// as arguments. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * Both `x` and `y` must be [valid]. /// -/// Ensure that these pointers are valid before calling `swap`. +/// * Both `x` and `y` must be properly aligned. +/// +/// [valid]: ../ptr/index.html#safety /// /// # Examples /// @@ -243,10 +310,38 @@ unsafe fn swap_nonoverlapping_bytes(x: *mut u8, y: *mut u8, len: usize) { /// /// Neither value is dropped. /// +/// This function is semantically equivalent to [`mem::replace`] except that it +/// operates on raw pointers instead of references. When references are +/// available, [`mem::replace`] should be preferred. +/// +/// [`mem::replace`]: ../mem/fn.replace.html +/// /// # Safety /// -/// This is only unsafe because it accepts a raw pointer. -/// Otherwise, this operation is identical to `mem::replace`. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `dest` must be [valid]. +/// +/// * `dest` must be properly aligned. +/// +/// [valid]: ../ptr/index.html#safety +/// +/// # Examples +/// +/// ``` +/// use std::ptr; +/// +/// let mut rust = vec!['b', 'u', 's', 't']; +/// +/// // `mem::replace` would have the same effect without requiring the unsafe +/// // block. +/// let b = unsafe { +/// ptr::replace(&mut rust[0], 'r') +/// }; +/// +/// assert_eq!(b, 'b'); +/// assert_eq!(rust, &['r', 'u', 's', 't']); +/// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn replace(dest: *mut T, mut src: T) -> T { @@ -259,14 +354,52 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// /// # Safety /// -/// Beyond accepting a raw pointer, this is unsafe because it semantically -/// moves the value out of `src` without preventing further usage of `src`. -/// If `T` is not `Copy`, then care must be taken to ensure that the value at -/// `src` is not used before the data is overwritten again (e.g. with `write`, -/// `write_bytes`, or `copy`). Note that `*src = foo` counts as a use -/// because it will attempt to drop the value previously at `*src`. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `src` must be [valid]. +/// +/// * `src` must be properly aligned. Use [`read_unaligned`] if this is not the +/// case. +/// +/// ## Ownership of the Returned Value +/// +/// `read` creates a bitwise copy of `T`, regardless of whether `T` is [`Copy`]. +/// If `T` is not [`Copy`], using both the returned value and the value at +/// `*src` can violate memory safety. Note that assigning to `src` counts as a +/// use because it will attempt to drop the value at `*src`. +/// +/// [`write`] can be used to overwrite data without causing it to be dropped. +/// +/// [valid]: ../ptr/index.html#safety +/// [`Copy`]: ../marker/trait.Copy.html +/// [`read_unaligned`]: ./fn.read_unaligned.html +/// [`write`]: ./fn.write.html +/// +/// ``` +/// use std::ptr; +/// +/// let mut s = String::new("foo"); +/// unsafe { +/// // `s2` now points to the same underlying memory as `s1`. +/// let mut s2 = ptr::read(&s); +/// +/// assert_eq!(s2, "foo"); +/// +/// // Assigning to `s2` causes its original value to be dropped. Beyond +/// // this point, `s` must no longer be used, as the underlying memory has +/// // been freed. +/// s2 = String::default(); /// -/// The pointer must be aligned; use `read_unaligned` if that is not the case. +/// // Assigning to `s` would cause the old value to be dropped again, +/// // resulting in undefined behavior. +/// // s = String::new("bar"); // ERROR +/// +/// // `ptr::write` can be used to overwrite a value without dropping it. +/// ptr::write(&s, String::new("bar")); +/// } +/// +/// assert_eq!(s, "bar"); +/// ``` /// /// # Examples /// @@ -280,6 +413,44 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// assert_eq!(std::ptr::read(y), 12); /// } /// ``` +/// +/// Manually implement [`mem::swap`]: +/// +/// ``` +/// use std::ptr; +/// +/// fn swap(a: &mut T, b: &mut T) { +/// unsafe { +/// // Create a bitwise copy of the value at `a` in `tmp`. +/// let tmp = ptr::read(a); +/// +/// // Exiting at this point (either by explicitly returning or by +/// // calling a function which panics) would cause the value in `tmp` to +/// // be dropped while the same value is still referenced by `a`. This +/// // could trigger undefined behavior if `T` is not `Copy`. +/// +/// // Create a bitwise copy of the value at `b` in `a`. +/// // This is safe because mutable references cannot alias. +/// ptr::copy_nonoverlapping(b, a, 1); +/// +/// // As above, exiting here could trigger undefined behavior because +/// // the same value is referenced by `a` and `b`. +/// +/// // Move `tmp` into `b`. +/// ptr::write(b, tmp); +/// } +/// } +/// +/// let mut foo = "foo".to_owned(); +/// let mut bar = "bar".to_owned(); +/// +/// swap(&mut foo, &mut bar); +/// +/// assert_eq!(foo, "bar"); +/// assert_eq!(bar, "foo"); +/// ``` +/// +/// [`mem::swap`]: ../mem/fn.swap.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn read(src: *const T) -> T { @@ -291,28 +462,59 @@ pub unsafe fn read(src: *const T) -> T { /// Reads the value from `src` without moving it. This leaves the /// memory in `src` unchanged. /// -/// Unlike `read`, the pointer may be unaligned. +/// Unlike [`read`], `read_unaligned` works with unaligned pointers. /// /// # Safety /// -/// Beyond accepting a raw pointer, this is unsafe because it semantically -/// moves the value out of `src` without preventing further usage of `src`. -/// If `T` is not `Copy`, then care must be taken to ensure that the value at -/// `src` is not used before the data is overwritten again (e.g. with `write`, -/// `write_bytes`, or `copy`). Note that `*src = foo` counts as a use -/// because it will attempt to drop the value previously at `*src`. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `src` must be [valid]. +/// +/// Like [`read`], `read_unaligned` creates a bitwise copy of `T`, regardless of +/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned +/// value and the value at `*src` can [violate memory safety][read-ownership]. +/// +/// [`Copy`]: ../marker/trait.Copy.html +/// [`read`]: ./fn.read.html +/// [`write_unaligned`]: ./fn.write_unaligned.html +/// [read-ownership]: ./fn.read.html#ownership-of-the-returned-value +/// [valid]: ../ptr/index.html#safety /// /// # Examples /// -/// Basic usage: +/// Access members of a packed struct by reference: /// /// ``` -/// let x = 12; -/// let y = &x as *const i32; +/// use std::ptr; /// -/// unsafe { -/// assert_eq!(std::ptr::read_unaligned(y), 12); +/// #[repr(packed, C)] +/// #[derive(Default)] +/// struct Packed { +/// _padding: u8, +/// unaligned: u32, /// } +/// +/// let x = Packed { +/// _padding: 0x00, +/// unaligned: 0x01020304, +/// }; +/// +/// let v = unsafe { +/// // Take a reference to a 32-bit integer which is not aligned. +/// let unaligned = &x.unaligned; +/// +/// // Dereferencing normally will emit an unaligned load instruction, +/// // causing undefined behavior. +/// // let v = *unaligned; // ERROR +/// +/// // Instead, use `read_unaligned` to read improperly aligned values. +/// let v = ptr::read_unaligned(unaligned); +/// +/// v +/// }; +/// +/// // Accessing unaligned values directly is safe. +/// assert!(x.unaligned == v); /// ``` #[inline] #[stable(feature = "ptr_unaligned", since = "1.17.0")] @@ -327,11 +529,7 @@ pub unsafe fn read_unaligned(src: *const T) -> T { /// Overwrites a memory location with the given value without reading or /// dropping the old value. /// -/// # Safety -/// -/// This operation is marked unsafe because it accepts a raw pointer. -/// -/// It does not drop the contents of `dst`. This is safe, but it could leak +/// `write` does not drop the contents of `dst`. This is safe, but it could leak /// allocations or resources, so care must be taken not to overwrite an object /// that should be dropped. /// @@ -339,9 +537,21 @@ pub unsafe fn read_unaligned(src: *const T) -> T { /// location pointed to by `dst`. /// /// This is appropriate for initializing uninitialized memory, or overwriting -/// memory that has previously been `read` from. +/// memory that has previously been [`read`] from. /// -/// The pointer must be aligned; use `write_unaligned` if that is not the case. +/// [`read`]: ./fn.read.html +/// +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `dst` must be [valid]. +/// +/// * `dst` must be properly aligned. Use [`write_unaligned`] if this is not the +/// case. +/// +/// [valid]: ../ptr/index.html#safety +/// [`write_unaligned`]: ./fn.write_unaligned.html /// /// # Examples /// @@ -357,6 +567,30 @@ pub unsafe fn read_unaligned(src: *const T) -> T { /// assert_eq!(std::ptr::read(y), 12); /// } /// ``` +/// +/// Manually implement [`mem::swap`]: +/// +/// ``` +/// use std::ptr; +/// +/// fn swap(a: &mut T, b: &mut T) { +/// unsafe { +/// let tmp = ptr::read(a); +/// ptr::copy_nonoverlapping(b, a, 1); +/// ptr::write(b, tmp); +/// } +/// } +/// +/// let mut foo = "foo".to_owned(); +/// let mut bar = "bar".to_owned(); +/// +/// swap(&mut foo, &mut bar); +/// +/// assert_eq!(foo, "bar"); +/// assert_eq!(bar, "foo"); +/// ``` +/// +/// [`mem::swap`]: ../mem/fn.swap.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn write(dst: *mut T, src: T) { @@ -366,36 +600,60 @@ pub unsafe fn write(dst: *mut T, src: T) { /// Overwrites a memory location with the given value without reading or /// dropping the old value. /// -/// Unlike `write`, the pointer may be unaligned. -/// -/// # Safety +/// Unlike [`write`], the pointer may be unaligned. /// -/// This operation is marked unsafe because it accepts a raw pointer. -/// -/// It does not drop the contents of `dst`. This is safe, but it could leak -/// allocations or resources, so care must be taken not to overwrite an object -/// that should be dropped. +/// `write_unaligned` does not drop the contents of `dst`. This is safe, but it +/// could leak allocations or resources, so care must be taken not to overwrite +/// an object that should be dropped. /// /// Additionally, it does not drop `src`. Semantically, `src` is moved into the /// location pointed to by `dst`. /// /// This is appropriate for initializing uninitialized memory, or overwriting -/// memory that has previously been `read` from. +/// memory that has previously been read with [`read_unaligned`]. +/// +/// [`write`]: ./fn.write.html +/// [`read_unaligned`]: ./fn.read_unaligned.html +/// +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `dst` must be [valid]. +/// +/// [valid]: ../ptr/index.html#safety /// /// # Examples /// -/// Basic usage: +/// Access fields in a packed struct: /// /// ``` -/// let mut x = 0; -/// let y = &mut x as *mut i32; -/// let z = 12; +/// use std::{mem, ptr}; +/// +/// #[repr(packed, C)] +/// #[derive(Default)] +/// struct Packed { +/// _padding: u8, +/// unaligned: u32, +/// } +/// +/// let v = 0x01020304; +/// let mut x: Packed = unsafe { mem::zeroed() }; /// /// unsafe { -/// std::ptr::write_unaligned(y, z); -/// assert_eq!(std::ptr::read_unaligned(y), 12); +/// // Take a reference to a 32-bit integer which is not aligned. +/// let unaligned = &mut x.unaligned; +/// +/// // Dereferencing normally will emit an unaligned store instruction, +/// // causing undefined behavior. +/// // *unaligned = v; // ERROR +/// +/// // Instead, use `write_unaligned` to write improperly aligned values. +/// ptr::write_unaligned(unaligned, v); /// } -/// ``` +/// +/// // Accessing unaligned values directly is safe. +/// assert!(x.unaligned == v); #[inline] #[stable(feature = "ptr_unaligned", since = "1.17.0")] pub unsafe fn write_unaligned(dst: *mut T, src: T) { @@ -412,6 +670,11 @@ pub unsafe fn write_unaligned(dst: *mut T, src: T) { /// to not be elided or reordered by the compiler across other volatile /// operations. /// +/// Memory read with `read_volatile` should almost always be written to using +/// [`write_volatile`]. +/// +/// [`write_volatile`]: ./fn.write_volatile.html +/// /// # Notes /// /// Rust does not currently have a rigorously and formally defined memory model, @@ -428,12 +691,21 @@ pub unsafe fn write_unaligned(dst: *mut T, src: T) { /// /// # Safety /// -/// Beyond accepting a raw pointer, this is unsafe because it semantically -/// moves the value out of `src` without preventing further usage of `src`. -/// If `T` is not `Copy`, then care must be taken to ensure that the value at -/// `src` is not used before the data is overwritten again (e.g. with `write`, -/// `write_bytes`, or `copy`). Note that `*src = foo` counts as a use -/// because it will attempt to drop the value previously at `*src`. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `src` must be [valid]. +/// +/// * `src` must be properly aligned. +/// +/// Like [`read`], `read_unaligned` creates a bitwise copy of `T`, regardless of +/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned +/// value and the value at `*src` can [violate memory safety][read-ownership]. +/// However, storing non-[`Copy`] types in volatile memory is almost certainly +/// incorrect. +/// +/// [valid]: ../ptr/index.html#safety +/// [`Copy`]: ../marker/trait.Copy.html +/// [`read`]: ./fn.read.html /// /// # Examples /// @@ -460,6 +732,18 @@ pub unsafe fn read_volatile(src: *const T) -> T { /// to not be elided or reordered by the compiler across other volatile /// operations. /// +/// Memory written with `write_volatile` should almost always be read from using +/// [`read_volatile`]. +/// +/// `write_volatile` does not drop the contents of `dst`. This is safe, but it +/// could leak allocations or resources, so care must be taken not to overwrite +/// an object that should be dropped. +/// +/// Additionally, it does not drop `src`. Semantically, `src` is moved into the +/// location pointed to by `dst`. +/// +/// [`read_volatile`]: ./fn.read_volatile.html +/// /// # Notes /// /// Rust does not currently have a rigorously and formally defined memory model, @@ -476,14 +760,13 @@ pub unsafe fn read_volatile(src: *const T) -> T { /// /// # Safety /// -/// This operation is marked unsafe because it accepts a raw pointer. +/// Behavior is undefined if any of the following conditions are violated: /// -/// It does not drop the contents of `dst`. This is safe, but it could leak -/// allocations or resources, so care must be taken not to overwrite an object -/// that should be dropped. +/// * `dst` must be [valid]. /// -/// This is appropriate for initializing uninitialized memory, or overwriting -/// memory that has previously been `read` from. +/// * `dst` must be properly aligned. +/// +/// [valid]: ../ptr/index.html#safety /// /// # Examples /// From ac53367d2696564454b18f166636d458e51dc44d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 23 May 2018 20:55:39 -0700 Subject: [PATCH 02/13] Mention alignment in top-level docs This also removes the overlong link that failed tidy xD. --- src/libcore/ptr.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index ac3fd4492bab9..65ff03883e716 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -16,18 +16,23 @@ //! //! # Safety //! -//! Most functions in this module [dereference raw pointers]. -//! -//! In order for a pointer dereference to be safe, the pointer must be "valid". -//! A valid pointer is one that satisfies **all** of the following conditions: +//! Many functions in this module take raw pointers as arguments and dereference +//! them. For this to be safe, these pointers must be valid. A valid pointer +//! is one that satisfies **all** of the following conditions: //! //! * The pointer is not null. //! * The pointer is not dangling (it does not point to memory which has been //! freed). //! * The pointer satisfies [LLVM's pointer aliasing rules]. //! -//! [dereference raw pointers]: https://doc.rust-lang.org/book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer +//! Valid pointers are not necessarily properly aligned. However, except for +//! [`read_unaligned`] and [`write_unaligned`], most functions require their +//! arguments to be aligned. Any alignment requirements will be explicitly +//! stated in the function's documentation. +//! //! [LLVM's pointer aliasing rules]: https://llvm.org/docs/LangRef.html#pointer-aliasing-rules +//! [`read_unaligned`]: ./fn.read_unaligned.html +//! [`write_unaligned`]: ./fn.write_unaligned.html #![stable(feature = "rust1", since = "1.0.0")] @@ -654,6 +659,7 @@ pub unsafe fn write(dst: *mut T, src: T) { /// /// // Accessing unaligned values directly is safe. /// assert!(x.unaligned == v); +/// ``` #[inline] #[stable(feature = "ptr_unaligned", since = "1.17.0")] pub unsafe fn write_unaligned(dst: *mut T, src: T) { From eecae1dbe0197aa5bbb4cd5e06f78659b0839a9c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 23 May 2018 22:38:22 -0700 Subject: [PATCH 03/13] Fix failing doctests --- src/libcore/intrinsics.rs | 2 +- src/libcore/ptr.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index c7307f2f7f346..e6ac6c4ccef5d 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1149,7 +1149,7 @@ extern "rust-intrinsic" { /// Creating an invalid value: /// /// ```no_run - /// use std::{mem, ptr}; + /// use std::ptr; /// /// let mut v = Box::new(0i32); /// diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 65ff03883e716..4a26289fcc6cb 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -383,7 +383,7 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// ``` /// use std::ptr; /// -/// let mut s = String::new("foo"); +/// let mut s = String::from("foo"); /// unsafe { /// // `s2` now points to the same underlying memory as `s1`. /// let mut s2 = ptr::read(&s); @@ -397,10 +397,10 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// /// // Assigning to `s` would cause the old value to be dropped again, /// // resulting in undefined behavior. -/// // s = String::new("bar"); // ERROR +/// // s = String::from("bar"); // ERROR /// /// // `ptr::write` can be used to overwrite a value without dropping it. -/// ptr::write(&s, String::new("bar")); +/// ptr::write(&mut s, String::from("bar")); /// } /// /// assert_eq!(s, "bar"); From de165bbce655bc681dbc7e30aac619cdc2e9d751 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 24 May 2018 09:05:56 -0700 Subject: [PATCH 04/13] Fix unused variable warning in doctest --- src/libcore/ptr.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 4a26289fcc6cb..3713f705ab170 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -394,6 +394,7 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// // this point, `s` must no longer be used, as the underlying memory has /// // been freed. /// s2 = String::default(); +/// assert_eq!(s2, ""); /// /// // Assigning to `s` would cause the old value to be dropped again, /// // resulting in undefined behavior. From 9e93c6b792bd4c99bffea8fb1a83bc85ea293db9 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 28 May 2018 17:41:19 -0700 Subject: [PATCH 05/13] Update docs for `swap_nonoverlapping` They closely mirror the docs for `copy_nonoverlapping` --- src/libcore/ptr.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 3713f705ab170..850de7f76021b 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -226,12 +226,28 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { mem::forget(tmp); } -/// Swaps a sequence of values at two mutable locations of the same type. +/// Swaps `count * size_of::()` bytes between the two regions of memory +/// beginning at `x` and `y`. The two regions must *not* overlap. /// /// # Safety /// -/// The two arguments must each point to the beginning of `count` locations -/// of valid memory, and the two memory ranges must not overlap. +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * Both `x` and `y` must be [valid]. +/// +/// * Both `x` and `y` must be properly aligned. +/// +/// * `x.offset(count)` must be [valid]. In other words, the region of memory +/// which begins at `x` and has a length of `count * size_of::()` bytes +/// must belong to a single, live allocation. +/// +/// * `y.offset(count)` must be [valid]. In other words, the region of memory +/// which begins at `y` and has a length of `count * size_of::()` bytes +/// must belong to a single, live allocation. +/// +/// * The two regions of memory must *not* overlap. +/// +/// [valid]: ../ptr/index.html#safety /// /// # Examples /// From 29c8358f4a11f9ccb7bd40a26ffd0c67e118ea36 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 5 Jun 2018 11:22:40 -0700 Subject: [PATCH 06/13] Reword module level docs re: alignment --- src/libcore/ptr.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 850de7f76021b..697588e46f211 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -25,10 +25,10 @@ //! freed). //! * The pointer satisfies [LLVM's pointer aliasing rules]. //! -//! Valid pointers are not necessarily properly aligned. However, except for -//! [`read_unaligned`] and [`write_unaligned`], most functions require their -//! arguments to be aligned. Any alignment requirements will be explicitly -//! stated in the function's documentation. +//! Valid pointers are not necessarily properly aligned. However, most functions +//! require their arguments to be properly aligned, and will explicitly state +//! this requirement in the `Safety` section. Notable exceptions to this are +//! [`read_unaligned`] and [`write_unaligned`]. //! //! [LLVM's pointer aliasing rules]: https://llvm.org/docs/LangRef.html#pointer-aliasing-rules //! [`read_unaligned`]: ./fn.read_unaligned.html From 086e1e4f65c64d7a6d48bd9def90fa1a8f0a417a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 5 Jun 2018 13:17:24 -0700 Subject: [PATCH 07/13] Fix off-by-one error when specifying a valid range --- src/libcore/intrinsics.rs | 10 +++++----- src/libcore/ptr.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index e6ac6c4ccef5d..c1ac20c47bfaa 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -980,11 +980,11 @@ extern "rust-intrinsic" { /// /// * Both `src` and `dst` must be properly aligned. /// - /// * `src.offset(count)` must be [valid]. In other words, the region of + /// * `src.offset(count-1)` must be [valid]. In other words, the region of /// memory which begins at `src` and has a length of `count * /// size_of::()` bytes must belong to a single, live allocation. /// - /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// * `dst.offset(count-1)` must be [valid]. In other words, the region of /// memory which begins at `dst` and has a length of `count * /// size_of::()` bytes must belong to a single, live allocation. /// @@ -1068,11 +1068,11 @@ extern "rust-intrinsic" { /// /// * Both `src` and `dst` must be properly aligned. /// - /// * `src.offset(count)` must be [valid]. In other words, the region of + /// * `src.offset(count-1)` must be [valid]. In other words, the region of /// memory which begins at `src` and has a length of `count * /// size_of::()` bytes must belong to a single, live allocation. /// - /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// * `dst.offset(count-1)` must be [valid]. In other words, the region of /// memory which begins at `dst` and has a length of `count * /// size_of::()` bytes must belong to a single, live allocation. /// @@ -1118,7 +1118,7 @@ extern "rust-intrinsic" { /// /// * `dst` must be [valid]. /// - /// * `dst.offset(count)` must be [valid]. In other words, the region of + /// * `dst.offset(count-1)` must be [valid]. In other words, the region of /// memory which begins at `dst` and has a length of `count * /// size_of::()` bytes must belong to a single, live allocation. /// diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 697588e46f211..10bb7d6d1f6e8 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -237,11 +237,11 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { /// /// * Both `x` and `y` must be properly aligned. /// -/// * `x.offset(count)` must be [valid]. In other words, the region of memory +/// * `x.offset(count-1)` must be [valid]. In other words, the region of memory /// which begins at `x` and has a length of `count * size_of::()` bytes /// must belong to a single, live allocation. /// -/// * `y.offset(count)` must be [valid]. In other words, the region of memory +/// * `y.offset(count-1)` must be [valid]. In other words, the region of memory /// which begins at `y` and has a length of `count * size_of::()` bytes /// must belong to a single, live allocation. /// From d31eb2abc2301d6a5752850c76c475b4d45928cf Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 15 Jun 2018 16:00:07 -0700 Subject: [PATCH 08/13] Remove definiton of valid pointer The enumerated list of conditions is replaced by an explanation that rust doesn't have a formal memory model. It does say that pointers created directly from references are guaranteed to be valid, and links to both the "Unsafe Code" section of the book and the "Undefined Behavior" section of the reference. --- src/libcore/ptr.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 10bb7d6d1f6e8..15ec4719815ca 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -17,20 +17,27 @@ //! # Safety //! //! Many functions in this module take raw pointers as arguments and dereference -//! them. For this to be safe, these pointers must be valid. A valid pointer -//! is one that satisfies **all** of the following conditions: +//! them. For this to be safe, these pointers must be valid. However, because +//! rust does not yet have a formal memory model, determining whether an +//! arbitrary pointer is a valid one can be tricky. One thing is certain: +//! creating a raw pointer from a reference (e.g. `&x as *const _`) *always* +//! results in a valid pointer. By exploiting this—and by taking care when +//! using [pointer arithmetic]—users can be confident in the correctness of +//! their unsafe code. //! -//! * The pointer is not null. -//! * The pointer is not dangling (it does not point to memory which has been -//! freed). -//! * The pointer satisfies [LLVM's pointer aliasing rules]. +//! For more information on dereferencing raw pointers, see the both the [book] +//! and the section in the reference devoted to [undefined behavior][ub]. +//! +//! ## Alignment //! //! Valid pointers are not necessarily properly aligned. However, most functions //! require their arguments to be properly aligned, and will explicitly state //! this requirement in the `Safety` section. Notable exceptions to this are //! [`read_unaligned`] and [`write_unaligned`]. //! -//! [LLVM's pointer aliasing rules]: https://llvm.org/docs/LangRef.html#pointer-aliasing-rules +//! [ub]: ../../reference/behavior-considered-undefined.html +//! [book]: ../../book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer +//! [pointer arithmetic]: ../../std/primitive.pointer.html#method.offset //! [`read_unaligned`]: ./fn.read_unaligned.html //! [`write_unaligned`]: ./fn.write_unaligned.html From af2a716f71ac41aa01d9b14a98f814b21bb4885c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 15 Jun 2018 22:33:00 -0700 Subject: [PATCH 09/13] Redefine range validity Uses `x.offset(i)` must be valid for all `i` in `0..count`. --- src/libcore/intrinsics.rs | 43 +++++++++++++++++++-------------------- src/libcore/ptr.rs | 14 ++++++------- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index c1ac20c47bfaa..d8ab71602d7c6 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -976,17 +976,17 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * Both `src` and `dst` must be [valid]. - /// /// * Both `src` and `dst` must be properly aligned. /// - /// * `src.offset(count-1)` must be [valid]. In other words, the region of - /// memory which begins at `src` and has a length of `count * - /// size_of::()` bytes must belong to a single, live allocation. + /// * `src.offset(i)` must be [valid] for all `i` in `0..count`. In other + /// words, the region of memory which begins at `src` and has a length of + /// `count * size_of::()` bytes must belong to a single, live + /// allocation. /// - /// * `dst.offset(count-1)` must be [valid]. In other words, the region of - /// memory which begins at `dst` and has a length of `count * - /// size_of::()` bytes must belong to a single, live allocation. + /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other + /// words, the region of memory which begins at `dst` and has a length of + /// `count * size_of::()` bytes must belong to a single, live + /// allocation. /// /// * The two regions of memory must *not* overlap. /// @@ -1064,17 +1064,17 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * Both `src` and `dst` must be [valid]. - /// /// * Both `src` and `dst` must be properly aligned. /// - /// * `src.offset(count-1)` must be [valid]. In other words, the region of - /// memory which begins at `src` and has a length of `count * - /// size_of::()` bytes must belong to a single, live allocation. + /// * `src.offset(i)` must be [valid] for all `i` in `0..count`. In other + /// words, the region of memory which begins at `src` and has a length of + /// `count * size_of::()` bytes must belong to a single, live + /// allocation. /// - /// * `dst.offset(count-1)` must be [valid]. In other words, the region of - /// memory which begins at `dst` and has a length of `count * - /// size_of::()` bytes must belong to a single, live allocation. + /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other + /// words, the region of memory which begins at `dst` and has a length of + /// `count * size_of::()` bytes must belong to a single, live + /// allocation. /// /// Like [`read`], `copy` creates a bitwise copy of `T`, regardless of /// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the values @@ -1116,14 +1116,13 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * `dst` must be [valid]. - /// - /// * `dst.offset(count-1)` must be [valid]. In other words, the region of - /// memory which begins at `dst` and has a length of `count * - /// size_of::()` bytes must belong to a single, live allocation. - /// /// * `dst` must be properly aligned. /// + /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other + /// words, the region of memory which begins at `dst` and has a length of + /// `count * size_of::()` bytes must belong to a single, live + /// allocation. + /// /// Additionally, the caller must ensure that writing `count * /// size_of::()` bytes to the given region of memory results in a valid /// value of `T`. Creating an invalid value of `T` can result in undefined diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 15ec4719815ca..51a6e1c2ac3b4 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -240,17 +240,15 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * Both `x` and `y` must be [valid]. -/// /// * Both `x` and `y` must be properly aligned. /// -/// * `x.offset(count-1)` must be [valid]. In other words, the region of memory -/// which begins at `x` and has a length of `count * size_of::()` bytes -/// must belong to a single, live allocation. +/// * `x.offset(i)` must be [valid] for all `i` in `0..count`. In other words, +/// the region of memory which begins at `x` and has a length of `count * +/// size_of::()` bytes must belong to a single, live allocation. /// -/// * `y.offset(count-1)` must be [valid]. In other words, the region of memory -/// which begins at `y` and has a length of `count * size_of::()` bytes -/// must belong to a single, live allocation. +/// * `y.offset(i)` must be [valid] for all `i` in `0..count`. In other words, +/// the region of memory which begins at `y` and has a length of `count * +/// size_of::()` bytes must belong to a single, live allocation. /// /// * The two regions of memory must *not* overlap. /// From 2031652fd2e8bd0f0efdf715f571e2f189c444b7 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 17 Jun 2018 02:45:38 -0700 Subject: [PATCH 10/13] Incorporate RalfJung's suggestions This splits "valid" into "valid for reads" and "valid for writes", and also adds the concept of operation size to validity. Now functions which operate on sequences state that e.g. pointer args must be "valid for reads of size x". --- src/libcore/intrinsics.rs | 35 +++++++-------------- src/libcore/ptr.rs | 66 ++++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index d8ab71602d7c6..db8c880c5bd18 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -976,19 +976,15 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * Both `src` and `dst` must be properly aligned. + /// * `src` must be [valid] for reads of `count * size_of::()` bytes. /// - /// * `src.offset(i)` must be [valid] for all `i` in `0..count`. In other - /// words, the region of memory which begins at `src` and has a length of - /// `count * size_of::()` bytes must belong to a single, live - /// allocation. + /// * `dst` must be [valid] for writes of `count * size_of::()` bytes. /// - /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other - /// words, the region of memory which begins at `dst` and has a length of - /// `count * size_of::()` bytes must belong to a single, live - /// allocation. + /// * Both `src` and `dst` must be properly aligned. /// - /// * The two regions of memory must *not* overlap. + /// * The region of memory beginning at `src` with a size of `count * + /// size_of::()` bytes must *not* overlap with the region of memory + /// beginning at `dst` with the same size. /// /// Like [`read`], `copy` creates a bitwise copy of `T`, regardless of /// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the values @@ -1064,17 +1060,11 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * Both `src` and `dst` must be properly aligned. + /// * `src` must be [valid] for reads of `count * size_of::()` bytes. /// - /// * `src.offset(i)` must be [valid] for all `i` in `0..count`. In other - /// words, the region of memory which begins at `src` and has a length of - /// `count * size_of::()` bytes must belong to a single, live - /// allocation. + /// * `dst` must be [valid] for writes of `count * size_of::()` bytes. /// - /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other - /// words, the region of memory which begins at `dst` and has a length of - /// `count * size_of::()` bytes must belong to a single, live - /// allocation. + /// * 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 @@ -1116,12 +1106,9 @@ extern "rust-intrinsic" { /// /// Behavior is undefined if any of the following conditions are violated: /// - /// * `dst` must be properly aligned. + /// * `dst` must be [valid] for writes of `count * size_of::()` bytes. /// - /// * `dst.offset(i)` must be [valid] for all `i` in `0..count`. In other - /// words, the region of memory which begins at `dst` and has a length of - /// `count * size_of::()` bytes must belong to a single, live - /// allocation. + /// * `dst` must be properly aligned. /// /// Additionally, the caller must ensure that writing `count * /// size_of::()` bytes to the given region of memory results in a valid diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 51a6e1c2ac3b4..8fea8977e0a83 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -19,25 +19,38 @@ //! Many functions in this module take raw pointers as arguments and dereference //! them. For this to be safe, these pointers must be valid. However, because //! rust does not yet have a formal memory model, determining whether an -//! arbitrary pointer is a valid one can be tricky. One thing is certain: -//! creating a raw pointer from a reference (e.g. `&x as *const _`) *always* -//! results in a valid pointer. By exploiting this—and by taking care when -//! using [pointer arithmetic]—users can be confident in the correctness of -//! their unsafe code. +//! arbitrary pointer is valid for a given operation can be tricky. //! -//! For more information on dereferencing raw pointers, see the both the [book] -//! and the section in the reference devoted to [undefined behavior][ub]. +//! There are two types of operations on memory, reads and writes. It is +//! possible for a `*mut` to be valid for one operation and not the other. Since +//! a `*const` can only be read and not written, it has no such ambiguity. For +//! example, a `*mut` is not valid for writes if a a reference exists which +//! [refers to the same memory][aliasing]. Therefore, each function in this +//! module will document which operations must be valid on any `*mut` arguments. +//! +//! Additionally, some functions (e.g. [`copy`]) take a single pointer but +//! operate on many values. In this case, the function will state the size of +//! the operation for which the pointer must be valid. For example, +//! `copy::(&src, &mut dst, 3)` requires `dst` to be valid for writes of +//! `size_of::() * 3` bytes. When the documentation requires that a pointer +//! be valid for an operation but omits the size of that operation, the size is +//! implied to be `size_of::()` bytes. +//! +//! For more information on the safety implications of dereferencing raw +//! pointers, see the both the [book] and the section in the reference devoted +//! to [undefined behavior][ub]. //! //! ## Alignment //! //! Valid pointers are not necessarily properly aligned. However, most functions //! require their arguments to be properly aligned, and will explicitly state -//! this requirement in the `Safety` section. Notable exceptions to this are +//! this requirement in their documentation. Notable exceptions to this are //! [`read_unaligned`] and [`write_unaligned`]. //! -//! [ub]: ../../reference/behavior-considered-undefined.html +//! [aliasing]: ../../nomicon/aliasing.html //! [book]: ../../book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer -//! [pointer arithmetic]: ../../std/primitive.pointer.html#method.offset +//! [ub]: ../../reference/behavior-considered-undefined.html +//! [`copy`]: ./fn.copy.html //! [`read_unaligned`]: ./fn.read_unaligned.html //! [`write_unaligned`]: ./fn.write_unaligned.html @@ -83,7 +96,7 @@ pub use intrinsics::write_bytes; /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `to_drop` must be [valid]. +/// * `to_drop` must be [valid] for reads. /// /// * `to_drop` must be properly aligned. /// @@ -178,7 +191,7 @@ pub const fn null_mut() -> *mut T { 0 as *mut T } /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * Both `x` and `y` must be [valid]. +/// * Both `x` and `y` must be [valid] for reads and writes. /// /// * Both `x` and `y` must be properly aligned. /// @@ -240,17 +253,14 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * Both `x` and `y` must be properly aligned. -/// -/// * `x.offset(i)` must be [valid] for all `i` in `0..count`. In other words, -/// the region of memory which begins at `x` and has a length of `count * -/// size_of::()` bytes must belong to a single, live allocation. +/// * Both `x` and `y` must be [valid] for reads and writes of `count * +/// size_of::()` bytes. /// -/// * `y.offset(i)` must be [valid] for all `i` in `0..count`. In other words, -/// the region of memory which begins at `y` and has a length of `count * -/// size_of::()` bytes must belong to a single, live allocation. +/// * Both `x` and `y` must be properly aligned. /// -/// * The two regions of memory must *not* overlap. +/// * The region of memory beginning at `x` with a size of `count * +/// size_of::()` bytes must *not* overlap with the region of memory +/// beginning at `y` with the same size. /// /// [valid]: ../ptr/index.html#safety /// @@ -346,7 +356,7 @@ unsafe fn swap_nonoverlapping_bytes(x: *mut u8, y: *mut u8, len: usize) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `dest` must be [valid]. +/// * `dest` must be [valid] for writes. /// /// * `dest` must be properly aligned. /// @@ -382,7 +392,7 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `src` must be [valid]. +/// * `src` must be [valid] for reads. /// /// * `src` must be properly aligned. Use [`read_unaligned`] if this is not the /// case. @@ -495,7 +505,7 @@ pub unsafe fn read(src: *const T) -> T { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `src` must be [valid]. +/// * `src` must be [valid] for reads. /// /// Like [`read`], `read_unaligned` creates a bitwise copy of `T`, regardless of /// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned @@ -572,7 +582,7 @@ pub unsafe fn read_unaligned(src: *const T) -> T { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `dst` must be [valid]. +/// * `dst` must be [valid] for writes. /// /// * `dst` must be properly aligned. Use [`write_unaligned`] if this is not the /// case. @@ -646,7 +656,7 @@ pub unsafe fn write(dst: *mut T, src: T) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `dst` must be [valid]. +/// * `dst` must be [valid] for writes. /// /// [valid]: ../ptr/index.html#safety /// @@ -721,7 +731,7 @@ pub unsafe fn write_unaligned(dst: *mut T, src: T) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `src` must be [valid]. +/// * `src` must be [valid] for reads. /// /// * `src` must be properly aligned. /// @@ -790,7 +800,7 @@ pub unsafe fn read_volatile(src: *const T) -> T { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `dst` must be [valid]. +/// * `dst` must be [valid] for writes. /// /// * `dst` must be properly aligned. /// From 0ff407e5d21280963640d29acd183918db162381 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 17 Jun 2018 10:44:29 -0700 Subject: [PATCH 11/13] You can't make an omlette without breaking a few links --- src/libcore/ptr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 8fea8977e0a83..da7d29679b6dc 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -50,7 +50,7 @@ //! [aliasing]: ../../nomicon/aliasing.html //! [book]: ../../book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer //! [ub]: ../../reference/behavior-considered-undefined.html -//! [`copy`]: ./fn.copy.html +//! [`copy`]: ../../std/ptr/fn.copy.html //! [`read_unaligned`]: ./fn.read_unaligned.html //! [`write_unaligned`]: ./fn.write_unaligned.html From 5b6a3ebc2e3c78d8f07771400cd94255ceb0f7c6 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 4 Jul 2018 11:30:23 -0700 Subject: [PATCH 12/13] Add a list of known facts re: validity Also rewrites the reads/writes section to be less reliant on `*const`, `*mut` --- src/libcore/ptr.rs | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index da7d29679b6dc..a1f7c1481697a 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -21,14 +21,15 @@ //! rust does not yet have a formal memory model, determining whether an //! arbitrary pointer is valid for a given operation can be tricky. //! -//! There are two types of operations on memory, reads and writes. It is -//! possible for a `*mut` to be valid for one operation and not the other. Since -//! a `*const` can only be read and not written, it has no such ambiguity. For -//! example, a `*mut` is not valid for writes if a a reference exists which -//! [refers to the same memory][aliasing]. Therefore, each function in this -//! module will document which operations must be valid on any `*mut` arguments. +//! There are two types of operations on memory, reads and writes. A single +//! pointer can be valid for any combination of these operations. For example, a +//! pointer is not valid for writes if a `&mut` exists which [refers to the same +//! memory][aliasing]. The set of operations for which a pointer argument must +//! be valid is explicitly documented for each function. This is not strictly +//! necessary for `*const` arguments, as they can only be used for reads and +//! never for writes. //! -//! Additionally, some functions (e.g. [`copy`]) take a single pointer but +//! Some functions (e.g. [`copy`]) take a single pointer but //! operate on many values. In this case, the function will state the size of //! the operation for which the pointer must be valid. For example, //! `copy::(&src, &mut dst, 3)` requires `dst` to be valid for writes of @@ -36,8 +37,21 @@ //! be valid for an operation but omits the size of that operation, the size is //! implied to be `size_of::()` bytes. //! -//! For more information on the safety implications of dereferencing raw -//! pointers, see the both the [book] and the section in the reference devoted +//! While we can't yet define whether an arbitrary pointer is a valid one, there +//! are a few rules regarding validity: +//! +//! * The result of casting a reference to a pointer is valid for as long as the +//! underlying object is live. +//! * All pointers to types with a [size of zero][zst] are valid for all +//! operations of size zero. +//! * A [null] pointer is *never* valid, except when it points to a zero-sized +//! type. +//! +//! These axioms, along with careful use of [`offset`] for pointer arithmentic, +//! are enough to correctly implement many useful things in unsafe code. Still, +//! unsafe code should be carefully examined since some of the finer +//! details—notably the [aliasing] rules—are not yet settled. For more +//! information, see the [book] as well as the section in the reference devoted //! to [undefined behavior][ub]. //! //! ## Alignment @@ -50,7 +64,10 @@ //! [aliasing]: ../../nomicon/aliasing.html //! [book]: ../../book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer //! [ub]: ../../reference/behavior-considered-undefined.html +//! [null]: ./fn.null.html +//! [zst]: ../../nomicon/exotic-sizes.html#zero-sized-types-zsts //! [`copy`]: ../../std/ptr/fn.copy.html +//! [`offset`]: ../../std/primitive.pointer.html#method.offset //! [`read_unaligned`]: ./fn.read_unaligned.html //! [`write_unaligned`]: ./fn.write_unaligned.html From 46407edbbd94c559d6b51b08f64748d86a492239 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 4 Jul 2018 12:02:26 -0700 Subject: [PATCH 13/13] Resolve null/ZST conflict correctly (whoops) --- src/libcore/ptr.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index a1f7c1481697a..03ec3c6fbfdb5 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -42,10 +42,9 @@ //! //! * The result of casting a reference to a pointer is valid for as long as the //! underlying object is live. -//! * All pointers to types with a [size of zero][zst] are valid for all -//! operations of size zero. -//! * A [null] pointer is *never* valid, except when it points to a zero-sized -//! type. +//! * A [null] pointer is *never* valid. +//! * All pointers (except for the null pointer) are valid for all operations of +//! [size zero][zst]. //! //! These axioms, along with careful use of [`offset`] for pointer arithmentic, //! are enough to correctly implement many useful things in unsafe code. Still,