diff --git a/src/data_race.rs b/src/data_race.rs index 7211f3176350..2fff13a80661 100644 --- a/src/data_race.rs +++ b/src/data_race.rs @@ -9,6 +9,9 @@ //! Relaxed stores now unconditionally block all currently active release sequences and so per-thread tracking of release //! sequences is not needed. //! +//! The implementation also models races with memory allocation and deallocation via treating allocation and +//! deallocation as a type of write internally for detecting data-races. +//! //! This does not explore weak memory orders and so can still miss data-races //! but should not report false-positives //! @@ -192,13 +195,22 @@ struct AtomicMemoryCellClocks { sync_vector: VClock, } +/// Type of write operation: allocating memory +/// non-atomic writes and deallocating memory +/// are all treated as writes for the purpose +/// of the data-race detector. #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum WriteType { /// Allocate memory. Allocate, + /// Standard unsynchronized write. Write, - /// Deallocate memory + + /// Deallocate memory. + /// Some races with deallocation will be missed and instead + /// reported as invalid accesses of freed memory due to + /// the order of checks. Deallocate, } impl WriteType { @@ -681,7 +693,7 @@ impl VClockAlloc { let (alloc_index, clocks) = global.current_thread_state(); let alloc_timestamp = clocks.clock[alloc_index]; (alloc_timestamp, alloc_index) - }else{ + } else { (0, VectorIdx::MAX_INDEX) }; VClockAlloc { @@ -695,7 +707,7 @@ impl VClockAlloc { // Find an index, if one exists where the value // in `l` is greater than the value in `r`. fn find_gt_index(l: &VClock, r: &VClock) -> Option { - log::info!("Find index where not {:?} <= {:?}", l, r); + log::trace!("Find index where not {:?} <= {:?}", l, r); let l_slice = l.as_slice(); let r_slice = r.as_slice(); l_slice @@ -1168,7 +1180,7 @@ impl GlobalState { vector_info.push(thread) }; - log::info!("Creating thread = {:?} with vector index = {:?}", thread, created_index); + log::trace!("Creating thread = {:?} with vector index = {:?}", thread, created_index); // Mark the chosen vector index as in use by the thread. thread_info[thread].vector_index = Some(created_index); diff --git a/src/machine.rs b/src/machine.rs index 159e08c87d3f..9825467a2cae 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -480,7 +480,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { let race_alloc = if let Some(data_race) = &memory_extra.data_race { match kind { // V-Table generation is lazy and so racy, so do not track races. - // Also V-Tables are read only so no data races can be detected. + // Also V-Tables are read only so no data races can be occur. + // Must be disabled since V-Tables are initialized via interpreter + // writes on demand and can incorrectly cause the data-race detector + // to trigger. MemoryKind::Vtable => None, // User allocated and stack memory should track allocation. MemoryKind::Machine( diff --git a/tests/compile-fail/data_race/alloc_read_race.rs b/tests/compile-fail/data_race/alloc_read_race.rs index 620a019b65c7..fc1e9d30e637 100644 --- a/tests/compile-fail/data_race/alloc_read_race.rs +++ b/tests/compile-fail/data_race/alloc_read_race.rs @@ -3,6 +3,7 @@ use std::thread::spawn; use std::ptr::null_mut; use std::sync::atomic::{Ordering, AtomicPtr}; +use std::mem::MaybeUninit; #[derive(Copy, Clone)] struct EvilSend(pub T); @@ -12,8 +13,8 @@ unsafe impl Sync for EvilSend {} pub fn main() { // Shared atomic pointer - let pointer = AtomicPtr::new(null_mut::()); - let ptr = EvilSend(&pointer as *const AtomicPtr); + let pointer = AtomicPtr::new(null_mut::>()); + let ptr = EvilSend(&pointer as *const AtomicPtr>); // Note: this is scheduler-dependent // the operations need to occur in @@ -28,14 +29,14 @@ pub fn main() { // Uses relaxed semantics to not generate // a release sequence. let pointer = &*ptr.0; - pointer.store(Box::into_raw(Box::new(0usize)), Ordering::Relaxed); + pointer.store(Box::into_raw(Box::new(MaybeUninit::uninit())), Ordering::Relaxed); }); let j2 = spawn(move || { let pointer = &*ptr.0; - //Note detects with write due to the initialization of memory - *pointer.load(Ordering::Relaxed) //~ ERROR Data race detected between Read on Thread(id = 2) and Write on Thread(id = 1) + // Note: could also error due to reading uninitialized memory, but the data-race detector triggers first. + *pointer.load(Ordering::Relaxed) //~ ERROR Data race detected between Read on Thread(id = 2) and Allocate on Thread(id = 1) }); j1.join().unwrap(); diff --git a/tests/compile-fail/data_race/dangling_thread_async_race.rs b/tests/compile-fail/data_race/dangling_thread_async_race.rs index 61587dc6384a..ad539ec5b083 100644 --- a/tests/compile-fail/data_race/dangling_thread_async_race.rs +++ b/tests/compile-fail/data_race/dangling_thread_async_race.rs @@ -26,7 +26,7 @@ fn main() { // Detach the thread and sleep until it terminates mem::drop(join); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); // Spawn and immediately join a thread // to execute the join code-path diff --git a/tests/compile-fail/data_race/dangling_thread_race.rs b/tests/compile-fail/data_race/dangling_thread_race.rs index c14b68080cc8..755ba8efdae9 100644 --- a/tests/compile-fail/data_race/dangling_thread_race.rs +++ b/tests/compile-fail/data_race/dangling_thread_race.rs @@ -24,9 +24,9 @@ fn main() { }) }; - // Detatch the thread and sleep until it terminates + // Detach the thread and sleep until it terminates mem::drop(join); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); // Spawn and immediately join a thread // to execute the join code-path diff --git a/tests/compile-fail/data_race/dealloc_read_race.rs b/tests/compile-fail/data_race/dealloc_read_race1.rs similarity index 100% rename from tests/compile-fail/data_race/dealloc_read_race.rs rename to tests/compile-fail/data_race/dealloc_read_race1.rs diff --git a/tests/compile-fail/data_race/dealloc_read_race2.rs b/tests/compile-fail/data_race/dealloc_read_race2.rs new file mode 100644 index 000000000000..a4bf210ef439 --- /dev/null +++ b/tests/compile-fail/data_race/dealloc_read_race2.rs @@ -0,0 +1,34 @@ +// ignore-windows: Concurrency on Windows is not supported yet. + +use std::thread::spawn; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +extern "Rust" { + fn __rust_dealloc(ptr: *mut u8, size: usize, align: usize); +} + +pub fn main() { + // Shared atomic pointer + let pointer: *mut usize = Box::into_raw(Box::new(0usize)); + let ptr = EvilSend(pointer); + + unsafe { + let j1 = spawn(move || { + __rust_dealloc(ptr.0 as *mut _, std::mem::size_of::(), std::mem::align_of::()) + }); + + let j2 = spawn(move || { + // Also an error of the form: Data race detected between Read on Thread(id = 2) and Deallocate on Thread(id = 1) + // but the invalid allocation is detected first. + *ptr.0 //~ ERROR dereferenced after this allocation got freed + }); + + j1.join().unwrap(); + j2.join().unwrap(); + } +} diff --git a/tests/compile-fail/data_race/dealloc_read_race_stack.rs b/tests/compile-fail/data_race/dealloc_read_race_stack.rs index 8a770563c753..31960fb2162b 100644 --- a/tests/compile-fail/data_race/dealloc_read_race_stack.rs +++ b/tests/compile-fail/data_race/dealloc_read_race_stack.rs @@ -36,7 +36,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); } //~ ERROR Data race detected between Deallocate on Thread(id = 1) and Read on Thread(id = 2) }); diff --git a/tests/compile-fail/data_race/dealloc_read_race_stack_drop.rs b/tests/compile-fail/data_race/dealloc_read_race_stack_drop.rs index 315d5eef3f01..44950a34db2f 100644 --- a/tests/compile-fail/data_race/dealloc_read_race_stack_drop.rs +++ b/tests/compile-fail/data_race/dealloc_read_race_stack_drop.rs @@ -36,7 +36,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); drop(stack_var); }); //~ ERROR Data race detected between Deallocate on Thread(id = 1) and Read on Thread(id = 2) diff --git a/tests/compile-fail/data_race/dealloc_write_race.rs b/tests/compile-fail/data_race/dealloc_write_race1.rs similarity index 100% rename from tests/compile-fail/data_race/dealloc_write_race.rs rename to tests/compile-fail/data_race/dealloc_write_race1.rs diff --git a/tests/compile-fail/data_race/dealloc_write_race2.rs b/tests/compile-fail/data_race/dealloc_write_race2.rs new file mode 100644 index 000000000000..20c05fa8f17b --- /dev/null +++ b/tests/compile-fail/data_race/dealloc_write_race2.rs @@ -0,0 +1,33 @@ +// ignore-windows: Concurrency on Windows is not supported yet. + +use std::thread::spawn; + +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +extern "Rust" { + fn __rust_dealloc(ptr: *mut u8, size: usize, align: usize); +} +pub fn main() { + // Shared atomic pointer + let pointer: *mut usize = Box::into_raw(Box::new(0usize)); + let ptr = EvilSend(pointer); + + unsafe { + let j1 = spawn(move || { + __rust_dealloc(ptr.0 as *mut _, std::mem::size_of::(), std::mem::align_of::()); + }); + + let j2 = spawn(move || { + // Also an error of the form: Data race detected between Write on Thread(id = 2) and Deallocate on Thread(id = 1) + // but the invalid allocation is detected first. + *ptr.0 = 2; //~ ERROR dereferenced after this allocation got freed + }); + + j1.join().unwrap(); + j2.join().unwrap(); + } +} diff --git a/tests/compile-fail/data_race/dealloc_write_race_stack.rs b/tests/compile-fail/data_race/dealloc_write_race_stack.rs index a245522b06f3..25dea65fe7e0 100644 --- a/tests/compile-fail/data_race/dealloc_write_race_stack.rs +++ b/tests/compile-fail/data_race/dealloc_write_race_stack.rs @@ -36,7 +36,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); } //~ ERROR Data race detected between Deallocate on Thread(id = 1) and Write on Thread(id = 2) }); diff --git a/tests/compile-fail/data_race/dealloc_write_race_stack_drop.rs b/tests/compile-fail/data_race/dealloc_write_race_stack_drop.rs index e936a2154ad1..1d239e9eb74d 100644 --- a/tests/compile-fail/data_race/dealloc_write_race_stack_drop.rs +++ b/tests/compile-fail/data_race/dealloc_write_race_stack_drop.rs @@ -36,7 +36,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); // Note: Implicit read for drop(_) races with write, would detect race with deallocate after. drop(stack_var); //~ ERROR Data race detected between Read on Thread(id = 1) and Write on Thread(id = 2) diff --git a/tests/compile-fail/data_race/read_write_race_stack.rs b/tests/compile-fail/data_race/read_write_race_stack.rs index 0fc45c8fafc7..0cf915cdef2b 100644 --- a/tests/compile-fail/data_race/read_write_race_stack.rs +++ b/tests/compile-fail/data_race/read_write_race_stack.rs @@ -1,6 +1,9 @@ // ignore-windows: Concurrency on Windows is not supported yet. // compile-flags: -Zmiri-disable-isolation -Zmir-opt-level=0 +// Note: mir-opt-level set to 0 to prevent the read of stack_var in thread 1 +// from being optimized away and preventing the detection of the data-race. + use std::thread::{spawn, sleep}; use std::ptr::null_mut; use std::sync::atomic::{Ordering, AtomicPtr}; @@ -38,7 +41,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); stack_var //~ ERROR Data race detected between Read on Thread(id = 1) and Write on Thread(id = 2) }); diff --git a/tests/compile-fail/data_race/release_seq_race.rs b/tests/compile-fail/data_race/release_seq_race.rs index 91235280d210..29c428b388d4 100644 --- a/tests/compile-fail/data_race/release_seq_race.rs +++ b/tests/compile-fail/data_race/release_seq_race.rs @@ -30,7 +30,7 @@ pub fn main() { let j1 = spawn(move || { *c.0 = 1; SYNC.store(1, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); SYNC.store(3, Ordering::Relaxed); }); @@ -40,7 +40,7 @@ pub fn main() { }); let j3 = spawn(move || { - sleep(Duration::from_millis(5000)); + sleep(Duration::from_millis(500)); if SYNC.load(Ordering::Acquire) == 3 { *c.0 //~ ERROR Data race detected between Read on Thread(id = 3) and Write on Thread(id = 1) } else { diff --git a/tests/compile-fail/data_race/write_write_race_stack.rs b/tests/compile-fail/data_race/write_write_race_stack.rs index 91ac51787fbe..aa1428f8a74b 100644 --- a/tests/compile-fail/data_race/write_write_race_stack.rs +++ b/tests/compile-fail/data_race/write_write_race_stack.rs @@ -38,7 +38,7 @@ pub fn main() { pointer.store(&mut stack_var as *mut _, Ordering::Release); - sleep(Duration::from_millis(1000)); + sleep(Duration::from_millis(200)); stack_var = 1usize; //~ ERROR Data race detected between Write on Thread(id = 1) and Write on Thread(id = 2)