fix and slightly improve data race reports

Author: RalfJung

src/concurrency/data_race.rs

@@ -882,7 +882,7 @@ impl VClockAlloc {
     ) -> InterpResult<'tcx> {
         let (current_index, current_clocks) = global.current_thread_state(thread_mgr);
         let write_clock;
-        let (other_action, other_thread, other_clock) = if range.write
+        let (other_action, other_thread, _other_clock) = if range.write
             > current_clocks.clock[range.write_index]
         {
             // Convert the write action into the vector clock it
@@ -920,14 +920,12 @@ impl VClockAlloc {
 
         // Throw the data-race detection.
         throw_ub_format!(
-            "Data race detected between {} on {} and {} on {} at {:?} (current vector clock = {:?}, conflicting timestamp = {:?})",
+            "Data race detected between {} on {} and {} on {} at {:?}",
             action,
             current_thread_info,
             other_action,
             other_thread_info,
             ptr_dbg,
-            current_clocks.clock,
-            other_clock
         )
     }
 
@@ -1208,8 +1206,7 @@ impl GlobalState {
         };
 
         // Setup the main-thread since it is not explicitly created:
-        // uses vector index and thread-id 0, also the rust runtime gives
-        // the main-thread a name of "main".
+        // uses vector index and thread-id 0.
         let index = global_state.vector_clocks.get_mut().push(ThreadClockSet::default());
         global_state.vector_info.get_mut().push(ThreadId::new(0));
         global_state
@@ -1448,12 +1445,8 @@ impl GlobalState {
         vector: VectorIdx,
     ) -> String {
         let thread = self.vector_info.borrow()[vector];
-        let thread_name = thread_mgr.get_thread_name();
-        format!(
-            "Thread(id = {:?}, name = {:?})",
-            thread.to_u32(),
-            String::from_utf8_lossy(thread_name)
-        )
+        let thread_name = thread_mgr.get_thread_name(thread);
+        format!("thread `{}`", String::from_utf8_lossy(thread_name))
     }
 
     /// Acquire a lock, express that the previous call of

src/concurrency/weak_memory.rs

@@ -9,7 +9,7 @@
 //! Note that this implementation does not take into account of C++20's memory model revision to SC accesses
 //! and fences introduced by P0668 (<https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0668r5.html>).
 //! This implementation is not fully correct under the revised C++20 model and may generate behaviours C++20
-//! disallows.
+//! disallows (<https://github.com/rust-lang/miri/issues/2301>).
 //!
 //! Rust follows the C++20 memory model (except for the Consume ordering and some operations not performable through C++'s
 //! std::atomic<T> API). It is therefore possible for this implementation to generate behaviours never observable when the

src/thread.rs

@@ -170,6 +170,14 @@ impl<'mir, 'tcx> Default for Thread<'mir, 'tcx> {
     }
 }
 
+impl<'mir, 'tcx> Thread<'mir, 'tcx> {
+    fn new(name: &str) -> Self {
+        let mut thread = Thread::default();
+        thread.thread_name = Some(Vec::from(name.as_bytes()));
+        thread
+    }
+}
+
 /// A specific moment in time.
 #[derive(Debug)]
 pub enum Time {
@@ -230,7 +238,7 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {
     fn default() -> Self {
         let mut threads = IndexVec::new();
         // Create the main thread and add it to the list of threads.
-        let mut main_thread = Thread::default();
+        let mut main_thread = Thread::new("main");
         // The main thread can *not* be joined on.
         main_thread.join_status = ThreadJoinStatus::Detached;
         threads.push(main_thread);
@@ -379,15 +387,20 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
     }
 
     /// Set the name of the active thread.
-    fn set_thread_name(&mut self, new_thread_name: Vec<u8>) {
+    fn set_active_thread_name(&mut self, new_thread_name: Vec<u8>) {
         self.active_thread_mut().thread_name = Some(new_thread_name);
     }
 
     /// Get the name of the active thread.
-    pub fn get_thread_name(&self) -> &[u8] {
+    pub fn get_active_thread_name(&self) -> &[u8] {
         self.active_thread_ref().thread_name()
     }
 
+    /// Get the name of the given thread.
+    pub fn get_thread_name(&self, thread: ThreadId) -> &[u8] {
+        self.threads[thread].thread_name()
+    }
+
     /// Put the thread into the blocked state.
     fn block_thread(&mut self, thread: ThreadId) {
         let state = &mut self.threads[thread].state;
@@ -475,7 +488,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
         for (i, thread) in self.threads.iter_enumerated_mut() {
             if thread.state == ThreadState::BlockedOnJoin(self.active_thread) {
                 // The thread has terminated, mark happens-before edge to joining thread
-                if let Some(_) = data_race {
+                if data_race.is_some() {
                     joined_threads.push(i);
                 }
                 trace!("unblocking {:?} because {:?} terminated", i, self.active_thread);
@@ -683,7 +696,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     #[inline]
     fn set_active_thread_name(&mut self, new_thread_name: Vec<u8>) {
         let this = self.eval_context_mut();
-        this.machine.threads.set_thread_name(new_thread_name);
+        this.machine.threads.set_active_thread_name(new_thread_name);
     }
 
     #[inline]
@@ -692,7 +705,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         'mir: 'c,
     {
         let this = self.eval_context_ref();
-        this.machine.threads.get_thread_name()
+        this.machine.threads.get_active_thread_name()
     }
 
     #[inline]

tests/compiletest.rs

@@ -94,8 +94,6 @@ regexes! {
     "([0-9]+: ) +0x[0-9a-f]+ - (.*)" => "$1$2",
     // erase long hexadecimals
     r"0x[0-9a-fA-F]+[0-9a-fA-F]{2,2}" => "$$HEX",
-    // erase clocks
-    r"VClock\(\[[^\]]+\]\)"      => "VClock",
     // erase specific alignments
     "alignment [0-9]+"               => "alignment ALIGN",
     // erase thread caller ids

tests/fail/data_race/alloc_read_race.rs

@@ -38,7 +38,7 @@ pub fn main() {
             let pointer = &*ptr.0;
 
             // 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)
+            *pointer.load(Ordering::Relaxed) //~ ERROR Data race detected between Read on thread `<unnamed>` and Allocate on thread `<unnamed>`
         });
 
         j1.join().unwrap();

tests/fail/data_race/alloc_read_race.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: Data race detected between Read on Thread(id = 2) and Allocate on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+error: Undefined Behavior: Data race detected between Read on thread `<unnamed>` and Allocate on thread `<unnamed>` at ALLOC
   --> $DIR/alloc_read_race.rs:LL:CC
    |
 LL |             *pointer.load(Ordering::Relaxed)
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Read on Thread(id = 2) and Allocate on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Read on thread `<unnamed>` and Allocate on thread `<unnamed>` at ALLOC
    |
    = 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

tests/fail/data_race/alloc_write_race.rs

@@ -36,7 +36,7 @@ pub fn main() {
 
         let j2 = spawn(move || {
             let pointer = &*ptr.0;
-            *pointer.load(Ordering::Relaxed) = 2; //~ ERROR Data race detected between Write on Thread(id = 2) and Allocate on Thread(id = 1)
+            *pointer.load(Ordering::Relaxed) = 2; //~ ERROR Data race detected between Write on thread `<unnamed>` and Allocate on thread `<unnamed>`
         });
 
         j1.join().unwrap();

tests/fail/data_race/alloc_write_race.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: Data race detected between Write on Thread(id = 2) and Allocate on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+error: Undefined Behavior: Data race detected between Write on thread `<unnamed>` and Allocate on thread `<unnamed>` at ALLOC
   --> $DIR/alloc_write_race.rs:LL:CC
    |
 LL |             *pointer.load(Ordering::Relaxed) = 2;
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Write on Thread(id = 2) and Allocate on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Write on thread `<unnamed>` and Allocate on thread `<unnamed>` at ALLOC
    |
    = 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

tests/fail/data_race/atomic_read_na_write_race1.rs

@@ -22,7 +22,7 @@ pub fn main() {
 
         let j2 = spawn(move || {
             //Equivalent to: (&*c.0).load(Ordering::SeqCst)
-            intrinsics::atomic_load_seqcst(c.0 as *mut usize) //~ ERROR Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1)
+            intrinsics::atomic_load_seqcst(c.0 as *mut usize) //~ ERROR Data race detected between Atomic Load on thread `<unnamed>` and Write on thread `<unnamed>`
         });
 
         j1.join().unwrap();

tests/fail/data_race/atomic_read_na_write_race1.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+error: Undefined Behavior: Data race detected between Atomic Load on thread `<unnamed>` and Write on thread `<unnamed>` at ALLOC
   --> $DIR/atomic_read_na_write_race1.rs:LL:CC
    |
 LL |             intrinsics::atomic_load_seqcst(c.0 as *mut usize)
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at ALLOC (current vector clock = VClock, conflicting timestamp = VClock)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Atomic Load on thread `<unnamed>` and Write on thread `<unnamed>` at ALLOC
    |
    = 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