ensure atomics happen on mutable allocations, and fix futex test

Author: RalfJung

src/concurrency/data_race.rs

@@ -46,6 +46,7 @@ use std::{
     mem,
 };
 
+use rustc_ast::Mutability;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_index::vec::{Idx, IndexVec};
 use rustc_middle::{mir, ty::layout::TyAndLayout};
@@ -476,6 +477,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
             align,
             CheckInAllocMsg::MemoryAccessTest,
         )?;
+        // Ensure the allocation is mutable. Even failing (read-only) compare_exchange need mutable
+        // memory on many targets (i.e., they segfault if taht memory is mapped read-only), and
+        // atomic loads can be implemented via compare_exchange on some targets. See
+        // <https://github.com/rust-lang/miri/issues/2463>.
+        // We avoid `get_ptr_alloc` since we do *not* want to run the access hooks -- the actual
+        // access will happen later.
+        let (alloc_id, _offset, _prov) =
+            this.ptr_try_get_alloc_id(place.ptr).expect("there are no zero-sized atomic accesses");
+        if this.get_alloc_mutability(alloc_id)? == Mutability::Not {
+            throw_ub_format!("atomic operations cannot be performed on read-only memory");
+        }
         Ok(())
     }
 

tests/fail/concurrency/read_only_atomic_cmpxchg.rs

@@ -0,0 +1,11 @@
+// Should not rely on the aliasing model for its failure.
+//@compile-flags: -Zmiri-disable-stacked-borrows
+
+use std::sync::atomic::{AtomicI32, Ordering};
+
+fn main() {
+    static X: i32 = 0;
+    let x = &X as *const i32 as *const AtomicI32;
+    let x = unsafe { &*x };
+    x.compare_exchange(1, 2, Ordering::Relaxed, Ordering::Relaxed).unwrap_err(); //~ERROR: atomic operations cannot be performed on read-only memory
+}

tests/fail/concurrency/read_only_atomic_cmpxchg.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: atomic operations cannot be performed on read-only memory
+  --> $DIR/read_only_atomic_cmpxchg.rs:LL:CC
+   |
+LL |     x.compare_exchange(1, 2, Ordering::Relaxed, Ordering::Relaxed).unwrap_err();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ atomic operations cannot be performed on read-only memory
+   |
+   = 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:
+   = note: inside `main` at $DIR/read_only_atomic_cmpxchg.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

tests/fail/concurrency/read_only_atomic_load.rs

@@ -0,0 +1,13 @@
+// Should not rely on the aliasing model for its failure.
+//@compile-flags: -Zmiri-disable-stacked-borrows
+
+use std::sync::atomic::{AtomicI32, Ordering};
+
+fn main() {
+    static X: i32 = 0;
+    let x = &X as *const i32 as *const AtomicI32;
+    let x = unsafe { &*x };
+    // Some targets can implement atomic loads via compare_exchange, so we cannot allow them on
+    // read-only memory.
+    x.load(Ordering::Relaxed); //~ERROR: atomic operations cannot be performed on read-only memory
+}

tests/fail/concurrency/read_only_atomic_load.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: atomic operations cannot be performed on read-only memory
+  --> $DIR/read_only_atomic_load.rs:LL:CC
+   |
+LL |     x.load(Ordering::Relaxed);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ atomic operations cannot be performed on read-only memory
+   |
+   = 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:
+   = note: inside `main` at $DIR/read_only_atomic_load.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

tests/pass/concurrency/linux-futex.rs

@@ -130,7 +130,7 @@ fn wait_absolute_timeout() {
 fn wait_wake() {
     let start = Instant::now();
 
-    static FUTEX: i32 = 0;
+    static mut FUTEX: i32 = 0;
 
     let t = thread::spawn(move || {
         thread::sleep(Duration::from_millis(200));
@@ -167,7 +167,7 @@ fn wait_wake() {
 fn wait_wake_bitset() {
     let start = Instant::now();
 
-    static FUTEX: i32 = 0;
+    static mut FUTEX: i32 = 0;
 
     let t = thread::spawn(move || {
         thread::sleep(Duration::from_millis(200));
@@ -277,8 +277,8 @@ fn concurrent_wait_wake() {
 
     // Make sure we got the interesting case (of having woken a thread) at least once, but not *each* time.
     let woken = WOKEN.load(Ordering::Relaxed);
-    assert!(woken > 0 && woken < rounds);
     //eprintln!("waking happened {woken} times");
+    assert!(woken > 0 && woken < rounds);
 }
 
 fn main() {