From f2af41ab8c8a6519bd0175da9fcd873575810def Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Nov 2018 09:32:00 +0100 Subject: [PATCH 01/10] use MaybeUninit instead of mem::uninitialized for Windows Mutex --- src/libstd/sys/windows/mutex.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index b0e7331e2b651..3ba19a40d48b9 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -30,7 +30,7 @@ //! detect recursive locks. use cell::UnsafeCell; -use mem; +use mem::{self, MaybeUninit}; use sync::atomic::{AtomicUsize, Ordering}; use sys::c; use sys::compat; @@ -157,34 +157,34 @@ fn kind() -> Kind { return ret; } -pub struct ReentrantMutex { inner: UnsafeCell } +pub struct ReentrantMutex { inner: MaybeUninit> } unsafe impl Send for ReentrantMutex {} unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { - mem::uninitialized() + pub fn uninitialized() -> ReentrantMutex { + MaybeUninit::uninitialized() } pub unsafe fn init(&mut self) { - c::InitializeCriticalSection(self.inner.get()); + c::InitializeCriticalSection(self.inner.get_ref().get()); } pub unsafe fn lock(&self) { - c::EnterCriticalSection(self.inner.get()); + c::EnterCriticalSection(self.inner.get_ref().get()); } #[inline] pub unsafe fn try_lock(&self) -> bool { - c::TryEnterCriticalSection(self.inner.get()) != 0 + c::TryEnterCriticalSection(self.inner.get_ref().get()) != 0 } pub unsafe fn unlock(&self) { - c::LeaveCriticalSection(self.inner.get()); + c::LeaveCriticalSection(self.inner.get_ref().get()); } pub unsafe fn destroy(&self) { - c::DeleteCriticalSection(self.inner.get()); + c::DeleteCriticalSection(self.inner.get_ref().get()); } } From a81027515069ac500dbbb6ea57bfb72a9eb948ac Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Nov 2018 13:48:40 +0100 Subject: [PATCH 02/10] fix build --- src/libstd/sys/windows/mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index 3ba19a40d48b9..a8eb82a8a6649 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -164,7 +164,7 @@ unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub fn uninitialized() -> ReentrantMutex { - MaybeUninit::uninitialized() + ReentrantMutex { inner: MaybeUninit::uninitialized() } } pub unsafe fn init(&mut self) { From 2f2f37983d14c53c3328540d6cf44499c1699521 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Nov 2018 16:11:45 +0100 Subject: [PATCH 03/10] add missing feature --- src/libstd/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 575903d576a23..8c8e7cb760e5a 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -310,6 +310,7 @@ #![feature(panic_info_message)] #![feature(non_exhaustive)] #![feature(alloc_layout_extra)] +#![feature(maybe_uninit)] #![default_lib_allocator] From a4f12344c68530d1f42c5b00c10ab417137c0491 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Nov 2018 16:12:08 +0100 Subject: [PATCH 04/10] add comments explaining our uses of get_ref/get_mut for MaybeUninit --- src/libcore/fmt/float.rs | 3 +++ src/libcore/mem.rs | 6 ++++++ src/libstd/sys/windows/mutex.rs | 3 +++ 3 files changed, 12 insertions(+) diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index d01cd012031db..0c1ac90dfd2d7 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -22,6 +22,9 @@ fn float_to_decimal_common_exact(fmt: &mut Formatter, num: &T, unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninitialized(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part; 4]>::uninitialized(); + // FIXME: Technically, this is calling `get_mut` on an uninitialized + // `MaybeUninit` (here and elsewhere in this file). Revisit this once + // we decided whether that is valid or not. let formatted = flt2dec::to_exact_fixed_str(flt2dec::strategy::grisu::format_exact, *num, sign, precision, false, buf.get_mut(), parts.get_mut()); diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index a5603ff6a62e7..626db7806dfe4 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1106,6 +1106,9 @@ impl MaybeUninit { /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state, otherwise this will immediately cause undefined behavior. + // FIXME: We currently rely on the above being incorrect, i.e., we have references + // to uninitialized data (e.g. in `libstd/sys/windows/mutex.rs`). We should make + // a final decision about the rules before stabilization. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub unsafe fn get_ref(&self) -> &T { @@ -1118,6 +1121,9 @@ impl MaybeUninit { /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state, otherwise this will immediately cause undefined behavior. + // FIXME: 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. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub unsafe fn get_mut(&mut self) -> &mut T { diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index a8eb82a8a6649..0c228f5097ee3 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -168,6 +168,9 @@ impl ReentrantMutex { } pub unsafe fn init(&mut self) { + // FIXME: Technically, this is calling `get_ref` on an uninitialized + // `MaybeUninit`. Revisit this once we decided whether that is valid + // or not. c::InitializeCriticalSection(self.inner.get_ref().get()); } From 12d90aa949f34712a374984bfaf88a5bf2f08685 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 09:29:56 +0100 Subject: [PATCH 05/10] put the MaybeUninit inside the UnsafeCell --- src/libcore/mem.rs | 3 --- src/libstd/sys/windows/mutex.rs | 17 +++++++---------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 626db7806dfe4..b61a5c973a766 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1106,9 +1106,6 @@ impl MaybeUninit { /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state, otherwise this will immediately cause undefined behavior. - // FIXME: We currently rely on the above being incorrect, i.e., we have references - // to uninitialized data (e.g. in `libstd/sys/windows/mutex.rs`). We should make - // a final decision about the rules before stabilization. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub unsafe fn get_ref(&self) -> &T { diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index 0c228f5097ee3..38ba0c7e035e5 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -157,37 +157,34 @@ fn kind() -> Kind { return ret; } -pub struct ReentrantMutex { inner: MaybeUninit> } +pub struct ReentrantMutex { inner: UnsafeCell> } unsafe impl Send for ReentrantMutex {} unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub fn uninitialized() -> ReentrantMutex { - ReentrantMutex { inner: MaybeUninit::uninitialized() } + ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninitialized()) } } pub unsafe fn init(&mut self) { - // FIXME: Technically, this is calling `get_ref` on an uninitialized - // `MaybeUninit`. Revisit this once we decided whether that is valid - // or not. - c::InitializeCriticalSection(self.inner.get_ref().get()); + c::InitializeCriticalSection(self.inner.get().as_mut_ptr()); } pub unsafe fn lock(&self) { - c::EnterCriticalSection(self.inner.get_ref().get()); + c::EnterCriticalSection(self.inner.get().get_ref()); } #[inline] pub unsafe fn try_lock(&self) -> bool { - c::TryEnterCriticalSection(self.inner.get_ref().get()) != 0 + c::TryEnterCriticalSection(self.inner.get().get_ref()) != 0 } pub unsafe fn unlock(&self) { - c::LeaveCriticalSection(self.inner.get_ref().get()); + c::LeaveCriticalSection(self.inner.get().get_ref()); } pub unsafe fn destroy(&self) { - c::DeleteCriticalSection(self.inner.get_ref().get()); + c::DeleteCriticalSection(self.inner.get().get_ref()); } } From 965fdb029460346553cd9ddc59f5bb3b93e2f508 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 10:35:56 +0100 Subject: [PATCH 06/10] fix build --- src/libstd/sys/windows/mutex.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index 38ba0c7e035e5..e54bbfca05617 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -168,23 +168,31 @@ impl ReentrantMutex { } pub unsafe fn init(&mut self) { - c::InitializeCriticalSection(self.inner.get().as_mut_ptr()); + c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr()); } pub unsafe fn lock(&self) { - c::EnterCriticalSection(self.inner.get().get_ref()); + // `init` must have been called, so this is now initialized and + // we can call `get_ref`. + c::EnterCriticalSection((&mut *self.inner.get()).get_ref()); } #[inline] pub unsafe fn try_lock(&self) -> bool { - c::TryEnterCriticalSection(self.inner.get().get_ref()) != 0 + // `init` must have been called, so this is now initialized and + // we can call `get_ref`. + c::TryEnterCriticalSection((&mut *self.inner.get()).get_ref()) != 0 } pub unsafe fn unlock(&self) { - c::LeaveCriticalSection(self.inner.get().get_ref()); + // `init` must have been called, so this is now initialized and + // we can call `get_ref`. + c::LeaveCriticalSection((&mut *self.inner.get()).get_ref()); } pub unsafe fn destroy(&self) { - c::DeleteCriticalSection(self.inner.get().get_ref()); + // `init` must have been called, so this is now initialized and + // we can call `get_ref`. + c::DeleteCriticalSection((&mut *self.inner.get()).get_ref()); } } From dd593d3ab85826436dec593ce6ac06932291fd0e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 12:49:11 +0100 Subject: [PATCH 07/10] get_ref -> get_mut --- src/libstd/sys/windows/mutex.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index e54bbfca05617..c2107c28e034c 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -173,26 +173,26 @@ impl ReentrantMutex { pub unsafe fn lock(&self) { // `init` must have been called, so this is now initialized and - // we can call `get_ref`. - c::EnterCriticalSection((&mut *self.inner.get()).get_ref()); + // we can call `get_mut`. + c::EnterCriticalSection((&mut *self.inner.get()).get_mut()); } #[inline] pub unsafe fn try_lock(&self) -> bool { // `init` must have been called, so this is now initialized and - // we can call `get_ref`. - c::TryEnterCriticalSection((&mut *self.inner.get()).get_ref()) != 0 + // we can call `get_mut`. + c::TryEnterCriticalSection((&mut *self.inner.get()).get_mut()) != 0 } pub unsafe fn unlock(&self) { // `init` must have been called, so this is now initialized and - // we can call `get_ref`. - c::LeaveCriticalSection((&mut *self.inner.get()).get_ref()); + // we can call `get_mut`. + c::LeaveCriticalSection((&mut *self.inner.get()).get_mut()); } pub unsafe fn destroy(&self) { // `init` must have been called, so this is now initialized and - // we can call `get_ref`. - c::DeleteCriticalSection((&mut *self.inner.get()).get_ref()); + // we can call `get_mut`. + c::DeleteCriticalSection((&mut *self.inner.get()).get_mut()); } } From f9fb8d64350efdff400c54176a9ac1c3e5da5afd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 2 Dec 2018 12:16:43 +0100 Subject: [PATCH 08/10] no reason to use mutable references here at all --- src/libstd/sys/windows/mutex.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index c2107c28e034c..2bd5dee63e883 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -172,27 +172,19 @@ impl ReentrantMutex { } pub unsafe fn lock(&self) { - // `init` must have been called, so this is now initialized and - // we can call `get_mut`. - c::EnterCriticalSection((&mut *self.inner.get()).get_mut()); + c::EnterCriticalSection((&mut *self.inner.get()).as_mut_ptr()); } #[inline] pub unsafe fn try_lock(&self) -> bool { - // `init` must have been called, so this is now initialized and - // we can call `get_mut`. - c::TryEnterCriticalSection((&mut *self.inner.get()).get_mut()) != 0 + c::TryEnterCriticalSection((&mut *self.inner.get()).as_mut_ptr()) != 0 } pub unsafe fn unlock(&self) { - // `init` must have been called, so this is now initialized and - // we can call `get_mut`. - c::LeaveCriticalSection((&mut *self.inner.get()).get_mut()); + c::LeaveCriticalSection((&mut *self.inner.get()).as_mut_ptr()); } pub unsafe fn destroy(&self) { - // `init` must have been called, so this is now initialized and - // we can call `get_mut`. - c::DeleteCriticalSection((&mut *self.inner.get()).get_mut()); + c::DeleteCriticalSection((&mut *self.inner.get()).as_mut_ptr()); } } From f4f8b211a8650feff62c69a1dfb5156f692003c3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 2 Dec 2018 12:29:54 +0100 Subject: [PATCH 09/10] let FIXME refer to tracking issue --- src/libcore/fmt/float.rs | 2 +- src/libcore/mem.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index 0c1ac90dfd2d7..3717a783f2411 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -22,7 +22,7 @@ fn float_to_decimal_common_exact(fmt: &mut Formatter, num: &T, unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninitialized(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part; 4]>::uninitialized(); - // FIXME: Technically, this is calling `get_mut` on an uninitialized + // FIXME(#53491): Technically, this is calling `get_mut` on an uninitialized // `MaybeUninit` (here and elsewhere in this file). Revisit this once // we decided whether that is valid or not. let formatted = flt2dec::to_exact_fixed_str(flt2dec::strategy::grisu::format_exact, diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index b61a5c973a766..e4b2800ae2117 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1118,7 +1118,7 @@ impl MaybeUninit { /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state, otherwise this will immediately cause undefined behavior. - // FIXME: We currently rely on the above being incorrect, i.e., we have references + // 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. #[unstable(feature = "maybe_uninit", issue = "53491")] From ebe69c06b38e0d1d20c79ee4342715514e917107 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 2 Dec 2018 12:34:39 +0100 Subject: [PATCH 10/10] avoid MaybeUninit::get_mut where it is not needed --- src/liballoc/collections/btree/node.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index f9b455fe796d1..215689dfc48c9 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -602,7 +602,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } else { unsafe { slice::from_raw_parts_mut( - self.as_leaf_mut().keys.get_mut() as *mut [K] as *mut K, + self.as_leaf_mut().keys.as_mut_ptr() as *mut K, self.len() ) } @@ -613,7 +613,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { debug_assert!(!self.is_shared_root()); unsafe { slice::from_raw_parts_mut( - self.as_leaf_mut().vals.get_mut() as *mut [V] as *mut V, + self.as_leaf_mut().vals.as_mut_ptr() as *mut V, self.len() ) }