epoch: Fix stacked borrows violations

ci/miri.sh

@@ -12,29 +12,24 @@ export RUSTFLAGS="${RUSTFLAGS:-} -Z randomize-layout"
 MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation" \
  cargo miri test \
  -p crossbeam-queue \
- -p crossbeam-utils 2>&1 | ts -i '%.s '
+ -p crossbeam-utils \
+ -p crossbeam-epoch \
+ -p crossbeam 2>&1 | ts -i '%.s '
 
 # -Zmiri-ignore-leaks is needed because we use detached threads in tests/docs: https://github.com/rust-lang/miri/issues/1371
 MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation -Zmiri-ignore-leaks" \
  cargo miri test \
  -p crossbeam-channel 2>&1 | ts -i '%.s '
 
-# Use Tree Borrows instead of Stacked Borrows because epoch is not compatible with Stacked Borrows: https://github.com/crossbeam-rs/crossbeam/issues/545#issuecomment-1192785003
-MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation -Zmiri-tree-borrows" \
- cargo miri test \
- -p crossbeam-epoch \
- -p crossbeam 2>&1 | ts -i '%.s '
-
-# Use Tree Borrows instead of Stacked Borrows because epoch is not compatible with Stacked Borrows: https://github.com/crossbeam-rs/crossbeam/issues/545#issuecomment-1192785003
+# Use Tree Borrows instead of Stacked Borrows because skiplist is not compatible with Stacked Borrows: https://github.com/crossbeam-rs/crossbeam/issues/878
 # -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/614
 MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation -Zmiri-tree-borrows -Zmiri-ignore-leaks" \
  cargo miri test \
  -p crossbeam-skiplist 2>&1 | ts -i '%.s '
 
-# Use Tree Borrows instead of Stacked Borrows because epoch is not compatible with Stacked Borrows: https://github.com/crossbeam-rs/crossbeam/issues/545#issuecomment-1192785003
 # -Zmiri-compare-exchange-weak-failure-rate=0.0 is needed because some sequential tests (e.g.,
 # doctest of Stealer::steal) incorrectly assume that sequential weak CAS will never fail.
 # -Zmiri-preemption-rate=0 is needed because this code technically has UB and Miri catches that.
-MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation -Zmiri-tree-borrows -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \
+MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-retag-fields -Zmiri-disable-isolation -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \
  cargo miri test \
  -p crossbeam-deque 2>&1 | ts -i '%.s '

crossbeam-epoch/src/atomic.rs

@@ -14,7 +14,9 @@ use crate::guard::Guard;
 use crate::primitive::sync::atomic::AtomicPtr;
 #[cfg(not(miri))]
 use crate::primitive::sync::atomic::AtomicUsize;
+
 use crossbeam_utils::atomic::AtomicConsume;
+use memoffset::offset_of;
 
 /// The error returned on failed compare-and-swap operation.
 pub struct CompareExchangeError<'g, T: ?Sized + Pointable, P: Pointer<T>> {
@@ -113,26 +115,26 @@ pub trait Pointable {
  ///
  /// - The given `ptr` should have been initialized with [`Pointable::init`].
  /// - `ptr` should not have yet been dropped by [`Pointable::drop`].
- /// - `ptr` should not be mutably dereferenced by [`Pointable::deref_mut`] concurrently.
- unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self;
+ /// - `ptr` should not be mutably dereferenced by [`Pointable::as_mut_ptr`] concurrently.
+ unsafe fn as_ptr(ptr: *mut ()) -> *const Self;
 
  /// Mutably dereferences the given pointer.
  ///
  /// # Safety
  ///
  /// - The given `ptr` should have been initialized with [`Pointable::init`].
  /// - `ptr` should not have yet been dropped by [`Pointable::drop`].
- /// - `ptr` should not be dereferenced by [`Pointable::deref`] or [`Pointable::deref_mut`]
+ /// - `ptr` should not be dereferenced by [`Pointable::as_ptr`] or [`Pointable::as_mut_ptr`]
  /// concurrently.
- unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self;
+ unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self;
 
  /// Drops the object pointed to by the given pointer.
  ///
  /// # Safety
  ///
  /// - The given `ptr` should have been initialized with [`Pointable::init`].
  /// - `ptr` should not have yet been dropped by [`Pointable::drop`].
- /// - `ptr` should not be dereferenced by [`Pointable::deref`] or [`Pointable::deref_mut`]
+ /// - `ptr` should not be dereferenced by [`Pointable::as_ptr`] or [`Pointable::as_mut_ptr`]
  /// concurrently.
  unsafe fn drop(ptr: *mut ());
 }
@@ -146,12 +148,12 @@ impl<T> Pointable for T {
  Box::into_raw(Box::new(init)).cast::<()>()
  }
 
- unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self {
- &*(ptr as *const T)
+ unsafe fn as_ptr(ptr: *mut ()) -> *const Self {
+ ptr as *const T
  }
 
- unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self {
- &mut *ptr.cast::<T>()
+ unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
+ ptr.cast::<T>()
  }
 
  unsafe fn drop(ptr: *mut ()) {
@@ -206,13 +208,19 @@ impl<T> Pointable for [MaybeUninit<T>] {
  ptr.cast::<()>()
  }
 
- unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self {
- let array = &*(ptr as *const Array<T>);
- slice::from_raw_parts(array.elements.as_ptr(), array.len)
+ unsafe fn as_ptr(ptr: *mut ()) -> *const Self {
+ let array = ptr.cast::<Array<T>>();
+ let elements = array
+ .cast::<u8>()
+ .add(offset_of!(Array<T>, elements))
+ .cast::<MaybeUninit<T>>();
+ // TODO: use ptr::slice_from_raw_parts once we bump MSRV to 1.42
+ slice::from_raw_parts(elements, (*array).len)
  }
 
- unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self {
+ unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
  let array = &mut *ptr.cast::<Array<T>>();
+ // TODO: use ptr::slice_from_raw_parts_mut once we bump MSRV to 1.42
  slice::from_raw_parts_mut(array.elements.as_mut_ptr(), array.len)
  }
 
@@ -808,7 +816,7 @@ impl<T: ?Sized + Pointable> fmt::Pointer for Atomic<T> {
  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
  let data = self.data.load(Ordering::SeqCst);
  let (raw, _) = decompose_tag::<T>(data);
- fmt::Pointer::fmt(&(unsafe { T::deref(raw) as *const _ }), f)
+ fmt::Pointer::fmt(&(unsafe { T::as_ptr(raw) }), f)
  }
 }
 
@@ -1096,14 +1104,14 @@ impl<T: ?Sized + Pointable> Deref for Owned<T> {
 
  fn deref(&self) -> &T {
  let (raw, _) = decompose_tag::<T>(self.data);
- unsafe { T::deref(raw) }
+ unsafe { &*T::as_ptr(raw) }
  }
 }
 
 impl<T: ?Sized + Pointable> DerefMut for Owned<T> {
  fn deref_mut(&mut self) -> &mut T {
  let (raw, _) = decompose_tag::<T>(self.data);
- unsafe { T::deref_mut(raw) }
+ unsafe { &mut *T::as_mut_ptr(raw) }
  }
 }
 
@@ -1256,6 +1264,16 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
  raw.is_null()
  }
 
+ pub(crate) unsafe fn as_ptr(&self) -> *const T {
+ let (raw, _) = decompose_tag::<T>(self.data);
+ T::as_ptr(raw)
+ }
+
+ pub(crate) unsafe fn as_mut_ptr(&self) -> *mut T {
+ let (raw, _) = decompose_tag::<T>(self.data);
+ T::as_mut_ptr(raw)
+ }
+
  /// Dereferences the pointer.
  ///
  /// Returns a reference to the pointee that is valid during the lifetime `'g`.
@@ -1289,8 +1307,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
  /// # unsafe { drop(a.into_owned()); } // avoid leak
  /// ```
  pub unsafe fn deref(&self) -> &'g T {
- let (raw, _) = decompose_tag::<T>(self.data);
- T::deref(raw)
+ &*self.as_ptr()
  }
 
  /// Dereferences the pointer.
@@ -1331,8 +1348,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
  /// # unsafe { drop(a.into_owned()); } // avoid leak
  /// ```
  pub unsafe fn deref_mut(&mut self) -> &'g mut T {
- let (raw, _) = decompose_tag::<T>(self.data);
- T::deref_mut(raw)
+ &mut *self.as_mut_ptr()
  }
 
  /// Converts the pointer to a reference.
@@ -1372,7 +1388,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
  if raw.is_null() {
  None
  } else {
- Some(T::deref(raw))
+ Some(&*T::as_ptr(raw))
  }
  }
 
@@ -1534,7 +1550,7 @@ impl<T: ?Sized + Pointable> fmt::Debug for Shared<'_, T> {
 
 impl<T: ?Sized + Pointable> fmt::Pointer for Shared<'_, T> {
  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- fmt::Pointer::fmt(&(unsafe { self.deref() as *const _ }), f)
+ fmt::Pointer::fmt(&(unsafe { self.as_ptr() }), f)
  }
 }
 

crossbeam-epoch/src/internal.rs

@@ -536,24 +536,24 @@ impl Local {
 }
 
 impl IsElement<Local> for Local {
- fn entry_of(local: &Local) -> &Entry {
+ fn entry_of(local: *const Local) -> *const Entry {
  unsafe {
- let entry_ptr = (local as *const Local as *const u8)
+ local
+ .cast::<u8>()
  .add(offset_of!(Local, entry))
- .cast::<Entry>();
- &*entry_ptr
+ .cast::<Entry>()
  }
  }
 
- unsafe fn element_of(entry: &Entry) -> &Local {
- let local_ptr = (entry as *const Entry as *const u8)
+ unsafe fn element_of(entry: *const Entry) -> *const Local {
+ entry
+ .cast::<u8>()
  .sub(offset_of!(Local, entry))
- .cast::<Local>();
- &*local_ptr
+ .cast::<Local>()
  }
 
- unsafe fn finalize(entry: &Entry, guard: &Guard) {
- guard.defer_destroy(Shared::from(Self::element_of(entry) as *const _));
+ unsafe fn finalize(entry: *const Entry, guard: &Guard) {
+ guard.defer_destroy(Shared::from(Self::element_of(entry)));
  }
 }
 

crossbeam-epoch/src/sync/list.rs

@@ -4,6 +4,7 @@
 //! 2002. <http://dl.acm.org/citation.cfm?id=564870.564881>
 
 use core::marker::PhantomData;
+use core::ptr::NonNull;
 use core::sync::atomic::Ordering::{Acquire, Relaxed, Release};
 
 use crate::{unprotected, Atomic, Guard, Shared};
@@ -66,7 +67,7 @@ pub(crate) struct Entry {
 ///
 pub(crate) trait IsElement<T> {
  /// Returns a reference to this element's `Entry`.
- fn entry_of(_: &T) -> &Entry;
+ fn entry_of(_: *const T) -> *const Entry;
 
  /// Given a reference to an element's entry, returns that element.
  ///
@@ -80,15 +81,15 @@ pub(crate) trait IsElement<T> {
  ///
  /// The caller has to guarantee that the `Entry` is called with was retrieved from an instance
  /// of the element type (`T`).
- unsafe fn element_of(_: &Entry) -> &T;
+ unsafe fn element_of(_: *const Entry) -> *const T;
 
  /// The function that is called when an entry is unlinked from list.
  ///
  /// # Safety
  ///
  /// The caller has to guarantee that the `Entry` is called with was retrieved from an instance
  /// of the element type (`T`).
- unsafe fn finalize(_: &Entry, _: &Guard);
+ unsafe fn finalize(_: *const Entry, _: &Guard);
 }
 
 /// A lock-free, intrusive linked list of type `T`.
@@ -173,16 +174,16 @@ impl<T, C: IsElement<T>> List<T, C> {
  // Insert right after head, i.e. at the beginning of the list.
  let to = &self.head;
  // Get the intrusively stored Entry of the new element to insert.
- let entry: &Entry = C::entry_of(container.deref());
+ let entry: *const Entry = C::entry_of(container.as_ptr());
  // Make a Shared ptr to that Entry.
- let entry_ptr = Shared::from(entry as *const _);
+ let entry_ptr = Shared::from(entry);
  // Read the current successor of where we want to insert.
  let mut next = to.load(Relaxed, guard);
 
  loop {
  // Set the Entry of the to-be-inserted element to point to the previous successor of
  // `to`.
- entry.next.store(next, Relaxed);
+ (*entry).next.store(next, Relaxed);
  match to.compare_exchange_weak(next, entry_ptr, Release, Relaxed, guard) {
  Ok(_) => break,
  // We lost the race or weak CAS failed spuriously. Update the successor and try
@@ -225,7 +226,7 @@ impl<T, C: IsElement<T>> Drop for List<T, C> {
  // Verify that all elements have been removed from the list.
  assert_eq!(succ.tag(), 1);
 
- C::finalize(curr.deref(), guard);
+ C::finalize(curr.as_ptr(), guard);
  curr = succ;
  }
  }
@@ -236,8 +237,8 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
  type Item = Result<&'g T, IterError>;
 
  fn next(&mut self) -> Option<Self::Item> {
- while let Some(c) = unsafe { self.curr.as_ref() } {
- let succ = c.next.load(Acquire, self.guard);
+ while let Some(c) = unsafe { NonNull::new(self.curr.as_ptr() as *mut Entry) } {
+ let succ = unsafe { c.as_ref().next.load(Acquire, self.guard) };
 
  if succ.tag() == 1 {
  // This entry was removed. Try unlinking it from the list.
@@ -257,7 +258,7 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
  // deallocation. Deferred drop is okay, because `list.delete()` can only be
  // called if `T: 'static`.
  unsafe {
- C::finalize(self.curr.deref(), self.guard);
+ C::finalize(self.curr.as_ptr(), self.guard);
  }
 
  // `succ` is the new value of `self.pred`.
@@ -284,10 +285,10 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
  }
 
  // Move one step forward.
- self.pred = &c.next;
+ self.pred = unsafe { &(*c.as_ptr()).next };
  self.curr = succ;
 
- return Some(Ok(unsafe { C::element_of(c) }));
+ return Some(Ok(unsafe { &*C::element_of(c.as_ptr()) }));
  }
 
  // We reached the end of the list.
@@ -303,16 +304,16 @@ mod tests {
  use std::sync::Barrier;
 
  impl IsElement<Entry> for Entry {
- fn entry_of(entry: &Entry) -> &Entry {
+ fn entry_of(entry: *const Entry) -> *const Entry {
  entry
  }
 
- unsafe fn element_of(entry: &Entry) -> &Entry {
+ unsafe fn element_of(entry: *const Entry) -> *const Entry {
  entry
  }
 
- unsafe fn finalize(entry: &Entry, guard: &Guard) {
- guard.defer_destroy(Shared::from(Self::element_of(entry) as *const _));
+ unsafe fn finalize(entry: *const Entry, guard: &Guard) {
+ guard.defer_destroy(Shared::from(Self::element_of(entry)));
  }
  }