From d2e1c3738e292c46184e0d64841607bc88dce9d6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 20 Dec 2022 22:48:23 -0500 Subject: [PATCH] Fix span management --- src/tools/miri/src/concurrency/data_race.rs | 16 ++++++-------- .../miri/src/concurrency/vector_clock.rs | 21 +++++++------------ .../fail/data_race/alloc_read_race.stderr | 2 +- .../atomic_write_na_read_race1.stderr | 6 +++--- .../atomic_write_na_read_race2.stderr | 6 +++--- .../fail/data_race/dealloc_read_race1.stderr | 6 +++--- .../data_race/dealloc_read_race_stack.stderr | 2 +- .../fail/data_race/read_write_race.stderr | 6 +++--- .../data_race/read_write_race_stack.stderr | 4 ++-- .../fail/data_race/relax_acquire_race.stderr | 4 ++-- .../fail/data_race/release_seq_race.stderr | 4 ++-- .../release_seq_race_same_thread.stderr | 4 ++-- .../miri/tests/fail/data_race/rmw_race.stderr | 4 ++-- .../fail/data_race/stack_pop_race.stderr | 6 +++--- .../retag_data_race_read.stderr | 4 ++-- 15 files changed, 42 insertions(+), 53 deletions(-) diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 8b343dd2fe8..2d825b369c2 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -51,7 +51,6 @@ use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::mir; use rustc_span::Span; -use rustc_span::DUMMY_SP; use rustc_target::abi::{Align, Size}; use crate::diagnostics::RacingOp; @@ -398,7 +397,10 @@ fn read_race_detect( current_span: Span, ) -> Result<(), DataRace> { log::trace!("Unsynchronized read with vectors: {:#?} :: {:#?}", self, clocks); - let res = if self.write <= clocks.clock[self.write_index] { + if !current_span.is_dummy() { + clocks.clock[index].span = current_span; + } + if self.write <= clocks.clock[self.write_index] { let race_free = if let Some(atomic) = self.atomic() { atomic.write_vector <= clocks.clock } else { @@ -408,11 +410,7 @@ fn read_race_detect( if race_free { Ok(()) } else { Err(DataRace) } } else { Err(DataRace) - }; - if res.is_ok() && current_span != DUMMY_SP { - clocks.clock[index].span = current_span; } - res } /// Detect races for non-atomic write operations at the current memory cell @@ -425,7 +423,7 @@ fn write_race_detect( current_span: Span, ) -> Result<(), DataRace> { log::trace!("Unsynchronized write with vectors: {:#?} :: {:#?}", self, clocks); - if current_span != DUMMY_SP { + if !current_span.is_dummy() { clocks.clock[index].span = current_span; } if self.write <= clocks.clock[self.write_index] && self.read <= clocks.clock { @@ -712,9 +710,7 @@ pub fn new_allocation( | MemoryKind::Stack => { let (alloc_index, clocks) = global.current_thread_state(thread_mgr); let mut alloc_timestamp = clocks.clock[alloc_index]; - if current_span != DUMMY_SP { - alloc_timestamp.span = current_span; - } + alloc_timestamp.span = current_span; (alloc_timestamp, alloc_index) } // Other global memory should trace races but be allocated at the 0 timestamp. diff --git a/src/tools/miri/src/concurrency/vector_clock.rs b/src/tools/miri/src/concurrency/vector_clock.rs index 191bb1449af..0dcd6830b4e 100644 --- a/src/tools/miri/src/concurrency/vector_clock.rs +++ b/src/tools/miri/src/concurrency/vector_clock.rs @@ -45,8 +45,9 @@ fn from(id: u32) -> Self { /// clock vectors larger than this will be stored on the heap const SMALL_VECTOR: usize = 4; -/// The type of the time-stamps recorded in the data-race detector -/// set to a type of unsigned integer +/// 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, Eq)] pub struct VTimestamp { time: u32, @@ -128,7 +129,7 @@ pub fn increment_index(&mut self, idx: VectorIdx, current_span: Span) { let mut_slice = self.get_mut_with_min_len(idx + 1); let idx_ref = &mut mut_slice[idx]; idx_ref.time = idx_ref.time.checked_add(1).expect("Vector clock overflow"); - if current_span != DUMMY_SP { + if !current_span.is_dummy() { idx_ref.span = current_span; } } @@ -143,14 +144,7 @@ pub fn join(&mut self, other: &Self) { let l_span = l.span; let r_span = r.span; *l = r.max(*l); - if l.span == DUMMY_SP { - if r_span != DUMMY_SP { - l.span = r_span; - } - if l_span != DUMMY_SP { - l.span = l_span; - } - } + l.span = l.span.substitute_dummy(r_span).substitute_dummy(l_span); } } @@ -162,9 +156,8 @@ pub fn set_at_index(&mut self, other: &Self, idx: VectorIdx) { mut_slice[idx.index()] = other[idx]; - if other[idx].span == DUMMY_SP { - mut_slice[idx.index()].span = prev_span; - } + let span = &mut mut_slice[idx.index()].span; + *span = span.substitute_dummy(prev_span); } /// Set the vector to the all-zero vector diff --git a/src/tools/miri/tests/fail/data_race/alloc_read_race.stderr b/src/tools/miri/tests/fail/data_race/alloc_read_race.stderr index d0ec8c1f128..510b177fc7e 100644 --- a/src/tools/miri/tests/fail/data_race/alloc_read_race.stderr +++ b/src/tools/miri/tests/fail/data_race/alloc_read_race.stderr @@ -8,7 +8,7 @@ help: The Read on thread `` is here --> $DIR/alloc_read_race.rs:LL:CC | LL | ... *pointer.load(Ordering::Relaxed) - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: The Allocate on thread `` is here --> $DIR/alloc_read_race.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.stderr b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.stderr index 32485d3f8f3..ffc985c04cb 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.stderr +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.stderr @@ -5,10 +5,10 @@ LL | *atomic_ref.get_mut() | ^^^^^^^^^^^^^^^^^^^^^ Data race detected between Read on thread `` and Atomic Store on thread `` at ALLOC | help: The Read on thread `` is here - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + --> $DIR/atomic_write_na_read_race1.rs:LL:CC | -LL | } - | ^ +LL | *atomic_ref.get_mut() + | ^^^^^^^^^^^^^^^^^^^^^ help: The Atomic Store on thread `` is here --> $DIR/atomic_write_na_read_race1.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.stderr b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.stderr index 5217f3d8a70..5b5c8d07f8b 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.stderr +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.stderr @@ -10,10 +10,10 @@ help: The Atomic Store on thread `` is here LL | ... (&*c.0).store(32, Ordering::SeqCst); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: The Read on thread `` is here - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + --> $DIR/atomic_write_na_read_race2.rs:LL:CC | -LL | } - | ^ +LL | let _val = *(c.0 as *mut usize); + | ^^^^^^^^^^^^^^^^^^^^ = 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: diff --git a/src/tools/miri/tests/fail/data_race/dealloc_read_race1.stderr b/src/tools/miri/tests/fail/data_race/dealloc_read_race1.stderr index ac20d05bf12..02a9bb63b5d 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_read_race1.stderr +++ b/src/tools/miri/tests/fail/data_race/dealloc_read_race1.stderr @@ -20,10 +20,10 @@ LL | | std::mem::align_of::(), LL | | ); | |_____________^ help: The Read on thread `` is here - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + --> $DIR/dealloc_read_race1.rs:LL:CC | -LL | } - | ^ +LL | let _val = *ptr.0; + | ^^^^^^ = 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: diff --git a/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.stderr b/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.stderr index 93c7db38751..18a30bd5a68 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.stderr +++ b/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.stderr @@ -13,7 +13,7 @@ help: The Read on thread `` is here --> $DIR/dealloc_read_race_stack.rs:LL:CC | LL | *pointer.load(Ordering::Acquire) - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = 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: diff --git a/src/tools/miri/tests/fail/data_race/read_write_race.stderr b/src/tools/miri/tests/fail/data_race/read_write_race.stderr index b1441dabaf0..afba81ee8f7 100644 --- a/src/tools/miri/tests/fail/data_race/read_write_race.stderr +++ b/src/tools/miri/tests/fail/data_race/read_write_race.stderr @@ -10,10 +10,10 @@ help: The Write on thread `` is here LL | *c.0 = 64; | ^^^^^^^^^ help: The Read on thread `` is here - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + --> $DIR/read_write_race.rs:LL:CC | -LL | } - | ^ +LL | let _val = *c.0; + | ^^^^ = 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: diff --git a/src/tools/miri/tests/fail/data_race/read_write_race_stack.stderr b/src/tools/miri/tests/fail/data_race/read_write_race_stack.stderr index 2a2572576db..e1e4604aa6e 100644 --- a/src/tools/miri/tests/fail/data_race/read_write_race_stack.stderr +++ b/src/tools/miri/tests/fail/data_race/read_write_race_stack.stderr @@ -7,8 +7,8 @@ LL | stack_var help: The Read on thread `` is here --> $DIR/read_write_race_stack.rs:LL:CC | -LL | sleep(Duration::from_millis(200)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | stack_var + | ^^^^^^^^^ help: The Write on thread `` is here --> $DIR/read_write_race_stack.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/relax_acquire_race.stderr b/src/tools/miri/tests/fail/data_race/relax_acquire_race.stderr index e4819614ddb..aae90e0c9e9 100644 --- a/src/tools/miri/tests/fail/data_race/relax_acquire_race.stderr +++ b/src/tools/miri/tests/fail/data_race/relax_acquire_race.stderr @@ -7,8 +7,8 @@ LL | *c.0 help: The Read on thread `` is here --> $DIR/relax_acquire_race.rs:LL:CC | -LL | if SYNC.load(Ordering::Acquire) == 2 { - | ^ +LL | *c.0 + | ^^^^ help: The Write on thread `` is here --> $DIR/relax_acquire_race.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/release_seq_race.stderr b/src/tools/miri/tests/fail/data_race/release_seq_race.stderr index 6bc8bccbbf0..02413e6baa2 100644 --- a/src/tools/miri/tests/fail/data_race/release_seq_race.stderr +++ b/src/tools/miri/tests/fail/data_race/release_seq_race.stderr @@ -7,8 +7,8 @@ LL | *c.0 help: The Read on thread `` is here --> $DIR/release_seq_race.rs:LL:CC | -LL | if SYNC.load(Ordering::Acquire) == 3 { - | ^ +LL | *c.0 + | ^^^^ help: The Write on thread `` is here --> $DIR/release_seq_race.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.stderr b/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.stderr index 6997685dc3e..9573ca95867 100644 --- a/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.stderr +++ b/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.stderr @@ -7,8 +7,8 @@ LL | *c.0 help: The Read on thread `` is here --> $DIR/release_seq_race_same_thread.rs:LL:CC | -LL | if SYNC.load(Ordering::Acquire) == 2 { - | ^ +LL | *c.0 + | ^^^^ help: The Write on thread `` is here --> $DIR/release_seq_race_same_thread.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/rmw_race.stderr b/src/tools/miri/tests/fail/data_race/rmw_race.stderr index e4f3ecf89ce..ae53cc0c75f 100644 --- a/src/tools/miri/tests/fail/data_race/rmw_race.stderr +++ b/src/tools/miri/tests/fail/data_race/rmw_race.stderr @@ -7,8 +7,8 @@ LL | *c.0 help: The Read on thread `` is here --> $DIR/rmw_race.rs:LL:CC | -LL | if SYNC.load(Ordering::Acquire) == 3 { - | ^ +LL | *c.0 + | ^^^^ help: The Write on thread `` is here --> $DIR/rmw_race.rs:LL:CC | diff --git a/src/tools/miri/tests/fail/data_race/stack_pop_race.stderr b/src/tools/miri/tests/fail/data_race/stack_pop_race.stderr index b156179da4d..0853e0830c7 100644 --- a/src/tools/miri/tests/fail/data_race/stack_pop_race.stderr +++ b/src/tools/miri/tests/fail/data_race/stack_pop_race.stderr @@ -10,10 +10,10 @@ help: The Deallocate on thread `main` is here LL | } | ^ help: The Read on thread `` is here - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + --> $DIR/stack_pop_race.rs:LL:CC | -LL | } - | ^ +LL | let _val = unsafe { *ptr.0 }; + | ^^^^^^ = 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: diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr index 6b280a16a37..ff874bd9e82 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr @@ -12,8 +12,8 @@ LL | *p = 5; help: The Read on thread `` is here --> $DIR/retag_data_race_read.rs:LL:CC | -LL | let t1 = std::thread::spawn(move || thread_1(p)); - | ^ +LL | let _r = &*p; + | ^^^ = 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: