diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 1117b69116a..bd3bc8dbb41 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -282,6 +282,14 @@ fn main() { }; miri_config.tracked_alloc_id = Some(miri::AllocId(id)); } + arg if arg.starts_with("-Zmiri-compare-exchange-weak-failure-rate=") => { + let rate = match arg.strip_prefix("-Zmiri-compare-exchange-weak-failure-rate=").unwrap().parse::() { + Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate, + Ok(_) => panic!("-Zmiri-compare-exchange-weak-failure-rate must be between `0.0` and `1.0`"), + Err(err) => panic!("-Zmiri-compare-exchange-weak-failure-rate requires a `f64` between `0.0` and `1.0`: {}", err), + }; + miri_config.cmpxchg_weak_failure_rate = rate; + } _ => { // Forward to rustc. rustc_args.push(arg); diff --git a/src/data_race.rs b/src/data_race.rs index d204d60f156..f79775e12fe 100644 --- a/src/data_race.rs +++ b/src/data_race.rs @@ -75,7 +75,7 @@ use rustc_target::abi::Size; use crate::{ ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MiriEvalContext, MiriEvalContextExt, - OpTy, Pointer, RangeMap, ScalarMaybeUninit, Tag, ThreadId, VClock, VTimestamp, + OpTy, Pointer, RangeMap, Scalar, ScalarMaybeUninit, Tag, ThreadId, VClock, VTimestamp, VectorIdx, MemoryKind, MiriMemoryKind }; @@ -544,7 +544,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { /// Perform an atomic compare and exchange at a given memory location. /// On success an atomic RMW operation is performed and on failure - /// only an atomic read occurs. + /// only an atomic read occurs. If `can_fail_spuriously` is true, + /// then we treat it as a "compare_exchange_weak" operation, and + /// some portion of the time fail even when the values are actually + /// identical. fn atomic_compare_exchange_scalar( &mut self, place: MPlaceTy<'tcx, Tag>, @@ -552,7 +555,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { new: ScalarMaybeUninit, success: AtomicRwOp, fail: AtomicReadOp, + can_fail_spuriously: bool, ) -> InterpResult<'tcx, Immediate> { + use rand::Rng as _; let this = self.eval_context_mut(); // Failure ordering cannot be stronger than success ordering, therefore first attempt @@ -560,15 +565,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { // read ordering and write in the success case. // Read as immediate for the sake of `binary_op()` let old = this.allow_data_races_mut(|this| this.read_immediate(place.into()))?; - // `binary_op` will bail if either of them is not a scalar. let eq = this.overflowing_binary_op(mir::BinOp::Eq, old, expect_old)?.0; - let res = Immediate::ScalarPair(old.to_scalar_or_uninit(), eq.into()); + // If the operation would succeed, but is "weak", fail some portion + // of the time, based on `rate`. + let rate = this.memory.extra.cmpxchg_weak_failure_rate; + let cmpxchg_success = eq.to_bool()? + && (!can_fail_spuriously || this.memory.extra.rng.borrow_mut().gen::() < rate); + let res = Immediate::ScalarPair( + old.to_scalar_or_uninit(), + Scalar::from_bool(cmpxchg_success).into(), + ); // Update ptr depending on comparison. // if successful, perform a full rw-atomic validation // otherwise treat this as an atomic load with the fail ordering. - if eq.to_bool()? { + if cmpxchg_success { this.allow_data_races_mut(|this| this.write_scalar(new, place.into()))?; this.validate_atomic_rmw(place, success)?; } else { diff --git a/src/eval.rs b/src/eval.rs index 0a62f14dd3a..b6d4fa05e1e 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -50,6 +50,9 @@ pub struct MiriConfig { pub track_raw: bool, /// Determine if data race detection should be enabled pub data_race_detector: bool, + /// Rate of spurious failures for compare_exchange_weak atomic operations, + /// between 0.0 and 1.0, defaulting to 0.8 (80% chance of failure). + pub cmpxchg_weak_failure_rate: f64, } impl Default for MiriConfig { @@ -68,6 +71,7 @@ impl Default for MiriConfig { tracked_alloc_id: None, track_raw: false, data_race_detector: true, + cmpxchg_weak_failure_rate: 0.8, } } } diff --git a/src/machine.rs b/src/machine.rs index d28aa34c75e..60a6dae0f81 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -135,6 +135,9 @@ pub struct MemoryExtra { /// Controls whether alignment of memory accesses is being checked. pub(crate) check_alignment: AlignmentCheck, + + /// Failure rate of compare_exchange_weak, between 0.0 and 1.0 + pub(crate) cmpxchg_weak_failure_rate: f64, } impl MemoryExtra { @@ -162,6 +165,7 @@ impl MemoryExtra { rng: RefCell::new(rng), tracked_alloc_id: config.tracked_alloc_id, check_alignment: config.check_alignment, + cmpxchg_weak_failure_rate: config.cmpxchg_weak_failure_rate, } } diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index 840dd9f1433..73419a9f976 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -518,9 +518,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } - fn atomic_compare_exchange( + fn atomic_compare_exchange_impl( &mut self, args: &[OpTy<'tcx, Tag>], dest: PlaceTy<'tcx, Tag>, - success: AtomicRwOp, fail: AtomicReadOp + success: AtomicRwOp, fail: AtomicReadOp, can_fail_spuriously: bool ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); @@ -538,7 +538,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let old = this.atomic_compare_exchange_scalar( - place, expect_old, new, success, fail + place, expect_old, new, success, fail, can_fail_spuriously )?; // Return old value. @@ -546,14 +546,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } + fn atomic_compare_exchange( + &mut self, args: &[OpTy<'tcx, Tag>], dest: PlaceTy<'tcx, Tag>, + success: AtomicRwOp, fail: AtomicReadOp + ) -> InterpResult<'tcx> { + self.atomic_compare_exchange_impl(args, dest, success, fail, false) + } + fn atomic_compare_exchange_weak( &mut self, args: &[OpTy<'tcx, Tag>], dest: PlaceTy<'tcx, Tag>, success: AtomicRwOp, fail: AtomicReadOp ) -> InterpResult<'tcx> { - - // FIXME: the weak part of this is currently not modelled, - // it is assumed to always succeed unconditionally. - self.atomic_compare_exchange(args, dest, success, fail) + self.atomic_compare_exchange_impl(args, dest, success, fail, true) } fn float_to_int_unchecked( diff --git a/tests/run-pass/atomic.rs b/tests/run-pass/atomic.rs index b03a8c8901e..4f27c2bd54d 100644 --- a/tests/run-pass/atomic.rs +++ b/tests/run-pass/atomic.rs @@ -5,6 +5,7 @@ fn main() { atomic_isize(); atomic_u64(); atomic_fences(); + weak_sometimes_fails(); } fn atomic_bool() { @@ -24,7 +25,20 @@ fn atomic_bool() { assert_eq!(*ATOMIC.get_mut(), false); } } - +// There isn't a trait to use to make this generic, so just use a macro +macro_rules! compare_exchange_weak_loop { + ($atom:expr, $from:expr, $to:expr, $succ_order:expr, $fail_order:expr) => { + loop { + match $atom.compare_exchange_weak($from, $to, $succ_order, $fail_order) { + Ok(n) => { + assert_eq!(n, $from); + break; + } + Err(n) => assert_eq!(n, $from), + } + } + }; +} fn atomic_isize() { static ATOMIC: AtomicIsize = AtomicIsize::new(0); @@ -40,11 +54,11 @@ fn atomic_isize() { ATOMIC.compare_exchange(0, 1, SeqCst, SeqCst).ok(); ATOMIC.store(0, SeqCst); - - assert_eq!(ATOMIC.compare_exchange_weak(0, 1, Relaxed, Relaxed), Ok(0)); + compare_exchange_weak_loop!(ATOMIC, 0, 1, Relaxed, Relaxed); assert_eq!(ATOMIC.compare_exchange_weak(0, 2, Acquire, Relaxed), Err(1)); assert_eq!(ATOMIC.compare_exchange_weak(0, 1, Release, Relaxed), Err(1)); - assert_eq!(ATOMIC.compare_exchange_weak(1, 0, AcqRel, Relaxed), Ok(1)); + compare_exchange_weak_loop!(ATOMIC, 1, 0, AcqRel, Relaxed); + assert_eq!(ATOMIC.load(Relaxed), 0); ATOMIC.compare_exchange_weak(0, 1, AcqRel, Relaxed).ok(); ATOMIC.compare_exchange_weak(0, 1, SeqCst, Relaxed).ok(); ATOMIC.compare_exchange_weak(0, 1, Acquire, Acquire).ok(); @@ -58,10 +72,7 @@ fn atomic_u64() { ATOMIC.store(1, SeqCst); assert_eq!(ATOMIC.compare_exchange(0, 0x100, AcqRel, Acquire), Err(1)); - assert_eq!( - ATOMIC.compare_exchange_weak(1, 0x100, AcqRel, Acquire), - Ok(1) - ); + compare_exchange_weak_loop!(ATOMIC, 1, 0x100, AcqRel, Acquire); assert_eq!(ATOMIC.load(Relaxed), 0x100); } @@ -75,3 +86,17 @@ fn atomic_fences() { compiler_fence(Acquire); compiler_fence(AcqRel); } + +fn weak_sometimes_fails() { + let atomic = AtomicBool::new(false); + let tries = 100; + for _ in 0..tries { + let cur = atomic.load(Relaxed); + // Try (weakly) to flip the flag. + if atomic.compare_exchange_weak(cur, !cur, Relaxed, Relaxed).is_err() { + // We failed, so return and skip the panic. + return; + } + } + panic!("compare_exchange_weak succeeded {} tries in a row", tries); +}