Skip to content

Commit a1d94d4

Browse files
committed
impl condvars for windows
1 parent 4492c02 commit a1d94d4

File tree

7 files changed

+185
-30
lines changed

7 files changed

+185
-30
lines changed

src/tools/miri/src/concurrency/sync.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,10 @@ declare_id!(CondvarId);
121121
struct CondvarWaiter {
122122
/// The thread that is waiting on this variable.
123123
thread: ThreadId,
124-
/// The mutex on which the thread is waiting.
125-
mutex: MutexId,
124+
/// The mutex or rwlock on which the thread is waiting.
125+
lock: u32,
126+
/// If the lock is shared or exclusive
127+
shared: bool,
126128
}
127129

128130
/// The conditional variable state.
@@ -569,16 +571,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
569571
}
570572

571573
/// Mark that the thread is waiting on the conditional variable.
572-
fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, mutex: MutexId) {
574+
fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, lock: u32, shared: bool) {
573575
let this = self.eval_context_mut();
574576
let waiters = &mut this.machine.threads.sync.condvars[id].waiters;
575577
assert!(waiters.iter().all(|waiter| waiter.thread != thread), "thread is already waiting");
576-
waiters.push_back(CondvarWaiter { thread, mutex });
578+
waiters.push_back(CondvarWaiter { thread, lock, shared });
577579
}
578580

579581
/// Wake up some thread (if there is any) sleeping on the conditional
580582
/// variable.
581-
fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, MutexId)> {
583+
fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, u32, bool)> {
582584
let this = self.eval_context_mut();
583585
let current_thread = this.get_active_thread();
584586
let condvar = &mut this.machine.threads.sync.condvars[id];
@@ -592,7 +594,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
592594
if let Some(data_race) = data_race {
593595
data_race.validate_lock_acquire(&condvar.data_race, waiter.thread);
594596
}
595-
(waiter.thread, waiter.mutex)
597+
(waiter.thread, waiter.lock, waiter.shared)
596598
})
597599
}
598600

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -696,8 +696,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
696696
fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> {
697697
let this = self.eval_context_mut();
698698
let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?;
699-
if let Some((thread, mutex)) = this.condvar_signal(id) {
700-
post_cond_signal(this, thread, mutex)?;
699+
if let Some((thread, mutex, shared)) = this.condvar_signal(id) {
700+
assert!(!shared);
701+
post_cond_signal(this, thread, MutexId::from_u32(mutex))?;
701702
}
702703

703704
Ok(0)
@@ -710,8 +711,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
710711
let this = self.eval_context_mut();
711712
let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?;
712713

713-
while let Some((thread, mutex)) = this.condvar_signal(id) {
714-
post_cond_signal(this, thread, mutex)?;
714+
while let Some((thread, mutex, shared)) = this.condvar_signal(id) {
715+
assert!(!shared);
716+
post_cond_signal(this, thread, MutexId::from_u32(mutex))?;
715717
}
716718

717719
Ok(0)
@@ -729,7 +731,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
729731
let active_thread = this.get_active_thread();
730732

731733
release_cond_mutex_and_block(this, active_thread, mutex_id)?;
732-
this.condvar_wait(id, active_thread, mutex_id);
734+
this.condvar_wait(id, active_thread, mutex_id.to_u32(), false);
733735

734736
Ok(0)
735737
}
@@ -768,7 +770,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
768770
};
769771

770772
release_cond_mutex_and_block(this, active_thread, mutex_id)?;
771-
this.condvar_wait(id, active_thread, mutex_id);
773+
this.condvar_wait(id, active_thread, mutex_id.to_u32(), false);
772774

773775
// We return success for now and override it in the timeout callback.
774776
this.write_scalar(Scalar::from_i32(0), dest)?;

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
273273
let result = this.InitOnceComplete(ptr, flags, context)?;
274274
this.write_scalar(result, dest)?;
275275
}
276+
"SleepConditionVariableSRW" => {
277+
let [condvar, lock, timeout, flags] =
278+
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
279+
280+
let result = this.SleepConditionVariableSRW(condvar, lock, timeout, flags, dest)?;
281+
this.write_scalar(result, dest)?;
282+
}
283+
"WakeConditionVariable" => {
284+
let [condvar] =
285+
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
286+
287+
this.WakeConditionVariable(condvar)?;
288+
}
289+
"WakeAllConditionVariable" => {
290+
let [condvar] =
291+
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
292+
293+
this.WakeAllConditionVariable(condvar)?;
294+
}
276295

277296
// Dynamic symbol loading
278297
"GetProcAddress" => {

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

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,38 @@ use crate::*;
88

99
const SRWLOCK_ID_OFFSET: u64 = 0;
1010
const INIT_ONCE_ID_OFFSET: u64 = 0;
11+
const CONDVAR_ID_OFFSET: u64 = 0;
12+
13+
impl<'mir, 'tcx> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
14+
pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
15+
/// Try to reacquire the lock associated with the condition variable after we
16+
/// were signaled.
17+
fn reacquire_cond_lock(
18+
&mut self,
19+
thread: ThreadId,
20+
lock: RwLockId,
21+
shared: bool,
22+
) -> InterpResult<'tcx> {
23+
let this = self.eval_context_mut();
24+
this.unblock_thread(thread);
25+
26+
if shared {
27+
if this.rwlock_is_locked(lock) {
28+
this.rwlock_enqueue_and_block_reader(lock, thread);
29+
} else {
30+
this.rwlock_reader_lock(lock, thread);
31+
}
32+
} else {
33+
if this.rwlock_is_write_locked(lock) {
34+
this.rwlock_enqueue_and_block_writer(lock, thread);
35+
} else {
36+
this.rwlock_writer_lock(lock, thread);
37+
}
38+
}
39+
40+
Ok(())
41+
}
42+
}
1143

1244
impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
1345
#[allow(non_snake_case)]
@@ -327,4 +359,118 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
327359

328360
Ok(())
329361
}
362+
363+
fn SleepConditionVariableSRW(
364+
&mut self,
365+
condvar_op: &OpTy<'tcx, Provenance>,
366+
lock_op: &OpTy<'tcx, Provenance>,
367+
timeout_op: &OpTy<'tcx, Provenance>,
368+
flags_op: &OpTy<'tcx, Provenance>,
369+
dest: &PlaceTy<'tcx, Provenance>,
370+
) -> InterpResult<'tcx, Scalar<Provenance>> {
371+
let this = self.eval_context_mut();
372+
373+
let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?;
374+
let lock_id = this.rwlock_get_or_create_id(lock_op, SRWLOCK_ID_OFFSET)?;
375+
let timeout_ms = this.read_scalar(timeout_op)?.to_u32()?;
376+
let flags = this.read_scalar(flags_op)?.to_u32()?;
377+
378+
let timeout_time = if timeout_ms == this.eval_windows("c", "INFINITE")?.to_u32()? {
379+
None
380+
} else {
381+
let duration = Duration::from_millis(timeout_ms.into());
382+
Some(this.machine.clock.now().checked_add(duration).unwrap())
383+
};
384+
385+
let shared_mode = 0x1; // CONDITION_VARIABLE_LOCKMODE_SHARED is not in std
386+
let shared = flags == shared_mode;
387+
388+
let active_thread = this.get_active_thread();
389+
390+
let was_locked = if shared {
391+
this.rwlock_reader_unlock(lock_id, active_thread)
392+
} else {
393+
this.rwlock_writer_unlock(lock_id, active_thread)
394+
};
395+
396+
if !was_locked {
397+
throw_ub_format!(
398+
"calling SleepConditionVariableSRW with an SRWLock that is not locked by the current thread"
399+
);
400+
}
401+
402+
this.block_thread(active_thread);
403+
this.condvar_wait(condvar_id, active_thread, lock_id.to_u32(), shared);
404+
405+
if let Some(timeout_time) = timeout_time {
406+
struct Callback<'tcx> {
407+
thread: ThreadId,
408+
condvar_id: CondvarId,
409+
lock_id: RwLockId,
410+
shared: bool,
411+
dest: PlaceTy<'tcx, Provenance>,
412+
}
413+
414+
impl<'tcx> VisitTags for Callback<'tcx> {
415+
fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
416+
let Callback { thread: _, condvar_id: _, lock_id: _, shared: _, dest } = self;
417+
dest.visit_tags(visit);
418+
}
419+
}
420+
421+
impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> {
422+
fn call(&self, this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
423+
this.reacquire_cond_lock(self.thread, self.lock_id, self.shared)?;
424+
425+
this.condvar_remove_waiter(self.condvar_id, self.thread);
426+
427+
let error_timeout = this.eval_windows("c", "ERROR_TIMEOUT")?;
428+
this.set_last_error(error_timeout)?;
429+
this.write_scalar(this.eval_windows("c", "FALSE")?, &self.dest)?;
430+
Ok(())
431+
}
432+
}
433+
434+
this.register_timeout_callback(
435+
active_thread,
436+
Time::Monotonic(timeout_time),
437+
Box::new(Callback {
438+
thread: active_thread,
439+
condvar_id,
440+
lock_id,
441+
shared,
442+
dest: dest.clone(),
443+
}),
444+
);
445+
}
446+
447+
this.eval_windows("c", "TRUE")
448+
}
449+
450+
fn WakeConditionVariable(&mut self, condvar_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> {
451+
let this = self.eval_context_mut();
452+
let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?;
453+
454+
if let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) {
455+
this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?;
456+
this.unregister_timeout_callback_if_exists(thread);
457+
}
458+
459+
Ok(())
460+
}
461+
462+
fn WakeAllConditionVariable(
463+
&mut self,
464+
condvar_op: &OpTy<'tcx, Provenance>,
465+
) -> InterpResult<'tcx> {
466+
let this = self.eval_context_mut();
467+
let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?;
468+
469+
while let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) {
470+
this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?;
471+
this.unregister_timeout_callback_if_exists(thread);
472+
}
473+
474+
Ok(())
475+
}
330476
}

src/tools/miri/tests/pass/concurrency/sync.rs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -230,20 +230,8 @@ fn main() {
230230
check_once();
231231
park_timeout();
232232
park_unpark();
233-
234-
if !cfg!(windows) {
235-
// ignore-target-windows: Condvars on Windows are not supported yet
236-
check_barriers();
237-
check_conditional_variables_notify_one();
238-
check_conditional_variables_timed_wait_timeout();
239-
check_conditional_variables_timed_wait_notimeout();
240-
} else {
241-
// We need to fake the same output...
242-
for _ in 0..10 {
243-
println!("before wait");
244-
}
245-
for _ in 0..10 {
246-
println!("after wait");
247-
}
248-
}
233+
check_barriers();
234+
check_conditional_variables_notify_one();
235+
check_conditional_variables_timed_wait_timeout();
236+
check_conditional_variables_timed_wait_notimeout();
249237
}

src/tools/miri/tests/pass/concurrency/sync_nopreempt.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//@ignore-target-windows: Condvars on Windows are not supported yet.
21
// We are making scheduler assumptions here.
32
//@compile-flags: -Zmiri-strict-provenance -Zmiri-preemption-rate=0
43

src/tools/miri/tests/pass/panic/concurrent-panic.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//@ignore-target-windows: Condvars on Windows are not supported yet.
21
// We are making scheduler assumptions here.
32
//@compile-flags: -Zmiri-preemption-rate=0
43

0 commit comments

Comments
 (0)