Auto merge of rust-lang#103150 - joboet:remove_lock_wrappers, r=m-ou-se

Remove lock wrappers in `sys_common`

This moves the lazy allocation to `sys` (SGX and UNIX). While this leads to a bit more verbosity, it will simplify future improvements by making room in `sys_common` for platform-independent implementations.

This also removes the condvar check on SGX as it is not necessary for soundness and will be removed anyway once mutex has been made movable.

For simplicity's sake, `libunwind` also uses lazy allocation now on SGX. This will require an update to the C definitions before merging this (CC `@raoulstrackx).`

r? `@m-ou-se`

Author: bors

Diff for: library/std/src/sync/condvar.rs

@@ -3,7 +3,7 @@ mod tests;
 
 use crate::fmt;
 use crate::sync::{mutex, poison, LockResult, MutexGuard, PoisonError};
-use crate::sys_common::condvar as sys;
+use crate::sys::locks as sys;
 use crate::time::{Duration, Instant};
 
 /// A type indicating whether a timed wait on a condition variable returned

Diff for: library/std/src/sync/mutex.rs

@@ -5,7 +5,7 @@ use crate::cell::UnsafeCell;
 use crate::fmt;
 use crate::ops::{Deref, DerefMut};
 use crate::sync::{poison, LockResult, TryLockError, TryLockResult};
-use crate::sys_common::mutex as sys;
+use crate::sys::locks as sys;
 
 /// A mutual exclusion primitive useful for protecting shared data
 ///
@@ -163,7 +163,7 @@ use crate::sys_common::mutex as sys;
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "Mutex")]
 pub struct Mutex<T: ?Sized> {
-    inner: sys::MovableMutex,
+    inner: sys::Mutex,
     poison: poison::Flag,
     data: UnsafeCell<T>,
 }
@@ -217,11 +217,7 @@ impl<T> Mutex<T> {
     #[rustc_const_stable(feature = "const_locks", since = "1.63.0")]
     #[inline]
     pub const fn new(t: T) -> Mutex<T> {
-        Mutex {
-            inner: sys::MovableMutex::new(),
-            poison: poison::Flag::new(),
-            data: UnsafeCell::new(t),
-        }
+        Mutex { inner: sys::Mutex::new(), poison: poison::Flag::new(), data: UnsafeCell::new(t) }
     }
 }
 
@@ -264,7 +260,7 @@ impl<T: ?Sized> Mutex<T> {
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn lock(&self) -> LockResult<MutexGuard<'_, T>> {
         unsafe {
-            self.inner.raw_lock();
+            self.inner.lock();
             MutexGuard::new(self)
         }
     }
@@ -526,7 +522,7 @@ impl<T: ?Sized> Drop for MutexGuard<'_, T> {
     fn drop(&mut self) {
         unsafe {
             self.lock.poison.done(&self.poison);
-            self.lock.inner.raw_unlock();
+            self.lock.inner.unlock();
         }
     }
 }
@@ -545,7 +541,7 @@ impl<T: ?Sized + fmt::Display> fmt::Display for MutexGuard<'_, T> {
     }
 }
 
-pub fn guard_lock<'a, T: ?Sized>(guard: &MutexGuard<'a, T>) -> &'a sys::MovableMutex {
+pub fn guard_lock<'a, T: ?Sized>(guard: &MutexGuard<'a, T>) -> &'a sys::Mutex {
     &guard.lock.inner
 }
 

Diff for: library/std/src/sync/rwlock.rs

@@ -6,7 +6,7 @@ use crate::fmt;
 use crate::ops::{Deref, DerefMut};
 use crate::ptr::NonNull;
 use crate::sync::{poison, LockResult, TryLockError, TryLockResult};
-use crate::sys_common::rwlock as sys;
+use crate::sys::locks as sys;
 
 /// A reader-writer lock
 ///
@@ -78,7 +78,7 @@ use crate::sys_common::rwlock as sys;
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "RwLock")]
 pub struct RwLock<T: ?Sized> {
-    inner: sys::MovableRwLock,
+    inner: sys::RwLock,
     poison: poison::Flag,
     data: UnsafeCell<T>,
 }
@@ -109,7 +109,7 @@ pub struct RwLockReadGuard<'a, T: ?Sized + 'a> {
     // `NonNull` is also covariant over `T`, just like we would have with `&T`. `NonNull`
     // is preferable over `const* T` to allow for niche optimization.
     data: NonNull<T>,
-    inner_lock: &'a sys::MovableRwLock,
+    inner_lock: &'a sys::RwLock,
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -158,11 +158,7 @@ impl<T> RwLock<T> {
     #[rustc_const_stable(feature = "const_locks", since = "1.63.0")]
     #[inline]
     pub const fn new(t: T) -> RwLock<T> {
-        RwLock {
-            inner: sys::MovableRwLock::new(),
-            poison: poison::Flag::new(),
-            data: UnsafeCell::new(t),
-        }
+        RwLock { inner: sys::RwLock::new(), poison: poison::Flag::new(), data: UnsafeCell::new(t) }
     }
 }
 

Diff for: library/std/src/sys/hermit/mod.rs

@@ -51,9 +51,9 @@ pub mod locks {
     mod futex_condvar;
     mod futex_mutex;
     mod futex_rwlock;
-    pub(crate) use futex_condvar::MovableCondvar;
-    pub(crate) use futex_mutex::{MovableMutex, Mutex};
-    pub(crate) use futex_rwlock::{MovableRwLock, RwLock};
+    pub(crate) use futex_condvar::Condvar;
+    pub(crate) use futex_mutex::Mutex;
+    pub(crate) use futex_rwlock::RwLock;
 }
 
 use crate::io::ErrorKind;

Diff for: library/std/src/sys/itron/condvar.rs

@@ -12,18 +12,13 @@ pub struct Condvar {
 unsafe impl Send for Condvar {}
 unsafe impl Sync for Condvar {}
 
-pub type MovableCondvar = Condvar;
-
 impl Condvar {
     #[inline]
     pub const fn new() -> Condvar {
         Condvar { waiters: SpinMutex::new(waiter_queue::WaiterQueue::new()) }
     }
 
-    #[inline]
-    pub unsafe fn init(&mut self) {}
-
-    pub unsafe fn notify_one(&self) {
+    pub fn notify_one(&self) {
         self.waiters.with_locked(|waiters| {
             if let Some(task) = waiters.pop_front() {
                 // Unpark the task
@@ -39,7 +34,7 @@ impl Condvar {
         });
     }
 
-    pub unsafe fn notify_all(&self) {
+    pub fn notify_all(&self) {
         self.waiters.with_locked(|waiters| {
             while let Some(task) = waiters.pop_front() {
                 // Unpark the task

Diff for: library/std/src/sys/itron/mutex.rs

@@ -11,8 +11,6 @@ pub struct Mutex {
     mtx: SpinIdOnceCell<()>,
 }
 
-pub type MovableMutex = Mutex;
-
 /// Create a mutex object. This function never panics.
 fn new_mtx() -> Result<abi::ID, ItronError> {
     ItronError::err_if_negative(unsafe {
@@ -39,7 +37,7 @@ impl Mutex {
         }
     }
 
-    pub unsafe fn lock(&self) {
+    pub fn lock(&self) {
         let mtx = self.raw();
         expect_success(unsafe { abi::loc_mtx(mtx) }, &"loc_mtx");
     }
@@ -49,7 +47,7 @@ impl Mutex {
         expect_success_aborting(unsafe { abi::unl_mtx(mtx) }, &"unl_mtx");
     }
 
-    pub unsafe fn try_lock(&self) -> bool {
+    pub fn try_lock(&self) -> bool {
         let mtx = self.raw();
         match unsafe { abi::ploc_mtx(mtx) } {
             abi::E_TMOUT => false,

Diff for: library/std/src/sys/sgx/condvar.rs

@@ -4,42 +4,43 @@ use crate::time::Duration;
 
 use super::waitqueue::{SpinMutex, WaitQueue, WaitVariable};
 
+/// FIXME: `UnsafeList` is not movable.
+struct AllocatedCondvar(SpinMutex<WaitVariable<()>>);
+
 pub struct Condvar {
-    inner: SpinMutex<WaitVariable<()>>,
+    inner: LazyBox<AllocatedCondvar>,
 }
 
-pub(crate) type MovableCondvar = LazyBox<Condvar>;
-
-impl LazyInit for Condvar {
+impl LazyInit for AllocatedCondvar {
     fn init() -> Box<Self> {
-        Box::new(Self::new())
+        Box::new(AllocatedCondvar(SpinMutex::new(WaitVariable::new(()))))
     }
 }
 
 impl Condvar {
     pub const fn new() -> Condvar {
-        Condvar { inner: SpinMutex::new(WaitVariable::new(())) }
+        Condvar { inner: LazyBox::new() }
     }
 
     #[inline]
-    pub unsafe fn notify_one(&self) {
-        let _ = WaitQueue::notify_one(self.inner.lock());
+    pub fn notify_one(&self) {
+        let _ = WaitQueue::notify_one(self.inner.0.lock());
     }
 
     #[inline]
-    pub unsafe fn notify_all(&self) {
-        let _ = WaitQueue::notify_all(self.inner.lock());
+    pub fn notify_all(&self) {
+        let _ = WaitQueue::notify_all(self.inner.0.lock());
     }
 
     pub unsafe fn wait(&self, mutex: &Mutex) {
-        let guard = self.inner.lock();
+        let guard = self.inner.0.lock();
         WaitQueue::wait(guard, || unsafe { mutex.unlock() });
-        unsafe { mutex.lock() }
+        mutex.lock()
     }
 
     pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
-        let success = WaitQueue::wait_timeout(&self.inner, dur, || unsafe { mutex.unlock() });
-        unsafe { mutex.lock() };
+        let success = WaitQueue::wait_timeout(&self.inner.0, dur, || unsafe { mutex.unlock() });
+        mutex.lock();
         success
     }
 }

Diff for: library/std/src/sys/sgx/mutex.rs

@@ -1,28 +1,28 @@
 use super::waitqueue::{try_lock_or_false, SpinMutex, WaitQueue, WaitVariable};
 use crate::sys_common::lazy_box::{LazyBox, LazyInit};
 
+/// FIXME: `UnsafeList` is not movable.
+struct AllocatedMutex(SpinMutex<WaitVariable<bool>>);
+
 pub struct Mutex {
-    inner: SpinMutex<WaitVariable<bool>>,
+    inner: LazyBox<AllocatedMutex>,
 }
 
-// not movable: see UnsafeList implementation
-pub(crate) type MovableMutex = LazyBox<Mutex>;
-
-impl LazyInit for Mutex {
+impl LazyInit for AllocatedMutex {
     fn init() -> Box<Self> {
-        Box::new(Self::new())
+        Box::new(AllocatedMutex(SpinMutex::new(WaitVariable::new(false))))
     }
 }
 
 // Implementation according to “Operating Systems: Three Easy Pieces”, chapter 28
 impl Mutex {
     pub const fn new() -> Mutex {
-        Mutex { inner: SpinMutex::new(WaitVariable::new(false)) }
+        Mutex { inner: LazyBox::new() }
     }
 
     #[inline]
-    pub unsafe fn lock(&self) {
-        let mut guard = self.inner.lock();
+    pub fn lock(&self) {
+        let mut guard = self.inner.0.lock();
         if *guard.lock_var() {
             // Another thread has the lock, wait
             WaitQueue::wait(guard, || {})
@@ -35,7 +35,7 @@ impl Mutex {
 
     #[inline]
     pub unsafe fn unlock(&self) {
-        let guard = self.inner.lock();
+        let guard = self.inner.0.lock();
         if let Err(mut guard) = WaitQueue::notify_one(guard) {
             // No other waiters, unlock
             *guard.lock_var_mut() = false;
@@ -45,8 +45,8 @@ impl Mutex {
     }
 
     #[inline]
-    pub unsafe fn try_lock(&self) -> bool {
-        let mut guard = try_lock_or_false!(self.inner);
+    pub fn try_lock(&self) -> bool {
+        let mut guard = try_lock_or_false!(self.inner.0);
         if *guard.lock_var() {
             // Another thread has the lock
             false