Skip to content

Commit 0690641

Browse files
committed
Use NonNull pointer in the place of references in AtomicRef and
`AtomicRefMut` to avoid `noalias` related soundness bug.
1 parent d0aefef commit 0690641

File tree

1 file changed

+42
-17
lines changed

1 file changed

+42
-17
lines changed

src/lib.rs

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ use core::cell::UnsafeCell;
4848
use core::cmp;
4949
use core::fmt;
5050
use core::fmt::{Debug, Display};
51+
use core::marker::PhantomData;
5152
use core::ops::{Deref, DerefMut};
53+
use core::ptr::NonNull;
5254
use core::sync::atomic;
5355
use core::sync::atomic::AtomicUsize;
5456

@@ -116,7 +118,7 @@ impl<T: ?Sized> AtomicRefCell<T> {
116118
pub fn borrow(&self) -> AtomicRef<T> {
117119
match AtomicBorrowRef::try_new(&self.borrow) {
118120
Ok(borrow) => AtomicRef {
119-
value: unsafe { &*self.value.get() },
121+
value: unsafe { NonNull::new_unchecked(self.value.get()) },
120122
borrow,
121123
},
122124
Err(s) => panic!("{}", s),
@@ -129,7 +131,7 @@ impl<T: ?Sized> AtomicRefCell<T> {
129131
pub fn try_borrow(&self) -> Result<AtomicRef<T>, BorrowError> {
130132
match AtomicBorrowRef::try_new(&self.borrow) {
131133
Ok(borrow) => Ok(AtomicRef {
132-
value: unsafe { &*self.value.get() },
134+
value: unsafe { NonNull::new_unchecked(self.value.get()) },
133135
borrow,
134136
}),
135137
Err(_) => Err(BorrowError { _private: () }),
@@ -141,8 +143,9 @@ impl<T: ?Sized> AtomicRefCell<T> {
141143
pub fn borrow_mut(&self) -> AtomicRefMut<T> {
142144
match AtomicBorrowRefMut::try_new(&self.borrow) {
143145
Ok(borrow) => AtomicRefMut {
144-
value: unsafe { &mut *self.value.get() },
146+
value: unsafe { NonNull::new_unchecked(self.value.get()) },
145147
borrow,
148+
marker: PhantomData,
146149
},
147150
Err(s) => panic!("{}", s),
148151
}
@@ -154,8 +157,9 @@ impl<T: ?Sized> AtomicRefCell<T> {
154157
pub fn try_borrow_mut(&self) -> Result<AtomicRefMut<T>, BorrowMutError> {
155158
match AtomicBorrowRefMut::try_new(&self.borrow) {
156159
Ok(borrow) => Ok(AtomicRefMut {
157-
value: unsafe { &mut *self.value.get() },
160+
value: unsafe { NonNull::new_unchecked(self.value.get()) },
158161
borrow,
162+
marker: PhantomData,
159163
}),
160164
Err(_) => Err(BorrowMutError { _private: () }),
161165
}
@@ -366,16 +370,22 @@ impl<'b> Clone for AtomicBorrowRef<'b> {
366370

367371
/// A wrapper type for an immutably borrowed value from an `AtomicRefCell<T>`.
368372
pub struct AtomicRef<'b, T: ?Sized + 'b> {
369-
value: &'b T,
373+
value: NonNull<T>,
370374
borrow: AtomicBorrowRef<'b>,
371375
}
372376

377+
// SAFETY: `AtomicRef<'_, T> acts as a reference. `AtomicBorrowRef` is a
378+
// reference to an atomic.
379+
unsafe impl<'b, T: ?Sized + 'b> Sync for AtomicRef<'b, T> where for<'a> &'a T: Sync {}
380+
unsafe impl<'b, T: ?Sized + 'b> Send for AtomicRef<'b, T> where for<'a> &'a T: Send {}
381+
373382
impl<'b, T: ?Sized> Deref for AtomicRef<'b, T> {
374383
type Target = T;
375384

376385
#[inline]
377386
fn deref(&self) -> &T {
378-
self.value
387+
// SAFETY: We hold shared borrow of the value.
388+
unsafe { self.value.as_ref() }
379389
}
380390
}
381391

@@ -396,7 +406,7 @@ impl<'b, T: ?Sized> AtomicRef<'b, T> {
396406
F: FnOnce(&T) -> &U,
397407
{
398408
AtomicRef {
399-
value: f(orig.value),
409+
value: NonNull::from(f(&*orig)),
400410
borrow: orig.borrow,
401411
}
402412
}
@@ -408,7 +418,7 @@ impl<'b, T: ?Sized> AtomicRef<'b, T> {
408418
F: FnOnce(&T) -> Option<&U>,
409419
{
410420
Some(AtomicRef {
411-
value: f(orig.value)?,
421+
value: NonNull::from(f(&*orig)?),
412422
borrow: orig.borrow,
413423
})
414424
}
@@ -418,60 +428,75 @@ impl<'b, T: ?Sized> AtomicRefMut<'b, T> {
418428
/// Make a new `AtomicRefMut` for a component of the borrowed data, e.g. an enum
419429
/// variant.
420430
#[inline]
421-
pub fn map<U: ?Sized, F>(orig: AtomicRefMut<'b, T>, f: F) -> AtomicRefMut<'b, U>
431+
pub fn map<U: ?Sized, F>(mut orig: AtomicRefMut<'b, T>, f: F) -> AtomicRefMut<'b, U>
422432
where
423433
F: FnOnce(&mut T) -> &mut U,
424434
{
425435
AtomicRefMut {
426-
value: f(orig.value),
436+
value: NonNull::from(f(&mut *orig)),
427437
borrow: orig.borrow,
438+
marker: PhantomData,
428439
}
429440
}
430441

431442
/// Make a new `AtomicRefMut` for an optional component of the borrowed data.
432443
#[inline]
433-
pub fn filter_map<U: ?Sized, F>(orig: AtomicRefMut<'b, T>, f: F) -> Option<AtomicRefMut<'b, U>>
444+
pub fn filter_map<U: ?Sized, F>(
445+
mut orig: AtomicRefMut<'b, T>,
446+
f: F,
447+
) -> Option<AtomicRefMut<'b, U>>
434448
where
435449
F: FnOnce(&mut T) -> Option<&mut U>,
436450
{
437451
Some(AtomicRefMut {
438-
value: f(orig.value)?,
452+
value: NonNull::from(f(&mut *orig)?),
439453
borrow: orig.borrow,
454+
marker: PhantomData,
440455
})
441456
}
442457
}
443458

444459
/// A wrapper type for a mutably borrowed value from an `AtomicRefCell<T>`.
445460
pub struct AtomicRefMut<'b, T: ?Sized + 'b> {
446-
value: &'b mut T,
461+
value: NonNull<T>,
447462
borrow: AtomicBorrowRefMut<'b>,
463+
// `NonNull` is covariant over `T`, but this is used in place of a mutable
464+
// reference so we need to be invariant over `T`.
465+
marker: PhantomData<&'b mut T>,
448466
}
449467

468+
// SAFETY: `AtomicRefMut<'_, T> acts as a mutable reference.
469+
// `AtomicBorrowRefMut` is a reference to an atomic.
470+
unsafe impl<'b, T: ?Sized + 'b> Sync for AtomicRefMut<'b, T> where for<'a> &'a mut T: Sync {}
471+
unsafe impl<'b, T: ?Sized + 'b> Send for AtomicRefMut<'b, T> where for<'a> &'a mut T: Send {}
472+
450473
impl<'b, T: ?Sized> Deref for AtomicRefMut<'b, T> {
451474
type Target = T;
452475

453476
#[inline]
454477
fn deref(&self) -> &T {
455-
self.value
478+
// SAFETY: We hold an exclusive borrow of the value.
479+
unsafe { self.value.as_ref() }
456480
}
457481
}
458482

459483
impl<'b, T: ?Sized> DerefMut for AtomicRefMut<'b, T> {
460484
#[inline]
461485
fn deref_mut(&mut self) -> &mut T {
462-
self.value
486+
// SAFETY: We hold an exclusive borrow of the value.
487+
unsafe { self.value.as_mut() }
463488
}
464489
}
465490

466491
impl<'b, T: ?Sized + Debug + 'b> Debug for AtomicRef<'b, T> {
467492
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
468-
self.value.fmt(f)
493+
<T as Debug>::fmt(self, f)
469494
}
470495
}
471496

472497
impl<'b, T: ?Sized + Debug + 'b> Debug for AtomicRefMut<'b, T> {
473498
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
474-
self.value.fmt(f)
499+
<T as Debug>::fmt(self, f)
475500
}
476501
}
477502

0 commit comments

Comments
 (0)