From cd2edbfd098b6c9351f8c168c8f36c190ce5e675 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Aug 2022 17:47:28 -0400 Subject: [PATCH] ensure atomics happen on mutable allocations, and fix futex test --- src/concurrency/data_race.rs | 12 ++++++++++++ .../fail/concurrency/read_only_atomic_cmpxchg.rs | 11 +++++++++++ .../concurrency/read_only_atomic_cmpxchg.stderr | 15 +++++++++++++++ tests/fail/concurrency/read_only_atomic_load.rs | 13 +++++++++++++ .../fail/concurrency/read_only_atomic_load.stderr | 15 +++++++++++++++ tests/pass/concurrency/linux-futex.rs | 6 +++--- 6 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 tests/fail/concurrency/read_only_atomic_cmpxchg.rs create mode 100644 tests/fail/concurrency/read_only_atomic_cmpxchg.stderr create mode 100644 tests/fail/concurrency/read_only_atomic_load.rs create mode 100644 tests/fail/concurrency/read_only_atomic_load.stderr diff --git a/src/concurrency/data_race.rs b/src/concurrency/data_race.rs index 65f198f3c9d..51105ec98df 100644 --- a/src/concurrency/data_race.rs +++ b/src/concurrency/data_race.rs @@ -46,6 +46,7 @@ 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 @@ fn atomic_access_check(&self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResul 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 + // . + // 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(()) } diff --git a/tests/fail/concurrency/read_only_atomic_cmpxchg.rs b/tests/fail/concurrency/read_only_atomic_cmpxchg.rs new file mode 100644 index 00000000000..cb6aeea665d --- /dev/null +++ b/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 +} diff --git a/tests/fail/concurrency/read_only_atomic_cmpxchg.stderr b/tests/fail/concurrency/read_only_atomic_cmpxchg.stderr new file mode 100644 index 00000000000..2753c492266 --- /dev/null +++ b/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 + diff --git a/tests/fail/concurrency/read_only_atomic_load.rs b/tests/fail/concurrency/read_only_atomic_load.rs new file mode 100644 index 00000000000..6e92453e3c1 --- /dev/null +++ b/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 +} diff --git a/tests/fail/concurrency/read_only_atomic_load.stderr b/tests/fail/concurrency/read_only_atomic_load.stderr new file mode 100644 index 00000000000..588081afc62 --- /dev/null +++ b/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 + diff --git a/tests/pass/concurrency/linux-futex.rs b/tests/pass/concurrency/linux-futex.rs index 43216481e76..2c99bfa1000 100644 --- a/tests/pass/concurrency/linux-futex.rs +++ b/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() {