Skip to content

Commit 278965a

Browse files
RalfJungRalfJung
committed
give some more help for the unusual data races
1 parent 052539e commit 278965a

12 files changed

+54
-29
lines changed

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,13 @@ impl AccessType {
248248
AccessType::NaRead | AccessType::NaWrite(_) => false,
249249
}
250250
}
251+
252+
fn is_read(self) -> bool {
253+
match self {
254+
AccessType::AtomicLoad | AccessType::NaRead => true,
255+
AccessType::NaWrite(_) | AccessType::AtomicStore | AccessType::AtomicRmw => false,
256+
}
257+
}
251258
}
252259

253260
/// Memory Cell vector clock metadata
@@ -872,9 +879,8 @@ impl VClockAlloc {
872879
) -> InterpResult<'tcx> {
873880
let (current_index, current_clocks) = global.current_thread_state(thread_mgr);
874881
let mut other_size = None; // if `Some`, this was a size-mismatch race
875-
let mut involves_non_atomic = true;
876882
let write_clock;
877-
let (other_action, other_thread, other_clock) =
883+
let (other_access, other_thread, other_clock) =
878884
// First check the atomic-nonatomic cases. If it looks like multiple
879885
// cases apply, this one should take precedence, else it might look like
880886
// we are reporting races between two non-atomic reads.
@@ -898,7 +904,6 @@ impl VClockAlloc {
898904
} else if access.is_atomic() && let Some(atomic) = mem_clocks.atomic() && atomic.size != access_size {
899905
// This is only a race if we are not synchronized with all atomic accesses, so find
900906
// the one we are not synchronized with.
901-
involves_non_atomic = false;
902907
other_size = Some(atomic.size);
903908
if let Some(idx) = Self::find_gt_index(&atomic.write_vector, &current_clocks.clock)
904909
{
@@ -919,27 +924,36 @@ impl VClockAlloc {
919924
// Load elaborated thread information about the racing thread actions.
920925
let current_thread_info = global.print_thread_metadata(thread_mgr, current_index);
921926
let other_thread_info = global.print_thread_metadata(thread_mgr, other_thread);
927+
let involves_non_atomic = !access.is_atomic() || !other_access.is_atomic();
922928

923929
// Throw the data-race detection.
930+
let extra = if other_size.is_some() {
931+
assert!(!involves_non_atomic);
932+
Some("overlapping unsynchronized atomic accesses must use the same access size")
933+
} else if access.is_read() && other_access.is_read() {
934+
assert!(involves_non_atomic);
935+
Some(
936+
"overlapping atomic and non-atomic accesses must be synchronized, even if both are read-only",
937+
)
938+
} else {
939+
None
940+
};
924941
Err(err_machine_stop!(TerminationInfo::DataRace {
925942
involves_non_atomic,
943+
extra,
926944
ptr: ptr_dbg,
927945
op1: RacingOp {
928946
action: if let Some(other_size) = other_size {
929-
format!("{}-byte {}", other_size.bytes(), other_action.description())
947+
format!("{}-byte {}", other_size.bytes(), other_access.description())
930948
} else {
931-
other_action.description().to_owned()
949+
other_access.description().to_owned()
932950
},
933951
thread_info: other_thread_info,
934952
span: other_clock.as_slice()[other_thread.index()].span_data(),
935953
},
936954
op2: RacingOp {
937955
action: if other_size.is_some() {
938-
format!(
939-
"{}-byte (different-size) {}",
940-
access_size.bytes(),
941-
access.description()
942-
)
956+
format!("{}-byte {}", access_size.bytes(), access.description())
943957
} else {
944958
access.description().to_owned()
945959
},

‎src/tools/miri/src/diagnostics.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub enum TerminationInfo {
4747
ptr: Pointer,
4848
op1: RacingOp,
4949
op2: RacingOp,
50+
extra: Option<&'static str>,
5051
},
5152
}
5253

@@ -75,7 +76,7 @@ impl fmt::Display for TerminationInfo {
7576
write!(f, "multiple definitions of symbol `{link_name}`"),
7677
SymbolShimClashing { link_name, .. } =>
7778
write!(f, "found `{link_name}` symbol definition that clashes with a built-in shim",),
78-
DataRace { involves_non_atomic, ptr, op1, op2 } =>
79+
DataRace { involves_non_atomic, ptr, op1, op2, .. } =>
7980
write!(
8081
f,
8182
"{} detected between (1) {} on {} and (2) {} on {} at {ptr:?}. (2) just happened here",
@@ -266,12 +267,16 @@ pub fn report_error<'tcx, 'mir>(
266267
vec![(Some(*span), format!("the `{link_name}` symbol is defined here"))],
267268
Int2PtrWithStrictProvenance =>
268269
vec![(None, format!("use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead"))],
269-
DataRace { op1, .. } =>
270-
vec![
271-
(Some(op1.span), format!("and (1) occurred earlier here")),
272-
(None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")),
273-
(None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")),
274-
],
270+
DataRace { op1, extra, .. } => {
271+
let mut helps = vec![(Some(op1.span), format!("and (1) occurred earlier here"))];
272+
if let Some(extra) = extra {
273+
helps.push((None, format!("{extra}")))
274+
}
275+
helps.push((None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")));
276+
helps.push((None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")));
277+
helps
278+
}
279+
,
275280
_ => vec![],
276281
};
277282
(title, helps)

‎src/tools/miri/tests/fail/data_race/mixed_size_read.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn main() {
1919
});
2020
s.spawn(|| {
2121
a8[0].load(Ordering::SeqCst);
22-
//~^ ERROR: Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte (different-size) atomic load on thread `<unnamed>`
22+
//~^ ERROR: Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte atomic load on thread `<unnamed>`
2323
});
2424
});
2525
}

‎src/tools/miri/tests/fail/data_race/mixed_size_read.stderr

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
error: Undefined Behavior: Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte (different-size) atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
1+
error: Undefined Behavior: Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
22
--> $DIR/mixed_size_read.rs:LL:CC
33
|
44
LL | a8[0].load(Ordering::SeqCst);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte (different-size) atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic load on thread `<unnamed>` and (2) 1-byte atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
66
|
77
help: and (1) occurred earlier here
88
--> $DIR/mixed_size_read.rs:LL:CC
99
|
1010
LL | a16.load(Ordering::SeqCst);
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
= help: overlapping unsynchronized atomic accesses must use the same access size
1213
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1314
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
1415
= note: BACKTRACE (of the first span):

‎src/tools/miri/tests/fail/data_race/mixed_size_write.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn main() {
1919
});
2020
s.spawn(|| {
2121
a8[0].store(1, Ordering::SeqCst);
22-
//~^ ERROR: Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte (different-size) atomic store on thread `<unnamed>`
22+
//~^ ERROR: Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte atomic store on thread `<unnamed>`
2323
});
2424
});
2525
}

‎src/tools/miri/tests/fail/data_race/mixed_size_write.stderr

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
error: Undefined Behavior: Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte (different-size) atomic store on thread `<unnamed>` at ALLOC. (2) just happened here
1+
error: Undefined Behavior: Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte atomic store on thread `<unnamed>` at ALLOC. (2) just happened here
22
--> $DIR/mixed_size_write.rs:LL:CC
33
|
44
LL | a8[0].store(1, Ordering::SeqCst);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte (different-size) atomic store on thread `<unnamed>` at ALLOC. (2) just happened here
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 2-byte atomic store on thread `<unnamed>` and (2) 1-byte atomic store on thread `<unnamed>` at ALLOC. (2) just happened here
66
|
77
help: and (1) occurred earlier here
88
--> $DIR/mixed_size_write.rs:LL:CC
99
|
1010
LL | a16.store(1, Ordering::SeqCst);
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
= help: overlapping unsynchronized atomic accesses must use the same access size
1213
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1314
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
1415
= note: BACKTRACE (of the first span):

‎src/tools/miri/tests/fail/data_race/read_read_race1.stderr

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ help: and (1) occurred earlier here
99
|
1010
LL | unsafe { ptr.read() };
1111
| ^^^^^^^^^^
12+
= help: overlapping atomic and non-atomic accesses must be synchronized, even if both are read-only
1213
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1314
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
1415
= note: BACKTRACE (of the first span):

‎src/tools/miri/tests/fail/data_race/read_read_race2.stderr

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ help: and (1) occurred earlier here
99
|
1010
LL | a.load(Ordering::SeqCst);
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^
12+
= help: overlapping atomic and non-atomic accesses must be synchronized, even if both are read-only
1213
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1314
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
1415
= note: BACKTRACE (of the first span):

‎src/tools/miri/tests/fail/weak_memory/racing_mixed_size.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub fn main() {
3131
let x_split = split_u32_ptr(x_ptr);
3232
unsafe {
3333
let hi = ptr::addr_of!((*x_split)[0]);
34-
std::intrinsics::atomic_load_relaxed(hi); //~ ERROR: different-size
34+
std::intrinsics::atomic_load_relaxed(hi); //~ ERROR: (1) 4-byte atomic store on thread `<unnamed>` and (2) 2-byte atomic load
3535
}
3636
});
3737

‎src/tools/miri/tests/fail/weak_memory/racing_mixed_size.stderr

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
error: Undefined Behavior: Race condition detected between (1) 4-byte atomic store on thread `<unnamed>` and (2) 2-byte (different-size) atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
1+
error: Undefined Behavior: Race condition detected between (1) 4-byte atomic store on thread `<unnamed>` and (2) 2-byte atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
22
--> $DIR/racing_mixed_size.rs:LL:CC
33
|
44
LL | std::intrinsics::atomic_load_relaxed(hi);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 4-byte atomic store on thread `<unnamed>` and (2) 2-byte (different-size) atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Race condition detected between (1) 4-byte atomic store on thread `<unnamed>` and (2) 2-byte atomic load on thread `<unnamed>` at ALLOC. (2) just happened here
66
|
77
help: and (1) occurred earlier here
88
--> $DIR/racing_mixed_size.rs:LL:CC
99
|
1010
LL | x.store(1, Relaxed);
1111
| ^^^^^^^^^^^^^^^^^^^
12+
= help: overlapping unsynchronized atomic accesses must use the same access size
1213
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1314
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
1415
= note: BACKTRACE (of the first span):

0 commit comments

Comments
 (0)