diff --git a/src/concurrency/data_race.rs b/src/concurrency/data_race.rs index 51105ec98df..bcbdb616514 100644 --- a/src/concurrency/data_race.rs +++ b/src/concurrency/data_race.rs @@ -464,33 +464,6 @@ fn write_scalar_at_offset_atomic( this.write_scalar_atomic(value.into(), &value_place, atomic) } - /// Checks that an atomic access is legal at the given place. - fn atomic_access_check(&self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_ref(); - // Check alignment requirements. Atomics must always be aligned to their size, - // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must - // be 8-aligned). - let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.check_ptr_access_align( - place.ptr, - place.layout.size, - 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(()) - } - /// Perform an atomic read operation at the memory location. fn read_scalar_atomic( &self, @@ -682,80 +655,8 @@ fn atomic_compare_exchange_scalar( Ok(res) } - /// Update the data-race detector for an atomic read occurring at the - /// associated memory-place and on the current thread. - fn validate_atomic_load( - &self, - place: &MPlaceTy<'tcx, Provenance>, - atomic: AtomicReadOrd, - ) -> InterpResult<'tcx> { - let this = self.eval_context_ref(); - this.validate_overlapping_atomic(place)?; - this.validate_atomic_op( - place, - atomic, - "Atomic Load", - move |memory, clocks, index, atomic| { - if atomic == AtomicReadOrd::Relaxed { - memory.load_relaxed(&mut *clocks, index) - } else { - memory.load_acquire(&mut *clocks, index) - } - }, - ) - } - - /// Update the data-race detector for an atomic write occurring at the - /// associated memory-place and on the current thread. - fn validate_atomic_store( - &mut self, - place: &MPlaceTy<'tcx, Provenance>, - atomic: AtomicWriteOrd, - ) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - this.validate_overlapping_atomic(place)?; - this.validate_atomic_op( - place, - atomic, - "Atomic Store", - move |memory, clocks, index, atomic| { - if atomic == AtomicWriteOrd::Relaxed { - memory.store_relaxed(clocks, index) - } else { - memory.store_release(clocks, index) - } - }, - ) - } - - /// Update the data-race detector for an atomic read-modify-write occurring - /// at the associated memory place and on the current thread. - fn validate_atomic_rmw( - &mut self, - place: &MPlaceTy<'tcx, Provenance>, - atomic: AtomicRwOrd, - ) -> InterpResult<'tcx> { - use AtomicRwOrd::*; - let acquire = matches!(atomic, Acquire | AcqRel | SeqCst); - let release = matches!(atomic, Release | AcqRel | SeqCst); - let this = self.eval_context_mut(); - this.validate_overlapping_atomic(place)?; - this.validate_atomic_op(place, atomic, "Atomic RMW", move |memory, clocks, index, _| { - if acquire { - memory.load_acquire(clocks, index)?; - } else { - memory.load_relaxed(clocks, index)?; - } - if release { - memory.rmw_release(clocks, index) - } else { - memory.rmw_relaxed(clocks, index) - } - }) - } - /// Update the data-race detector for an atomic fence on the current thread. - fn validate_atomic_fence(&mut self, atomic: AtomicFenceOrd) -> InterpResult<'tcx> { + fn atomic_fence(&mut self, atomic: AtomicFenceOrd) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if let Some(data_race) = &mut this.machine.data_race { data_race.maybe_perform_sync_operation(&this.machine.threads, |index, mut clocks| { @@ -1081,6 +982,105 @@ fn allow_data_races_mut( result } + /// Checks that an atomic access is legal at the given place. + fn atomic_access_check(&self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.check_ptr_access_align( + place.ptr, + place.layout.size, + 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(()) + } + + /// Update the data-race detector for an atomic read occurring at the + /// associated memory-place and on the current thread. + fn validate_atomic_load( + &self, + place: &MPlaceTy<'tcx, Provenance>, + atomic: AtomicReadOrd, + ) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + this.validate_overlapping_atomic(place)?; + this.validate_atomic_op( + place, + atomic, + "Atomic Load", + move |memory, clocks, index, atomic| { + if atomic == AtomicReadOrd::Relaxed { + memory.load_relaxed(&mut *clocks, index) + } else { + memory.load_acquire(&mut *clocks, index) + } + }, + ) + } + + /// Update the data-race detector for an atomic write occurring at the + /// associated memory-place and on the current thread. + fn validate_atomic_store( + &mut self, + place: &MPlaceTy<'tcx, Provenance>, + atomic: AtomicWriteOrd, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + this.validate_overlapping_atomic(place)?; + this.validate_atomic_op( + place, + atomic, + "Atomic Store", + move |memory, clocks, index, atomic| { + if atomic == AtomicWriteOrd::Relaxed { + memory.store_relaxed(clocks, index) + } else { + memory.store_release(clocks, index) + } + }, + ) + } + + /// Update the data-race detector for an atomic read-modify-write occurring + /// at the associated memory place and on the current thread. + fn validate_atomic_rmw( + &mut self, + place: &MPlaceTy<'tcx, Provenance>, + atomic: AtomicRwOrd, + ) -> InterpResult<'tcx> { + use AtomicRwOrd::*; + let acquire = matches!(atomic, Acquire | AcqRel | SeqCst); + let release = matches!(atomic, Release | AcqRel | SeqCst); + let this = self.eval_context_mut(); + this.validate_overlapping_atomic(place)?; + this.validate_atomic_op(place, atomic, "Atomic RMW", move |memory, clocks, index, _| { + if acquire { + memory.load_acquire(clocks, index)?; + } else { + memory.load_relaxed(clocks, index)?; + } + if release { + memory.rmw_release(clocks, index) + } else { + memory.rmw_relaxed(clocks, index) + } + }) + } + /// Generic atomic operation implementation fn validate_atomic_op( &self, diff --git a/src/shims/intrinsics/atomic.rs b/src/shims/intrinsics/atomic.rs index 752bc0e302e..86f132f73fc 100644 --- a/src/shims/intrinsics/atomic.rs +++ b/src/shims/intrinsics/atomic.rs @@ -67,8 +67,8 @@ fn fence_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicFenceOrd> { ["load", ord] => this.atomic_load(args, dest, read_ord(ord)?)?, ["store", ord] => this.atomic_store(args, write_ord(ord)?)?, - ["fence", ord] => this.atomic_fence(args, fence_ord(ord)?)?, - ["singlethreadfence", ord] => this.compiler_fence(args, fence_ord(ord)?)?, + ["fence", ord] => this.atomic_fence_intrinsic(args, fence_ord(ord)?)?, + ["singlethreadfence", ord] => this.compiler_fence_intrinsic(args, fence_ord(ord)?)?, ["xchg", ord] => this.atomic_exchange(args, dest, rw_ord(ord)?)?, ["cxchg", ord1, ord2] => @@ -117,7 +117,10 @@ fn fence_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicFenceOrd> { } Ok(()) } +} +impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for MiriEvalContext<'mir, 'tcx> {} +trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { fn atomic_load( &mut self, args: &[OpTy<'tcx, Provenance>], @@ -153,7 +156,7 @@ fn atomic_store( Ok(()) } - fn compiler_fence( + fn compiler_fence_intrinsic( &mut self, args: &[OpTy<'tcx, Provenance>], atomic: AtomicFenceOrd, @@ -164,14 +167,14 @@ fn compiler_fence( Ok(()) } - fn atomic_fence( + fn atomic_fence_intrinsic( &mut self, args: &[OpTy<'tcx, Provenance>], atomic: AtomicFenceOrd, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let [] = check_arg_count(args)?; - this.validate_atomic_fence(atomic)?; + this.atomic_fence(atomic)?; Ok(()) } diff --git a/src/shims/unix/linux/sync.rs b/src/shims/unix/linux/sync.rs index 59de1049874..b33553f4663 100644 --- a/src/shims/unix/linux/sync.rs +++ b/src/shims/unix/linux/sync.rs @@ -169,7 +169,7 @@ pub fn futex<'tcx>( // // Thankfully, preemptions cannot happen inside a Miri shim, so we do not need to // do anything special to guarantee fence-load-comparison atomicity. - this.validate_atomic_fence(AtomicFenceOrd::SeqCst)?; + this.atomic_fence(AtomicFenceOrd::SeqCst)?; // Read an `i32` through the pointer, regardless of any wrapper types. // It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`. let futex_val = this @@ -240,7 +240,7 @@ pub fn futex<'tcx>( // Together with the SeqCst fence in futex_wait, this makes sure that futex_wait // will see the latest value on addr which could be changed by our caller // before doing the syscall. - this.validate_atomic_fence(AtomicFenceOrd::SeqCst)?; + this.atomic_fence(AtomicFenceOrd::SeqCst)?; let mut n = 0; for _ in 0..val { if let Some(thread) = this.futex_wake(addr_usize, bitset) {