Merge pull request rust-lang#4796 from RalfJung/data-race-ice

fix ICE for particular data race situations

Author: RalfJung

src/tools/miri/src/concurrency/data_race.rs

@@ -1116,6 +1116,7 @@ impl VClockAlloc {
             {
                 (AccessType::AtomicStore, idx, &atomic.write_vector)
             } else if !access.is_atomic() &&
+                !access.is_read() &&
                 let Some(atomic) = mem_clocks.atomic() &&
                 let Some(idx) = Self::find_gt_index(&atomic.read_vector, &active_clocks.clock)
             {
@@ -1124,7 +1125,7 @@ impl VClockAlloc {
             } else if mem_clocks.write.1 > active_clocks.clock[mem_clocks.write.0] {
                 write_clock = mem_clocks.write();
                 (AccessType::NaWrite(mem_clocks.write_type), mem_clocks.write.0, &write_clock)
-            } else if let Some(idx) = Self::find_gt_index(&mem_clocks.read, &active_clocks.clock) {
+            } else if !access.is_read() && 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 != Some(access_size) {
@@ -1157,7 +1158,9 @@ impl VClockAlloc {
             assert!(!involves_non_atomic);
             Some("overlapping unsynchronized atomic accesses must use the same access size")
         } else if access.is_read() && other_access.is_read() {
-            panic!("there should be no same-size read-read races")
+            panic!(
+                "there should be no same-size read-read races\naccess: {access:?}\nother_access: {other_access:?}"
+            )
         } else {
             None
         };

src/tools/miri/src/concurrency/vector_clock.rs

@@ -48,14 +48,24 @@ const SMALL_VECTOR: usize = 4;
 /// The time-stamps recorded in the data-race detector consist of both
 /// a 32-bit unsigned integer which is the actual timestamp, and a `Span`
 /// so that diagnostics can report what code was responsible for an operation.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub(super) struct VTimestamp {
     /// The lowest bit indicates read type, the rest is the time.
     /// `1` indicates a retag read, `0` a regular read.
     time_and_read_type: u32,
     pub span: Span,
 }
 
+impl Debug for VTimestamp {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("VTimestamp")
+            .field("time", &self.time())
+            .field("read_type", &self.read_type())
+            .field("span", &self.span)
+            .finish()
+    }
+}
+
 impl VTimestamp {
     pub const ZERO: VTimestamp = VTimestamp::new(0, NaReadType::Read, DUMMY_SP);
 

src/tools/miri/tests/fail/data_race/mixed_size_read_write_read.rs

@@ -0,0 +1,27 @@
+//@compile-flags: -Zmiri-deterministic-concurrency
+// A case that is not covered by `mixed_size_read_write`.
+#![feature(ptr_as_ref_unchecked)]
+
+use std::sync::atomic::*;
+use std::thread;
+
+fn main() {
+    let data = AtomicI32::new(0);
+
+    thread::scope(|s| {
+        s.spawn(|| unsafe {
+            let _val = (&raw const data).read();
+            let _val = (&raw const data).cast::<AtomicI8>().as_ref_unchecked().compare_exchange(
+                0,
+                1,
+                Ordering::Relaxed,
+                Ordering::Relaxed,
+            );
+            thread::yield_now();
+            unreachable!();
+        });
+        s.spawn(|| {
+            let _val = data.load(Ordering::Relaxed); //~ERROR: /Race condition detected between \(1\) 1-byte atomic store .* and \(2\) 4-byte atomic load/
+        });
+    });
+}

src/tools/miri/tests/fail/data_race/mixed_size_read_write_read.stderr

@@ -0,0 +1,26 @@
+error: Undefined Behavior: Race condition detected between (1) 1-byte atomic store on thread `unnamed-ID` and (2) 4-byte atomic load on thread `unnamed-ID` at ALLOC
+  --> tests/fail/data_race/mixed_size_read_write_read.rs:LL:CC
+   |
+LL | ...   let _val = data.load(Ordering::Relaxed);
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (2) just happened here
+   |
+help: and (1) occurred earlier here
+  --> tests/fail/data_race/mixed_size_read_write_read.rs:LL:CC
+   |
+LL |               let _val = (&raw const data).cast::<AtomicI8>().as_ref_unchecked().compare_exchange(
+   |  ________________________^
+LL | |                 0,
+LL | |                 1,
+LL | |                 Ordering::Relaxed,
+LL | |                 Ordering::Relaxed,
+LL | |             );
+   | |_____________^
+   = 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: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+