diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 3535eacb447..b038ac33df8 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -50,15 +50,6 @@ fn mutexattr_set_kind<'tcx>( /// in `pthread_mutexattr_settype` function. const PTHREAD_MUTEX_NORMAL_FLAG: i32 = 0x8000000; -fn is_mutex_kind_default<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, bool> { - Ok(kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT")) -} - -fn is_mutex_kind_normal<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, bool> { - let mutex_normal_kind = ecx.eval_libc_i32("PTHREAD_MUTEX_NORMAL"); - Ok(kind == (mutex_normal_kind | PTHREAD_MUTEX_NORMAL_FLAG)) -} - /// The mutex kind. #[derive(Debug, Clone, Copy)] pub enum MutexKind { @@ -78,7 +69,7 @@ pub struct AdditionalMutexData { pub address: u64, } -// pthread_mutex_t is between 24 and 48 bytes, depending on the platform. +// pthread_mutex_t is between 4 and 48 bytes, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 @@ -131,7 +122,7 @@ fn mutex_create<'tcx>( ) -> InterpResult<'tcx> { let mutex = ecx.deref_pointer(mutex_ptr)?; let address = mutex.ptr().addr().bytes(); - let kind = translate_kind(ecx, kind)?; + let kind = mutex_translate_kind(ecx, kind)?; let data = Box::new(AdditionalMutexData { address, kind }); ecx.mutex_create(&mutex, mutex_id_offset(ecx)?, Some(data))?; Ok(()) @@ -151,7 +142,7 @@ fn mutex_get_id<'tcx>( let id = ecx.mutex_get_or_create_id(&mutex, mutex_id_offset(ecx)?, |ecx| { // This is called if a static initializer was used and the lock has not been assigned // an ID yet. We have to determine the mutex kind from the static initializer. - let kind = kind_from_static_initializer(ecx, &mutex)?; + let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; Ok(Some(Box::new(AdditionalMutexData { kind, address }))) })?; @@ -168,12 +159,12 @@ fn mutex_get_id<'tcx>( } /// Returns the kind of a static initializer. -fn kind_from_static_initializer<'tcx>( +fn mutex_kind_from_static_initializer<'tcx>( ecx: &MiriInterpCx<'tcx>, mutex: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, MutexKind> { - // Only linux has static initializers other than PTHREAD_MUTEX_DEFAULT. let kind = match &*ecx.tcx.sess.target.os { + // Only linux has static initializers other than PTHREAD_MUTEX_DEFAULT. "linux" => { let offset = if ecx.pointer_size().bytes() == 8 { 16 } else { 12 }; let kind_place = @@ -184,13 +175,16 @@ fn kind_from_static_initializer<'tcx>( os => throw_unsup_format!("`pthread_mutex` is not supported on {os}"), }; - translate_kind(ecx, kind) + mutex_translate_kind(ecx, kind) } -fn translate_kind<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, MutexKind> { - Ok(if is_mutex_kind_default(ecx, kind)? { +fn mutex_translate_kind<'tcx>( + ecx: &MiriInterpCx<'tcx>, + kind: i32, +) -> InterpResult<'tcx, MutexKind> { + Ok(if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") { MutexKind::Default - } else if is_mutex_kind_normal(ecx, kind)? { + } else if kind == (ecx.eval_libc_i32("PTHREAD_MUTEX_NORMAL") | PTHREAD_MUTEX_NORMAL_FLAG) { MutexKind::Normal } else if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_ERRORCHECK") { MutexKind::ErrorCheck @@ -201,7 +195,7 @@ fn translate_kind<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tc }) } -// pthread_rwlock_t is between 32 and 56 bytes, depending on the platform. +// pthread_rwlock_t is between 4 and 56 bytes, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 @@ -286,11 +280,11 @@ fn condattr_get_clock_id<'tcx>( .to_i32() } -fn translate_clock_id<'tcx>(ecx: &MiriInterpCx<'tcx>, raw_id: i32) -> InterpResult<'tcx, ClockId> { - // To ensure compatibility with PTHREAD_COND_INITIALIZER on all platforms, - // we can't just compare with CLOCK_REALTIME: on Solarish, PTHREAD_COND_INITIALIZER - // makes the clock 0 but CLOCK_REALTIME is 3. - Ok(if raw_id == ecx.eval_libc_i32("CLOCK_REALTIME") || raw_id == 0 { +fn cond_translate_clock_id<'tcx>( + ecx: &MiriInterpCx<'tcx>, + raw_id: i32, +) -> InterpResult<'tcx, ClockId> { + Ok(if raw_id == ecx.eval_libc_i32("CLOCK_REALTIME") { ClockId::Realtime } else if raw_id == ecx.eval_libc_i32("CLOCK_MONOTONIC") { ClockId::Monotonic @@ -313,10 +307,9 @@ fn condattr_set_clock_id<'tcx>( ) } -// pthread_cond_t. +// pthread_cond_t can be only 4 bytes in size, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 -// - clock: i32 fn cond_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { let offset = match &*ecx.tcx.sess.target.os { @@ -344,30 +337,6 @@ fn cond_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { Ok(offset) } -fn cond_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> u64 { - // macOS doesn't have a clock attribute, but to keep the code uniform we store - // a clock ID in the pthread_cond_t anyway. There's enough space. - let offset = 8; - - // Sanity-check this against PTHREAD_COND_INITIALIZER (but only once): - // the clock must start out as CLOCK_REALTIME. - static SANITY: AtomicBool = AtomicBool::new(false); - if !SANITY.swap(true, Ordering::Relaxed) { - let static_initializer = ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]); - let id_field = static_initializer - .offset(Size::from_bytes(offset), ecx.machine.layouts.i32, ecx) - .unwrap(); - let id = ecx.read_scalar(&id_field).unwrap().to_i32().unwrap(); - let id = translate_clock_id(ecx, id).expect("static initializer should be valid"); - assert!( - matches!(id, ClockId::Realtime), - "PTHREAD_COND_INITIALIZER is incompatible with our pthread_cond layout: clock is not CLOCK_REALTIME" - ); - } - - offset -} - #[derive(Debug, Clone, Copy)] enum ClockId { Realtime, @@ -390,14 +359,9 @@ fn cond_get_id<'tcx>( ) -> InterpResult<'tcx, CondvarId> { let cond = ecx.deref_pointer(cond_ptr)?; let address = cond.ptr().addr().bytes(); - let id = ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?, |ecx| { - let raw_id = if ecx.tcx.sess.target.os == "macos" { - ecx.eval_libc_i32("CLOCK_REALTIME") - } else { - cond_get_clock_id(ecx, cond_ptr)? - }; - let clock_id = translate_clock_id(ecx, raw_id)?; - Ok(Some(Box::new(AdditionalCondData { address, clock_id }))) + let id = ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?, |_ecx| { + // This used the static initializer. The clock there is always CLOCK_REALTIME. + Ok(Some(Box::new(AdditionalCondData { address, clock_id: ClockId::Realtime }))) })?; // Check that the mutex has not been moved since last use. @@ -411,19 +375,6 @@ fn cond_get_id<'tcx>( Ok(id) } -fn cond_get_clock_id<'tcx>( - ecx: &MiriInterpCx<'tcx>, - cond_ptr: &OpTy<'tcx>, -) -> InterpResult<'tcx, i32> { - ecx.deref_pointer_and_read( - cond_ptr, - cond_clock_offset(ecx), - ecx.libc_ty_layout("pthread_cond_t"), - ecx.machine.layouts.i32, - )? - .to_i32() -} - impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_mutexattr_init(&mut self, attr_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { @@ -624,15 +575,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_mutex_destroy(&mut self, mutex_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = mutex_get_id(this, mutex_op)?; if this.mutex_is_locked(id) { throw_ub_format!("destroyed a locked mutex"); } - // Destroying an uninit pthread_mutex is UB, so check to make sure it's not uninit. - mutex_get_id(this, mutex_op)?; - // This might lead to false positives, see comment in pthread_mutexattr_destroy this.write_uninit( &this.deref_pointer_as(mutex_op, this.libc_ty_layout("pthread_mutex_t"))?, @@ -734,15 +684,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_rwlock_destroy(&mut self, rwlock_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = rwlock_get_id(this, rwlock_op)?; if this.rwlock_is_locked(id) { throw_ub_format!("destroyed a locked rwlock"); } - // Destroying an uninit pthread_rwlock is UB, so check to make sure it's not uninit. - rwlock_get_id(this, rwlock_op)?; - // This might lead to false positives, see comment in pthread_mutexattr_destroy this.write_uninit( &this.deref_pointer_as(rwlock_op, this.libc_ty_layout("pthread_rwlock_t"))?, @@ -832,7 +781,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } else { condattr_get_clock_id(this, attr_op)? }; - let clock_id = translate_clock_id(this, clock_id)?; + let clock_id = cond_translate_clock_id(this, clock_id)?; let cond = this.deref_pointer(cond_op)?; let address = cond.ptr().addr().bytes(); @@ -930,11 +879,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } fn pthread_cond_destroy(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { - //NOTE: Destroying an uninit pthread_cond is UB. Make sure it's not uninit, - // by accessing at least once all of its fields that we use. - let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = cond_get_id(this, cond_op)?; if this.condvar_is_awaited(id) { throw_ub_format!("destroying an awaited conditional variable");