Skip to content

Commit

Permalink
Auto merge of #98674 - RalfJung:miri-stacktrace-pruning, r=Mark-Simul…
Browse files Browse the repository at this point in the history
…acrum

miri: prune some atomic operation and raw pointer details from stacktrace

Since Miri removes `track_caller` frames from the stacktrace, adding that attribute can help make backtraces more readable (similar to how it makes panic locations better). I made them only show up with `cfg(miri)` to make sure the extra arguments induced by `track_caller` do not cause any runtime performance trouble.

This is also testing the waters for whether the libs team is okay with having these attributes in their code, or whether you'd prefer if we find some other way to do this. If you are fine with this, we will probably want to add it to a lot more functions (all the other atomic operations, to start).

Before:
```
error: Undefined Behavior: Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at alloc1727 (current vector clock = VClock([9, 0, 6]), conflicting timestamp = VClock([0, 6]))
    --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:2594:23
     |
2594 |             SeqCst => intrinsics::atomic_load_seqcst(dst),
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at alloc1727 (current vector clock = VClock([9, 0, 6]), conflicting timestamp = VClock([0, 6]))
     |
     = 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: inside `std::sync::atomic::atomic_load::<usize>` at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:2594:23
     = note: inside `std::sync::atomic::AtomicUsize::load` at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:1719:26
note: inside closure at ../miri/tests/fail/data_race/atomic_read_na_write_race1.rs:22:13
    --> ../miri/tests/fail/data_race/atomic_read_na_write_race1.rs:22:13
     |
22   |             (&*c.0).load(Ordering::SeqCst)
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

After:
```
error: Undefined Behavior: Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at alloc1727 (current vector clock = VClock([9, 0, 6]), conflicting timestamp = VClock([0, 6]))
  --> tests/fail/data_race/atomic_read_na_write_race1.rs:22:13
   |
22 |             (&*c.0).load(Ordering::SeqCst)
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Atomic Load on Thread(id = 2) and Write on Thread(id = 1) at alloc1727 (current vector clock = VClock([9, 0, 6]), conflicting timestamp = VClock([0, 6]))
   |
   = 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: inside closure at tests/fail/data_race/atomic_read_na_write_race1.rs:22:13
```
  • Loading branch information
bors committed Jul 24, 2022
2 parents 4dbc89d + 13877a9 commit 35a0617
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 0 deletions.
1 change: 1 addition & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2625,6 +2625,7 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
#[cfg_attr(not(bootstrap), rustc_allowed_through_unstable_modules)]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
extern "rust-intrinsic" {
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,7 @@ impl<T: ?Sized> *const T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read(self) -> T
where
T: Sized,
Expand All @@ -1164,6 +1165,7 @@ impl<T: ?Sized> *const T {
/// [`ptr::read_volatile`]: crate::ptr::read_volatile()
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn read_volatile(self) -> T
where
T: Sized,
Expand All @@ -1183,6 +1185,7 @@ impl<T: ?Sized> *const T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read_unaligned(self) -> T
where
T: Sized,
Expand Down
6 changes: 6 additions & 0 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,7 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read<T>(src: *const T) -> T {
// We are calling the intrinsics directly to avoid function calls in the generated code
// as `intrinsics::copy_nonoverlapping` is a wrapper function.
Expand Down Expand Up @@ -1194,6 +1195,7 @@ pub const unsafe fn read<T>(src: *const T) -> T {
#[inline]
#[stable(feature = "ptr_unaligned", since = "1.17.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read_unaligned<T>(src: *const T) -> T {
let mut tmp = MaybeUninit::<T>::uninit();
// SAFETY: the caller must guarantee that `src` is valid for reads.
Expand Down Expand Up @@ -1290,6 +1292,7 @@ pub const unsafe fn read_unaligned<T>(src: *const T) -> T {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write<T>(dst: *mut T, src: T) {
// We are calling the intrinsics directly to avoid function calls in the generated code
// as `intrinsics::copy_nonoverlapping` is a wrapper function.
Expand Down Expand Up @@ -1387,6 +1390,7 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
#[inline]
#[stable(feature = "ptr_unaligned", since = "1.17.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write_unaligned<T>(dst: *mut T, src: T) {
// SAFETY: the caller must guarantee that `dst` is valid for writes.
// `dst` cannot overlap `src` because the caller has mutable access
Expand Down Expand Up @@ -1460,6 +1464,7 @@ pub const unsafe fn write_unaligned<T>(dst: *mut T, src: T) {
/// ```
#[inline]
#[stable(feature = "volatile", since = "1.9.0")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn read_volatile<T>(src: *const T) -> T {
// SAFETY: the caller must uphold the safety contract for `volatile_load`.
unsafe {
Expand Down Expand Up @@ -1530,6 +1535,7 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
/// ```
#[inline]
#[stable(feature = "volatile", since = "1.9.0")]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
// SAFETY: the caller must uphold the safety contract for `volatile_store`.
unsafe {
Expand Down
7 changes: 7 additions & 0 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read(self) -> T
where
T: Sized,
Expand All @@ -1278,6 +1279,7 @@ impl<T: ?Sized> *mut T {
/// [`ptr::read_volatile`]: crate::ptr::read_volatile()
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn read_volatile(self) -> T
where
T: Sized,
Expand All @@ -1297,6 +1299,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read_unaligned(self) -> T
where
T: Sized,
Expand Down Expand Up @@ -1402,6 +1405,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write(self, val: T)
where
T: Sized,
Expand All @@ -1420,6 +1424,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write_bytes(self, val: u8, count: usize)
where
T: Sized,
Expand All @@ -1440,6 +1445,7 @@ impl<T: ?Sized> *mut T {
/// [`ptr::write_volatile`]: crate::ptr::write_volatile()
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn write_volatile(self, val: T)
where
T: Sized,
Expand All @@ -1459,6 +1465,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write_unaligned(self, val: T)
where
T: Sized,
Expand Down
Loading

0 comments on commit 35a0617

Please sign in to comment.