From 82dc3aa5fb6642fe0450305015ee68e0c2d1d492 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:32:47 +0100 Subject: [PATCH 1/5] link from raw slice creation methods to safety requirements --- src/libcore/ptr/mod.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 1355ce1aa43b7..649244a468305 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -221,10 +221,15 @@ pub(crate) struct FatPtr { pub(crate) len: usize, } -/// Forms a slice from a pointer and a length. +/// Forms a raw slice from a pointer and a length. /// /// The `len` argument is the number of **elements**, not the number of bytes. /// +/// This function is safe, but actually using the return value is unsafe. +/// See the documentation of [`from_raw_parts`] for slice safety requirements. +/// +/// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html +/// /// # Examples /// /// ```rust @@ -243,12 +248,16 @@ pub fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { unsafe { Repr { raw: FatPtr { data, len } }.rust } } -/// Performs the same functionality as [`from_raw_parts`], except that a -/// mutable slice is returned. +/// Performs the same functionality as [`slice_from_raw_parts`], except that a +/// raw mutable slice is returned. /// -/// See the documentation of [`from_raw_parts`] for more details. +/// See the documentation of [`slice_from_raw_parts`] for more details. /// -/// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html +/// This function is safe, but actually using the return value is unsafe. +/// See the documentation of [`from_raw_parts_mut`] for slice safety requirements. +/// +/// [`slice_from_raw_parts`]: fn.slice_from_raw_parts.html +/// [`from_raw_parts_mut`]: ../../std/slice/fn.from_raw_parts_mut.html #[inline] #[unstable(feature = "slice_from_raw_parts", reason = "recently added", issue = "36925")] pub fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { From 1a254e4f434e0cbb9ffcb24971416cb574df4751 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:55:33 +0100 Subject: [PATCH 2/5] expand slice from_raw_part docs --- src/libcore/ptr/mod.rs | 4 +++ src/libcore/slice/mod.rs | 60 ++++++++++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 649244a468305..407f8a6218e4f 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -18,6 +18,10 @@ //! * A [null] pointer is *never* valid, not even for accesses of [size zero][zst]. //! * All pointers (except for the null pointer) are valid for all operations of //! [size zero][zst]. +//! * For a pointer to be valid, it is necessary (but not always sufficient) that the pointer +//! be *dereferencable*: the memory range of the given size starting at the pointer must all be +//! within the bounds of a single allocated object. Note that in Rust, +//! every (stack-allocated) variable is considered a separate allocated object. //! * All accesses performed by functions in this module are *non-atomic* in the sense //! of [atomic operations] used to synchronize between threads. This means it is //! undefined behavior to perform two concurrent accesses to the same location from different diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index cdada1252d2bf..a7b759f523103 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -5272,18 +5272,24 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// /// # Safety /// -/// This function is unsafe as there is no guarantee that the given pointer is -/// valid for `len` elements, nor whether the lifetime inferred is a suitable -/// lifetime for the returned slice. +/// Behavior is undefined if any of the following conditions are violated: /// -/// `data` must be non-null and aligned, even for zero-length slices. One -/// reason for this is that enum layout optimizations may rely on references -/// (including slices of any length) being aligned and non-null to distinguish -/// them from other data. You can obtain a pointer that is usable as `data` -/// for zero-length slices using [`NonNull::dangling()`]. +/// * `data` must be [valid] for reads for `len * mem::size_of::()` many bytes, +/// and it must be properly aligned. This means in particular: /// -/// The total size of the slice must be no larger than `isize::MAX` **bytes** -/// in memory. See the safety documentation of [`pointer::offset`]. +/// * The entire memory range of this slice must be contained within a single allocated object! +/// Slices can never span across multiple allocated objects. +/// * `data` must be non-null and aligned even for zero-length slices. One +/// reason for this is that enum layout optimizations may rely on references +/// (including slices of any length) being aligned and non-null to distinguish +/// them from other data. You can obtain a pointer that is usable as `data` +/// for zero-length slices using [`NonNull::dangling()`]. +/// +/// * The memory referenced by the returned slice must not be mutated for the duration +/// of lifetime `'a`, except inside an `UnsafeCell`. +/// +/// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. +/// See the safety documentation of [`pointer::offset`]. /// /// # Caveat /// @@ -5305,6 +5311,7 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// assert_eq!(slice[0], 42); /// ``` /// +/// [valid]: ../ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset #[inline] @@ -5312,28 +5319,45 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, - "attempt to create slice covering half the address space"); + "attempt to create slice covering at least half the address space"); &*ptr::slice_from_raw_parts(data, len) } /// Performs the same functionality as [`from_raw_parts`], except that a /// mutable slice is returned. /// -/// This function is unsafe for the same reasons as [`from_raw_parts`], as well -/// as not being able to provide a non-aliasing guarantee of the returned -/// mutable slice. `data` must be non-null and aligned even for zero-length -/// slices as with [`from_raw_parts`]. The total size of the slice must be no -/// larger than `isize::MAX` **bytes** in memory. +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `data` must be [valid] for writes for `len * mem::size_of::()` many bytes, +/// and it must be properly aligned. This means in particular: /// -/// See the documentation of [`from_raw_parts`] for more details. +/// * The entire memory range of this slice must be contained within a single allocated object! +/// Slices can never span across multiple allocated objects. +/// * `data` must be non-null and aligned even for zero-length slices. One +/// reason for this is that enum layout optimizations may rely on references +/// (including slices of any length) being aligned and non-null to distinguish +/// them from other data. You can obtain a pointer that is usable as `data` +/// for zero-length slices using [`NonNull::dangling()`]. /// +/// * The memory referenced by the returned slice must not be accessed through any other pointer +/// (not derived from the return value) for the duration of lifetime `'a`. +/// Both read and write accesses are forbidden. +/// +/// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. +/// See the safety documentation of [`pointer::offset`]. +/// +/// [valid]: ../ptr/index.html#safety +/// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling +/// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset /// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] { debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, - "attempt to create slice covering half the address space"); + "attempt to create slice covering at least half the address space"); &mut *ptr::slice_from_raw_parts_mut(data, len) } From 6a1f303b98fe77775f3f201bef792a98e31e79e8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:57:52 +0100 Subject: [PATCH 3/5] also edit String::from_raw_parts while we are at it --- src/liballoc/string.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index d9927c642b2d8..62e58b6facd01 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -687,7 +687,7 @@ impl String { /// checked: /// /// * The memory at `ptr` needs to have been previously allocated by the - /// same allocator the standard library uses. + /// same allocator the standard library uses, with a required alignment of exactly 1. /// * `length` needs to be less than or equal to `capacity`. /// * `capacity` needs to be the correct value. /// From b1d0a68fd78f79682121559b3f8225fd56fa1440 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 13:22:43 +0100 Subject: [PATCH 4/5] fix link to ptr docs --- src/libcore/slice/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index a7b759f523103..7655b2f806532 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -5311,7 +5311,7 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// assert_eq!(slice[0], 42); /// ``` /// -/// [valid]: ../ptr/index.html#safety +/// [valid]: ../../std/ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset #[inline] @@ -5348,7 +5348,7 @@ pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { /// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// -/// [valid]: ../ptr/index.html#safety +/// [valid]: ../../std/ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset /// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html From 11a48a0423410376dbe9a6080b41aa90e43cead2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 21:50:55 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/libcore/ptr/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 407f8a6218e4f..1a13ac465f436 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -18,7 +18,7 @@ //! * A [null] pointer is *never* valid, not even for accesses of [size zero][zst]. //! * All pointers (except for the null pointer) are valid for all operations of //! [size zero][zst]. -//! * For a pointer to be valid, it is necessary (but not always sufficient) that the pointer +//! * For a pointer to be valid, it is necessary, but not always sufficient, that the pointer //! be *dereferencable*: the memory range of the given size starting at the pointer must all be //! within the bounds of a single allocated object. Note that in Rust, //! every (stack-allocated) variable is considered a separate allocated object. @@ -253,7 +253,7 @@ pub fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { } /// Performs the same functionality as [`slice_from_raw_parts`], except that a -/// raw mutable slice is returned. +/// raw mutable slice is returned, as opposed to a raw immutable slice. /// /// See the documentation of [`slice_from_raw_parts`] for more details. ///