Fix stacked borrows violations

ci/miri.sh

@@ -19,21 +19,19 @@ MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-t
     cargo miri test \
     -p crossbeam-channel
 
-# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
-MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" \
+MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-tag-raw-pointers -Zmiri-disable-isolation" \
     cargo miri test \
     -p crossbeam-epoch
 
 # -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/614
-# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
-MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation-Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \
+MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks" \
     cargo miri test \
     -p crossbeam-skiplist
 
-MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-compare-exchange-weak-failure-rate=1.0" \
+MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-tag-raw-pointers -Zmiri-compare-exchange-weak-failure-rate=1.0" \
     cargo miri test \
     -p crossbeam-deque
 
-MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check" \
+MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-tag-raw-pointers" \
     cargo miri test \
     -p crossbeam

crossbeam-epoch/src/atomic.rs

@@ -203,7 +203,7 @@ 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;
+    unsafe fn as_ptr(ptr: *mut ()) -> *const Self;
 
     /// Mutably dereferences the given pointer.
     ///
@@ -213,7 +213,7 @@ pub trait Pointable {
     /// - `ptr` should not have yet been dropped by [`Pointable::drop`].
     /// - `ptr` should not be dereferenced by [`Pointable::deref`] or [`Pointable::deref_mut`]
     ///   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.
     ///
@@ -235,12 +235,12 @@ impl<T> Pointable for T {
         Box::into_raw(Box::new(init)) as *mut ()
     }
 
-    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 as *mut T)
+    unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
+        ptr as *mut T
     }
 
     unsafe fn drop(ptr: *mut ()) {
@@ -295,18 +295,19 @@ impl<T> Pointable for [MaybeUninit<T>] {
         ptr as *mut ()
     }
 
-    unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self {
-        let array = &*(ptr as *const Array<T>);
-        slice::from_raw_parts(array.elements.as_ptr() as *const _, array.len)
+    unsafe fn as_ptr(ptr: *mut ()) -> *const Self {
+        let array = ptr as *mut Array<T>;
+        let data = ptr::addr_of_mut!((*array).elements);
+        slice::from_raw_parts(data as *const _, (*array).len)
     }
 
-    unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self {
-        let array = &*(ptr as *mut Array<T>);
-        slice::from_raw_parts_mut(array.elements.as_ptr() as *mut _, array.len)
+    unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
+        let array = &mut *(ptr as *mut Array<T>);
+        slice::from_raw_parts_mut(array.elements.as_mut_ptr() as *mut _, array.len)
     }
 
     unsafe fn drop(ptr: *mut ()) {
-        let array = &*(ptr as *mut Array<T>);
+        let array = &mut *(ptr as *mut Array<T>);
         let size = mem::size_of::<Array<T>>() + mem::size_of::<MaybeUninit<T>>() * array.len;
         let align = mem::align_of::<Array<T>>();
         let layout = alloc::Layout::from_size_align(size, align).unwrap();
@@ -960,7 +961,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)
     }
 }
 
@@ -1246,14 +1247,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) }
     }
 }
 
@@ -1439,7 +1440,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
     /// ```
     pub unsafe fn deref(&self) -> &'g T {
         let (raw, _) = decompose_tag::<T>(self.data);
-        T::deref(raw)
+        &*T::as_ptr(raw)
     }
 
     /// Dereferences the pointer.
@@ -1481,7 +1482,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
     /// ```
     pub unsafe fn deref_mut(&mut self) -> &'g mut T {
         let (raw, _) = decompose_tag::<T>(self.data);
-        T::deref_mut(raw)
+        &mut *T::as_mut_ptr(raw)
     }
 
     /// Converts the pointer to a reference.
@@ -1521,10 +1522,25 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
         if raw.is_null() {
             None
         } else {
-            Some(T::deref(raw))
+            Some(&*T::as_ptr(raw))
         }
     }
 
+    pub(crate) unsafe fn as_ptr(&self) -> Option<*const T> {
+        let (raw, _) = decompose_tag::<T>(self.data);
+        if raw.is_null() {
+            None
+        } else {
+            Some(T::as_ptr(raw))
+        }
+    }
+
+    pub(crate) unsafe fn as_mut_ptr(&mut self) -> *mut T {
+        let (raw, _) = decompose_tag::<T>(self.data);
+        debug_assert!(!raw.is_null());
+        T::as_mut_ptr(raw)
+    }
+
     /// Takes ownership of the pointee.
     ///
     /// # Panics

crossbeam-epoch/src/collector.rs

@@ -98,7 +98,7 @@ impl Drop for LocalHandle {
     #[inline]
     fn drop(&mut self) {
         unsafe {
-            Local::release_handle(&*self.local);
+            Local::release_handle(self.local);
         }
     }
 }
@@ -111,7 +111,7 @@ impl fmt::Debug for LocalHandle {
 
 #[cfg(all(test, not(crossbeam_loom)))]
 mod tests {
-    use std::mem;
+    use std::mem::ManuallyDrop;
     use std::sync::atomic::{AtomicUsize, Ordering};
 
     use crossbeam_utils::thread;
@@ -402,15 +402,13 @@ mod tests {
                 v.push(i as i32);
             }
 
-            let ptr = v.as_mut_ptr() as usize;
             let len = v.len();
+            let ptr = ManuallyDrop::new(v).as_mut_ptr();
             guard.defer_unchecked(move || {
-                drop(Vec::from_raw_parts(ptr as *const i32 as *mut i32, len, len));
+                drop(Vec::from_raw_parts(ptr, len, len));
                 DESTROYS.fetch_add(len, Ordering::Relaxed);
             });
             guard.flush();
-
-            mem::forget(v);
         }
 
         while DESTROYS.load(Ordering::Relaxed) < COUNT {

crossbeam-epoch/src/guard.rs

@@ -370,14 +370,16 @@ impl Guard {
             // We need to acquire a handle here to ensure the Local doesn't
             // disappear from under us.
             local.acquire_handle();
-            local.unpin();
+            unsafe {
+                Local::unpin(self.local);
+            }
         }
 
         // Ensure the Guard is re-pinned even if the function panics
         defer! {
             if let Some(local) = unsafe { self.local.as_ref() } {
                 mem::forget(local.pin());
-                local.release_handle();
+                unsafe { Local::release_handle(self.local) };
             }
         }
 
@@ -408,8 +410,10 @@ impl Guard {
 impl Drop for Guard {
     #[inline]
     fn drop(&mut self) {
-        if let Some(local) = unsafe { self.local.as_ref() } {
-            local.unpin();
+        if !self.local.is_null() {
+            unsafe {
+                Local::unpin(self.local);
+            }
         }
     }
 }

crossbeam-epoch/src/internal.rs

@@ -515,15 +515,16 @@ impl Local {
 
     /// Unpins the `Local`.
     #[inline]
-    pub(crate) fn unpin(&self) {
-        let guard_count = self.guard_count.get();
-        self.guard_count.set(guard_count - 1);
+    pub(crate) unsafe fn unpin(ptr: *const Self) {
+        let this = &*ptr;
+        let guard_count = this.guard_count.get();
+        this.guard_count.set(guard_count - 1);
 
         if guard_count == 1 {
-            self.epoch.store(Epoch::starting(), Ordering::Release);
+            this.epoch.store(Epoch::starting(), Ordering::Release);
 
-            if self.handle_count.get() == 0 {
-                self.finalize();
+            if this.handle_count.get() == 0 {
+                Self::finalize(ptr);
             }
         }
     }
@@ -561,44 +562,46 @@ impl Local {
 
     /// Decrements the handle count.
     #[inline]
-    pub(crate) fn release_handle(&self) {
-        let guard_count = self.guard_count.get();
-        let handle_count = self.handle_count.get();
+    pub(crate) unsafe fn release_handle(ptr: *const Self) {
+        let this = &*ptr;
+        let guard_count = this.guard_count.get();
+        let handle_count = this.handle_count.get();
         debug_assert!(handle_count >= 1);
-        self.handle_count.set(handle_count - 1);
+        this.handle_count.set(handle_count - 1);
 
         if guard_count == 0 && handle_count == 1 {
-            self.finalize();
+            Self::finalize(ptr);
         }
     }
 
     /// Removes the `Local` from the global linked list.
     #[cold]
-    fn finalize(&self) {
-        debug_assert_eq!(self.guard_count.get(), 0);
-        debug_assert_eq!(self.handle_count.get(), 0);
+    unsafe fn finalize(ptr: *const Self) {
+        let this = unsafe { &*ptr };
+        debug_assert_eq!(this.guard_count.get(), 0);
+        debug_assert_eq!(this.handle_count.get(), 0);
 
         // Temporarily increment handle count. This is required so that the following call to `pin`
         // doesn't call `finalize` again.
-        self.handle_count.set(1);
+        this.handle_count.set(1);
         unsafe {
             // Pin and move the local bag into the global queue. It's important that `push_bag`
             // doesn't defer destruction on any new garbage.
-            let guard = &self.pin();
-            self.global()
-                .push_bag(self.bag.with_mut(|b| &mut *b), guard);
+            let guard = &this.pin();
+            this.global()
+                .push_bag(this.bag.with_mut(|b| &mut *b), guard);
         }
         // Revert the handle count back to zero.
-        self.handle_count.set(0);
+        this.handle_count.set(0);
 
         unsafe {
             // Take the reference to the `Global` out of this `Local`. Since we're not protected
             // by a guard at this time, it's crucial that the reference is read before marking the
             // `Local` as deleted.
-            let collector: Collector = ptr::read(self.collector.with(|c| &*(*c)));
+            let collector: Collector = ptr::read(this.collector.with(|c| &*(*c)));
 
             // Mark this node in the linked list as deleted.
-            self.entry.delete(unprotected());
+            this.entry.delete(unprotected());
 
             // Finally, drop the reference to the global. Note that this might be the last reference
             // to the `Global`. If so, the global data will be destroyed and all deferred functions
@@ -609,22 +612,18 @@ 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).add(offset_of!(Local, entry)) as *const Entry;
-            &*entry_ptr
+            (local as *const Local as *const u8).add(offset_of!(Local, entry)) as *const Entry
         }
     }
 
-    unsafe fn element_of(entry: &Entry) -> &Local {
-        let local_ptr =
-            (entry as *const Entry as *const u8).sub(offset_of!(Local, entry)) as *const Local;
-        &*local_ptr
+    unsafe fn element_of(entry: *const Entry) -> *const Local {
+        (entry as *const Entry as *const u8).sub(offset_of!(Local, entry)) as *const 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/lib.rs

@@ -61,6 +61,7 @@
     rust_2018_idioms,
     unreachable_pub
 )]
+#![allow(unused_unsafe)] // TODO: use unsafe_op_in_unsafe_fn
 #![cfg_attr(not(feature = "std"), no_std)]
 #![cfg_attr(feature = "nightly", feature(const_fn_trait_bound))]