From b4a11d4dd971be4a2e055e4f92fece030615a692 Mon Sep 17 00:00:00 2001 From: cohenarthur Date: Thu, 23 Apr 2020 22:22:27 +0200 Subject: [PATCH 1/2] doc-example: Refactor pointer name to avoid confusion Changed raw pointer name from ptr to raw_pointer to avoid confusion with the `use std::ptr` statement a few lines above. This way the crate name and pointer name are well differenciated. --- src/libcore/ptr/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 84f28488c74b6..54d44222d3f57 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -65,8 +65,6 @@ //! [`write_volatile`]: ./fn.write_volatile.html //! [`NonNull::dangling`]: ./struct.NonNull.html#method.dangling -// ignore-tidy-undocumented-unsafe - #![stable(feature = "rust1", since = "1.0.0")] use crate::cmp::Ordering; @@ -248,8 +246,8 @@ pub(crate) struct FatPtr { /// /// // create a slice pointer when starting out with a pointer to the first element /// let x = [5, 6, 7]; -/// let ptr = x.as_ptr(); -/// let slice = ptr::slice_from_raw_parts(ptr, 3); +/// let raw_pointer = x.as_ptr(); +/// let slice = ptr::slice_from_raw_parts(raw_pointer, 3); /// assert_eq!(unsafe { &*slice }[2], 7); /// ``` #[inline] From 8558ccd5c4d3d7ca78aaff4a81c48f7a04b0f1f2 Mon Sep 17 00:00:00 2001 From: cohenarthur Date: Thu, 23 Apr 2020 22:32:20 +0200 Subject: [PATCH 2/2] safety-ptr: Add SAFETY on some unsafe blocks from libcore/ptr Add documentation example to slice_from_raw_parts_mut() Add SAFETY explanations to some unsafe blocks in libcore/ptr * libcore/ptr/mod.rs * libcore/ptr/unique.rs * libcore/ptr/non_null.rs safety-mod.rs: Add SAFETY to slice_from_raw_parts(), slice_from_raw_parts_mut() slice_from_raw_parts_mut: Add documentation example safety-ptr-unique.rs: Add SAFETY to new() and cast() safety-ptr-non_null.rs: Add SAFETY to new() safety-ptr-non_null.rs: Add SAFETY to cast() safety-ptr-non_null.rs: Add SAFETY to from() impls safety-ptr-unique.rs: Add SAFETY to from() impls safety-ptr-non_null.rs: Add SAFETY to new() safety-ptr-unique.rs: Add SAFETY to new() safety-ptr-mod.rs: Fix safety explanation https://github.com/rust-lang/rust/pull/71507#discussion_r414488884 safety-prt-non_null.rs: Fix SAFETY comment syntax safety-ptr-unique.rs: Fix syntax for empty() safety-ptr-non_null.rs: Fix misuse of non-null for align_of() safety-ptr-non_null.rs: Remove incorrect SAFETY comment safety-ptr-unique.rs: Remove unsound SAFETY comment safety-ptr-mod.rs: Add std comment on slice_from_raw_parts guarantee safety-ptr-unique.rs: Remove incorrect safety comment Creating a Unique from a NonNull has strict guarantees that the current implementation does not guarantee https://github.com/rust-lang/rust/pull/71507#discussion_r415035952 safety-ptr: Re-adding ignore-tidy directive --- src/libcore/ptr/mod.rs | 21 +++++++++++++++++++++ src/libcore/ptr/non_null.rs | 18 +++++++++++++++--- src/libcore/ptr/unique.rs | 7 +++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 54d44222d3f57..58f779106f716 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -254,6 +254,9 @@ pub(crate) struct FatPtr { #[stable(feature = "slice_from_raw_parts", since = "1.42.0")] #[rustc_const_unstable(feature = "const_slice_from_raw_parts", issue = "67456")] pub const fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { + // SAFETY: Accessing the value from the `Repr` union is safe since *const [T] + // and FatPtr have the same memory layouts. Only std can make this + // guarantee. unsafe { Repr { raw: FatPtr { data, len } }.rust } } @@ -267,10 +270,28 @@ pub const fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { /// /// [`slice_from_raw_parts`]: fn.slice_from_raw_parts.html /// [`from_raw_parts_mut`]: ../../std/slice/fn.from_raw_parts_mut.html +/// +/// # Examples +/// +/// ```rust +/// use std::ptr; +/// +/// let x = &mut [5, 6, 7]; +/// let raw_pointer = x.as_mut_ptr(); +/// let slice = ptr::slice_from_raw_parts_mut(raw_pointer, 3); +/// +/// unsafe { +/// (*slice)[2] = 99; // assign a value at an index in the slice +/// }; +/// +/// assert_eq!(unsafe { &*slice }[2], 99); +/// ``` #[inline] #[stable(feature = "slice_from_raw_parts", since = "1.42.0")] #[rustc_const_unstable(feature = "const_slice_from_raw_parts", issue = "67456")] pub const fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { + // SAFETY: Accessing the value from the `Repr` union is safe since *mut [T] + // and FatPtr have the same memory layouts unsafe { Repr { raw: FatPtr { data, len } }.rust_mut } } diff --git a/src/libcore/ptr/non_null.rs b/src/libcore/ptr/non_null.rs index 626e58d49306e..7d08503215ed0 100644 --- a/src/libcore/ptr/non_null.rs +++ b/src/libcore/ptr/non_null.rs @@ -7,8 +7,6 @@ use crate::mem; use crate::ops::{CoerceUnsized, DispatchFromDyn}; use crate::ptr::Unique; -// ignore-tidy-undocumented-unsafe - /// `*mut T` but non-zero and covariant. /// /// This is often the correct thing to use when building data structures using @@ -69,6 +67,9 @@ impl NonNull { #[rustc_const_stable(feature = "const_nonnull_dangling", since = "1.32.0")] #[inline] pub const fn dangling() -> Self { + // SAFETY: mem::align_of() returns a non-zero usize which is then casted + // to a *mut T. Therefore, `ptr` is not null and the conditions for + // calling new_unchecked() are respected. unsafe { let ptr = mem::align_of::() as *mut T; NonNull::new_unchecked(ptr) @@ -93,7 +94,12 @@ impl NonNull { #[stable(feature = "nonnull", since = "1.25.0")] #[inline] pub fn new(ptr: *mut T) -> Option { - if !ptr.is_null() { Some(unsafe { Self::new_unchecked(ptr) }) } else { None } + if !ptr.is_null() { + // SAFETY: The pointer is already checked and is not null + Some(unsafe { Self::new_unchecked(ptr) }) + } else { + None + } } /// Acquires the underlying `*mut` pointer. @@ -131,6 +137,7 @@ impl NonNull { #[rustc_const_stable(feature = "const_nonnull_cast", since = "1.32.0")] #[inline] pub const fn cast(self) -> NonNull { + // SAFETY: `self` is a `NonNull` pointer which is necessarily non-null unsafe { NonNull::new_unchecked(self.as_ptr() as *mut U) } } } @@ -205,6 +212,8 @@ impl hash::Hash for NonNull { impl From> for NonNull { #[inline] fn from(unique: Unique) -> Self { + // SAFETY: A Unique pointer cannot be null, so the conditions for + // new_unchecked() are respected. unsafe { NonNull::new_unchecked(unique.as_ptr()) } } } @@ -213,6 +222,7 @@ impl From> for NonNull { impl From<&mut T> for NonNull { #[inline] fn from(reference: &mut T) -> Self { + // SAFETY: A mutable reference cannot be null. unsafe { NonNull { pointer: reference as *mut T } } } } @@ -221,6 +231,8 @@ impl From<&mut T> for NonNull { impl From<&T> for NonNull { #[inline] fn from(reference: &T) -> Self { + // SAFETY: A reference cannot be null, so the conditions for + // new_unchecked() are respected. unsafe { NonNull { pointer: reference as *const T } } } } diff --git a/src/libcore/ptr/unique.rs b/src/libcore/ptr/unique.rs index 87b56d951c6ce..7d823f91fecb8 100644 --- a/src/libcore/ptr/unique.rs +++ b/src/libcore/ptr/unique.rs @@ -74,6 +74,8 @@ impl Unique { // FIXME: rename to dangling() to match NonNull? #[inline] pub const fn empty() -> Self { + // SAFETY: mem::align_of() returns a valid, non-null pointer. The + // conditions to call new_unchecked() are thus respected. unsafe { Unique::new_unchecked(mem::align_of::() as *mut T) } } } @@ -94,6 +96,7 @@ impl Unique { #[inline] pub fn new(ptr: *mut T) -> Option { if !ptr.is_null() { + // SAFETY: The pointer has already been checked and is not null. Some(unsafe { Unique { pointer: ptr as _, _marker: PhantomData } }) } else { None @@ -129,6 +132,9 @@ impl Unique { /// Casts to a pointer of another type. #[inline] pub const fn cast(self) -> Unique { + // SAFETY: Unique::new_unchecked() creates a new unique and needs + // the given pointer to not be null. + // Since we are passing self as a pointer, it cannot be null. unsafe { Unique::new_unchecked(self.as_ptr() as *mut U) } } } @@ -168,6 +174,7 @@ impl fmt::Pointer for Unique { impl From<&mut T> for Unique { #[inline] fn from(reference: &mut T) -> Self { + // SAFETY: A mutable reference cannot be null unsafe { Unique { pointer: reference as *mut T, _marker: PhantomData } } } }