From 60671268c897cecac8e93e667cfe48cdd848d58b Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Tue, 29 Oct 2019 23:32:51 +0100 Subject: [PATCH 1/4] Improved MaybeUninit::get_{ref,mut} documentation --- src/libcore/mem/maybe_uninit.rs | 159 ++++++++++++++++++++++++++++++-- 1 file changed, 151 insertions(+), 8 deletions(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 792ce9dfad419..e72a922a418f5 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -509,26 +509,169 @@ impl MaybeUninit { self.as_ptr().read() } - /// Gets a reference to the contained value. + /// Gets a shared reference to the contained value. + /// + /// This can be useful when we want to access a `MaybeUninit` that has been + /// initialized but don't have ownership of the `MaybeUninit` (preventing the use + /// of `.assume_init()`). /// /// # Safety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized - /// state. Calling this when the content is not yet fully initialized causes undefined - /// behavior. + /// Calling this when the content is not yet fully initialized causes undefined + /// behavior: it is up to the caller to guarantee that the `MaybeUninit` really + /// is in an initialized state. + /// + /// # Examples + /// + /// ### Correct usage of this method: + /// + /// ```rust + /// use ::std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninit(); + /// // Initialize `x`: + /// unsafe { x.as_mut_ptr().write(vec![1, 2, 3]); } + /// /* The above line can also be done without unsafe: + /// x = MaybeUninit::new(vec![1, 2, 3]); // */ + /// // Now that our `MaybeUninit<_>` is known to be initialized, it is okay to + /// // create a shared reference to it: + /// let x: &Vec = unsafe { + /// // # Safety + /// // + /// // - `x` has been initialized. + /// x.get_ref() + /// }; + /// assert_eq!(x, &vec![1, 2, 3]); + /// ``` + /// + /// ### *Incorrect* usages of this method: + /// + /// ```rust,no_run + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::>::uninit(); + /// let x_vec: &Vec = unsafe { x.get_ref() }; + /// // We have created a reference to an uninitialized vector! This is undefined behavior. + /// ``` + /// + /// ```rust,no_run + /// use std::{cell::Cell, mem::MaybeUninit}; + /// + /// let b = MaybeUninit::>::uninit(); + /// // Initialize the `MaybeUninit` using `Cell::set`: + /// unsafe { + /// b.get_ref().set(true); + /// // ^^^^^^^^^^^ + /// // Reference to an uninitialized `Cell`: UB! + /// } + /// ``` #[unstable(feature = "maybe_uninit_ref", issue = "63568")] #[inline(always)] pub unsafe fn get_ref(&self) -> &T { &*self.value } - /// Gets a mutable reference to the contained value. + /// Gets a mutable (unique) reference to the contained value. + /// + /// This can be useful when we want to access a `MaybeUninit` that has been + /// initialized but don't have ownership of the `MaybeUninit` (preventing the use + /// of `.assume_init()`). /// /// # Safety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized - /// state. Calling this when the content is not yet fully initialized causes undefined - /// behavior. + /// Calling this when the content is not yet fully initialized causes undefined + /// behavior: it is up to the caller to guarantee that the `MaybeUninit` really + /// is in an initialized state. For instance, `.get_mut()` cannot be used to + /// initialize a `MaybeUninit`. + /// + /// # Examples + /// + /// ### Correct usage of this method: + /// + /// ```rust + /// use ::std::mem::MaybeUninit; + /// + /// # unsafe extern "C" fn initialize_buffer (buf: *mut [u8; 2048]) { *buf = [0; 2048] } + /// # #[cfg(FALSE)] + /// extern "C" { + /// /// Initializes *all* the bytes of the input buffer. + /// fn initialize_buffer (buf: *mut [u8; 2048]); + /// } + /// + /// let mut buf = MaybeUninit::<[u8; 2048]>::uninit(); + /// // Initialize `buf`: + /// unsafe { initialize_buffer(buf.as_mut_ptr()); } + /// // Now we know that `buf` has been initialized; so we could `.assume_init()` it. + /// // However, using `.assume_init()` may trigger a `memcpy` of the 2048 bytes. + /// // To assert our buffer has been initialized without copying it, we upgrade + /// // the `&mut MaybeUninit<[u8; 2048]>` to a `&mut [u8; 2048]`: + /// let buf: &mut [u8; 2048] = unsafe { + /// // # Safety + /// // + /// // - `buf` has been initialized. + /// buf.get_mut() + /// }; + /// // Now we can use `buf` as a normal slice: + /// buf.sort_unstable(); + /// assert!(buf.is_sorted()); + /// ``` + /// + /// ### *Incorrect* usages of this method: + /// + /// Do not use `.get_mut()` to initialize a value + /// + /// ```rust,no_run + /// use std::mem::MaybeUninit; + /// + /// let mut b = MaybeUninit::::uninit(); + /// unsafe { + /// *b.get_mut() = true; + /// // We have created a (mutable) reference to an uninitialized `bool`! + /// // This is undefined behavior. + /// } + /// ``` + /// + /// For instance, you cannot [`Read`] into an uninitialized buffer. + /// + /// [`Read`]: https://doc.rust-lang.org/std/io/trait.Read.html + /// + /// ```rust,no_run + /// use std::{io, mem::MaybeUninit}; + /// + /// fn read_chunk (reader: &'_ mut dyn io::Read) -> io::Result<[u8; 64]> + /// { + /// let mut buffer = MaybeUninit::<[u8; 64]>::uninit(); + /// reader.read_exact(unsafe { buffer.get_mut() })?; + /// // ^^^^^^^^^^^^^^^^ + /// // (mutable) reference to uninitialized memory! + /// // This is undefined behavior. + /// Ok(buffer.assume_init()) + /// } + /// ``` + /// + /// Nor can you use direct field access to do field-by-field gradual initialization. + /// + /// ```rust,no_run + /// use std::mem::MaybeUninit; + /// + /// struct Foo { + /// a: u32, + /// b: u8, + /// } + /// + /// let foo: Foo = unsafe { + /// let foo = MaybeUninit::::uninit(); + /// ptr::write(&mut foo.get_mut().a as *mut u32, 1337); + /// // ^^^^^^^^^^^^^ + /// // (mutable) reference to uninitialized memory! + /// // This is undefined behavior. + /// ptr::write(&mut foo.get_mut().b as *mut u8, 42); + /// // ^^^^^^^^^^^^^ + /// // (mutable) reference to uninitialized memory! + /// // This is undefined behavior. + /// foo.assume_init() + /// }; + /// ``` // FIXME(#53491): We currently rely on the above being incorrect, i.e., we have references // to uninitialized data (e.g., in `libcore/fmt/float.rs`). We should make // a final decision about the rules before stabilization. From 2ebf5e6e2f8ff1e5eea56e471303746ec626fb92 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Wed, 30 Oct 2019 11:45:43 +0100 Subject: [PATCH 2/4] Fix doctests --- src/libcore/mem/maybe_uninit.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index e72a922a418f5..60d20735db467 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -526,6 +526,7 @@ impl MaybeUninit { /// ### Correct usage of this method: /// /// ```rust + /// #![feature(maybe_uninit_ref)] /// use ::std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninit(); @@ -547,6 +548,7 @@ impl MaybeUninit { /// ### *Incorrect* usages of this method: /// /// ```rust,no_run + /// #![feature(maybe_uninit_ref)] /// use std::mem::MaybeUninit; /// /// let x = MaybeUninit::>::uninit(); @@ -555,6 +557,7 @@ impl MaybeUninit { /// ``` /// /// ```rust,no_run + /// #![feature(maybe_uninit_ref)] /// use std::{cell::Cell, mem::MaybeUninit}; /// /// let b = MaybeUninit::>::uninit(); @@ -589,6 +592,7 @@ impl MaybeUninit { /// ### Correct usage of this method: /// /// ```rust + /// #![feature(maybe_uninit_ref)] /// use ::std::mem::MaybeUninit; /// /// # unsafe extern "C" fn initialize_buffer (buf: *mut [u8; 2048]) { *buf = [0; 2048] } @@ -599,6 +603,7 @@ impl MaybeUninit { /// } /// /// let mut buf = MaybeUninit::<[u8; 2048]>::uninit(); + /// /// // Initialize `buf`: /// unsafe { initialize_buffer(buf.as_mut_ptr()); } /// // Now we know that `buf` has been initialized; so we could `.assume_init()` it. @@ -611,16 +616,21 @@ impl MaybeUninit { /// // - `buf` has been initialized. /// buf.get_mut() /// }; + /// /// // Now we can use `buf` as a normal slice: /// buf.sort_unstable(); - /// assert!(buf.is_sorted()); + /// assert!( + /// buf.chunks(2).all(|chunk| chunk[0] <= chunk[1]), + /// "buffer is sorted", + /// ); /// ``` /// /// ### *Incorrect* usages of this method: /// - /// Do not use `.get_mut()` to initialize a value + /// You cannot use `.get_mut()` to initialize a value: /// /// ```rust,no_run + /// #![feature(maybe_uninit_ref)] /// use std::mem::MaybeUninit; /// /// let mut b = MaybeUninit::::uninit(); @@ -631,11 +641,12 @@ impl MaybeUninit { /// } /// ``` /// - /// For instance, you cannot [`Read`] into an uninitialized buffer. + /// For instance, you cannot [`Read`] into an uninitialized buffer: /// /// [`Read`]: https://doc.rust-lang.org/std/io/trait.Read.html /// /// ```rust,no_run + /// #![feature(maybe_uninit_ref)] /// use std::{io, mem::MaybeUninit}; /// /// fn read_chunk (reader: &'_ mut dyn io::Read) -> io::Result<[u8; 64]> @@ -645,14 +656,15 @@ impl MaybeUninit { /// // ^^^^^^^^^^^^^^^^ /// // (mutable) reference to uninitialized memory! /// // This is undefined behavior. - /// Ok(buffer.assume_init()) + /// Ok(unsafe { buffer.assume_init() }) /// } /// ``` /// - /// Nor can you use direct field access to do field-by-field gradual initialization. + /// Nor can you use direct field access to do field-by-field gradual initialization: /// /// ```rust,no_run - /// use std::mem::MaybeUninit; + /// #![feature(maybe_uninit_ref)] + /// use std::{mem::MaybeUninit, ptr}; /// /// struct Foo { /// a: u32, @@ -660,7 +672,7 @@ impl MaybeUninit { /// } /// /// let foo: Foo = unsafe { - /// let foo = MaybeUninit::::uninit(); + /// let mut foo = MaybeUninit::::uninit(); /// ptr::write(&mut foo.get_mut().a as *mut u32, 1337); /// // ^^^^^^^^^^^^^ /// // (mutable) reference to uninitialized memory! From d9087cb388c00ec9a53f7a3049afb2ce00ce56fa Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Wed, 30 Oct 2019 14:52:56 +0100 Subject: [PATCH 3/4] Added a panic-on-uninhabited guard on get_ref and get_mut --- src/libcore/mem/maybe_uninit.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 60d20735db467..d5893dd1d58f5 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -571,6 +571,7 @@ impl MaybeUninit { #[unstable(feature = "maybe_uninit_ref", issue = "63568")] #[inline(always)] pub unsafe fn get_ref(&self) -> &T { + intrinsics::panic_if_uninhabited::(); &*self.value } @@ -690,6 +691,7 @@ impl MaybeUninit { #[unstable(feature = "maybe_uninit_ref", issue = "63568")] #[inline(always)] pub unsafe fn get_mut(&mut self) -> &mut T { + intrinsics::panic_if_uninhabited::(); &mut *self.value } From 67f2200f4a1836742a605dca551408db56976b69 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sun, 3 Nov 2019 21:53:21 +0100 Subject: [PATCH 4/4] Minor style improvements Co-Authored-By: Ralf Jung --- src/libcore/mem/maybe_uninit.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index d5893dd1d58f5..51ba260589f62 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -527,19 +527,15 @@ impl MaybeUninit { /// /// ```rust /// #![feature(maybe_uninit_ref)] - /// use ::std::mem::MaybeUninit; + /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninit(); /// // Initialize `x`: /// unsafe { x.as_mut_ptr().write(vec![1, 2, 3]); } - /// /* The above line can also be done without unsafe: - /// x = MaybeUninit::new(vec![1, 2, 3]); // */ /// // Now that our `MaybeUninit<_>` is known to be initialized, it is okay to /// // create a shared reference to it: /// let x: &Vec = unsafe { - /// // # Safety - /// // - /// // - `x` has been initialized. + /// // Safety: `x` has been initialized. /// x.get_ref() /// }; /// assert_eq!(x, &vec![1, 2, 3]); @@ -594,27 +590,25 @@ impl MaybeUninit { /// /// ```rust /// #![feature(maybe_uninit_ref)] - /// use ::std::mem::MaybeUninit; + /// use std::mem::MaybeUninit; /// - /// # unsafe extern "C" fn initialize_buffer (buf: *mut [u8; 2048]) { *buf = [0; 2048] } + /// # unsafe extern "C" fn initialize_buffer(buf: *mut [u8; 2048]) { *buf = [0; 2048] } /// # #[cfg(FALSE)] /// extern "C" { /// /// Initializes *all* the bytes of the input buffer. - /// fn initialize_buffer (buf: *mut [u8; 2048]); + /// fn initialize_buffer(buf: *mut [u8; 2048]); /// } /// /// let mut buf = MaybeUninit::<[u8; 2048]>::uninit(); /// /// // Initialize `buf`: /// unsafe { initialize_buffer(buf.as_mut_ptr()); } - /// // Now we know that `buf` has been initialized; so we could `.assume_init()` it. + /// // Now we know that `buf` has been initialized, so we could `.assume_init()` it. /// // However, using `.assume_init()` may trigger a `memcpy` of the 2048 bytes. /// // To assert our buffer has been initialized without copying it, we upgrade /// // the `&mut MaybeUninit<[u8; 2048]>` to a `&mut [u8; 2048]`: /// let buf: &mut [u8; 2048] = unsafe { - /// // # Safety - /// // - /// // - `buf` has been initialized. + /// // Safety: `buf` has been initialized. /// buf.get_mut() /// }; ///