diff --git a/ci/miri.sh b/ci/miri.sh index 98a9b56d6..9c747ab24 100755 --- a/ci/miri.sh +++ b/ci/miri.sh @@ -3,37 +3,40 @@ set -euxo pipefail IFS=$'\n\t' cd "$(dirname "$0")"/.. +# We need 'ts' for the per-line timing +sudo apt-get -y install moreutils +echo + export RUSTFLAGS="${RUSTFLAGS:-} -Z randomize-layout" -# disable preemption due to https://github.com/rust-lang/rust/issues/55005 -MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-preemption-rate=0" \ +# -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" \ cargo miri test \ -p crossbeam-queue \ - -p crossbeam-utils + -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 -# disable preemption due to https://github.com/rust-lang/rust/issues/55005 -MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-preemption-rate=0" \ +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks" \ cargo miri test \ - -p crossbeam-channel + -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 -# disable preemption due to https://github.com/rust-lang/rust/issues/55005 -MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks -Zmiri-preemption-rate=0" \ +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \ cargo miri test \ -p crossbeam-epoch \ - -p crossbeam-skiplist + -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 -# disable preemption due to https://github.com/rust-lang/rust/issues/55005 -MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks -Zmiri-compare-exchange-weak-failure-rate=0.0 -Zmiri-preemption-rate=0" \ +# -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" \ cargo miri test \ - -p crossbeam-deque + -p crossbeam-deque 2>&1 | ts -i '%.s ' # -Zmiri-ignore-leaks is needed for https://github.com/crossbeam-rs/crossbeam/issues/579 -# disable preemption due to https://github.com/rust-lang/rust/issues/55005 -MIRIFLAGS="-Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zmiri-ignore-leaks -Zmiri-preemption-rate=0" \ +MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-ignore-leaks" \ cargo miri test \ - -p crossbeam + -p crossbeam 2>&1 | ts -i '%.s ' diff --git a/crossbeam-channel/tests/array.rs b/crossbeam-channel/tests/array.rs index de843cd32..6fd8ffcc6 100644 --- a/crossbeam-channel/tests/array.rs +++ b/crossbeam-channel/tests/array.rs @@ -377,7 +377,7 @@ fn spsc() { #[test] fn mpmc() { #[cfg(miri)] - const COUNT: usize = 100; + const COUNT: usize = 50; #[cfg(not(miri))] const COUNT: usize = 25_000; const THREADS: usize = 4; @@ -532,16 +532,12 @@ fn drops() { scope.spawn(|_| { for _ in 0..steps { r.recv().unwrap(); - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } }); scope.spawn(|_| { for _ in 0..steps { s.send(DropCounter).unwrap(); - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } }); }) diff --git a/crossbeam-channel/tests/golang.rs b/crossbeam-channel/tests/golang.rs index 6a46c0353..4ffda545f 100644 --- a/crossbeam-channel/tests/golang.rs +++ b/crossbeam-channel/tests/golang.rs @@ -959,7 +959,7 @@ mod chan_test { #[test] fn test_chan() { #[cfg(miri)] - const N: i32 = 20; + const N: i32 = 12; #[cfg(not(miri))] const N: i32 = 200; @@ -1489,7 +1489,7 @@ mod chan_test { fn test_multi_consumer() { const NWORK: usize = 23; #[cfg(miri)] - const NITER: usize = 100; + const NITER: usize = 50; #[cfg(not(miri))] const NITER: usize = 271828; diff --git a/crossbeam-channel/tests/list.rs b/crossbeam-channel/tests/list.rs index a0b908722..535d624e5 100644 --- a/crossbeam-channel/tests/list.rs +++ b/crossbeam-channel/tests/list.rs @@ -433,8 +433,6 @@ fn drops() { scope.spawn(|_| { for _ in 0..steps { r.recv().unwrap(); - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } }); diff --git a/crossbeam-channel/tests/mpsc.rs b/crossbeam-channel/tests/mpsc.rs index 3db4812c6..d7cc8e25f 100644 --- a/crossbeam-channel/tests/mpsc.rs +++ b/crossbeam-channel/tests/mpsc.rs @@ -339,25 +339,22 @@ mod channel_tests { #[test] fn stress_shared() { - #[cfg(miri)] - const AMT: u32 = 100; - #[cfg(not(miri))] - const AMT: u32 = 10000; - const NTHREADS: u32 = 8; + let amt: u32 = if cfg!(miri) { 100 } else { 10_000 }; + let nthreads: u32 = if cfg!(miri) { 4 } else { 8 }; let (tx, rx) = channel::(); let t = thread::spawn(move || { - for _ in 0..AMT * NTHREADS { + for _ in 0..amt * nthreads { assert_eq!(rx.recv().unwrap(), 1); } assert!(rx.try_recv().is_err()); }); - let mut ts = Vec::with_capacity(NTHREADS as usize); - for _ in 0..NTHREADS { + let mut ts = Vec::with_capacity(nthreads as usize); + for _ in 0..nthreads { let tx = tx.clone(); let t = thread::spawn(move || { - for _ in 0..AMT { + for _ in 0..amt { tx.send(1).unwrap(); } }); diff --git a/crossbeam-channel/tests/thread_locals.rs b/crossbeam-channel/tests/thread_locals.rs index effb6a143..fb4e577f2 100644 --- a/crossbeam-channel/tests/thread_locals.rs +++ b/crossbeam-channel/tests/thread_locals.rs @@ -1,6 +1,6 @@ //! Tests that make sure accessing thread-locals while exiting the thread doesn't cause panics. -#![cfg(not(miri))] // error: abnormal termination: the evaluated program aborted execution +#![cfg(not(miri))] // Miri detects that this test is buggy: the destructor of `FOO` uses `std::thread::current()`! use std::thread; use std::time::Duration; diff --git a/crossbeam-channel/tests/zero.rs b/crossbeam-channel/tests/zero.rs index c90d74187..aed0e5752 100644 --- a/crossbeam-channel/tests/zero.rs +++ b/crossbeam-channel/tests/zero.rs @@ -403,7 +403,7 @@ fn drops() { #[cfg(not(miri))] const RUNS: usize = 100; #[cfg(miri)] - const STEPS: usize = 500; + const STEPS: usize = 100; #[cfg(not(miri))] const STEPS: usize = 10_000; diff --git a/crossbeam-deque/src/deque.rs b/crossbeam-deque/src/deque.rs index 802a2fef5..75d31acb2 100644 --- a/crossbeam-deque/src/deque.rs +++ b/crossbeam-deque/src/deque.rs @@ -53,6 +53,8 @@ impl Buffer { /// Returns a pointer to the task at the specified `index`. unsafe fn at(&self, index: isize) -> *mut T { // `self.cap` is always a power of two. + // We do all the loads at `MaybeUninit` because we might realize, after loading, that we + // don't actually have the right to access this memory. self.ptr.offset(index & (self.cap - 1) as isize) } @@ -62,8 +64,8 @@ impl Buffer { /// technically speaking a data race and therefore UB. We should use an atomic store here, but /// that would be more expensive and difficult to implement generically for all types `T`. /// Hence, as a hack, we use a volatile write instead. - unsafe fn write(&self, index: isize, task: T) { - ptr::write_volatile(self.at(index), task) + unsafe fn write(&self, index: isize, task: MaybeUninit) { + ptr::write_volatile(self.at(index) as *mut MaybeUninit, task) } /// Reads a task from the specified `index`. @@ -71,9 +73,9 @@ impl Buffer { /// This method might be concurrently called with another `write` at the same index, which is /// technically speaking a data race and therefore UB. We should use an atomic load here, but /// that would be more expensive and difficult to implement generically for all types `T`. - /// Hence, as a hack, we use a volatile write instead. - unsafe fn read(&self, index: isize) -> T { - ptr::read_volatile(self.at(index)) + /// Hence, as a hack, we use a volatile load instead. + unsafe fn read(&self, index: isize) -> MaybeUninit { + ptr::read_volatile(self.at(index) as *mut MaybeUninit) } } @@ -406,7 +408,7 @@ impl Worker { // Write `task` into the slot. unsafe { - buffer.write(b, task); + buffer.write(b, MaybeUninit::new(task)); } atomic::fence(Ordering::Release); @@ -461,7 +463,7 @@ impl Worker { unsafe { // Read the popped task. let buffer = self.buffer.get(); - let task = buffer.read(f); + let task = buffer.read(f).assume_init(); // Shrink the buffer if `len - 1` is less than one fourth of the capacity. if buffer.cap > MIN_CAP && len <= buffer.cap as isize / 4 { @@ -509,8 +511,8 @@ impl Worker { ) .is_err() { - // Failed. We didn't pop anything. - mem::forget(task.take()); + // Failed. We didn't pop anything. Reset to `None`. + task.take(); } // Restore the back index to the original task. @@ -524,7 +526,7 @@ impl Worker { } } - task + task.map(|t| unsafe { t.assume_init() }) } } } @@ -661,12 +663,11 @@ impl Stealer { .is_err() { // We didn't steal this task, forget it. - mem::forget(task); return Steal::Retry; } // Return the stolen task. - Steal::Success(task) + Steal::Success(unsafe { task.assume_init() }) } /// Steals a batch of tasks and pushes them into another worker. @@ -821,7 +822,6 @@ impl Stealer { .is_err() { // We didn't steal this task, forget it and break from the loop. - mem::forget(task); batch_size = i; break; } @@ -975,7 +975,6 @@ impl Stealer { .is_err() { // We didn't steal this task, forget it. - mem::forget(task); return Steal::Retry; } @@ -992,7 +991,6 @@ impl Stealer { .is_err() { // We didn't steal this task, forget it. - mem::forget(task); return Steal::Retry; } @@ -1037,7 +1035,6 @@ impl Stealer { .is_err() { // We didn't steal this task, forget it and break from the loop. - mem::forget(tmp); batch_size = i; break; } @@ -1077,7 +1074,7 @@ impl Stealer { dest.inner.back.store(dest_b, Ordering::Release); // Return with success. - Steal::Success(task) + Steal::Success(unsafe { task.assume_init() }) } } @@ -1535,7 +1532,7 @@ impl Injector { // Read the task. let slot = (*block).slots.get_unchecked(offset + i); slot.wait_write(); - let task = slot.task.get().read().assume_init(); + let task = slot.task.get().read(); // Write it into the destination queue. dest_buffer.write(dest_b.wrapping_add(i as isize), task); @@ -1547,7 +1544,7 @@ impl Injector { // Read the task. let slot = (*block).slots.get_unchecked(offset + i); slot.wait_write(); - let task = slot.task.get().read().assume_init(); + let task = slot.task.get().read(); // Write it into the destination queue. dest_buffer.write(dest_b.wrapping_add((batch_size - 1 - i) as isize), task); @@ -1689,7 +1686,7 @@ impl Injector { // Read the task. let slot = (*block).slots.get_unchecked(offset); slot.wait_write(); - let task = slot.task.get().read().assume_init(); + let task = slot.task.get().read(); match dest.flavor { Flavor::Fifo => { @@ -1698,7 +1695,7 @@ impl Injector { // Read the task. let slot = (*block).slots.get_unchecked(offset + i + 1); slot.wait_write(); - let task = slot.task.get().read().assume_init(); + let task = slot.task.get().read(); // Write it into the destination queue. dest_buffer.write(dest_b.wrapping_add(i as isize), task); @@ -1711,7 +1708,7 @@ impl Injector { // Read the task. let slot = (*block).slots.get_unchecked(offset + i + 1); slot.wait_write(); - let task = slot.task.get().read().assume_init(); + let task = slot.task.get().read(); // Write it into the destination queue. dest_buffer.write(dest_b.wrapping_add((batch_size - 1 - i) as isize), task); @@ -1744,7 +1741,7 @@ impl Injector { } } - Steal::Success(task) + Steal::Success(task.assume_init()) } } diff --git a/crossbeam-queue/tests/array_queue.rs b/crossbeam-queue/tests/array_queue.rs index d4abfdc7f..495a1f48f 100644 --- a/crossbeam-queue/tests/array_queue.rs +++ b/crossbeam-queue/tests/array_queue.rs @@ -103,8 +103,6 @@ fn len() { assert_eq!(x, i); break; } - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } let len = q.len(); assert!(len <= CAP); @@ -113,10 +111,7 @@ fn len() { scope.spawn(|_| { for i in 0..COUNT { - while q.push(i).is_err() { - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 - } + while q.push(i).is_err() {} let len = q.len(); assert!(len <= CAP); } @@ -129,7 +124,7 @@ fn len() { #[test] fn spsc() { #[cfg(miri)] - const COUNT: usize = 100; + const COUNT: usize = 50; #[cfg(not(miri))] const COUNT: usize = 100_000; @@ -143,8 +138,6 @@ fn spsc() { assert_eq!(x, i); break; } - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } } assert!(q.pop().is_none()); @@ -152,10 +145,7 @@ fn spsc() { scope.spawn(|_| { for i in 0..COUNT { - while q.push(i).is_err() { - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 - } + while q.push(i).is_err() {} } }); }) @@ -184,8 +174,6 @@ fn spsc_ring_buffer() { } } } - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 }); scope.spawn(|_| { @@ -288,14 +276,9 @@ fn mpmc_ring_buffer() { #[test] fn drops() { - #[cfg(miri)] - const RUNS: usize = 5; - #[cfg(not(miri))] - const RUNS: usize = 100; - #[cfg(miri)] - const STEPS: usize = 50; - #[cfg(not(miri))] - const STEPS: usize = 10_000; + let runs: usize = if cfg!(miri) { 3 } else { 100 }; + let steps: usize = if cfg!(miri) { 50 } else { 10_000 }; + let additional: usize = if cfg!(miri) { 10 } else { 50 }; static DROPS: AtomicUsize = AtomicUsize::new(0); @@ -310,9 +293,9 @@ fn drops() { let mut rng = thread_rng(); - for _ in 0..RUNS { - let steps = rng.gen_range(0..STEPS); - let additional = rng.gen_range(0..50); + for _ in 0..runs { + let steps = rng.gen_range(0..steps); + let additional = rng.gen_range(0..additional); DROPS.store(0, Ordering::SeqCst); let q = ArrayQueue::new(50); @@ -320,10 +303,7 @@ fn drops() { scope(|scope| { scope.spawn(|_| { for _ in 0..steps { - while q.pop().is_none() { - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 - } + while q.pop().is_none() {} } }); @@ -331,8 +311,6 @@ fn drops() { for _ in 0..steps { while q.push(DropCounter).is_err() { DROPS.fetch_sub(1, Ordering::SeqCst); - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } } }); diff --git a/crossbeam-queue/tests/seg_queue.rs b/crossbeam-queue/tests/seg_queue.rs index ef812b34d..0bb56eeb0 100644 --- a/crossbeam-queue/tests/seg_queue.rs +++ b/crossbeam-queue/tests/seg_queue.rs @@ -69,8 +69,6 @@ fn spsc() { assert_eq!(x, i); break; } - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 } } assert!(q.pop().is_none()); @@ -123,14 +121,9 @@ fn mpmc() { #[test] fn drops() { - #[cfg(miri)] - const RUNS: usize = 5; - #[cfg(not(miri))] - const RUNS: usize = 100; - #[cfg(miri)] - const STEPS: usize = 50; - #[cfg(not(miri))] - const STEPS: usize = 10_000; + let runs: usize = if cfg!(miri) { 5 } else { 100 }; + let steps: usize = if cfg!(miri) { 50 } else { 10_000 }; + let additional: usize = if cfg!(miri) { 100 } else { 1_000 }; static DROPS: AtomicUsize = AtomicUsize::new(0); @@ -145,9 +138,9 @@ fn drops() { let mut rng = thread_rng(); - for _ in 0..RUNS { - let steps = rng.gen_range(0..STEPS); - let additional = rng.gen_range(0..1000); + for _ in 0..runs { + let steps = rng.gen_range(0..steps); + let additional = rng.gen_range(0..additional); DROPS.store(0, Ordering::SeqCst); let q = SegQueue::new(); @@ -155,10 +148,7 @@ fn drops() { scope(|scope| { scope.spawn(|_| { for _ in 0..steps { - while q.pop().is_none() { - #[cfg(miri)] - std::thread::yield_now(); // https://github.com/rust-lang/miri/issues/1388 - } + while q.pop().is_none() {} } }); diff --git a/crossbeam-utils/tests/wait_group.rs b/crossbeam-utils/tests/wait_group.rs index b6c2a2437..00afcafda 100644 --- a/crossbeam-utils/tests/wait_group.rs +++ b/crossbeam-utils/tests/wait_group.rs @@ -36,6 +36,7 @@ fn wait() { } #[test] +#[cfg(not(miri))] // this test makes timing assumptions, but Miri is so slow it violates them fn wait_and_drop() { let wg = WaitGroup::new(); let (tx, rx) = mpsc::channel(); @@ -51,8 +52,8 @@ fn wait_and_drop() { }); } - // At this point, all spawned threads should be sleeping, so we shouldn't get anything from the - // channel. + // At this point, all spawned threads should be in `thread::sleep`, so we shouldn't get anything + // from the channel. assert!(rx.try_recv().is_err()); wg.wait();