diff --git a/src/concurrency/sync.rs b/src/concurrency/sync.rs index e42add0d50..11110c6e75 100644 --- a/src/concurrency/sync.rs +++ b/src/concurrency/sync.rs @@ -201,12 +201,11 @@ pub fn lazy_sync_init<'tcx, T: 'static + Copy>( init_offset: Size, data: T, ) -> InterpResult<'tcx> { - let init_field = primitive.offset(init_offset, ecx.machine.layouts.u32, ecx)?; - let (alloc, offset, _) = ecx.ptr_get_alloc_id(primitive.ptr(), 0)?; let (alloc_extra, _machine) = ecx.get_alloc_extra_mut(alloc)?; alloc_extra.sync.insert(offset, Box::new(data)); // Mark this as "initialized". + let init_field = primitive.offset(init_offset, ecx.machine.layouts.u32, ecx)?; ecx.write_scalar_atomic( Scalar::from_u32(LAZY_INIT_COOKIE), &init_field, @@ -217,18 +216,19 @@ pub fn lazy_sync_init<'tcx, T: 'static + Copy>( /// Helper for lazily initialized `alloc_extra.sync` data: /// Checks if the primitive is initialized, and return its associated data if so. -/// Otherwise, return None. +/// Otherwise, calls `new_data` to initialize the primitive. pub fn lazy_sync_get_data<'tcx, T: 'static + Copy>( ecx: &mut MiriInterpCx<'tcx>, primitive: &MPlaceTy<'tcx>, init_offset: Size, name: &str, -) -> InterpResult<'tcx, Option> { - let init_field = primitive.offset(init_offset, ecx.machine.layouts.u32, ecx)?; + new_data: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>, +) -> InterpResult<'tcx, T> { // Check if this is already initialized. Needs to be atomic because we can race with another // thread initializing. Needs to be an RMW operation to ensure we read the *latest* value. // So we just try to replace MUTEX_INIT_COOKIE with itself. let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); + let init_field = primitive.offset(init_offset, ecx.machine.layouts.u32, ecx)?; let (_init, success) = ecx .atomic_compare_exchange_scalar( &init_field, @@ -239,6 +239,7 @@ pub fn lazy_sync_get_data<'tcx, T: 'static + Copy>( /* can_fail_spuriously */ false, )? .to_scalar_pair(); + if success.to_bool()? { // If it is initialized, it must be found in the "sync primitive" table, // or else it has been moved illegally. @@ -247,9 +248,11 @@ pub fn lazy_sync_get_data<'tcx, T: 'static + Copy>( let data = alloc_extra .get_sync::(offset) .ok_or_else(|| err_ub_format!("`{name}` can't be moved after first use"))?; - interp_ok(Some(*data)) + interp_ok(*data) } else { - interp_ok(None) + let data = new_data(ecx)?; + lazy_sync_init(ecx, primitive, init_offset, data)?; + interp_ok(data) } } diff --git a/src/shims/unix/sync.rs b/src/shims/unix/sync.rs index efbe09a2a5..5b4c136c23 100644 --- a/src/shims/unix/sync.rs +++ b/src/shims/unix/sync.rs @@ -190,19 +190,11 @@ fn mutex_get_data<'tcx, 'a>( mutex_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, MutexData> { let mutex = ecx.deref_pointer(mutex_ptr)?; - - if let Some(data) = - lazy_sync_get_data::(ecx, &mutex, mutex_init_offset(ecx)?, "pthread_mutex_t")? - { - interp_ok(data) - } else { - // Not yet initialized. This must be a static initializer, figure out the kind - // from that. We don't need to worry about races since we are the interpreter - // and don't let any other tread take a step. + lazy_sync_get_data(ecx, &mutex, mutex_init_offset(ecx)?, "pthread_mutex_t", |ecx| { let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; - // And then create the mutex like this. - mutex_create(ecx, mutex_ptr, kind) - } + let id = ecx.machine.sync.mutex_create(); + interp_ok(MutexData { id, kind }) + }) } /// Returns the kind of a static initializer. @@ -271,13 +263,7 @@ fn rwlock_get_data<'tcx>( rwlock_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, RwLockData> { let rwlock = ecx.deref_pointer(rwlock_ptr)?; - let init_offset = rwlock_init_offset(ecx)?; - - if let Some(data) = - lazy_sync_get_data::(ecx, &rwlock, init_offset, "pthread_rwlock_t")? - { - interp_ok(data) - } else { + lazy_sync_get_data(ecx, &rwlock, rwlock_init_offset(ecx)?, "pthread_rwlock_t", |ecx| { if !bytewise_equal_atomic_relaxed( ecx, &rwlock, @@ -286,10 +272,8 @@ fn rwlock_get_data<'tcx>( throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`"); } let id = ecx.machine.sync.rwlock_create(); - let data = RwLockData { id }; - lazy_sync_init(ecx, &rwlock, init_offset, data)?; - interp_ok(data) - } + interp_ok(RwLockData { id }) + }) } // # pthread_condattr_t @@ -405,12 +389,7 @@ fn cond_get_data<'tcx>( cond_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, CondData> { let cond = ecx.deref_pointer(cond_ptr)?; - let init_offset = cond_init_offset(ecx)?; - - if let Some(data) = lazy_sync_get_data::(ecx, &cond, init_offset, "pthread_cond_t")? { - interp_ok(data) - } else { - // This used the static initializer. The clock there is always CLOCK_REALTIME. + lazy_sync_get_data(ecx, &cond, cond_init_offset(ecx)?, "pthread_cond_t", |ecx| { if !bytewise_equal_atomic_relaxed( ecx, &cond, @@ -418,8 +397,10 @@ fn cond_get_data<'tcx>( )? { throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`"); } - cond_create(ecx, cond_ptr, ClockId::Realtime) - } + // This used the static initializer. The clock there is always CLOCK_REALTIME. + let id = ecx.machine.sync.condvar_create(); + interp_ok(CondData { id, clock: ClockId::Realtime }) + }) } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} diff --git a/src/shims/windows/sync.rs b/src/shims/windows/sync.rs index de780361b5..d0e82112a5 100644 --- a/src/shims/windows/sync.rs +++ b/src/shims/windows/sync.rs @@ -3,7 +3,7 @@ use std::time::Duration; use rustc_target::abi::Size; use crate::concurrency::init_once::InitOnceStatus; -use crate::concurrency::sync::{lazy_sync_get_data, lazy_sync_init}; +use crate::concurrency::sync::lazy_sync_get_data; use crate::*; #[derive(Copy, Clone)] @@ -16,23 +16,20 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Windows sync primitives are pointer sized. // We only use the first 4 bytes for the id. - fn init_once_get_id(&mut self, init_once_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, InitOnceId> { + fn init_once_get_data( + &mut self, + init_once_ptr: &OpTy<'tcx>, + ) -> InterpResult<'tcx, InitOnceData> { let this = self.eval_context_mut(); let init_once = this.deref_pointer(init_once_ptr)?; let init_offset = Size::ZERO; - if let Some(data) = - lazy_sync_get_data::(this, &init_once, init_offset, "INIT_ONCE")? - { - interp_ok(data.id) - } else { + lazy_sync_get_data(this, &init_once, init_offset, "INIT_ONCE", |this| { // TODO: check that this is still all-zero. let id = this.machine.sync.init_once_create(); - let data = InitOnceData { id }; - lazy_sync_init(this, &init_once, init_offset, data)?; - interp_ok(id) - } + interp_ok(InitOnceData { id }) + }) } /// Returns `true` if we were succssful, `false` if we would block. @@ -74,7 +71,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let id = this.init_once_get_id(init_once_op)?; + let id = this.init_once_get_data(init_once_op)?.id; let flags = this.read_scalar(flags_op)?.to_u32()?; let pending_place = this.deref_pointer(pending_op)?; let context = this.read_pointer(context_op)?; @@ -120,7 +117,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let id = this.init_once_get_id(init_once_op)?; + let id = this.init_once_get_data(init_once_op)?.id; let flags = this.read_scalar(flags_op)?.to_u32()?; let context = this.read_pointer(context_op)?;