From 837715a674700113deb18c15f0abc1356c3af237 Mon Sep 17 00:00:00 2001 From: JCTyBlaidd Date: Mon, 2 Nov 2020 01:46:42 +0000 Subject: [PATCH] Add tests, and fix bug in atomic RMW relaxed stores --- src/data_race.rs | 5 +- src/shims/posix/thread.rs | 3 +- .../compile-fail/data_race/read_write_race.rs | 26 ++++ .../data_race/relax_acquire_race.rs | 42 +++++++ .../data_race/release_seq_race.rs | 46 +++++++ tests/compile-fail/data_race/rmw_race.rs | 43 +++++++ .../data_race/write_write_race.rs | 26 ++++ tests/run-pass/concurrency/data_race.rs | 119 ++++++++++++++++++ tests/run-pass/concurrency/data_race.stderr | 2 + tests/run-pass/concurrency/linux-futex.stderr | 2 +- tests/run-pass/concurrency/simple.stderr | 2 +- tests/run-pass/concurrency/sync.stderr | 2 +- .../run-pass/concurrency/thread_locals.stderr | 2 +- .../run-pass/concurrency/tls_lib_drop.stderr | 2 +- tests/run-pass/libc.stderr | 2 +- tests/run-pass/panic/concurrent-panic.stderr | 2 +- 16 files changed, 315 insertions(+), 11 deletions(-) create mode 100644 tests/compile-fail/data_race/read_write_race.rs create mode 100644 tests/compile-fail/data_race/relax_acquire_race.rs create mode 100644 tests/compile-fail/data_race/release_seq_race.rs create mode 100644 tests/compile-fail/data_race/rmw_race.rs create mode 100644 tests/compile-fail/data_race/write_write_race.rs create mode 100644 tests/run-pass/concurrency/data_race.rs create mode 100644 tests/run-pass/concurrency/data_race.stderr diff --git a/src/data_race.rs b/src/data_race.rs index 5952606394..ac928071be 100644 --- a/src/data_race.rs +++ b/src/data_race.rs @@ -404,8 +404,9 @@ impl AtomicReleaseSequences { fn clear_and_retain(&mut self, thread: ThreadId) { match self { Self::ReleaseOneOrEmpty(id, rel_clock) => { - // Keep or forget depending on id - if *id == Some(thread) { + // If the id is the same, then reatin the value + // otherwise delete and clear the release vector clock + if *id != Some(thread) { *id = None; rel_clock.set_zero_vector(); } diff --git a/src/shims/posix/thread.rs b/src/shims/posix/thread.rs index e420457765..e823a7d88d 100644 --- a/src/shims/posix/thread.rs +++ b/src/shims/posix/thread.rs @@ -15,8 +15,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); this.tcx.sess.warn( - "thread support is experimental. \ - For example, Miri does not detect data races yet.", + "thread support is experimental.", ); // Create the new thread diff --git a/tests/compile-fail/data_race/read_write_race.rs b/tests/compile-fail/data_race/read_write_race.rs new file mode 100644 index 0000000000..ece99b4a87 --- /dev/null +++ b/tests/compile-fail/data_race/read_write_race.rs @@ -0,0 +1,26 @@ + +use std::thread::spawn; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +pub fn main() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + unsafe { + let j1 = spawn(move || { + *c.0 + }); + + let j2 = spawn(move || { + *c.0 = 64; //~ ERROR Data race + }); + + j1.join().unwrap(); + j2.join().unwrap(); + } +} \ No newline at end of file diff --git a/tests/compile-fail/data_race/relax_acquire_race.rs b/tests/compile-fail/data_race/relax_acquire_race.rs new file mode 100644 index 0000000000..cc96083546 --- /dev/null +++ b/tests/compile-fail/data_race/relax_acquire_race.rs @@ -0,0 +1,42 @@ + +use std::thread::spawn; +use std::sync::atomic::{AtomicUsize, Ordering}; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +static SYNC: AtomicUsize = AtomicUsize::new(0); + +pub fn main() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + + unsafe { + let j1 = spawn(move || { + *c.0 = 1; + SYNC.store(1, Ordering::Release); + }); + + let j2 = spawn(move || { + if SYNC.load(Ordering::Acquire) == 1 { + SYNC.store(2, Ordering::Relaxed); + } + }); + + let j3 = spawn(move || { + if SYNC.load(Ordering::Acquire) == 2 { + *c.0 //~ ERROR Data race + }else{ + 0 + } + }); + + j1.join().unwrap(); + j2.join().unwrap(); + j3.join().unwrap(); + } +} \ No newline at end of file diff --git a/tests/compile-fail/data_race/release_seq_race.rs b/tests/compile-fail/data_race/release_seq_race.rs new file mode 100644 index 0000000000..8b3ffbcd9d --- /dev/null +++ b/tests/compile-fail/data_race/release_seq_race.rs @@ -0,0 +1,46 @@ +// compile-flags: -Zmiri-disable-isolation + +use std::thread::{spawn, sleep}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +static SYNC: AtomicUsize = AtomicUsize::new(0); + +pub fn main() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + + unsafe { + let j1 = spawn(move || { + *c.0 = 1; + SYNC.store(1, Ordering::Release); + sleep(Duration::from_millis(100)); + SYNC.store(3, Ordering::Relaxed); + }); + + let j2 = spawn(move || { + // Blocks the acquire-release sequence + SYNC.store(2, Ordering::Relaxed); + }); + + let j3 = spawn(move || { + sleep(Duration::from_millis(1000)); + if SYNC.load(Ordering::Acquire) == 3 { + *c.0 //~ ERROR Data race + }else{ + 0 + } + }); + + j1.join().unwrap(); + j2.join().unwrap(); + j3.join().unwrap(); + } +} \ No newline at end of file diff --git a/tests/compile-fail/data_race/rmw_race.rs b/tests/compile-fail/data_race/rmw_race.rs new file mode 100644 index 0000000000..9c31c79ebf --- /dev/null +++ b/tests/compile-fail/data_race/rmw_race.rs @@ -0,0 +1,43 @@ + +use std::thread::spawn; +use std::sync::atomic::{AtomicUsize, Ordering}; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +static SYNC: AtomicUsize = AtomicUsize::new(0); + +pub fn main() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + + unsafe { + let j1 = spawn(move || { + *c.0 = 1; + SYNC.store(1, Ordering::Release); + }); + + let j2 = spawn(move || { + if SYNC.swap(2, Ordering::Relaxed) == 1 { + // Blocks the acquire-release sequence + SYNC.store(3, Ordering::Relaxed); + } + }); + + let j3 = spawn(move || { + if SYNC.load(Ordering::Acquire) == 3 { + *c.0 //~ ERROR Data race + }else{ + 0 + } + }); + + j1.join().unwrap(); + j2.join().unwrap(); + j3.join().unwrap(); + } +} \ No newline at end of file diff --git a/tests/compile-fail/data_race/write_write_race.rs b/tests/compile-fail/data_race/write_write_race.rs new file mode 100644 index 0000000000..22caf5f0f7 --- /dev/null +++ b/tests/compile-fail/data_race/write_write_race.rs @@ -0,0 +1,26 @@ + +use std::thread::spawn; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +pub fn main() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + unsafe { + let j1 = spawn(move || { + *c.0 = 32; + }); + + let j2 = spawn(move || { + *c.0 = 64; //~ ERROR Data race + }); + + j1.join().unwrap(); + j2.join().unwrap(); + } +} \ No newline at end of file diff --git a/tests/run-pass/concurrency/data_race.rs b/tests/run-pass/concurrency/data_race.rs new file mode 100644 index 0000000000..bc4a4e30e8 --- /dev/null +++ b/tests/run-pass/concurrency/data_race.rs @@ -0,0 +1,119 @@ +use std::sync::atomic::{AtomicUsize, fence, Ordering}; +use std::thread::spawn; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +static SYNC: AtomicUsize = AtomicUsize::new(0); + +fn test_fence_sync() { + let mut var = 0u32; + let ptr = &mut var as *mut u32; + let evil_ptr = EvilSend(ptr); + + + let j1 = spawn(move || { + unsafe { *evil_ptr.0 = 1; } + fence(Ordering::Release); + SYNC.store(1, Ordering::Relaxed) + }); + + let j2 = spawn(move || { + if SYNC.load(Ordering::Relaxed) == 1 { + fence(Ordering::Acquire); + unsafe { *evil_ptr.0 } + }else{ + 0 + } + }); + + j1.join().unwrap(); + j2.join().unwrap(); +} + + +fn test_multiple_reads() { + let mut var = 42u32; + let ptr = &mut var as *mut u32; + let evil_ptr = EvilSend(ptr); + + let j1 = spawn(move || unsafe {*evil_ptr.0}); + let j2 = spawn(move || unsafe {*evil_ptr.0}); + let j3 = spawn(move || unsafe {*evil_ptr.0}); + let j4 = spawn(move || unsafe {*evil_ptr.0}); + + assert_eq!(j1.join().unwrap(), 42); + assert_eq!(j2.join().unwrap(), 42); + assert_eq!(j3.join().unwrap(), 42); + assert_eq!(j4.join().unwrap(), 42); + + var = 10; + assert_eq!(var, 10); +} + +pub fn test_rmw_no_block() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + + unsafe { + let j1 = spawn(move || { + *c.0 = 1; + SYNC.store(1, Ordering::Release); + }); + + let j2 = spawn(move || { + if SYNC.swap(2, Ordering::Relaxed) == 1 { + //No op, blocking store removed + } + }); + + let j3 = spawn(move || { + if SYNC.load(Ordering::Acquire) == 2 { + *c.0 + }else{ + 0 + } + }); + + j1.join().unwrap(); + j2.join().unwrap(); + let v = j3.join().unwrap(); + assert!(v == 1 || v == 2); + } +} + +pub fn test_release_no_block() { + let mut a = 0u32; + let b = &mut a as *mut u32; + let c = EvilSend(b); + + unsafe { + let j1 = spawn(move || { + *c.0 = 1; + SYNC.store(1, Ordering::Release); + SYNC.store(3, Ordering::Relaxed); + }); + + let j2 = spawn(move || { + if SYNC.load(Ordering::Acquire) == 3 { + *c.0 + }else{ + 0 + } + }); + + j1.join().unwrap(); + assert_eq!(j2.join().unwrap(),1); + } +} + +pub fn main() { + test_fence_sync(); + test_multiple_reads(); + test_rmw_no_block(); + test_release_no_block(); +} \ No newline at end of file diff --git a/tests/run-pass/concurrency/data_race.stderr b/tests/run-pass/concurrency/data_race.stderr new file mode 100644 index 0000000000..b01247aea4 --- /dev/null +++ b/tests/run-pass/concurrency/data_race.stderr @@ -0,0 +1,2 @@ +warning: thread support is experimental. + diff --git a/tests/run-pass/concurrency/linux-futex.stderr b/tests/run-pass/concurrency/linux-futex.stderr index 2dbfb7721d..b01247aea4 100644 --- a/tests/run-pass/concurrency/linux-futex.stderr +++ b/tests/run-pass/concurrency/linux-futex.stderr @@ -1,2 +1,2 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. diff --git a/tests/run-pass/concurrency/simple.stderr b/tests/run-pass/concurrency/simple.stderr index 7060411278..f1550dd25a 100644 --- a/tests/run-pass/concurrency/simple.stderr +++ b/tests/run-pass/concurrency/simple.stderr @@ -1,4 +1,4 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. thread '' panicked at 'Hello!', $DIR/simple.rs:54:9 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/tests/run-pass/concurrency/sync.stderr b/tests/run-pass/concurrency/sync.stderr index 2dbfb7721d..b01247aea4 100644 --- a/tests/run-pass/concurrency/sync.stderr +++ b/tests/run-pass/concurrency/sync.stderr @@ -1,2 +1,2 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. diff --git a/tests/run-pass/concurrency/thread_locals.stderr b/tests/run-pass/concurrency/thread_locals.stderr index 2dbfb7721d..b01247aea4 100644 --- a/tests/run-pass/concurrency/thread_locals.stderr +++ b/tests/run-pass/concurrency/thread_locals.stderr @@ -1,2 +1,2 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. diff --git a/tests/run-pass/concurrency/tls_lib_drop.stderr b/tests/run-pass/concurrency/tls_lib_drop.stderr index 2dbfb7721d..b01247aea4 100644 --- a/tests/run-pass/concurrency/tls_lib_drop.stderr +++ b/tests/run-pass/concurrency/tls_lib_drop.stderr @@ -1,2 +1,2 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. diff --git a/tests/run-pass/libc.stderr b/tests/run-pass/libc.stderr index 2dbfb7721d..b01247aea4 100644 --- a/tests/run-pass/libc.stderr +++ b/tests/run-pass/libc.stderr @@ -1,2 +1,2 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. diff --git a/tests/run-pass/panic/concurrent-panic.stderr b/tests/run-pass/panic/concurrent-panic.stderr index eb5b5f59a0..ca6031e57b 100644 --- a/tests/run-pass/panic/concurrent-panic.stderr +++ b/tests/run-pass/panic/concurrent-panic.stderr @@ -1,4 +1,4 @@ -warning: thread support is experimental. For example, Miri does not detect data races yet. +warning: thread support is experimental. Thread 1 starting, will block on mutex Thread 1 reported it has started