Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use MaybeUninit instead of mem::uninitialized for Windows Mutex #56275

Merged
merged 10 commits into from
Dec 2, 2018
3 changes: 3 additions & 0 deletions src/libcore/fmt/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ fn float_to_decimal_common_exact<T>(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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe file an issue about this specifically, and mention that issue number in code comments for all known occurences? That way when the question is resolved we can grep for the issue number to find places to fix (or to remove outdated FIXME comments).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this usually be part of the discussion in the tracking issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the tracking issue number could serve as well. My comment was more about having something to grep for to find known occurrences.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Is like this okay? This covers all get_mut uses that got added as part of my PR series. I grepped the codebase for other uses of MaybeUninit and found none.

// `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());
Expand Down
3 changes: 3 additions & 0 deletions src/libcore/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,9 @@ impl<T> MaybeUninit<T> {
///
/// 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 {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@
#![feature(panic_info_message)]
#![feature(non_exhaustive)]
#![feature(alloc_layout_extra)]
#![feature(maybe_uninit)]

#![default_lib_allocator]

Expand Down
26 changes: 17 additions & 9 deletions src/libstd/sys/windows/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -157,34 +157,42 @@ fn kind() -> Kind {
return ret;
}

pub struct ReentrantMutex { inner: UnsafeCell<c::CRITICAL_SECTION> }
pub struct ReentrantMutex { inner: UnsafeCell<MaybeUninit<c::CRITICAL_SECTION>> }

unsafe impl Send for ReentrantMutex {}
unsafe impl Sync for ReentrantMutex {}

impl ReentrantMutex {
pub unsafe fn uninitialized() -> ReentrantMutex {
mem::uninitialized()
pub fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninitialized()) }
}

pub unsafe fn init(&mut self) {
c::InitializeCriticalSection(self.inner.get());
c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}

pub unsafe fn lock(&self) {
c::EnterCriticalSection(self.inner.get());
// `init` must have been called, so this is now initialized and
// we can call `get_mut`.
c::EnterCriticalSection((&mut *self.inner.get()).get_mut());
}

#[inline]
pub unsafe fn try_lock(&self) -> bool {
c::TryEnterCriticalSection(self.inner.get()) != 0
// `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
}

pub unsafe fn unlock(&self) {
c::LeaveCriticalSection(self.inner.get());
// `init` must have been called, so this is now initialized and
// we can call `get_mut`.
c::LeaveCriticalSection((&mut *self.inner.get()).get_mut());
}

pub unsafe fn destroy(&self) {
c::DeleteCriticalSection(self.inner.get());
// `init` must have been called, so this is now initialized and
// we can call `get_mut`.
c::DeleteCriticalSection((&mut *self.inner.get()).get_mut());
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}
}