diff options
| -rw-r--r-- | src/tools/miri/src/alloc_addresses/mod.rs | 2 | ||||
| -rw-r--r-- | src/tools/miri/src/concurrency/data_race.rs | 22 | ||||
| -rw-r--r-- | src/tools/miri/src/concurrency/init_once.rs | 10 | ||||
| -rw-r--r-- | src/tools/miri/src/concurrency/sync.rs | 68 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/linux_like/epoll.rs | 2 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/linux_like/eventfd.rs | 2 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/macos/sync.rs | 4 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/sync.rs | 16 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/unnamed_socket.rs | 2 |
9 files changed, 52 insertions, 76 deletions
diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index 5d94a1f4138..f011ee71785 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -498,6 +498,8 @@ impl<'tcx> MiriMachine<'tcx> { // Also remember this address for future reuse. let thread = self.threads.active_thread(); global_state.reuse.add_addr(rng, addr, size, align, kind, thread, || { + // We already excluded GenMC above. We cannot use `self.release_clock` as + // `self.alloc_addresses` is borrowed. if let Some(data_race) = self.data_race.as_vclocks_ref() { data_race.release_clock(&self.threads, |clock| clock.clone()) } else { diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 30061a78594..de60e80b9f7 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -958,16 +958,20 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { /// with this program point. /// /// The closure will only be invoked if data race handling is on. - fn release_clock<R>(&self, callback: impl FnOnce(&VClock) -> R) -> Option<R> { + fn release_clock<R>( + &self, + callback: impl FnOnce(&VClock) -> R, + ) -> InterpResult<'tcx, Option<R>> { let this = self.eval_context_ref(); - // FIXME: make this a proper error instead of ICEing the interpreter. - assert!( - this.machine.data_race.as_genmc_ref().is_none(), - "this operation performs synchronization that is not supported in GenMC mode" - ); - Some( - this.machine.data_race.as_vclocks_ref()?.release_clock(&this.machine.threads, callback), - ) + interp_ok(match &this.machine.data_race { + GlobalDataRaceHandler::None => None, + GlobalDataRaceHandler::Genmc(_genmc_ctx) => + throw_unsup_format!( + "this operation performs synchronization that is not supported in GenMC mode" + ), + GlobalDataRaceHandler::Vclocks(data_race) => + Some(data_race.release_clock(&this.machine.threads, callback)), + }) } /// Acquire the given clock into the current thread, establishing synchronization with diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index 3fb3d890419..daea20b3779 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -96,10 +96,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { init_once.status = InitOnceStatus::Complete; // Each complete happens-before the end of the wait - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race - .release_clock(&this.machine.threads, |clock| init_once.clock.clone_from(clock)); - } + this.release_clock(|clock| init_once.clock.clone_from(clock))?; // Wake up everyone. // need to take the queue to avoid having `this` be borrowed multiple times @@ -125,10 +122,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { init_once.status = InitOnceStatus::Uninitialized; // Each complete happens-before the end of the wait - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race - .release_clock(&this.machine.threads, |clock| init_once.clock.clone_from(clock)); - } + this.release_clock(|clock| init_once.clock.clone_from(clock))?; // Wake up one waiting thread, so they can go ahead and try to init this. if let Some(waiter) = init_once.waiters.pop_front() { diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 179094db0fe..15d486c27e3 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -204,7 +204,7 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { this.mutex_enqueue_and_block(mutex_ref, Some((retval, dest))); } else { // We can have it right now! - this.mutex_lock(&mutex_ref); + this.mutex_lock(&mutex_ref)?; // Don't forget to write the return value. this.write_scalar(retval, &dest)?; } @@ -338,7 +338,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Lock by setting the mutex owner and increasing the lock count. - fn mutex_lock(&mut self, mutex_ref: &MutexRef) { + fn mutex_lock(&mut self, mutex_ref: &MutexRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let thread = this.active_thread(); let mut mutex = mutex_ref.0.borrow_mut(); @@ -352,9 +352,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { mutex.owner = Some(thread); } mutex.lock_count = mutex.lock_count.strict_add(1); - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.acquire_clock(&mutex.clock, &this.machine.threads); - } + this.acquire_clock(&mutex.clock)?; + interp_ok(()) } /// Try unlocking by decreasing the lock count and returning the old lock @@ -377,11 +376,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // The mutex is completely unlocked. Try transferring ownership // to another thread. - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.release_clock(&this.machine.threads, |clock| { - mutex.clock.clone_from(clock) - }); - } + this.release_clock(|clock| mutex.clock.clone_from(clock))?; let thread_id = mutex.queue.pop_front(); // We need to drop our mutex borrow before unblock_thread // because it will be borrowed again in the unblock callback. @@ -425,7 +420,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(unblock, UnblockKind::Ready); assert!(mutex_ref.owner().is_none()); - this.mutex_lock(&mutex_ref); + this.mutex_lock(&mutex_ref)?; if let Some((retval, dest)) = retval_dest { this.write_scalar(retval, &dest)?; @@ -439,7 +434,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Read-lock the lock by adding the `reader` the list of threads that own /// this lock. - fn rwlock_reader_lock(&mut self, rwlock_ref: &RwLockRef) { + fn rwlock_reader_lock(&mut self, rwlock_ref: &RwLockRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let thread = this.active_thread(); trace!("rwlock_reader_lock: now also held (one more time) by {:?}", thread); @@ -447,9 +442,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert!(!rwlock.is_write_locked(), "the lock is write locked"); let count = rwlock.readers.entry(thread).or_insert(0); *count = count.strict_add(1); - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.acquire_clock(&rwlock.clock_unlocked, &this.machine.threads); - } + this.acquire_clock(&rwlock.clock_unlocked)?; + interp_ok(()) } /// Try read-unlock the lock for the current threads and potentially give the lock to a new owner. @@ -472,12 +466,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } Entry::Vacant(_) => return interp_ok(false), // we did not even own this lock } - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - // Add this to the shared-release clock of all concurrent readers. - data_race.release_clock(&this.machine.threads, |clock| { - rwlock.clock_current_readers.join(clock) - }); - } + // Add this to the shared-release clock of all concurrent readers. + this.release_clock(|clock| rwlock.clock_current_readers.join(clock))?; // The thread was a reader. If the lock is not held any more, give it to a writer. if rwlock.is_locked().not() { @@ -521,7 +511,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } |this, unblock: UnblockKind| { assert_eq!(unblock, UnblockKind::Ready); - this.rwlock_reader_lock(&rwlock_ref); + this.rwlock_reader_lock(&rwlock_ref)?; this.write_scalar(retval, &dest)?; interp_ok(()) } @@ -531,7 +521,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Lock by setting the writer that owns the lock. #[inline] - fn rwlock_writer_lock(&mut self, rwlock_ref: &RwLockRef) { + fn rwlock_writer_lock(&mut self, rwlock_ref: &RwLockRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let thread = this.active_thread(); trace!("rwlock_writer_lock: now held by {:?}", thread); @@ -539,9 +529,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut rwlock = rwlock_ref.0.borrow_mut(); assert!(!rwlock.is_locked(), "the rwlock is already locked"); rwlock.writer = Some(thread); - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.acquire_clock(&rwlock.clock_unlocked, &this.machine.threads); - } + this.acquire_clock(&rwlock.clock_unlocked)?; + interp_ok(()) } /// Try to unlock an rwlock held by the current thread. @@ -559,11 +548,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { rwlock.writer = None; trace!("rwlock_writer_unlock: unlocked by {:?}", thread); // Record release clock for next lock holder. - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.release_clock(&this.machine.threads, |clock| { - rwlock.clock_unlocked.clone_from(clock) - }); - } + this.release_clock(|clock| rwlock.clock_unlocked.clone_from(clock))?; // The thread was a writer. // @@ -613,7 +598,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } |this, unblock: UnblockKind| { assert_eq!(unblock, UnblockKind::Ready); - this.rwlock_writer_lock(&rwlock_ref); + this.rwlock_writer_lock(&rwlock_ref)?; this.write_scalar(retval, &dest)?; interp_ok(()) } @@ -663,12 +648,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { match unblock { UnblockKind::Ready => { // The condvar was signaled. Make sure we get the clock for that. - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.acquire_clock( + this.acquire_clock( &condvar_ref.0.borrow().clock, - &this.machine.threads, - ); - } + )?; // Try to acquire the mutex. // The timeout only applies to the first wait (until the signal), not for mutex acquisition. this.condvar_reacquire_mutex(mutex_ref, retval_succ, dest) @@ -695,9 +677,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut condvar = condvar_ref.0.borrow_mut(); // Each condvar signal happens-before the end of the condvar wake - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.release_clock(&this.machine.threads, |clock| condvar.clock.clone_from(clock)); - } + this.release_clock(|clock| condvar.clock.clone_from(clock))?; let Some(waiter) = condvar.waiters.pop_front() else { return interp_ok(false); }; @@ -736,9 +716,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { UnblockKind::Ready => { let futex = futex_ref.0.borrow(); // Acquire the clock of the futex. - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.acquire_clock(&futex.clock, &this.machine.threads); - } + this.acquire_clock(&futex.clock)?; }, UnblockKind::TimedOut => { // Remove the waiter from the futex. @@ -766,9 +744,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut futex = futex_ref.0.borrow_mut(); // Each futex-wake happens-before the end of the futex wait - if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { - data_race.release_clock(&this.machine.threads, |clock| futex.clock.clone_from(clock)); - } + this.release_clock(|clock| futex.clock.clone_from(clock))?; // Remove `count` of the threads in the queue that match any of the bits in the bitset. // We collect all of them before unblocking because the unblock callback may access the diff --git a/src/tools/miri/src/shims/unix/linux_like/epoll.rs b/src/tools/miri/src/shims/unix/linux_like/epoll.rs index aac880feda4..3133c149293 100644 --- a/src/tools/miri/src/shims/unix/linux_like/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux_like/epoll.rs @@ -608,7 +608,7 @@ fn check_and_update_one_event_interest<'tcx>( // If we are tracking data races, remember the current clock so we can sync with it later. ecx.release_clock(|clock| { event_instance.clock.clone_from(clock); - }); + })?; // Triggers the notification by inserting it to the ready list. ready_list.insert(epoll_key, event_instance); interp_ok(true) diff --git a/src/tools/miri/src/shims/unix/linux_like/eventfd.rs b/src/tools/miri/src/shims/unix/linux_like/eventfd.rs index 54cc571884d..5d4f207d365 100644 --- a/src/tools/miri/src/shims/unix/linux_like/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux_like/eventfd.rs @@ -199,7 +199,7 @@ fn eventfd_write<'tcx>( // Future `read` calls will synchronize with this write, so update the FD clock. ecx.release_clock(|clock| { eventfd.clock.borrow_mut().join(clock); - }); + })?; // Store new counter value. eventfd.counter.set(new_count); diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 05616dd5a42..c4ddff7805e 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -296,7 +296,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.mutex_enqueue_and_block(mutex_ref, None); } else { - this.mutex_lock(&mutex_ref); + this.mutex_lock(&mutex_ref)?; } interp_ok(()) @@ -321,7 +321,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // reentrancy. this.write_scalar(Scalar::from_bool(false), dest)?; } else { - this.mutex_lock(&mutex_ref); + this.mutex_lock(&mutex_ref)?; this.write_scalar(Scalar::from_bool(true), dest)?; } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 5ad4fd501a6..cefd8b81ac1 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -498,14 +498,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { MutexKind::Normal => throw_machine_stop!(TerminationInfo::Deadlock), MutexKind::ErrorCheck => this.eval_libc_i32("EDEADLK"), MutexKind::Recursive => { - this.mutex_lock(&mutex.mutex_ref); + this.mutex_lock(&mutex.mutex_ref)?; 0 } } } } else { // The mutex is unlocked. Let's lock it. - this.mutex_lock(&mutex.mutex_ref); + this.mutex_lock(&mutex.mutex_ref)?; 0 }; this.write_scalar(Scalar::from_i32(ret), dest)?; @@ -525,14 +525,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { MutexKind::Default | MutexKind::Normal | MutexKind::ErrorCheck => this.eval_libc_i32("EBUSY"), MutexKind::Recursive => { - this.mutex_lock(&mutex.mutex_ref); + this.mutex_lock(&mutex.mutex_ref)?; 0 } } } } else { // The mutex is unlocked. Let's lock it. - this.mutex_lock(&mutex.mutex_ref); + this.mutex_lock(&mutex.mutex_ref)?; 0 })) } @@ -600,7 +600,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { dest.clone(), ); } else { - this.rwlock_reader_lock(&rwlock.rwlock_ref); + this.rwlock_reader_lock(&rwlock.rwlock_ref)?; this.write_null(dest)?; } @@ -615,7 +615,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if rwlock.rwlock_ref.is_write_locked() { interp_ok(Scalar::from_i32(this.eval_libc_i32("EBUSY"))) } else { - this.rwlock_reader_lock(&rwlock.rwlock_ref); + this.rwlock_reader_lock(&rwlock.rwlock_ref)?; interp_ok(Scalar::from_i32(0)) } } @@ -648,7 +648,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { dest.clone(), ); } else { - this.rwlock_writer_lock(&rwlock.rwlock_ref); + this.rwlock_writer_lock(&rwlock.rwlock_ref)?; this.write_null(dest)?; } @@ -663,7 +663,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if rwlock.rwlock_ref.is_locked() { interp_ok(Scalar::from_i32(this.eval_libc_i32("EBUSY"))) } else { - this.rwlock_writer_lock(&rwlock.rwlock_ref); + this.rwlock_writer_lock(&rwlock.rwlock_ref)?; interp_ok(Scalar::from_i32(0)) } } diff --git a/src/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs index 0585f3ba747..81703d6e176 100644 --- a/src/tools/miri/src/shims/unix/unnamed_socket.rs +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -259,7 +259,7 @@ fn anonsocket_write<'tcx>( // Remember this clock so `read` can synchronize with us. ecx.release_clock(|clock| { writebuf.clock.join(clock); - }); + })?; // Do full write / partial write based on the space available. let write_size = len.min(available_space); let actual_write_size = ecx.write_to_host(&mut writebuf.buf, write_size, ptr)?.unwrap(); |