From b05e1e3609aeffde70735858101c12103c06048a Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 22 Jul 2022 23:04:08 +0900 Subject: [PATCH 1/3] epoch: Remove ptr-to-int casts --- ci/miri.sh | 15 +- crossbeam-epoch/src/atomic.rs | 229 +++++++++++++++++++------------ crossbeam-epoch/src/collector.rs | 4 +- crossbeam-epoch/src/internal.rs | 14 +- crossbeam-epoch/src/lib.rs | 5 +- tests/subcrates.rs | 1 - 6 files changed, 167 insertions(+), 101 deletions(-) diff --git a/ci/miri.sh b/ci/miri.sh index 50b19b9e5..2ac6814f1 100755 --- a/ci/miri.sh +++ b/ci/miri.sh @@ -19,23 +19,26 @@ MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignor cargo miri test \ -p crossbeam-channel 2>&1 | ts -i '%.s ' -# -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/579 +# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545 +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" \ + cargo miri test \ + -p crossbeam-epoch 2>&1 | ts -i '%.s ' + +# -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-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \ cargo miri test \ - -p crossbeam-epoch \ -p crossbeam-skiplist 2>&1 | ts -i '%.s ' -# -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/579 # -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545 # -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-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \ +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \ cargo miri test \ -p crossbeam-deque 2>&1 | ts -i '%.s ' -# -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/579 -MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-ignore-leaks" \ +# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545 +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows" \ cargo miri test \ -p crossbeam 2>&1 | ts -i '%.s ' diff --git a/crossbeam-epoch/src/atomic.rs b/crossbeam-epoch/src/atomic.rs index 19bab4729..aeacca356 100644 --- a/crossbeam-epoch/src/atomic.rs +++ b/crossbeam-epoch/src/atomic.rs @@ -4,12 +4,15 @@ use core::fmt; use core::marker::PhantomData; use core::mem::{self, MaybeUninit}; use core::ops::{Deref, DerefMut}; +use core::ptr; use core::slice; use core::sync::atomic::Ordering; use crate::alloc::alloc; use crate::alloc::boxed::Box; use crate::guard::Guard; +use crate::primitive::sync::atomic::AtomicPtr; +#[cfg(not(miri))] use crate::primitive::sync::atomic::AtomicUsize; use crossbeam_utils::atomic::AtomicConsume; @@ -109,22 +112,35 @@ fn low_bits() -> usize { /// Panics if the pointer is not properly unaligned. #[inline] -fn ensure_aligned(raw: usize) { - assert_eq!(raw & low_bits::(), 0, "unaligned pointer"); +fn ensure_aligned(raw: *mut ()) { + assert_eq!(raw as usize & low_bits::(), 0, "unaligned pointer"); } /// Given a tagged pointer `data`, returns the same pointer, but tagged with `tag`. /// /// `tag` is truncated to fit into the unused bits of the pointer to `T`. #[inline] -fn compose_tag(data: usize, tag: usize) -> usize { - (data & !low_bits::()) | (tag & low_bits::()) +fn compose_tag(ptr: *mut (), tag: usize) -> *mut () { + int_to_ptr_with_provenance( + (ptr as usize & !low_bits::()) | (tag & low_bits::()), + ptr, + ) } /// Decomposes a tagged pointer `data` into the pointer and the tag. #[inline] -fn decompose_tag(data: usize) -> (usize, usize) { - (data & !low_bits::(), data & low_bits::()) +fn decompose_tag(ptr: *mut ()) -> (*mut (), usize) { + ( + int_to_ptr_with_provenance(ptr as usize & !low_bits::(), ptr), + ptr as usize & low_bits::(), + ) +} + +// HACK: https://github.com/rust-lang/miri/issues/1866#issuecomment-985802751 +#[inline] +fn int_to_ptr_with_provenance(addr: usize, prov: *mut T) -> *mut T { + let ptr = prov.cast::(); + ptr.wrapping_add(addr.wrapping_sub(ptr as usize)).cast() } /// Types that are pointed to by a single word. @@ -159,7 +175,7 @@ pub trait Pointable { /// # Safety /// /// The result should be a multiple of `ALIGN`. - unsafe fn init(init: Self::Init) -> usize; + unsafe fn init(init: Self::Init) -> *mut (); /// Dereferences the given pointer. /// @@ -168,7 +184,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: usize) -> &'a Self; + unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self; /// Mutably dereferences the given pointer. /// @@ -178,7 +194,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: usize) -> &'a mut Self; + unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self; /// Drops the object pointed to by the given pointer. /// @@ -188,7 +204,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 drop(ptr: usize); + unsafe fn drop(ptr: *mut ()); } impl Pointable for T { @@ -196,20 +212,20 @@ impl Pointable for T { type Init = T; - unsafe fn init(init: Self::Init) -> usize { - Box::into_raw(Box::new(init)) as usize + unsafe fn init(init: Self::Init) -> *mut () { + Box::into_raw(Box::new(init)).cast::<()>() } - unsafe fn deref<'a>(ptr: usize) -> &'a Self { + unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self { &*(ptr as *const T) } - unsafe fn deref_mut<'a>(ptr: usize) -> &'a mut Self { - &mut *(ptr as *mut T) + unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self { + &mut *ptr.cast::() } - unsafe fn drop(ptr: usize) { - drop(Box::from_raw(ptr as *mut T)); + unsafe fn drop(ptr: *mut ()) { + drop(Box::from_raw(ptr.cast::())); } } @@ -248,7 +264,7 @@ impl Pointable for [MaybeUninit] { type Init = usize; - unsafe fn init(len: Self::Init) -> usize { + unsafe fn init(len: Self::Init) -> *mut () { let size = mem::size_of::>() + mem::size_of::>() * len; let align = mem::align_of::>(); let layout = alloc::Layout::from_size_align(size, align).unwrap(); @@ -257,25 +273,25 @@ impl Pointable for [MaybeUninit] { alloc::handle_alloc_error(layout); } (*ptr).len = len; - ptr as usize + ptr.cast::<()>() } - unsafe fn deref<'a>(ptr: usize) -> &'a Self { + unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self { let array = &*(ptr as *const Array); - slice::from_raw_parts(array.elements.as_ptr() as *const _, array.len) + slice::from_raw_parts(array.elements.as_ptr(), array.len) } - unsafe fn deref_mut<'a>(ptr: usize) -> &'a mut Self { - let array = &*(ptr as *mut Array); - slice::from_raw_parts_mut(array.elements.as_ptr() as *mut _, array.len) + unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self { + let array = &mut *ptr.cast::>(); + slice::from_raw_parts_mut(array.elements.as_mut_ptr(), array.len) } - unsafe fn drop(ptr: usize) { - let array = &*(ptr as *mut Array); + unsafe fn drop(ptr: *mut ()) { + let array = &*ptr.cast::>(); let size = mem::size_of::>() + mem::size_of::>() * array.len; let align = mem::align_of::>(); let layout = alloc::Layout::from_size_align(size, align).unwrap(); - alloc::dealloc(ptr as *mut u8, layout); + alloc::dealloc(ptr.cast::(), layout); } } @@ -289,7 +305,7 @@ impl Pointable for [MaybeUninit] { /// /// Crossbeam supports dynamically sized types. See [`Pointable`] for details. pub struct Atomic { - data: AtomicUsize, + data: AtomicPtr<()>, _marker: PhantomData<*mut T>, } @@ -328,9 +344,9 @@ impl Atomic { } /// Returns a new atomic pointer pointing to the tagged pointer `data`. - fn from_usize(data: usize) -> Self { + fn from_ptr(data: *mut ()) -> Self { Self { - data: AtomicUsize::new(data), + data: AtomicPtr::new(data), _marker: PhantomData, } } @@ -347,7 +363,7 @@ impl Atomic { #[cfg(all(not(crossbeam_no_const_fn_trait_bound), not(crossbeam_loom)))] pub const fn null() -> Atomic { Self { - data: AtomicUsize::new(0), + data: AtomicPtr::new(ptr::null_mut()), _marker: PhantomData, } } @@ -356,7 +372,7 @@ impl Atomic { #[cfg(not(all(not(crossbeam_no_const_fn_trait_bound), not(crossbeam_loom))))] pub fn null() -> Atomic { Self { - data: AtomicUsize::new(0), + data: AtomicPtr::new(ptr::null_mut()), _marker: PhantomData, } } @@ -378,7 +394,7 @@ impl Atomic { /// # unsafe { drop(a.into_owned()); } // avoid leak /// ``` pub fn load<'g>(&self, ord: Ordering, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.load(ord)) } + unsafe { Shared::from_ptr(self.data.load(ord)) } } /// Loads a `Shared` from the atomic pointer using a "consume" memory ordering. @@ -404,7 +420,7 @@ impl Atomic { /// # unsafe { drop(a.into_owned()); } // avoid leak /// ``` pub fn load_consume<'g>(&self, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.load_consume()) } + unsafe { Shared::from_ptr(self.data.load_consume()) } } /// Stores a `Shared` or `Owned` pointer into the atomic pointer. @@ -425,7 +441,7 @@ impl Atomic { /// # unsafe { drop(a.into_owned()); } // avoid leak /// ``` pub fn store>(&self, new: P, ord: Ordering) { - self.data.store(new.into_usize(), ord); + self.data.store(new.into_ptr(), ord); } /// Stores a `Shared` or `Owned` pointer into the atomic pointer, returning the previous @@ -446,7 +462,7 @@ impl Atomic { /// # unsafe { drop(p.into_owned()); } // avoid leak /// ``` pub fn swap<'g, P: Pointer>(&self, new: P, ord: Ordering, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.swap(new.into_usize(), ord)) } + unsafe { Shared::from_ptr(self.data.swap(new.into_ptr(), ord)) } } /// Stores the pointer `new` (either `Shared` or `Owned`) into the atomic pointer if the current @@ -491,14 +507,14 @@ impl Atomic { where P: Pointer, { - let new = new.into_usize(); + let new = new.into_ptr(); self.data - .compare_exchange(current.into_usize(), new, success, failure) - .map(|_| unsafe { Shared::from_usize(new) }) + .compare_exchange(current.into_ptr(), new, success, failure) + .map(|_| unsafe { Shared::from_ptr(new) }) .map_err(|current| unsafe { CompareExchangeError { - current: Shared::from_usize(current), - new: P::from_usize(new), + current: Shared::from_ptr(current), + new: P::from_ptr(new), } }) } @@ -568,14 +584,14 @@ impl Atomic { where P: Pointer, { - let new = new.into_usize(); + let new = new.into_ptr(); self.data - .compare_exchange_weak(current.into_usize(), new, success, failure) - .map(|_| unsafe { Shared::from_usize(new) }) + .compare_exchange_weak(current.into_ptr(), new, success, failure) + .map(|_| unsafe { Shared::from_ptr(new) }) .map_err(|current| unsafe { CompareExchangeError { - current: Shared::from_usize(current), - new: P::from_usize(new), + current: Shared::from_ptr(current), + new: P::from_ptr(new), } }) } @@ -795,7 +811,21 @@ impl Atomic { /// assert_eq!(a.load(SeqCst, guard).tag(), 2); /// ``` pub fn fetch_and<'g>(&self, val: usize, ord: Ordering, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.fetch_and(val | !low_bits::(), ord)) } + // Ideally, we would always use AtomicPtr::fetch_* since it is strict-provenance + // compatible, but it is unstable. + // Code using AtomicUsize::fetch_* via casts is still permissive-provenance + // compatible and is sound. + #[cfg(miri)] + unsafe { + Shared::from_ptr(self.data.fetch_and(val | !low_bits::(), ord)) + } + #[cfg(not(miri))] + unsafe { + Shared::from_ptr( + (*(&self.data as *const AtomicPtr<_> as *const AtomicUsize)) + .fetch_and(val | !low_bits::(), ord) as *mut (), + ) + } } /// Bitwise "or" with the current tag. @@ -818,7 +848,21 @@ impl Atomic { /// assert_eq!(a.load(SeqCst, guard).tag(), 3); /// ``` pub fn fetch_or<'g>(&self, val: usize, ord: Ordering, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.fetch_or(val & low_bits::(), ord)) } + // Ideally, we would always use AtomicPtr::fetch_* since it is strict-provenance + // compatible, but it is unstable. + // Code using AtomicUsize::fetch_* via casts is still permissive-provenance + // compatible and is sound. + #[cfg(miri)] + unsafe { + Shared::from_ptr(self.data.fetch_or(val & low_bits::(), ord)) + } + #[cfg(not(miri))] + unsafe { + Shared::from_ptr( + (*(&self.data as *const AtomicPtr<_> as *const AtomicUsize)) + .fetch_or(val & low_bits::(), ord) as *mut (), + ) + } } /// Bitwise "xor" with the current tag. @@ -841,7 +885,21 @@ impl Atomic { /// assert_eq!(a.load(SeqCst, guard).tag(), 2); /// ``` pub fn fetch_xor<'g>(&self, val: usize, ord: Ordering, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.data.fetch_xor(val & low_bits::(), ord)) } + // Ideally, we would always use AtomicPtr::fetch_* since it is strict-provenance + // compatible, but it is unstable. + // Code using AtomicUsize::fetch_* via casts is still permissive-provenance + // compatible and is sound. + #[cfg(miri)] + unsafe { + Shared::from_ptr(self.data.fetch_xor(val & low_bits::(), ord)) + } + #[cfg(not(miri))] + unsafe { + Shared::from_ptr( + (*(&self.data as *const AtomicPtr<_> as *const AtomicUsize)) + .fetch_xor(val & low_bits::(), ord) as *mut (), + ) + } } /// Takes ownership of the pointee. @@ -884,11 +942,11 @@ impl Atomic { // FIXME: loom does not yet support into_inner, so we use unsync_load for now, // which should have the same synchronization properties: // https://github.com/tokio-rs/loom/issues/117 - Owned::from_usize(self.data.unsync_load()) + Owned::from_ptr(self.data.unsync_load()) } #[cfg(not(crossbeam_loom))] { - Owned::from_usize(self.data.into_inner()) + Owned::from_ptr(self.data.into_inner()) } } @@ -931,10 +989,10 @@ impl Atomic { let data = self.data.unsync_load(); #[cfg(not(crossbeam_loom))] let data = self.data.into_inner(); - if decompose_tag::(data).0 == 0 { + if decompose_tag::(data).0.is_null() { None } else { - Some(Owned::from_usize(data)) + Some(Owned::from_ptr(data)) } } } @@ -966,7 +1024,7 @@ impl Clone for Atomic { /// atomics or fences. fn clone(&self) -> Self { let data = self.data.load(Ordering::Relaxed); - Atomic::from_usize(data) + Atomic::from_ptr(data) } } @@ -990,7 +1048,7 @@ impl From> for Atomic { fn from(owned: Owned) -> Self { let data = owned.data; mem::forget(owned); - Self::from_usize(data) + Self::from_ptr(data) } } @@ -1017,7 +1075,7 @@ impl<'g, T: ?Sized + Pointable> From> for Atomic { /// let a = Atomic::::from(Shared::::null()); /// ``` fn from(ptr: Shared<'g, T>) -> Self { - Self::from_usize(ptr.data) + Self::from_ptr(ptr.data) } } @@ -1033,22 +1091,23 @@ impl From<*const T> for Atomic { /// let a = Atomic::::from(ptr::null::()); /// ``` fn from(raw: *const T) -> Self { - Self::from_usize(raw as usize) + Self::from_ptr(raw as *mut ()) } } /// A trait for either `Owned` or `Shared` pointers. +// TODO: seal this trait https://github.com/crossbeam-rs/crossbeam/issues/620 pub trait Pointer { /// Returns the machine representation of the pointer. - fn into_usize(self) -> usize; + fn into_ptr(self) -> *mut (); /// Returns a new pointer pointing to the tagged pointer `data`. /// /// # Safety /// - /// The given `data` should have been created by `Pointer::into_usize()`, and one `data` should - /// not be converted back by `Pointer::from_usize()` multiple times. - unsafe fn from_usize(data: usize) -> Self; + /// The given `data` should have been created by `Pointer::into_ptr()`, and one `data` should + /// not be converted back by `Pointer::from_ptr()` multiple times. + unsafe fn from_ptr(data: *mut ()) -> Self; } /// An owned heap-allocated object. @@ -1058,13 +1117,13 @@ pub trait Pointer { /// The pointer must be properly aligned. Since it is aligned, a tag can be stored into the unused /// least significant bits of the address. pub struct Owned { - data: usize, + data: *mut (), _marker: PhantomData>, } impl Pointer for Owned { #[inline] - fn into_usize(self) -> usize { + fn into_ptr(self) -> *mut () { let data = self.data; mem::forget(self); data @@ -1074,11 +1133,11 @@ impl Pointer for Owned { /// /// # Panics /// - /// Panics if the data is zero in debug mode. + /// Panics if the pointer is null, but only in debug mode. #[inline] - unsafe fn from_usize(data: usize) -> Self { - debug_assert!(data != 0, "converting zero into `Owned`"); - Owned { + unsafe fn from_ptr(data: *mut ()) -> Self { + debug_assert!(!data.is_null(), "converting null into `Owned`"); + Self { data, _marker: PhantomData, } @@ -1109,9 +1168,9 @@ impl Owned { /// let o = unsafe { Owned::from_raw(Box::into_raw(Box::new(1234))) }; /// ``` pub unsafe fn from_raw(raw: *mut T) -> Owned { - let raw = raw as usize; + let raw = raw.cast::<()>(); ensure_aligned::(raw); - Self::from_usize(raw) + Self::from_ptr(raw) } /// Converts the owned pointer into a `Box`. @@ -1128,7 +1187,7 @@ impl Owned { pub fn into_box(self) -> Box { let (raw, _) = decompose_tag::(self.data); mem::forget(self); - unsafe { Box::from_raw(raw as *mut _) } + unsafe { Box::from_raw(raw.cast::()) } } /// Allocates `value` on the heap and returns a new owned pointer pointing to it. @@ -1156,7 +1215,7 @@ impl Owned { /// let o = Owned::::init(1234); /// ``` pub fn init(init: T::Init) -> Owned { - unsafe { Self::from_usize(T::init(init)) } + unsafe { Self::from_ptr(T::init(init)) } } /// Converts the owned pointer into a [`Shared`]. @@ -1173,7 +1232,7 @@ impl Owned { /// ``` #[allow(clippy::needless_lifetimes)] pub fn into_shared<'g>(self, _: &'g Guard) -> Shared<'g, T> { - unsafe { Shared::from_usize(self.into_usize()) } + unsafe { Shared::from_ptr(self.into_ptr()) } } /// Returns the tag stored within the pointer. @@ -1204,8 +1263,8 @@ impl Owned { /// assert_eq!(o.tag(), 2); /// ``` pub fn with_tag(self, tag: usize) -> Owned { - let data = self.into_usize(); - unsafe { Self::from_usize(compose_tag::(data, tag)) } + let data = self.into_ptr(); + unsafe { Self::from_ptr(compose_tag::(data, tag)) } } } @@ -1307,7 +1366,7 @@ impl AsMut for Owned { /// The pointer must be properly aligned. Since it is aligned, a tag can be stored into the unused /// least significant bits of the address. pub struct Shared<'g, T: 'g + ?Sized + Pointable> { - data: usize, + data: *mut (), _marker: PhantomData<(&'g (), *const T)>, } @@ -1324,12 +1383,12 @@ impl Copy for Shared<'_, T> {} impl Pointer for Shared<'_, T> { #[inline] - fn into_usize(self) -> usize { + fn into_ptr(self) -> *mut () { self.data } #[inline] - unsafe fn from_usize(data: usize) -> Self { + unsafe fn from_ptr(data: *mut ()) -> Self { Shared { data, _marker: PhantomData, @@ -1374,7 +1433,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { /// ``` pub fn null() -> Shared<'g, T> { Shared { - data: 0, + data: ptr::null_mut(), _marker: PhantomData, } } @@ -1396,7 +1455,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { /// ``` pub fn is_null(&self) -> bool { let (raw, _) = decompose_tag::(self.data); - raw == 0 + raw.is_null() } /// Dereferences the pointer. @@ -1512,7 +1571,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { /// ``` pub unsafe fn as_ref(&self) -> Option<&'g T> { let (raw, _) = decompose_tag::(self.data); - if raw == 0 { + if raw.is_null() { None } else { Some(T::deref(raw)) @@ -1545,7 +1604,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { /// ``` pub unsafe fn into_owned(self) -> Owned { debug_assert!(!self.is_null(), "converting a null `Shared` into `Owned`"); - Owned::from_usize(self.data) + Owned::from_ptr(self.data) } /// Takes ownership of the pointee if it is not null. @@ -1574,7 +1633,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { if self.is_null() { None } else { - Some(Owned::from_usize(self.data)) + Some(Owned::from_ptr(self.data)) } } @@ -1617,7 +1676,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> { /// # unsafe { drop(a.into_owned()); } // avoid leak /// ``` pub fn with_tag(&self, tag: usize) -> Shared<'g, T> { - unsafe { Self::from_usize(compose_tag::(self.data, tag)) } + unsafe { Self::from_ptr(compose_tag::(self.data, tag)) } } } @@ -1638,9 +1697,9 @@ impl From<*const T> for Shared<'_, T> { /// # unsafe { drop(p.into_owned()); } // avoid leak /// ``` fn from(raw: *const T) -> Self { - let raw = raw as usize; + let raw = raw as *mut (); ensure_aligned::(raw); - unsafe { Self::from_usize(raw) } + unsafe { Self::from_ptr(raw) } } } diff --git a/crossbeam-epoch/src/collector.rs b/crossbeam-epoch/src/collector.rs index 5b0851184..129009995 100644 --- a/crossbeam-epoch/src/collector.rs +++ b/crossbeam-epoch/src/collector.rs @@ -403,9 +403,9 @@ mod tests { } let len = v.len(); - let ptr = ManuallyDrop::new(v).as_mut_ptr() as usize; + 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(); diff --git a/crossbeam-epoch/src/internal.rs b/crossbeam-epoch/src/internal.rs index 00c66a40a..a39b50033 100644 --- a/crossbeam-epoch/src/internal.rs +++ b/crossbeam-epoch/src/internal.rs @@ -537,14 +537,18 @@ impl Local { impl IsElement for Local { fn entry_of(local: &Local) -> &Entry { - let entry_ptr = (local as *const Local as usize + offset_of!(Local, entry)) as *const Entry; - unsafe { &*entry_ptr } + unsafe { + let entry_ptr = (local as *const Local as *const u8) + .add(offset_of!(Local, entry)) + .cast::(); + &*entry_ptr + } } unsafe fn element_of(entry: &Entry) -> &Local { - // offset_of! macro uses unsafe, but it's unnecessary in this context. - #[allow(unused_unsafe)] - let local_ptr = (entry as *const Entry as usize - offset_of!(Local, entry)) as *const Local; + let local_ptr = (entry as *const Entry as *const u8) + .sub(offset_of!(Local, entry)) + .cast::(); &*local_ptr } diff --git a/crossbeam-epoch/src/lib.rs b/crossbeam-epoch/src/lib.rs index 4cf982b6b..7c36d4c55 100644 --- a/crossbeam-epoch/src/lib.rs +++ b/crossbeam-epoch/src/lib.rs @@ -62,6 +62,7 @@ unreachable_pub )] #![cfg_attr(not(feature = "std"), no_std)] +#![cfg_attr(miri, feature(strict_provenance_atomic_ptr))] #[cfg(crossbeam_loom)] extern crate loom_crate as loom; @@ -77,7 +78,7 @@ mod primitive { pub(crate) mod sync { pub(crate) mod atomic { use core::sync::atomic::Ordering; - pub(crate) use loom::sync::atomic::{fence, AtomicUsize}; + pub(crate) use loom::sync::atomic::{fence, AtomicPtr, AtomicUsize}; // FIXME: loom does not support compiler_fence at the moment. // https://github.com/tokio-rs/loom/issues/117 @@ -127,7 +128,7 @@ mod primitive { pub(crate) mod atomic { pub(crate) use core::sync::atomic::compiler_fence; pub(crate) use core::sync::atomic::fence; - pub(crate) use core::sync::atomic::AtomicUsize; + pub(crate) use core::sync::atomic::{AtomicPtr, AtomicUsize}; } pub(crate) use alloc::sync::Arc; } diff --git a/tests/subcrates.rs b/tests/subcrates.rs index f7d2f5de8..21b99fb0e 100644 --- a/tests/subcrates.rs +++ b/tests/subcrates.rs @@ -20,7 +20,6 @@ fn deque() { } #[test] -#[cfg_attr(miri, ignore)] // Miri ICE: https://github.com/crossbeam-rs/crossbeam/pull/870#issuecomment-1189209073 fn epoch() { crossbeam::epoch::pin(); } From 46d0777ff4056809e9245a5b34b2e54d73a5021e Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 22 Jul 2022 23:04:11 +0900 Subject: [PATCH 2/3] Apply patch to once_cell --- Cargo.toml | 4 ++++ ci/miri.sh | 12 ++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6b6882a8c..0c935517b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,3 +86,7 @@ members = [ "crossbeam-skiplist", "crossbeam-utils", ] + +[patch.crates-io] +# https://github.com/matklad/once_cell/pull/185 +once_cell = { git = "https://github.com/saethlin/once_cell.git", rev = "4aa5ac4aa30b9eed4a0848f3ad22a5eeb025760d" } diff --git a/ci/miri.sh b/ci/miri.sh index 2ac6814f1..1e9dde6f4 100755 --- a/ci/miri.sh +++ b/ci/miri.sh @@ -9,24 +9,24 @@ echo export RUSTFLAGS="${RUSTFLAGS:-} -Z randomize-layout" -MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation" \ cargo miri test \ -p crossbeam-queue \ -p crossbeam-utils 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-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks" \ cargo miri test \ -p crossbeam-channel 2>&1 | ts -i '%.s ' # -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545 -MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" \ cargo miri test \ -p crossbeam-epoch 2>&1 | ts -i '%.s ' # -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-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \ cargo miri test \ -p crossbeam-skiplist 2>&1 | ts -i '%.s ' @@ -34,11 +34,11 @@ MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disab # -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-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \ cargo miri test \ -p crossbeam-deque 2>&1 | ts -i '%.s ' # -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545 -MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows" \ +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows" \ cargo miri test \ -p crossbeam 2>&1 | ts -i '%.s ' From d3187be4ae1d9fc88666d872fda8a311d4c5d5b6 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 23 Jul 2022 16:27:02 +0900 Subject: [PATCH 3/3] Remove deprecated items This removes the following deprecated items: - crossbeam-epoch: - `CompareAndSetError` - `CompareAndSetOrdering` - `Atomic::compare_and_set` - `Atomic::compare_and_set_weak` - crossbeam-utils: - `AtomicCell::compare_and_swap` --- crossbeam-epoch/src/atomic.rs | 205 ---------------------- crossbeam-epoch/src/lib.rs | 3 - crossbeam-utils/src/atomic/atomic_cell.rs | 28 --- 3 files changed, 236 deletions(-) diff --git a/crossbeam-epoch/src/atomic.rs b/crossbeam-epoch/src/atomic.rs index 19bab4729..588ea380c 100644 --- a/crossbeam-epoch/src/atomic.rs +++ b/crossbeam-epoch/src/atomic.rs @@ -13,23 +13,6 @@ use crate::guard::Guard; use crate::primitive::sync::atomic::AtomicUsize; use crossbeam_utils::atomic::AtomicConsume; -/// Given ordering for the success case in a compare-exchange operation, returns the strongest -/// appropriate ordering for the failure case. -#[inline] -fn strongest_failure_ordering(ord: Ordering) -> Ordering { - use self::Ordering::*; - match ord { - Relaxed | Release => Relaxed, - Acquire | AcqRel => Acquire, - _ => SeqCst, - } -} - -/// The error returned on failed compare-and-set operation. -// TODO: remove in the next major version. -#[deprecated(note = "Use `CompareExchangeError` instead")] -pub type CompareAndSetError<'g, T, P> = CompareExchangeError<'g, T, P>; - /// The error returned on failed compare-and-swap operation. pub struct CompareExchangeError<'g, T: ?Sized + Pointable, P: Pointer> { /// The value in the atomic pointer at the time of the failed operation. @@ -48,59 +31,6 @@ impl + fmt::Debug> fmt::Debug for CompareExchangeError<'_, T, P } } -/// Memory orderings for compare-and-set operations. -/// -/// A compare-and-set operation can have different memory orderings depending on whether it -/// succeeds or fails. This trait generalizes different ways of specifying memory orderings. -/// -/// The two ways of specifying orderings for compare-and-set are: -/// -/// 1. Just one `Ordering` for the success case. In case of failure, the strongest appropriate -/// ordering is chosen. -/// 2. A pair of `Ordering`s. The first one is for the success case, while the second one is -/// for the failure case. -// TODO: remove in the next major version. -#[deprecated( - note = "`compare_and_set` and `compare_and_set_weak` that use this trait are deprecated, \ - use `compare_exchange` or `compare_exchange_weak instead`" -)] -pub trait CompareAndSetOrdering { - /// The ordering of the operation when it succeeds. - fn success(&self) -> Ordering; - - /// The ordering of the operation when it fails. - /// - /// The failure ordering can't be `Release` or `AcqRel` and must be equivalent or weaker than - /// the success ordering. - fn failure(&self) -> Ordering; -} - -#[allow(deprecated)] -impl CompareAndSetOrdering for Ordering { - #[inline] - fn success(&self) -> Ordering { - *self - } - - #[inline] - fn failure(&self) -> Ordering { - strongest_failure_ordering(*self) - } -} - -#[allow(deprecated)] -impl CompareAndSetOrdering for (Ordering, Ordering) { - #[inline] - fn success(&self) -> Ordering { - self.0 - } - - #[inline] - fn failure(&self) -> Ordering { - self.1 - } -} - /// Returns a bitmask containing the unused least significant bits of an aligned pointer to `T`. #[inline] fn low_bits() -> usize { @@ -640,141 +570,6 @@ impl Atomic { Err(prev) } - /// Stores the pointer `new` (either `Shared` or `Owned`) into the atomic pointer if the current - /// value is the same as `current`. The tag is also taken into account, so two pointers to the - /// same object, but with different tags, will not be considered equal. - /// - /// The return value is a result indicating whether the new pointer was written. On success the - /// pointer that was written is returned. On failure the actual current value and `new` are - /// returned. - /// - /// This method takes a [`CompareAndSetOrdering`] argument which describes the memory - /// ordering of this operation. - /// - /// # Migrating to `compare_exchange` - /// - /// `compare_and_set` is equivalent to `compare_exchange` with the following mapping for - /// memory orderings: - /// - /// Original | Success | Failure - /// -------- | ------- | ------- - /// Relaxed | Relaxed | Relaxed - /// Acquire | Acquire | Acquire - /// Release | Release | Relaxed - /// AcqRel | AcqRel | Acquire - /// SeqCst | SeqCst | SeqCst - /// - /// # Examples - /// - /// ``` - /// # #![allow(deprecated)] - /// use crossbeam_epoch::{self as epoch, Atomic, Owned, Shared}; - /// use std::sync::atomic::Ordering::SeqCst; - /// - /// let a = Atomic::new(1234); - /// - /// let guard = &epoch::pin(); - /// let curr = a.load(SeqCst, guard); - /// let res1 = a.compare_and_set(curr, Shared::null(), SeqCst, guard); - /// let res2 = a.compare_and_set(curr, Owned::new(5678), SeqCst, guard); - /// # unsafe { drop(curr.into_owned()); } // avoid leak - /// ``` - // TODO: remove in the next major version. - #[allow(deprecated)] - #[deprecated(note = "Use `compare_exchange` instead")] - pub fn compare_and_set<'g, O, P>( - &self, - current: Shared<'_, T>, - new: P, - ord: O, - guard: &'g Guard, - ) -> Result, CompareAndSetError<'g, T, P>> - where - O: CompareAndSetOrdering, - P: Pointer, - { - self.compare_exchange(current, new, ord.success(), ord.failure(), guard) - } - - /// Stores the pointer `new` (either `Shared` or `Owned`) into the atomic pointer if the current - /// value is the same as `current`. The tag is also taken into account, so two pointers to the - /// same object, but with different tags, will not be considered equal. - /// - /// Unlike [`compare_and_set`], this method is allowed to spuriously fail even when comparison - /// succeeds, which can result in more efficient code on some platforms. The return value is a - /// result indicating whether the new pointer was written. On success the pointer that was - /// written is returned. On failure the actual current value and `new` are returned. - /// - /// This method takes a [`CompareAndSetOrdering`] argument which describes the memory - /// ordering of this operation. - /// - /// [`compare_and_set`]: Atomic::compare_and_set - /// - /// # Migrating to `compare_exchange_weak` - /// - /// `compare_and_set_weak` is equivalent to `compare_exchange_weak` with the following mapping for - /// memory orderings: - /// - /// Original | Success | Failure - /// -------- | ------- | ------- - /// Relaxed | Relaxed | Relaxed - /// Acquire | Acquire | Acquire - /// Release | Release | Relaxed - /// AcqRel | AcqRel | Acquire - /// SeqCst | SeqCst | SeqCst - /// - /// # Examples - /// - /// ``` - /// # #![allow(deprecated)] - /// use crossbeam_epoch::{self as epoch, Atomic, Owned, Shared}; - /// use std::sync::atomic::Ordering::SeqCst; - /// - /// let a = Atomic::new(1234); - /// let guard = &epoch::pin(); - /// - /// let mut new = Owned::new(5678); - /// let mut ptr = a.load(SeqCst, guard); - /// # unsafe { drop(a.load(SeqCst, guard).into_owned()); } // avoid leak - /// loop { - /// match a.compare_and_set_weak(ptr, new, SeqCst, guard) { - /// Ok(p) => { - /// ptr = p; - /// break; - /// } - /// Err(err) => { - /// ptr = err.current; - /// new = err.new; - /// } - /// } - /// } - /// - /// let mut curr = a.load(SeqCst, guard); - /// loop { - /// match a.compare_and_set_weak(curr, Shared::null(), SeqCst, guard) { - /// Ok(_) => break, - /// Err(err) => curr = err.current, - /// } - /// } - /// # unsafe { drop(curr.into_owned()); } // avoid leak - /// ``` - // TODO: remove in the next major version. - #[allow(deprecated)] - #[deprecated(note = "Use `compare_exchange_weak` instead")] - pub fn compare_and_set_weak<'g, O, P>( - &self, - current: Shared<'_, T>, - new: P, - ord: O, - guard: &'g Guard, - ) -> Result, CompareAndSetError<'g, T, P>> - where - O: CompareAndSetOrdering, - P: Pointer, - { - self.compare_exchange_weak(current, new, ord.success(), ord.failure(), guard) - } - /// Bitwise "and" with the current tag. /// /// Performs a bitwise "and" operation on the current tag and the argument `val`, and sets the diff --git a/crossbeam-epoch/src/lib.rs b/crossbeam-epoch/src/lib.rs index 4cf982b6b..1f8d6c403 100644 --- a/crossbeam-epoch/src/lib.rs +++ b/crossbeam-epoch/src/lib.rs @@ -155,9 +155,6 @@ cfg_if! { }; pub use self::collector::{Collector, LocalHandle}; pub use self::guard::{unprotected, Guard}; - - #[allow(deprecated)] - pub use self::atomic::{CompareAndSetError, CompareAndSetOrdering}; } } diff --git a/crossbeam-utils/src/atomic/atomic_cell.rs b/crossbeam-utils/src/atomic/atomic_cell.rs index 7941c5c87..c60efbc94 100644 --- a/crossbeam-utils/src/atomic/atomic_cell.rs +++ b/crossbeam-utils/src/atomic/atomic_cell.rs @@ -221,34 +221,6 @@ impl AtomicCell { } impl AtomicCell { - /// If the current value equals `current`, stores `new` into the atomic cell. - /// - /// The return value is always the previous value. If it is equal to `current`, then the value - /// was updated. - /// - /// # Examples - /// - /// ``` - /// # #![allow(deprecated)] - /// use crossbeam_utils::atomic::AtomicCell; - /// - /// let a = AtomicCell::new(1); - /// - /// assert_eq!(a.compare_and_swap(2, 3), 1); - /// assert_eq!(a.load(), 1); - /// - /// assert_eq!(a.compare_and_swap(1, 2), 1); - /// assert_eq!(a.load(), 2); - /// ``` - // TODO: remove in the next major version. - #[deprecated(note = "Use `compare_exchange` instead")] - pub fn compare_and_swap(&self, current: T, new: T) -> T { - match self.compare_exchange(current, new) { - Ok(v) => v, - Err(v) => v, - } - } - /// If the current value equals `current`, stores `new` into the atomic cell. /// /// The return value is a result indicating whether the new value was written and containing