Skip to content

Commit a81e099

Browse files
committed
epoch: Fix stacked borrows violations
1 parent 99a3451 commit a81e099

File tree

5 files changed

+71
-55
lines changed

5 files changed

+71
-55
lines changed

ci/miri.sh

+4-10
Original file line numberDiff line numberDiff line change
@@ -12,33 +12,27 @@ export RUSTFLAGS="${RUSTFLAGS:-} -Z randomize-layout"
1212
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation" \
1313
cargo miri test \
1414
-p crossbeam-queue \
15-
-p crossbeam-utils 2>&1 | ts -i '%.s '
15+
-p crossbeam-utils \
16+
-p crossbeam-epoch 2>&1 | ts -i '%.s '
1617

1718
# -Zmiri-ignore-leaks is needed because we use detached threads in tests/docs: https://github.com/rust-lang/miri/issues/1371
1819
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks" \
1920
cargo miri test \
2021
-p crossbeam-channel 2>&1 | ts -i '%.s '
2122

22-
# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
23-
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" \
24-
cargo miri test \
25-
-p crossbeam-epoch 2>&1 | ts -i '%.s '
26-
2723
# -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/614
2824
# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
2925
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \
3026
cargo miri test \
3127
-p crossbeam-skiplist 2>&1 | ts -i '%.s '
3228

33-
# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
3429
# -Zmiri-compare-exchange-weak-failure-rate=0.0 is needed because some sequential tests (e.g.,
3530
# doctest of Stealer::steal) incorrectly assume that sequential weak CAS will never fail.
3631
# -Zmiri-preemption-rate=0 is needed because this code technically has UB and Miri catches that.
37-
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \
32+
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \
3833
cargo miri test \
3934
-p crossbeam-deque 2>&1 | ts -i '%.s '
4035

41-
# -Zmiri-disable-stacked-borrows is needed for https://github.com/crossbeam-rs/crossbeam/issues/545
42-
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows" \
36+
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check" \
4337
cargo miri test \
4438
-p crossbeam 2>&1 | ts -i '%.s '

crossbeam-epoch/build.rs

+3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ fn main() {
4848
println!("cargo:rustc-cfg=crossbeam_no_atomic_cas");
4949
}
5050

51+
if !cfg.probe_rustc_version(1, 51) {
52+
println!("cargo:rustc-cfg=crossbeam_no_raw_ref_macros");
53+
}
5154
if !cfg.probe_rustc_version(1, 61) {
5255
println!("cargo:rustc-cfg=crossbeam_no_const_fn_trait_bound");
5356
}

crossbeam-epoch/src/atomic.rs

+38-19
Original file line numberDiff line numberDiff line change
@@ -183,26 +183,26 @@ pub trait Pointable {
183183
///
184184
/// - The given `ptr` should have been initialized with [`Pointable::init`].
185185
/// - `ptr` should not have yet been dropped by [`Pointable::drop`].
186-
/// - `ptr` should not be mutably dereferenced by [`Pointable::deref_mut`] concurrently.
187-
unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self;
186+
/// - `ptr` should not be mutably dereferenced by [`Pointable::as_mut_ptr`] concurrently.
187+
unsafe fn as_ptr(ptr: *mut ()) -> *const Self;
188188

189189
/// Mutably dereferences the given pointer.
190190
///
191191
/// # Safety
192192
///
193193
/// - The given `ptr` should have been initialized with [`Pointable::init`].
194194
/// - `ptr` should not have yet been dropped by [`Pointable::drop`].
195-
/// - `ptr` should not be dereferenced by [`Pointable::deref`] or [`Pointable::deref_mut`]
195+
/// - `ptr` should not be dereferenced by [`Pointable::as_ptr`] or [`Pointable::as_mut_ptr`]
196196
/// concurrently.
197-
unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self;
197+
unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self;
198198

199199
/// Drops the object pointed to by the given pointer.
200200
///
201201
/// # Safety
202202
///
203203
/// - The given `ptr` should have been initialized with [`Pointable::init`].
204204
/// - `ptr` should not have yet been dropped by [`Pointable::drop`].
205-
/// - `ptr` should not be dereferenced by [`Pointable::deref`] or [`Pointable::deref_mut`]
205+
/// - `ptr` should not be dereferenced by [`Pointable::as_ptr`] or [`Pointable::as_mut_ptr`]
206206
/// concurrently.
207207
unsafe fn drop(ptr: *mut ());
208208
}
@@ -216,12 +216,12 @@ impl<T> Pointable for T {
216216
Box::into_raw(Box::new(init)).cast::<()>()
217217
}
218218

219-
unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self {
220-
&*(ptr as *const T)
219+
unsafe fn as_ptr(ptr: *mut ()) -> *const Self {
220+
ptr as *const T
221221
}
222222

223-
unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self {
224-
&mut *ptr.cast::<T>()
223+
unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
224+
ptr.cast::<T>()
225225
}
226226

227227
unsafe fn drop(ptr: *mut ()) {
@@ -276,12 +276,17 @@ impl<T> Pointable for [MaybeUninit<T>] {
276276
ptr.cast::<()>()
277277
}
278278

279-
unsafe fn deref<'a>(ptr: *mut ()) -> &'a Self {
280-
let array = &*(ptr as *const Array<T>);
281-
slice::from_raw_parts(array.elements.as_ptr(), array.len)
279+
unsafe fn as_ptr(ptr: *mut ()) -> *const Self {
280+
let array = ptr.cast::<Array<T>>();
281+
// deallocation requires raw/mut provenance: <https://github.com/rust-lang/rust/pull/67339>
282+
#[cfg(not(crossbeam_no_raw_ref_macros))]
283+
let ptr = ptr::addr_of_mut!((*array).elements) as *const _;
284+
#[cfg(crossbeam_no_raw_ref_macros)]
285+
let ptr = (*array).elements.as_ptr();
286+
slice::from_raw_parts(ptr, (*array).len)
282287
}
283288

284-
unsafe fn deref_mut<'a>(ptr: *mut ()) -> &'a mut Self {
289+
unsafe fn as_mut_ptr(ptr: *mut ()) -> *mut Self {
285290
let array = &mut *ptr.cast::<Array<T>>();
286291
slice::from_raw_parts_mut(array.elements.as_mut_ptr(), array.len)
287292
}
@@ -1013,7 +1018,7 @@ impl<T: ?Sized + Pointable> fmt::Pointer for Atomic<T> {
10131018
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
10141019
let data = self.data.load(Ordering::SeqCst);
10151020
let (raw, _) = decompose_tag::<T>(data);
1016-
fmt::Pointer::fmt(&(unsafe { T::deref(raw) as *const _ }), f)
1021+
fmt::Pointer::fmt(&(unsafe { T::as_ptr(raw) }), f)
10171022
}
10181023
}
10191024

@@ -1299,14 +1304,14 @@ impl<T: ?Sized + Pointable> Deref for Owned<T> {
12991304

13001305
fn deref(&self) -> &T {
13011306
let (raw, _) = decompose_tag::<T>(self.data);
1302-
unsafe { T::deref(raw) }
1307+
unsafe { &*T::as_ptr(raw) }
13031308
}
13041309
}
13051310

13061311
impl<T: ?Sized + Pointable> DerefMut for Owned<T> {
13071312
fn deref_mut(&mut self) -> &mut T {
13081313
let (raw, _) = decompose_tag::<T>(self.data);
1309-
unsafe { T::deref_mut(raw) }
1314+
unsafe { &mut *T::as_mut_ptr(raw) }
13101315
}
13111316
}
13121317

@@ -1492,7 +1497,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
14921497
/// ```
14931498
pub unsafe fn deref(&self) -> &'g T {
14941499
let (raw, _) = decompose_tag::<T>(self.data);
1495-
T::deref(raw)
1500+
&*T::as_ptr(raw)
14961501
}
14971502

14981503
/// Dereferences the pointer.
@@ -1534,7 +1539,7 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
15341539
/// ```
15351540
pub unsafe fn deref_mut(&mut self) -> &'g mut T {
15361541
let (raw, _) = decompose_tag::<T>(self.data);
1537-
T::deref_mut(raw)
1542+
&mut *T::as_mut_ptr(raw)
15381543
}
15391544

15401545
/// Converts the pointer to a reference.
@@ -1574,10 +1579,24 @@ impl<'g, T: ?Sized + Pointable> Shared<'g, T> {
15741579
if raw.is_null() {
15751580
None
15761581
} else {
1577-
Some(T::deref(raw))
1582+
Some(&*T::as_ptr(raw))
15781583
}
15791584
}
15801585

1586+
pub(crate) unsafe fn as_non_null(&self) -> Option<*const T> {
1587+
let (raw, _) = decompose_tag::<T>(self.data);
1588+
if raw.is_null() {
1589+
None
1590+
} else {
1591+
Some(T::as_ptr(raw))
1592+
}
1593+
}
1594+
1595+
pub(crate) unsafe fn as_ptr(&self) -> *const T {
1596+
let (raw, _) = decompose_tag::<T>(self.data);
1597+
T::as_ptr(raw)
1598+
}
1599+
15811600
/// Takes ownership of the pointee.
15821601
///
15831602
/// # Panics

crossbeam-epoch/src/internal.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -534,24 +534,24 @@ impl Local {
534534
}
535535

536536
impl IsElement<Local> for Local {
537-
fn entry_of(local: &Local) -> &Entry {
537+
fn entry_of(local: *const Local) -> *const Entry {
538538
unsafe {
539-
let entry_ptr = (local as *const Local as *const u8)
539+
local
540+
.cast::<u8>()
540541
.add(offset_of!(Local, entry))
541-
.cast::<Entry>();
542-
&*entry_ptr
542+
.cast::<Entry>()
543543
}
544544
}
545545

546-
unsafe fn element_of(entry: &Entry) -> &Local {
547-
let local_ptr = (entry as *const Entry as *const u8)
546+
unsafe fn element_of(entry: *const Entry) -> *const Local {
547+
entry
548+
.cast::<u8>()
548549
.sub(offset_of!(Local, entry))
549-
.cast::<Local>();
550-
&*local_ptr
550+
.cast::<Local>()
551551
}
552552

553-
unsafe fn finalize(entry: &Entry, guard: &Guard) {
554-
guard.defer_destroy(Shared::from(Self::element_of(entry) as *const _));
553+
unsafe fn finalize(entry: *const Entry, guard: &Guard) {
554+
guard.defer_destroy(Shared::from(Self::element_of(entry)));
555555
}
556556
}
557557

crossbeam-epoch/src/sync/list.rs

+16-16
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ pub(crate) struct Entry {
6666
///
6767
pub(crate) trait IsElement<T> {
6868
/// Returns a reference to this element's `Entry`.
69-
fn entry_of(_: &T) -> &Entry;
69+
fn entry_of(_: *const T) -> *const Entry;
7070

7171
/// Given a reference to an element's entry, returns that element.
7272
///
@@ -80,15 +80,15 @@ pub(crate) trait IsElement<T> {
8080
///
8181
/// The caller has to guarantee that the `Entry` is called with was retrieved from an instance
8282
/// of the element type (`T`).
83-
unsafe fn element_of(_: &Entry) -> &T;
83+
unsafe fn element_of(_: *const Entry) -> *const T;
8484

8585
/// The function that is called when an entry is unlinked from list.
8686
///
8787
/// # Safety
8888
///
8989
/// The caller has to guarantee that the `Entry` is called with was retrieved from an instance
9090
/// of the element type (`T`).
91-
unsafe fn finalize(_: &Entry, _: &Guard);
91+
unsafe fn finalize(_: *const Entry, _: &Guard);
9292
}
9393

9494
/// A lock-free, intrusive linked list of type `T`.
@@ -173,16 +173,16 @@ impl<T, C: IsElement<T>> List<T, C> {
173173
// Insert right after head, i.e. at the beginning of the list.
174174
let to = &self.head;
175175
// Get the intrusively stored Entry of the new element to insert.
176-
let entry: &Entry = C::entry_of(container.deref());
176+
let entry: *const Entry = C::entry_of(container.as_ptr());
177177
// Make a Shared ptr to that Entry.
178-
let entry_ptr = Shared::from(entry as *const _);
178+
let entry_ptr = Shared::from(entry);
179179
// Read the current successor of where we want to insert.
180180
let mut next = to.load(Relaxed, guard);
181181

182182
loop {
183183
// Set the Entry of the to-be-inserted element to point to the previous successor of
184184
// `to`.
185-
entry.next.store(next, Relaxed);
185+
(*entry).next.store(next, Relaxed);
186186
match to.compare_exchange_weak(next, entry_ptr, Release, Relaxed, guard) {
187187
Ok(_) => break,
188188
// We lost the race or weak CAS failed spuriously. Update the successor and try
@@ -225,7 +225,7 @@ impl<T, C: IsElement<T>> Drop for List<T, C> {
225225
// Verify that all elements have been removed from the list.
226226
assert_eq!(succ.tag(), 1);
227227

228-
C::finalize(curr.deref(), guard);
228+
C::finalize(curr.as_ptr(), guard);
229229
curr = succ;
230230
}
231231
}
@@ -236,8 +236,8 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
236236
type Item = Result<&'g T, IterError>;
237237

238238
fn next(&mut self) -> Option<Self::Item> {
239-
while let Some(c) = unsafe { self.curr.as_ref() } {
240-
let succ = c.next.load(Acquire, self.guard);
239+
while let Some(c) = unsafe { self.curr.as_non_null() } {
240+
let succ = unsafe { (*c).next.load(Acquire, self.guard) };
241241

242242
if succ.tag() == 1 {
243243
// This entry was removed. Try unlinking it from the list.
@@ -257,7 +257,7 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
257257
// deallocation. Deferred drop is okay, because `list.delete()` can only be
258258
// called if `T: 'static`.
259259
unsafe {
260-
C::finalize(self.curr.deref(), self.guard);
260+
C::finalize(self.curr.as_ptr(), self.guard);
261261
}
262262

263263
// `succ` is the new value of `self.pred`.
@@ -284,10 +284,10 @@ impl<'g, T: 'g, C: IsElement<T>> Iterator for Iter<'g, T, C> {
284284
}
285285

286286
// Move one step forward.
287-
self.pred = &c.next;
287+
self.pred = unsafe { &(*c).next };
288288
self.curr = succ;
289289

290-
return Some(Ok(unsafe { C::element_of(c) }));
290+
return Some(Ok(unsafe { &*C::element_of(c) }));
291291
}
292292

293293
// We reached the end of the list.
@@ -303,16 +303,16 @@ mod tests {
303303
use std::sync::Barrier;
304304

305305
impl IsElement<Entry> for Entry {
306-
fn entry_of(entry: &Entry) -> &Entry {
306+
fn entry_of(entry: *const Entry) -> *const Entry {
307307
entry
308308
}
309309

310-
unsafe fn element_of(entry: &Entry) -> &Entry {
310+
unsafe fn element_of(entry: *const Entry) -> *const Entry {
311311
entry
312312
}
313313

314-
unsafe fn finalize(entry: &Entry, guard: &Guard) {
315-
guard.defer_destroy(Shared::from(Self::element_of(entry) as *const _));
314+
unsafe fn finalize(entry: *const Entry, guard: &Guard) {
315+
guard.defer_destroy(Shared::from(Self::element_of(entry)));
316316
}
317317
}
318318

0 commit comments

Comments
 (0)