Skip to content

Commit 0042528

Browse files
committed
Use AtomicPtr for head_available
This enables Miri to track pointer provenance even for our raw pointers, which allows us to check with `-Zmiri-tag-raw-pointers`. Yay! See rust-lang/miri#1993 for details.
1 parent ba8ffcc commit 0042528

File tree

2 files changed

+32
-25
lines changed

2 files changed

+32
-25
lines changed

.github/workflows/miri.yml

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,4 @@ jobs:
2222
command: miri
2323
args: test
2424
env:
25-
# I would _love_ to enable -Zmiri-track-raw-pointers, but it doesn't
26-
# work if the program casts pointers to integers and back, which we
27-
# don't do _explicitly_, but we do do _implicitly_ through
28-
# `AtomicPtr::compare_exchange`. `load` and `store` work correctly
29-
# (https://github.com/rust-lang/rust/pull/77611), as does `swap`
30-
# (https://github.com/rust-lang/rust/pull/80236), but we also need
31-
# atomic exchange internally. See also
32-
# https://github.com/rust-lang/miri/issues/1574.
33-
MIRIFLAGS: ""
25+
MIRIFLAGS: "-Zmiri-tag-raw-pointers"

src/domain.rs

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ macro_rules! new {
186186
Self {
187187
hazptrs: HazPtrRecords {
188188
head: AtomicPtr::new(core::ptr::null_mut()),
189-
head_available: AtomicUsize::new(0),
189+
head_available: AtomicPtr::new(core::ptr::null_mut()),
190190
count: AtomicIsize::new(0),
191191
},
192192
untagged,
@@ -257,24 +257,28 @@ impl<F> Domain<F> {
257257

258258
loop {
259259
let avail = self.hazptrs.head_available.load(Ordering::Acquire);
260-
if avail == core::ptr::null::<HazPtrRecord>() as usize {
261-
return (core::ptr::null_mut(), 0);
260+
if avail.is_null() {
261+
return (avail, 0);
262262
}
263-
debug_assert_ne!(avail, core::ptr::null::<HazPtrRecord>() as usize | LOCK_BIT);
263+
debug_assert_ne!(avail, LOCK_BIT as *mut _);
264264
if (avail as usize & LOCK_BIT) == 0 {
265-
// Definitely a valid pointer now.
266-
let avail: *const HazPtrRecord = avail as _;
267-
268265
// The available list is not currently locked.
269266
//
270267
// XXX: This could be a fetch_or and allow progress even if there's a new (but
271-
// unlocked) head.
268+
// unlocked) head. However, `AtomicPtr` doesn't support fetch_or at the moment, so
269+
// we'd have to convert it to an `AtomicUsize`. This will in turn make Miri fail
270+
// (with -Zmiri-tag-raw-pointers, which we want enabled) to track the provenance of
271+
// the pointer in question through the int-to-ptr conversion. The workaround is
272+
// probably to mock a type that is `AtomicUsize` with `fetch_or` with
273+
// `#[cfg(not(miri))]`, but is `AtomicPtr` with `compare_exchange` with
274+
// `#[cfg(miri)]`. It ain't pretty, but should do the job. The issue is tracked in
275+
// https://github.com/rust-lang/miri/issues/1993.
272276
if self
273277
.hazptrs
274278
.head_available
275279
.compare_exchange_weak(
276-
avail as usize,
277-
avail as usize | LOCK_BIT,
280+
avail,
281+
with_lock_bit(avail),
278282
Ordering::AcqRel,
279283
Ordering::Relaxed,
280284
)
@@ -317,9 +321,7 @@ impl<F> Domain<F> {
317321
}
318322

319323
// NOTE: This releases the lock
320-
self.hazptrs
321-
.head_available
322-
.store(next as usize, Ordering::Release);
324+
self.hazptrs.head_available.store(next, Ordering::Release);
323325
unsafe { &*tail }
324326
.available_next
325327
.store(core::ptr::null_mut(), Ordering::Relaxed);
@@ -335,15 +337,15 @@ impl<F> Domain<F> {
335337
debug_assert_eq!(head as *const _ as usize & LOCK_BIT, 0);
336338
loop {
337339
let avail = self.hazptrs.head_available.load(Ordering::Acquire);
338-
if (avail & LOCK_BIT) == 0 {
340+
if (avail as usize & LOCK_BIT) == 0 {
339341
tail.available_next
340342
.store(avail as *mut _, Ordering::Relaxed);
341343
if self
342344
.hazptrs
343345
.head_available
344346
.compare_exchange_weak(
345347
avail,
346-
head as *const _ as usize,
348+
head as *const _ as *mut _,
347349
Ordering::AcqRel,
348350
Ordering::Relaxed,
349351
)
@@ -724,7 +726,7 @@ impl<F> Drop for Domain<F> {
724726

725727
struct HazPtrRecords {
726728
head: AtomicPtr<HazPtrRecord>,
727-
head_available: AtomicUsize, // really *mut HazPtrRecord
729+
head_available: AtomicPtr<HazPtrRecord>,
728730
count: AtomicIsize,
729731
}
730732

@@ -815,6 +817,19 @@ impl RetiredList {
815817
}
816818
}
817819

820+
// Helpers to set and unset the lock bit on a `*mut HazPtrRecord` without losing pointer
821+
// provenance. See https://github.com/rust-lang/miri/issues/1993 for details.
822+
fn with_lock_bit(ptr: *mut HazPtrRecord) -> *mut HazPtrRecord {
823+
int_to_ptr_with_provenance(ptr as usize | LOCK_BIT, ptr)
824+
}
825+
fn without_lock_bit(ptr: *mut HazPtrRecord) -> *mut HazPtrRecord {
826+
int_to_ptr_with_provenance(ptr as usize & !LOCK_BIT, ptr)
827+
}
828+
fn int_to_ptr_with_provenance<T>(addr: usize, prov: *mut T) -> *mut T {
829+
let ptr = prov.cast::<u8>();
830+
ptr.wrapping_add(addr.wrapping_sub(ptr as usize)).cast()
831+
}
832+
818833
/*
819834
fn foo() {
820835
let domain = Domain::new();

0 commit comments

Comments
 (0)