diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index 9517e7e1f0325..51e9c73d41a74 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -4,6 +4,7 @@ mod tests; use crate::cell::UnsafeCell; 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; @@ -101,7 +102,12 @@ unsafe impl Sync for RwLock {} #[stable(feature = "rust1", since = "1.0.0")] #[clippy::has_significant_drop] pub struct RwLockReadGuard<'a, T: ?Sized + 'a> { - lock: &'a RwLock, + // NB: we use a pointer instead of `&'a T` to avoid `noalias` violations, because a + // `Ref` argument doesn't hold immutability for its whole scope, only until it drops. + // `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, + inner_lock: &'a sys::MovableRwLock, } #[stable(feature = "rust1", since = "1.0.0")] @@ -509,12 +515,21 @@ impl From for RwLock { } impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> { + /// Create a new instance of `RwLockReadGuard` from a `RwLock`. + // SAFETY: if and only if `lock.inner.read()` (or `lock.inner.try_read()`) has been + // successfully called from the same thread before instantiating this object. unsafe fn new(lock: &'rwlock RwLock) -> LockResult> { - poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard { lock }) + poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard { + data: NonNull::new_unchecked(lock.data.get()), + inner_lock: &lock.inner, + }) } } impl<'rwlock, T: ?Sized> RwLockWriteGuard<'rwlock, T> { + /// Create a new instance of `RwLockWriteGuard` from a `RwLock`. + // SAFETY: if and only if `lock.inner.write()` (or `lock.inner.try_write()`) has been + // successfully called from the same thread before instantiating this object. unsafe fn new(lock: &'rwlock RwLock) -> LockResult> { poison::map_result(lock.poison.guard(), |guard| RwLockWriteGuard { lock, poison: guard }) } @@ -553,7 +568,8 @@ impl Deref for RwLockReadGuard<'_, T> { type Target = T; fn deref(&self) -> &T { - unsafe { &*self.lock.data.get() } + // SAFETY: the conditions of `RwLockGuard::new` were satisfied when created. + unsafe { self.data.as_ref() } } } @@ -562,6 +578,7 @@ impl Deref for RwLockWriteGuard<'_, T> { type Target = T; fn deref(&self) -> &T { + // SAFETY: the conditions of `RwLockWriteGuard::new` were satisfied when created. unsafe { &*self.lock.data.get() } } } @@ -569,6 +586,7 @@ impl Deref for RwLockWriteGuard<'_, T> { #[stable(feature = "rust1", since = "1.0.0")] impl DerefMut for RwLockWriteGuard<'_, T> { fn deref_mut(&mut self) -> &mut T { + // SAFETY: the conditions of `RwLockWriteGuard::new` were satisfied when created. unsafe { &mut *self.lock.data.get() } } } @@ -576,8 +594,9 @@ impl DerefMut for RwLockWriteGuard<'_, T> { #[stable(feature = "rust1", since = "1.0.0")] impl Drop for RwLockReadGuard<'_, T> { fn drop(&mut self) { + // SAFETY: the conditions of `RwLockReadGuard::new` were satisfied when created. unsafe { - self.lock.inner.read_unlock(); + self.inner_lock.read_unlock(); } } } @@ -586,6 +605,7 @@ impl Drop for RwLockReadGuard<'_, T> { impl Drop for RwLockWriteGuard<'_, T> { fn drop(&mut self) { self.lock.poison.done(&self.poison); + // SAFETY: the conditions of `RwLockWriteGuard::new` were satisfied when created. unsafe { self.lock.inner.write_unlock(); } diff --git a/library/std/src/sync/rwlock/tests.rs b/library/std/src/sync/rwlock/tests.rs index 53aa2b1e38a91..08255c985f5eb 100644 --- a/library/std/src/sync/rwlock/tests.rs +++ b/library/std/src/sync/rwlock/tests.rs @@ -1,6 +1,6 @@ use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sync::mpsc::channel; -use crate::sync::{Arc, RwLock, TryLockError}; +use crate::sync::{Arc, RwLock, RwLockReadGuard, TryLockError}; use crate::thread; use rand::{self, Rng}; @@ -245,3 +245,15 @@ fn test_get_mut_poison() { Ok(x) => panic!("get_mut of poisoned RwLock is Ok: {x:?}"), } } + +#[test] +fn test_read_guard_covariance() { + fn do_stuff<'a>(_: RwLockReadGuard<'_, &'a i32>, _: &'a i32) {} + let j: i32 = 5; + let lock = RwLock::new(&j); + { + let i = 6; + do_stuff(lock.read().unwrap(), &i); + } + drop(lock); +} diff --git a/src/test/codegen/noalias-rwlockreadguard.rs b/src/test/codegen/noalias-rwlockreadguard.rs new file mode 100644 index 0000000000000..7f7b46c85a8b0 --- /dev/null +++ b/src/test/codegen/noalias-rwlockreadguard.rs @@ -0,0 +1,14 @@ +// compile-flags: -O -C no-prepopulate-passes -Z mutable-noalias=yes + +#![crate_type = "lib"] + +use std::sync::{RwLock, RwLockReadGuard}; + +// Make sure that `RwLockReadGuard` does not get a `noalias` attribute, because +// the `RwLock` might alias writes after it is dropped. + +// CHECK-LABEL: @maybe_aliased( +// CHECK-NOT: noalias +// CHECK-SAME: %_data +#[no_mangle] +pub unsafe fn maybe_aliased(_: RwLockReadGuard<'_, i32>, _data: &RwLock) {} diff --git a/src/test/debuginfo/rwlock-read.rs b/src/test/debuginfo/rwlock-read.rs index e1c10a4d37fc3..ed9aae16b0db1 100644 --- a/src/test/debuginfo/rwlock-read.rs +++ b/src/test/debuginfo/rwlock-read.rs @@ -15,11 +15,8 @@ // // cdb-command:dx r // cdb-check:r [Type: std::sync::rwlock::RwLockReadGuard] -// cdb-check: [...] lock : [...] [Type: std::sync::rwlock::RwLock *] -// -// cdb-command:dx r.lock->data,d -// cdb-check:r.lock->data,d : 0 [Type: core::cell::UnsafeCell] -// cdb-check: [] [Type: core::cell::UnsafeCell] +// cdb-check: [...] data : NonNull([...]: 0) [Type: core::ptr::non_null::NonNull] +// cdb-check: [...] inner_lock : [...] [Type: std::sys_common::rwlock::MovableRwLock *] #[allow(unused_variables)]