diff --git a/src/refs/immutable.rs b/src/refs/immutable.rs index 7ca7526..d5a0b85 100644 --- a/src/refs/immutable.rs +++ b/src/refs/immutable.rs @@ -4,6 +4,7 @@ use core::{ fmt::{self, Debug, Display}, hash::{Hash, Hasher}, ops::{Deref, RangeBounds}, + ptr::NonNull, }; use crate::borrow::AtomicBorrow; @@ -19,10 +20,14 @@ use crate::borrow::AtomicBorrow; /// [`AtomicCell`]: struct.AtomicCell.html /// [`&T`]: https://doc.rust-lang.org/core/primitive.reference.html pub struct Ref<'a, T: ?Sized> { - value: &'a T, + value: NonNull, borrow: AtomicBorrow<'a>, } +// SAFETY: `Ref<'_, T> acts as a reference. `AtomicBorrowR` is a reference to an atomic. +unsafe impl<'b, T: ?Sized + 'b> Sync for Ref<'b, T> where for<'a> &'a T: Sync {} +unsafe impl<'b, T: ?Sized + 'b> Send for Ref<'b, T> where for<'a> &'a T: Send {} + impl<'a, T> Clone for Ref<'a, T> where T: ?Sized, @@ -50,7 +55,8 @@ where #[inline(always)] fn deref(&self) -> &T { - self.value + // SAFETY: We hold a shared borrow lock. + unsafe { self.value.as_ref() } } } @@ -60,7 +66,7 @@ where { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Debug::fmt(self.value, f) + ::fmt(self, f) } } @@ -70,7 +76,7 @@ where { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(self.value, f) + ::fmt(self, f) } } @@ -80,7 +86,7 @@ where { #[inline(always)] fn eq(&self, other: &U) -> bool { - PartialEq::eq(self.value, other) + >::eq(self, other) } } @@ -90,7 +96,7 @@ where { #[inline(always)] fn partial_cmp(&self, other: &U) -> Option { - PartialOrd::partial_cmp(self.value, other) + >::partial_cmp(self, other) } } @@ -103,14 +109,14 @@ where where H: Hasher, { - Hash::hash(self.value, state) + ::hash(self, state) } } impl<'a, T> Borrow for Ref<'a, T> { #[inline(always)] fn borrow(&self) -> &T { - self.value + self } } @@ -120,7 +126,7 @@ where { #[inline(always)] fn as_ref(&self) -> &U { - self.value.as_ref() + >::as_ref(self) } } @@ -145,7 +151,7 @@ where #[inline] pub fn new(r: &'a T) -> Self { Ref { - value: r, + value: NonNull::from(r), borrow: AtomicBorrow::dummy(), } } @@ -171,16 +177,20 @@ where /// ``` #[inline] pub fn with_borrow(r: &'a T, borrow: AtomicBorrow<'a>) -> Self { - Ref { value: r, borrow } + Ref { + value: NonNull::from(r), + borrow, + } } - /// Splits wrapper into two parts. - /// One is reference to the value - /// and the other is [`AtomicBorrow`] that guards it from being borrowed mutably. + /// Splits wrapper into two parts. One is reference to the value and the other is + /// [`AtomicBorrow`] that guards it from being borrowed mutably. /// /// # Safety /// - /// User must ensure reference is not used after [`AtomicBorrow`] is dropped. + /// User must ensure [`NonNull`] is not dereferenced after [`AtomicBorrow`] is dropped. + /// + /// Also, the [`NonNull`] that is returned is still only valid for reads, not writes. /// /// # Examples /// @@ -192,7 +202,7 @@ where /// /// unsafe { /// let (r, borrow) = Ref::into_split(r); - /// assert_eq!(*r, 42); + /// assert_eq!(*r.as_ref(), 42); /// /// assert!(cell.try_borrow().is_some(), "Must be able to borrow immutably"); /// assert!(cell.try_borrow_mut().is_none(), "Must not be able to borrow mutably yet"); @@ -201,7 +211,7 @@ where /// } /// ``` #[inline] - pub unsafe fn into_split(r: Ref<'a, T>) -> (&'a T, AtomicBorrow<'a>) { + pub fn into_split(r: Ref<'a, T>) -> (NonNull, AtomicBorrow<'a>) { (r.value, r.borrow) } @@ -231,7 +241,7 @@ where U: ?Sized, { Ref { - value: f(r.value), + value: NonNull::from(f(&*r)), borrow: r.borrow, } } @@ -259,9 +269,9 @@ where where F: FnOnce(&T) -> Option<&U>, { - match f(r.value) { + match f(&*r) { Some(value) => Ok(Ref { - value, + value: NonNull::from(value), borrow: r.borrow, }), None => Err(r), @@ -297,15 +307,16 @@ where let borrow_u = r.borrow.clone(); let borrow_v = r.borrow; - let (u, v) = f(r.value); + // SAFETY: we have a shared reference lock on the pointer. + let (u, v) = f(unsafe { r.value.as_ref() }); ( Ref { - value: u, + value: NonNull::from(u), borrow: borrow_u, }, Ref { - value: v, + value: NonNull::from(v), borrow: borrow_v, }, ) @@ -337,7 +348,8 @@ where /// ``` pub fn leak(r: Ref<'a, T>) -> &'a T { core::mem::forget(r.borrow); - r.value + // SAFETY: we have a shared reference lock on the pointer. + unsafe { r.value.as_ref() } } /// Converts reference and returns result wrapped in the [`Ref`]. @@ -366,7 +378,7 @@ where T: AsRef, { Ref { - value: r.value.as_ref(), + value: NonNull::from(>::as_ref(&r)), borrow: r.borrow, } } @@ -396,7 +408,8 @@ where T: Deref, { Ref { - value: &r.value, + value: NonNull::from(::deref(&*r)), + borrow: r.borrow, } } @@ -432,7 +445,7 @@ impl<'a, T> Ref<'a, Option> { #[inline] pub fn transpose(r: Ref<'a, Option>) -> Option> { Some(Ref { - value: r.value.as_ref()?, + value: r.as_ref().map(NonNull::from)?, borrow: r.borrow, }) } @@ -464,8 +477,10 @@ impl<'a, T> Ref<'a, [T]> { R: RangeBounds, { let bounds = (range.start_bound().cloned(), range.end_bound().cloned()); + let slice = &*r; + let slice = &slice[bounds]; Ref { - value: &r.value[bounds], + value: NonNull::from(slice), borrow: r.borrow, } } diff --git a/src/refs/mutable.rs b/src/refs/mutable.rs index 0282af4..7319cfb 100644 --- a/src/refs/mutable.rs +++ b/src/refs/mutable.rs @@ -3,11 +3,39 @@ use core::{ cmp::Ordering, fmt::{self, Debug, Display}, hash::{Hash, Hasher}, + marker::PhantomData, ops::{Deref, DerefMut, RangeBounds}, + ptr::NonNull, }; use crate::borrow::AtomicBorrowMut; +/// A wrapper around [`NonNull`] that is invariant over T. +#[repr(transparent)] +pub struct NonNullMut { + inner: NonNull, + _invariant: PhantomData<*mut T>, +} +impl From<&mut T> for NonNullMut { + fn from(value: &mut T) -> Self { + Self { + inner: NonNull::from(value), + _invariant: PhantomData, + } + } +} +impl core::ops::Deref for NonNullMut { + type Target = NonNull; + fn deref(&self) -> &Self::Target { + &self.inner + } +} +impl core::ops::DerefMut for NonNullMut { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } +} + /// Wrapper for mutably borrowed [`AtomicCell`] that will released lock on drop. /// /// This type can be dereferenced to [`&mut T`]. @@ -19,10 +47,14 @@ use crate::borrow::AtomicBorrowMut; /// [`AtomicCell`]: struct.AtomicCell.html /// [`&T`]: https://doc.rust-lang.org/core/primitive.reference.html pub struct RefMut<'a, T: ?Sized> { - value: &'a mut T, + value: NonNullMut, borrow: AtomicBorrowMut<'a>, } +// SAFETY: `Ref<'_, T> acts as a reference. `AtomicBorrowR` is a reference to an atomic. +unsafe impl<'b, T: ?Sized + 'b> Sync for RefMut<'b, T> where for<'a> &'a mut T: Sync {} +unsafe impl<'b, T: ?Sized + 'b> Send for RefMut<'b, T> where for<'a> &'a mut T: Send {} + impl<'a, T> Deref for RefMut<'a, T> where T: ?Sized, @@ -31,7 +63,8 @@ where #[inline(always)] fn deref(&self) -> &T { - &self.value + // SAFETY: We hold an exclusive lock on the pointer. + unsafe { self.value.as_ref() } } } @@ -41,7 +74,8 @@ where { #[inline(always)] fn deref_mut(&mut self) -> &mut T { - &mut self.value + // SAFETY: We hold an exclusive lock on the pointer. + unsafe { self.value.as_mut() } } } @@ -51,7 +85,7 @@ where { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Debug::fmt(self.value, f) + ::fmt(self, f) } } @@ -61,7 +95,7 @@ where { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(self.value, f) + ::fmt(self, f) } } @@ -71,7 +105,7 @@ where { #[inline(always)] fn eq(&self, other: &U) -> bool { - PartialEq::eq(&*self.value, other) + >::eq(self, other) } } @@ -81,7 +115,7 @@ where { #[inline(always)] fn partial_cmp(&self, other: &U) -> Option { - PartialOrd::partial_cmp(&*self.value, other) + >::partial_cmp(self, other) } } @@ -94,7 +128,7 @@ where where H: Hasher, { - Hash::hash(self.value, state) + ::hash(self, state) } } @@ -104,7 +138,7 @@ where { #[inline(always)] fn borrow(&self) -> &T { - &self.value + self } } @@ -114,7 +148,7 @@ where { #[inline(always)] fn borrow_mut(&mut self) -> &mut T { - &mut self.value + self } } @@ -124,7 +158,7 @@ where { #[inline(always)] fn as_ref(&self) -> &U { - self.value.as_ref() + >::as_ref(self) } } @@ -134,7 +168,7 @@ where { #[inline(always)] fn as_mut(&mut self) -> &mut U { - self.value.as_mut() + >::as_mut(self) } } @@ -159,7 +193,7 @@ where #[inline] pub fn new(r: &'a mut T) -> Self { RefMut { - value: r, + value: NonNullMut::from(r), borrow: AtomicBorrowMut::dummy(), } } @@ -186,16 +220,18 @@ where /// ``` #[inline] pub fn with_borrow(r: &'a mut T, borrow: AtomicBorrowMut<'a>) -> Self { - RefMut { value: r, borrow } + RefMut { + value: NonNullMut::from(r), + borrow, + } } - /// Splits wrapper into two parts. - /// One is reference to the value - /// and the other is [`AtomicBorrowMut`] that guards it from being borrowed. + /// Splits wrapper into two parts. One is reference to the value and the other is + /// [`AtomicBorrowMut`] that guards it from being borrowed. /// /// # Safety /// - /// User must ensure reference is not used after [`AtomicBorrowMut`] is dropped. + /// User must ensure [`NonNullMut`] is not dereferenced after [`AtomicBorrowMut`] is dropped. /// /// # Examples /// @@ -207,7 +243,7 @@ where /// /// unsafe { /// let (r, borrow) = RefMut::into_split(r); - /// assert_eq!(*r, 42); + /// assert_eq!(*r.as_ref(), 42); /// /// assert!(cell.try_borrow().is_none(), "Must not be able to borrow mutably yet"); /// assert!(cell.try_borrow_mut().is_none(), "Must not be able to borrow mutably yet"); @@ -216,7 +252,7 @@ where /// } /// ``` #[inline] - pub unsafe fn into_split(r: RefMut<'a, T>) -> (&'a mut T, AtomicBorrowMut<'a>) { + pub fn into_split(r: RefMut<'a, T>) -> (NonNullMut, AtomicBorrowMut<'a>) { (r.value, r.borrow) } @@ -239,13 +275,13 @@ where /// let b2: RefMut = RefMut::map(b1, |t| &mut t.0); /// assert_eq!(*b2, 5) #[inline] - pub fn map(r: RefMut<'a, T>, f: F) -> RefMut<'a, U> + pub fn map(mut r: RefMut<'a, T>, f: F) -> RefMut<'a, U> where F: FnOnce(&mut T) -> &mut U, U: ?Sized, { RefMut { - value: f(r.value), + value: NonNullMut::from(f(&mut *r)), borrow: r.borrow, } } @@ -283,20 +319,20 @@ where F: FnOnce(&mut T) -> Option<&mut U>, { // FIXME(nll-rfc#40): fix borrow-check - let RefMut { value, borrow } = r; - let value = value as *mut T; + let RefMut { value, borrow, .. } = r; + let _ptr = value.as_ptr(); // SAFETY: function holds onto an exclusive reference for the duration // of its call through `r`, and the pointer is only de-referenced // inside of the function call never allowing the exclusive reference to // escape. - match f(unsafe { &mut *value }) { - Some(value) => Ok(RefMut { value, borrow }), + match f(unsafe { &mut *_ptr }) { + Some(value) => Ok(RefMut { + value: NonNullMut::from(value), + borrow, + }), None => { // SAFETY: same as above. - Err(RefMut { - value: unsafe { &mut *value }, - borrow, - }) + Err(RefMut { value, borrow }) } } } @@ -321,7 +357,7 @@ where /// assert_eq!(*begin, [1, 2]); /// assert_eq!(*end, [3, 4]); /// ``` - pub fn map_split(r: RefMut<'a, T>, f: F) -> (RefMut<'a, U>, RefMut<'a, V>) + pub fn map_split(mut r: RefMut<'a, T>, f: F) -> (RefMut<'a, U>, RefMut<'a, V>) where U: ?Sized, V: ?Sized, @@ -330,15 +366,16 @@ where let borrow_u = r.borrow.clone(); let borrow_v = r.borrow; - let (u, v) = f(r.value); + // SAFETY: We hold an exclusive lock on the pointer. + let (u, v) = f(unsafe { r.value.as_mut() }); ( RefMut { - value: u, + value: NonNullMut::from(u), borrow: borrow_u, }, RefMut { - value: v, + value: NonNullMut::from(v), borrow: borrow_v, }, ) @@ -370,9 +407,10 @@ where /// assert!(cell.try_borrow().is_none()); /// assert!(cell.try_borrow_mut().is_none()); /// ``` - pub fn leak(r: RefMut<'a, T>) -> &'a mut T { + pub fn leak(mut r: RefMut<'a, T>) -> &'a mut T { core::mem::forget(r.borrow); - r.value + // SAFETY: We hold an exclusive lock on the pointer. + unsafe { r.value.as_mut() } } /// Converts reference and returns result wrapped in the [`RefMut`]. @@ -396,13 +434,13 @@ where /// assert_eq!(*b2, *"HELLO") /// ``` #[inline] - pub fn as_mut(r: RefMut<'a, T>) -> RefMut<'a, U> + pub fn as_mut(mut r: RefMut<'a, T>) -> RefMut<'a, U> where U: ?Sized, T: AsMut, { RefMut { - value: r.value.as_mut(), + value: NonNullMut::from(>::as_mut(&mut *r)), borrow: r.borrow, } } @@ -428,12 +466,12 @@ where /// assert_eq!(*b2, *"HELLO") /// ``` #[inline] - pub fn as_deref_mut(r: RefMut<'a, T>) -> RefMut<'a, T::Target> + pub fn as_deref_mut(mut r: RefMut<'a, T>) -> RefMut<'a, T::Target> where T: DerefMut, { RefMut { - value: &mut *r.value, + value: NonNullMut::from(::deref_mut(&mut *r)), borrow: r.borrow, } } @@ -467,9 +505,9 @@ impl<'a, T> RefMut<'a, Option> { /// assert!(c.try_borrow_mut().is_some()); /// ``` #[inline] - pub fn transpose(r: RefMut<'a, Option>) -> Option> { + pub fn transpose(mut r: RefMut<'a, Option>) -> Option> { Some(RefMut { - value: r.value.as_mut()?, + value: r.as_mut().map(NonNullMut::from)?, borrow: r.borrow, }) } @@ -496,13 +534,15 @@ impl<'a, T> RefMut<'a, [T]> { /// assert_eq!(*b2, [3, 4]) /// ``` #[inline] - pub fn slice(r: RefMut<'a, [T]>, range: R) -> RefMut<'a, [T]> + pub fn slice(mut r: RefMut<'a, [T]>, range: R) -> RefMut<'a, [T]> where R: RangeBounds, { let bounds = (range.start_bound().cloned(), range.end_bound().cloned()); + let slice = &mut *r; + let slice = &mut slice[bounds]; RefMut { - value: &mut r.value[bounds], + value: NonNullMut::from(slice), borrow: r.borrow, } } diff --git a/src/tests.rs b/src/tests.rs index b8da900..4d31cf4 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,4 +1,7 @@ -use super::{cell::AtomicCell, refs::RefMut}; +use super::{ + cell::AtomicCell, + refs::{Ref, RefMut}, +}; #[test] fn test_borrow() { @@ -33,3 +36,35 @@ fn test_unsized() { cell.borrow_mut()[0] = 11; assert_eq!(cell.borrow()[0], 11); } + +// For Miri to catch issues when calling a function. +// +// See how this scenerio affects std::cell::RefCell implementation: +// https://github.com/rust-lang/rust/issues/63787 +// +// Also see relevant unsafe code guidelines issue: +// https://github.com/rust-lang/unsafe-code-guidelines/issues/125 +#[test] +fn drop_and_borrow_in_fn_call() { + { + fn drop_and_borrow(cell: &AtomicCell, borrow: Ref<'_, i32>) { + drop(borrow); + *cell.borrow_mut() = 0; + } + + let a = AtomicCell::new(0); + let borrow = a.borrow(); + drop_and_borrow(&a, borrow); + } + + { + fn drop_and_borrow_mut(cell: &AtomicCell, borrow: RefMut<'_, i32>) { + drop(borrow); + *cell.borrow_mut() = 0; + } + + let a = AtomicCell::new(0); + let borrow = a.borrow_mut(); + drop_and_borrow_mut(&a, borrow); + } +}