make lazy_sync_get_data also take a closure to initialize if needed

Author: RalfJung

src/tools/miri/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.
 pubfnlazy_sync_get_data<'tcx,T:'static + Copy>(
 ecx:&mutMiriInterpCx<'tcx>,
 primitive:&MPlaceTy<'tcx>,
 init_offset:Size,
 name:&str,
-) -> InterpResult<'tcx,Option<T>>{
-let init_field = primitive.offset(init_offset, ecx.machine.layouts.u32, ecx)?;
+new_data:implFnOnce(&mutMiriInterpCx<'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::<T>(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)
 }
 }
 

src/tools/miri/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)?;
-
-ifletSome(data) =
-lazy_sync_get_data::<MutexData>(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)?;
-
-ifletSome(data) =
-lazy_sync_get_data::<RwLockData>(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,21 +389,18 @@ 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)?;
-
-ifletSome(data) = lazy_sync_get_data::<CondData>(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,
 &ecx.eval_path(&["libc","PTHREAD_COND_INITIALIZER"]),
 )? {
 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>forcrate::MiriInterpCx<'tcx>{}

src/tools/miri/src/shims/windows/sync.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 use rustc_target::abi::Size;
 
 usecrate::concurrency::init_once::InitOnceStatus;
-usecrate::concurrency::sync::{lazy_sync_get_data, lazy_sync_init};
+usecrate::concurrency::sync::lazy_sync_get_data;
 usecrate::*;
 
 #[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.
 
-fninit_once_get_id(&mutself,init_once_ptr:&OpTy<'tcx>) -> InterpResult<'tcx,InitOnceId>{
+fninit_once_get_data(
+&mutself,
+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;
 
-ifletSome(data) =
-lazy_sync_get_data::<InitOnceData>(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)?;