diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index b3fe99f7f1a..2357fb932b4 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -33,6 +33,12 @@ //! atomic load (via the operations provided in this module). A "modification of an atomic object" //! refers to an atomic store. //! +//! The most important aspect of this model is that conflicting non-synchronized accesses are +//! Undefined Behavior unless both accesses are atomic. Here, accesses are *conflicting* if they +//! affect overlapping regions of memory and at least one of them is a write. They are +//! *non-synchronized* if neither of them *happens-before* the other, according to the +//! happens-before order of the memory model. +//! //! The end result is *almost* equivalent to saying that creating a *shared reference* to one of the //! Rust atomic types corresponds to creating an `atomic_ref` in C++, with the `atomic_ref` being //! destroyed when the lifetime of the shared reference ends. The main difference is that Rust @@ -41,9 +47,10 @@ //! objects" and "non-atomic objects" (with `atomic_ref` temporarily converting a non-atomic object //! into an atomic object). //! -//! That said, Rust *does* inherit the C++ limitation that non-synchronized atomic accesses may not -//! partially overlap: they must be either disjoint or access the exact same memory. This in -//! particular rules out non-synchronized differently-sized accesses to the same data. +//! That said, Rust *does* inherit the C++ limitation that non-synchronized conflicting atomic +//! accesses may not partially overlap: they must be either disjoint or access the exact same +//! memory. This in particular rules out non-synchronized differently-sized atomic accesses to the +//! same data unless all accesses are reads. //! //! [cpp]: https://en.cppreference.com/w/cpp/atomic //! [cpp-intro.races]: https://timsong-cpp.github.io/cppwp/n4868/intro.multithread#intro.races @@ -63,7 +70,7 @@ //! let atomic = AtomicU16::new(0); //! //! thread::scope(|s| { -//! // This is UB: conflicting concurrent accesses. +//! // This is UB: conflicting non-synchronized accesses, at least one of which is non-atomic. //! s.spawn(|| atomic.store(1, Ordering::Relaxed)); // atomic store //! s.spawn(|| unsafe { atomic.as_ptr().write(2) }); // non-atomic write //! }); @@ -77,16 +84,15 @@ //! }); //! //! thread::scope(|s| { -//! // This is fine, `join` synchronizes the code in a way such that atomic -//! // and non-atomic accesses can't happen "at the same time". +//! // This is fine: `join` synchronizes the code in a way such that the atomic +//! // store happens-before the non-atomic write. //! let handle = s.spawn(|| atomic.store(1, Ordering::Relaxed)); // atomic store //! handle.join().unwrap(); // synchronize //! s.spawn(|| unsafe { atomic.as_ptr().write(2) }); // non-atomic write //! }); //! //! thread::scope(|s| { -//! // This is UB: using differently-sized atomic accesses to the same data. -//! // (It would be UB even if these are both loads.) +//! // This is UB: non-synchronized conflicting differently-sized atomic accesses. //! s.spawn(|| atomic.store(1, Ordering::Relaxed)); //! s.spawn(|| unsafe { //! let differently_sized = transmute::<&AtomicU16, &AtomicU8>(&atomic); @@ -95,8 +101,8 @@ //! }); //! //! thread::scope(|s| { -//! // This is fine, `join` synchronizes the code in a way such that -//! // differently-sized accesses can't happen "at the same time". +//! // This is fine: `join` synchronizes the code in a way such that +//! // the 1-byte store happens-before the 2-byte store. //! let handle = s.spawn(|| atomic.store(1, Ordering::Relaxed)); //! handle.join().unwrap(); //! s.spawn(|| unsafe { diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 558a032d0bc..812602c3980 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -191,7 +191,8 @@ struct AtomicMemoryCellClocks { /// The size of accesses to this atomic location. /// We use this to detect non-synchronized mixed-size accesses. Since all accesses must be /// aligned to their size, this is sufficient to detect imperfectly overlapping accesses. - size: Size, + /// `None` indicates that we saw multiple different sizes, which is okay as long as all accesses are reads. + size: Option, } #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -265,6 +266,14 @@ fn description(self, ty: Option>, size: Option) -> String { let mut msg = String::new(); if let Some(size) = size { + if size == Size::ZERO { + // In this case there were multiple read accesss with different sizes and then a write. + // We will be reporting *one* of the other reads, but we don't have enough information + // to determine which one had which size. + assert!(self == AccessType::AtomicLoad); + assert!(ty.is_none()); + return format!("multiple differently-sized atomic loads, including one load"); + } msg.push_str(&format!("{}-byte {}", size.bytes(), msg)) } @@ -305,8 +314,7 @@ fn is_retag(self) -> bool { } } -/// Memory Cell vector clock metadata -/// for data-race detection. +/// Per-byte vector clock metadata for data-race detection. #[derive(Clone, PartialEq, Eq, Debug)] struct MemoryCellClocks { /// The vector-clock timestamp and the thread that did the last non-atomic write. We don't need @@ -325,8 +333,8 @@ struct MemoryCellClocks { read: VClock, /// Atomic access, acquire, release sequence tracking clocks. - /// For non-atomic memory in the common case this - /// value is set to None. + /// For non-atomic memory this value is set to None. + /// For atomic memory, each byte carries this information. atomic_ops: Option>, } @@ -336,7 +344,7 @@ fn new(size: Size) -> Self { read_vector: Default::default(), write_vector: Default::default(), sync_vector: Default::default(), - size, + size: Some(size), } } } @@ -383,17 +391,23 @@ fn atomic_access( &mut self, thread_clocks: &ThreadClockSet, size: Size, + write: bool, ) -> Result<&mut AtomicMemoryCellClocks, DataRace> { match self.atomic_ops { Some(ref mut atomic) => { // We are good if the size is the same or all atomic accesses are before our current time. - if atomic.size == size { + if atomic.size == Some(size) { Ok(atomic) } else if atomic.read_vector <= thread_clocks.clock && atomic.write_vector <= thread_clocks.clock { - // This is now the new size that must be used for accesses here. - atomic.size = size; + // We are fully ordered after all previous accesses, so we can change the size. + atomic.size = Some(size); + Ok(atomic) + } else if !write && atomic.write_vector <= thread_clocks.clock { + // This is a read, and it is ordered after the last write. It's okay for the + // sizes to mismatch, as long as no writes with a different size occur later. + atomic.size = None; Ok(atomic) } else { Err(DataRace) @@ -508,7 +522,7 @@ fn atomic_read_detect( access_size: Size, ) -> Result<(), DataRace> { trace!("Atomic read with vectors: {:#?} :: {:#?}", self, thread_clocks); - let atomic = self.atomic_access(thread_clocks, access_size)?; + let atomic = self.atomic_access(thread_clocks, access_size, /*write*/ false)?; atomic.read_vector.set_at_index(&thread_clocks.clock, index); // Make sure the last non-atomic write was before this access. if self.write_was_before(&thread_clocks.clock) { Ok(()) } else { Err(DataRace) } @@ -523,7 +537,7 @@ fn atomic_write_detect( access_size: Size, ) -> Result<(), DataRace> { trace!("Atomic write with vectors: {:#?} :: {:#?}", self, thread_clocks); - let atomic = self.atomic_access(thread_clocks, access_size)?; + let atomic = self.atomic_access(thread_clocks, access_size, /*write*/ true)?; atomic.write_vector.set_at_index(&thread_clocks.clock, index); // Make sure the last non-atomic write and all non-atomic reads were before this access. if self.write_was_before(&thread_clocks.clock) && self.read <= thread_clocks.clock { @@ -969,10 +983,10 @@ fn report_data_race<'tcx>( } else if let Some(idx) = Self::find_gt_index(&mem_clocks.read, &active_clocks.clock) { (AccessType::NaRead(mem_clocks.read[idx].read_type()), idx, &mem_clocks.read) // Finally, mixed-size races. - } else if access.is_atomic() && let Some(atomic) = mem_clocks.atomic() && atomic.size != access_size { + } else if access.is_atomic() && let Some(atomic) = mem_clocks.atomic() && atomic.size != Some(access_size) { // This is only a race if we are not synchronized with all atomic accesses, so find // the one we are not synchronized with. - other_size = Some(atomic.size); + other_size = Some(atomic.size.unwrap_or(Size::ZERO)); if let Some(idx) = Self::find_gt_index(&atomic.write_vector, &active_clocks.clock) { (AccessType::AtomicStore, idx, &atomic.write_vector) diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read.rs b/src/tools/miri/tests/fail/data_race/mixed_size_read.rs deleted file mode 100644 index 828b47f0a65..00000000000 --- a/src/tools/miri/tests/fail/data_race/mixed_size_read.rs +++ /dev/null @@ -1,28 +0,0 @@ -//@compile-flags: -Zmiri-preemption-rate=0.0 -Zmiri-disable-weak-memory-emulation -// Avoid accidental synchronization via address reuse inside `thread::spawn`. -//@compile-flags: -Zmiri-address-reuse-cross-thread-rate=0 - -use std::sync::atomic::{AtomicU8, AtomicU16, Ordering}; -use std::thread; - -fn convert(a: &AtomicU16) -> &[AtomicU8; 2] { - unsafe { std::mem::transmute(a) } -} - -// We can't allow mixed-size accesses; they are not possible in C++ and even -// Intel says you shouldn't do it. -fn main() { - let a = AtomicU16::new(0); - let a16 = &a; - let a8 = convert(a16); - - thread::scope(|s| { - s.spawn(|| { - a16.load(Ordering::SeqCst); - }); - s.spawn(|| { - a8[0].load(Ordering::SeqCst); - //~^ ERROR: Race condition detected between (1) 2-byte atomic load on thread `unnamed-1` and (2) 1-byte atomic load on thread `unnamed-2` - }); - }); -} diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_first_load.stderr b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_first_load.stderr new file mode 100644 index 00000000000..b829627c00a --- /dev/null +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_first_load.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-ID` and (2) 2-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + --> tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + | +LL | a16.store(0, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-ID` and (2) 2-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + | +LL | a16.load(Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: overlapping unsynchronized atomic accesses must use the same access size + = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span) on thread `unnamed-ID`: + = note: inside closure at tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_second_load.stderr b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_second_load.stderr new file mode 100644 index 00000000000..6bc38b14cb2 --- /dev/null +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.match_second_load.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-ID` and (2) 1-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + --> tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + | +LL | a8[0].store(0, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-ID` and (2) 1-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + | +LL | a16.load(Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: overlapping unsynchronized atomic accesses must use the same access size + = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span) on thread `unnamed-ID`: + = note: inside closure at tests/fail/data_race/mixed_size_read_read_write.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.rs b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.rs new file mode 100644 index 00000000000..ece5bd31274 --- /dev/null +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_read_write.rs @@ -0,0 +1,39 @@ +//@compile-flags: -Zmiri-preemption-rate=0.0 -Zmiri-disable-weak-memory-emulation +// Avoid accidental synchronization via address reuse inside `thread::spawn`. +//@compile-flags: -Zmiri-address-reuse-cross-thread-rate=0 +// Two variants: the atomic store matches the size of the first or second atomic load. +//@revisions: match_first_load match_second_load + +use std::sync::atomic::{AtomicU16, AtomicU8, Ordering}; +use std::thread; + +fn convert(a: &AtomicU16) -> &[AtomicU8; 2] { + unsafe { std::mem::transmute(a) } +} + +// We can't allow mixed-size accesses; they are not possible in C++ and even +// Intel says you shouldn't do it. +fn main() { + let a = AtomicU16::new(0); + let a16 = &a; + let a8 = convert(a16); + + thread::scope(|s| { + s.spawn(|| { + a16.load(Ordering::SeqCst); + }); + s.spawn(|| { + a8[0].load(Ordering::SeqCst); + }); + s.spawn(|| { + thread::yield_now(); // make sure this happens last + if cfg!(match_first_load) { + a16.store(0, Ordering::SeqCst); + //~[match_first_load]^ ERROR: Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-1` and (2) 2-byte atomic store on thread `unnamed-3` + } else { + a8[0].store(0, Ordering::SeqCst); + //~[match_second_load]^ ERROR: Race condition detected between (1) multiple differently-sized atomic loads, including one load on thread `unnamed-1` and (2) 1-byte atomic store on thread `unnamed-3` + } + }); + }); +} diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read_write.read_write.stderr b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.read_write.stderr new file mode 100644 index 00000000000..6f8dbd38ca8 --- /dev/null +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.read_write.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: Race condition detected between (1) 1-byte atomic load on thread `unnamed-ID` and (2) 2-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + --> tests/fail/data_race/mixed_size_read_write.rs:LL:CC + | +LL | a16.store(1, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 1-byte atomic load on thread `unnamed-ID` and (2) 2-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> tests/fail/data_race/mixed_size_read_write.rs:LL:CC + | +LL | a8[0].load(Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: overlapping unsynchronized atomic accesses must use the same access size + = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span) on thread `unnamed-ID`: + = note: inside closure at tests/fail/data_race/mixed_size_read_write.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read_write.rs b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.rs new file mode 100644 index 00000000000..acc12427b3f --- /dev/null +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.rs @@ -0,0 +1,39 @@ +//@compile-flags: -Zmiri-preemption-rate=0.0 -Zmiri-disable-weak-memory-emulation +// Avoid accidental synchronization via address reuse inside `thread::spawn`. +//@compile-flags: -Zmiri-address-reuse-cross-thread-rate=0 +// Two revisions, depending on which access goes first. +//@revisions: read_write write_read + +use std::sync::atomic::{AtomicU16, AtomicU8, Ordering}; +use std::thread; + +fn convert(a: &AtomicU16) -> &[AtomicU8; 2] { + unsafe { std::mem::transmute(a) } +} + +// We can't allow mixed-size accesses; they are not possible in C++ and even +// Intel says you shouldn't do it. +fn main() { + let a = AtomicU16::new(0); + let a16 = &a; + let a8 = convert(a16); + + thread::scope(|s| { + s.spawn(|| { + if cfg!(read_write) { + // Let the other one go first. + thread::yield_now(); + } + a16.store(1, Ordering::SeqCst); + //~[read_write]^ ERROR: Race condition detected between (1) 1-byte atomic load on thread `unnamed-2` and (2) 2-byte atomic store on thread `unnamed-1` + }); + s.spawn(|| { + if cfg!(write_read) { + // Let the other one go first. + thread::yield_now(); + } + a8[0].load(Ordering::SeqCst); + //~[write_read]^ ERROR: Race condition detected between (1) 2-byte atomic store on thread `unnamed-1` and (2) 1-byte atomic load on thread `unnamed-2` + }); + }); +} diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_read.stderr b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.write_read.stderr similarity index 66% rename from src/tools/miri/tests/fail/data_race/mixed_size_read.stderr rename to src/tools/miri/tests/fail/data_race/mixed_size_read_write.write_read.stderr index 31a798a89b1..990d2058bca 100644 --- a/src/tools/miri/tests/fail/data_race/mixed_size_read.stderr +++ b/src/tools/miri/tests/fail/data_race/mixed_size_read_write.write_read.stderr @@ -1,20 +1,20 @@ -error: Undefined Behavior: Race condition detected between (1) 2-byte atomic load on thread `unnamed-ID` and (2) 1-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here - --> tests/fail/data_race/mixed_size_read.rs:LL:CC +error: Undefined Behavior: Race condition detected between (1) 2-byte atomic store on thread `unnamed-ID` and (2) 1-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here + --> tests/fail/data_race/mixed_size_read_write.rs:LL:CC | LL | a8[0].load(Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic load on thread `unnamed-ID` and (2) 1-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic store on thread `unnamed-ID` and (2) 1-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here - --> tests/fail/data_race/mixed_size_read.rs:LL:CC + --> tests/fail/data_race/mixed_size_read_write.rs:LL:CC | -LL | a16.load(Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | a16.store(1, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: overlapping unsynchronized atomic accesses must use the same access size = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information = note: BACKTRACE (of the first span) on thread `unnamed-ID`: - = note: inside closure at tests/fail/data_race/mixed_size_read.rs:LL:CC + = note: inside closure at tests/fail/data_race/mixed_size_read_write.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_write.rs b/src/tools/miri/tests/fail/data_race/mixed_size_write_write.rs similarity index 100% rename from src/tools/miri/tests/fail/data_race/mixed_size_write.rs rename to src/tools/miri/tests/fail/data_race/mixed_size_write_write.rs diff --git a/src/tools/miri/tests/fail/data_race/mixed_size_write.stderr b/src/tools/miri/tests/fail/data_race/mixed_size_write_write.stderr similarity index 90% rename from src/tools/miri/tests/fail/data_race/mixed_size_write.stderr rename to src/tools/miri/tests/fail/data_race/mixed_size_write_write.stderr index c30b48c1f32..1f22413bc5f 100644 --- a/src/tools/miri/tests/fail/data_race/mixed_size_write.stderr +++ b/src/tools/miri/tests/fail/data_race/mixed_size_write_write.stderr @@ -1,11 +1,11 @@ error: Undefined Behavior: Race condition detected between (1) 2-byte atomic store on thread `unnamed-ID` and (2) 1-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here - --> tests/fail/data_race/mixed_size_write.rs:LL:CC + --> tests/fail/data_race/mixed_size_write_write.rs:LL:CC | LL | a8[0].store(1, Ordering::SeqCst); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic store on thread `unnamed-ID` and (2) 1-byte atomic store on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here - --> tests/fail/data_race/mixed_size_write.rs:LL:CC + --> tests/fail/data_race/mixed_size_write_write.rs:LL:CC | LL | a16.store(1, Ordering::SeqCst); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -14,7 +14,7 @@ LL | a16.store(1, Ordering::SeqCst); = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information = note: BACKTRACE (of the first span) on thread `unnamed-ID`: - = note: inside closure at tests/fail/data_race/mixed_size_write.rs:LL:CC + = note: inside closure at tests/fail/data_race/mixed_size_write_write.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size.rs b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size.rs deleted file mode 100644 index 1193dddc577..00000000000 --- a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size.rs +++ /dev/null @@ -1,41 +0,0 @@ -// We want to control preemption here. -// Avoid accidental synchronization via address reuse. -//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-address-reuse-cross-thread-rate=0 - -#![feature(core_intrinsics)] - -use std::ptr; -use std::sync::atomic::AtomicU32; -use std::sync::atomic::Ordering::*; -use std::thread::spawn; - -fn static_atomic_u32(val: u32) -> &'static AtomicU32 { - let ret = Box::leak(Box::new(AtomicU32::new(val))); - ret -} - -fn split_u32_ptr(dword: *const u32) -> *const [u16; 2] { - unsafe { std::mem::transmute::<*const u32, *const [u16; 2]>(dword) } -} - -// Wine's SRWLock implementation does this, which is definitely undefined in C++ memory model -// https://github.com/wine-mirror/wine/blob/303f8042f9db508adaca02ef21f8de4992cb9c03/dlls/ntdll/sync.c#L543-L566 -// It probably works just fine on x86, but Intel does document this as "don't do it!" -pub fn main() { - let x = static_atomic_u32(0); - let j1 = spawn(move || { - x.store(1, Relaxed); - }); - - let j2 = spawn(move || { - let x_ptr = x as *const AtomicU32 as *const u32; - let x_split = split_u32_ptr(x_ptr); - unsafe { - let hi = ptr::addr_of!((*x_split)[0]); - std::intrinsics::atomic_load_relaxed(hi); //~ ERROR: (1) 4-byte atomic store on thread `unnamed-1` and (2) 2-byte atomic load - } - }); - - j1.join().unwrap(); - j2.join().unwrap(); -} diff --git a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs deleted file mode 100644 index 0a0e372f1f3..00000000000 --- a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs +++ /dev/null @@ -1,39 +0,0 @@ -// We want to control preemption here. -// Avoid accidental synchronization via address reuse. -//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-address-reuse-cross-thread-rate=0 - -use std::sync::atomic::Ordering::*; -use std::sync::atomic::{AtomicU16, AtomicU32}; -use std::thread::spawn; - -fn static_atomic(val: u32) -> &'static AtomicU32 { - let ret = Box::leak(Box::new(AtomicU32::new(val))); - ret -} - -fn split_u32_ptr(dword: *const u32) -> *const [u16; 2] { - unsafe { std::mem::transmute::<*const u32, *const [u16; 2]>(dword) } -} - -// Racing mixed size reads may cause two loads to read-from -// the same store but observe different values, which doesn't make -// sense under the formal model so we forbid this. -pub fn main() { - let x = static_atomic(0); - - let j1 = spawn(move || { - x.load(Relaxed); - }); - - let j2 = spawn(move || { - let x_ptr = x as *const AtomicU32 as *const u32; - let x_split = split_u32_ptr(x_ptr); - unsafe { - let hi = x_split as *const u16 as *const AtomicU16; - (*hi).load(Relaxed); //~ ERROR: (1) 4-byte atomic load on thread `unnamed-1` and (2) 2-byte atomic load - } - }); - - j1.join().unwrap(); - j2.join().unwrap(); -} diff --git a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr deleted file mode 100644 index 9e6a6e80418..00000000000 --- a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr +++ /dev/null @@ -1,22 +0,0 @@ -error: Undefined Behavior: Race condition detected between (1) 4-byte atomic load on thread `unnamed-ID` and (2) 2-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here - --> tests/fail/weak_memory/racing_mixed_size_read.rs:LL:CC - | -LL | (*hi).load(Relaxed); - | ^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 4-byte atomic load on thread `unnamed-ID` and (2) 2-byte atomic load on thread `unnamed-ID` at ALLOC. (2) just happened here - | -help: and (1) occurred earlier here - --> tests/fail/weak_memory/racing_mixed_size_read.rs:LL:CC - | -LL | x.load(Relaxed); - | ^^^^^^^^^^^^^^^ - = help: overlapping unsynchronized atomic accesses must use the same access size - = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE (of the first span) on thread `unnamed-ID`: - = note: inside closure at tests/fail/weak_memory/racing_mixed_size_read.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/pass/concurrency/data_race.rs b/src/tools/miri/tests/pass/concurrency/data_race.rs index adf5a37891f..0918b94ed31 100644 --- a/src/tools/miri/tests/pass/concurrency/data_race.rs +++ b/src/tools/miri/tests/pass/concurrency/data_race.rs @@ -177,6 +177,26 @@ fn test_read_read_race2() { }); } +fn mixed_size_read_read() { + fn convert(a: &AtomicU16) -> &[AtomicU8; 2] { + unsafe { std::mem::transmute(a) } + } + + let a = AtomicU16::new(0); + let a16 = &a; + let a8 = convert(a16); + + // Just two different-sized atomic reads without any writes are fine. + thread::scope(|s| { + s.spawn(|| { + a16.load(Ordering::SeqCst); + }); + s.spawn(|| { + a8[0].load(Ordering::SeqCst); + }); + }); +} + pub fn main() { test_fence_sync(); test_multiple_reads(); @@ -185,4 +205,5 @@ pub fn main() { test_local_variable_lazy_write(); test_read_read_race1(); test_read_read_race2(); + mixed_size_read_read(); }