Skip to content

Commit

Permalink
move rwlock dequeuing to shared code, and use that code for Windows r…
Browse files Browse the repository at this point in the history 
…wlocks
  • Loading branch information
@RalfJung
RalfJung committed Jun 28, 2020
1 parent a9dc279 commit 3a5bcb9
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 165 deletions.
22 changes: 1 addition & 21 deletions src/shims/posix/sync.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::convert::TryInto;
use std::time::{Duration, SystemTime};
use std::ops::Not;

use crate::*;
use stacked_borrows::Tag;
Expand Down Expand Up @@ -548,27 +547,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let active_thread = this.get_active_thread();

if this.rwlock_reader_unlock(id, active_thread) {
// The thread was a reader.
if this.rwlock_is_locked(id).not() {
// No more readers owning the lock. Give it to a writer if there
// is any.
this.rwlock_dequeue_and_lock_writer(id);
}
Ok(0)
} else if Some(active_thread) == this.rwlock_writer_unlock(id) {
// The thread was a writer.
//
// We are prioritizing writers here against the readers. As a
// result, not only readers can starve writers, but also writers can
// starve readers.
if this.rwlock_dequeue_and_lock_writer(id) {
// Someone got the write lock, nice.
} else {
// Give the lock to all readers.
while this.rwlock_dequeue_and_lock_reader(id) {
// Rinse and repeat.
}
}
} else if this.rwlock_writer_unlock(id, active_thread) {
Ok(0)
} else {
throw_ub_format!("unlocked an rwlock that was not locked by the active thread");
Expand Down
6 changes: 3 additions & 3 deletions src/shims/windows/foreign_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// Better error for attempts to create a thread
"CreateThread" => {
throw_unsup_format!("Miri does not support threading");
throw_unsup_format!("Miri does not support concurrency on Windows");
}

// Incomplete shims that we "stub out" just to get pre-main initialization code to work.
Expand Down Expand Up @@ -292,7 +292,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if this.frame().instance.to_string().starts_with("std::sys::windows::") => {
#[allow(non_snake_case)]
let &[_lpCriticalSection] = check_arg_count(args)?;
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows not supported");
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");
// Nothing to do, not even a return value.
// (Windows locks are reentrant, and we have only 1 thread,
// so not doing any futher checks here is at least not incorrect.)
Expand All @@ -301,7 +301,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if this.frame().instance.to_string().starts_with("std::sys::windows::") => {
#[allow(non_snake_case)]
let &[_lpCriticalSection] = check_arg_count(args)?;
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows not supported");
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");
// There is only one thread, so this always succeeds and returns TRUE.
this.write_scalar(Scalar::from_i32(1), dest)?;
}
Expand Down
134 changes: 54 additions & 80 deletions src/shims/windows/sync.rs
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
use rustc_target::abi::Size;

use crate::*;

// Locks are pointer-sized pieces of data, initialized to 0.
// We use them to count readers, with usize::MAX representing the write-locked state.
// We use the first 4 bytes to store the RwLockId.

fn deref_lock<'mir, 'tcx: 'mir>(
fn srwlock_get_or_create_id<'mir, 'tcx: 'mir>(
ecx: &mut MiriEvalContext<'mir, 'tcx>,
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, Tag>> {
// `lock` is a pointer to `void*`; cast it to a pointer to `usize`.
let lock = ecx.deref_operand(lock_op)?;
let usize = ecx.machine.layouts.usize;
assert_eq!(lock.layout.size, usize.size);
Ok(lock.offset(Size::ZERO, MemPlaceMeta::None, usize, ecx)?)
) -> InterpResult<'tcx, RwLockId> {
let id = ecx.read_scalar_at_offset(lock_op, 0, ecx.machine.layouts.u32)?.to_u32()?;
if id == 0 {
// 0 is a default value and also not a valid rwlock id. Need to allocate
// a new rwlock.
let id = ecx.rwlock_create();
ecx.write_scalar_at_offset(lock_op, 0, id.to_u32_scalar(), ecx.machine.layouts.u32)?;
Ok(id)
} else {
Ok(RwLockId::from_u32(id))
}
}

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
Expand All @@ -24,17 +27,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == 0 {
// Currently not locked. Lock it.
let new_val = Scalar::from_machine_usize(this.machine_usize_max(), this);
this.write_scalar(new_val, lock.into())?;
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

if this.rwlock_is_locked(id) {
// Note: this will deadlock if the lock is already locked by this
// thread in any way.
//
// FIXME: Detect and report the deadlock proactively. (We currently
// report the deadlock only when no thread can continue execution,
// but we could detect that this lock is already locked and report
// an error.)
this.rwlock_enqueue_and_block_writer(id, active_thread);
} else {
// Lock is already held. This is a deadlock.
throw_machine_stop!(TerminationInfo::Deadlock);
this.rwlock_writer_lock(id, active_thread);
}

Ok(())
Expand All @@ -46,18 +52,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, u8> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == 0 {
// Currently not locked. Lock it.
let new_val = this.machine_usize_max();
this.write_scalar(Scalar::from_machine_usize(new_val, this), lock.into())?;
Ok(1)
} else {
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

if this.rwlock_is_locked(id) {
// Lock is already held.
Ok(0)
} else {
this.rwlock_writer_lock(id, active_thread);
Ok(1)
}
}

Expand All @@ -67,17 +70,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == this.machine_usize_max() {
// Currently locked. Unlock it.
let new_val = 0;
this.write_scalar(Scalar::from_machine_usize(new_val, this), lock.into())?;
} else {
// Lock is not locked.
throw_ub_format!("calling ReleaseSRWLockExclusive on an SRWLock that is not exclusively locked");
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

if !this.rwlock_writer_unlock(id, active_thread) {
// The docs do not say anything about this case, but it seems better to not allow it.
throw_ub_format!("calling ReleaseSRWLockExclusive on an SRWLock that is not exclusively locked by the current thread");
}

Ok(())
Expand All @@ -89,21 +87,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == this.machine_usize_max() {
// Currently write locked. This is a deadlock.
throw_machine_stop!(TerminationInfo::Deadlock);
if this.rwlock_is_write_locked(id) {
this.rwlock_enqueue_and_block_reader(id, active_thread);
} else {
// Bump up read counter (cannot overflow as we just checkd against usize::MAX);
let new_val = lock_val+1;
// Make sure this does not reach the "write locked" flag.
if new_val == this.machine_usize_max() {
throw_unsup_format!("SRWLock read-acquired too many times");
}
this.write_scalar(Scalar::from_machine_usize(new_val, this), lock.into())?;
this.rwlock_reader_lock(id, active_thread);
}

Ok(())
Expand All @@ -115,21 +105,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, u8> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == this.machine_usize_max() {
// Currently write locked.
if this.rwlock_is_write_locked(id) {
Ok(0)
} else {
// Bump up read counter (cannot overflow as we just checkd against usize::MAX);
let new_val = lock_val+1;
// Make sure this does not reach the "write locked" flag.
if new_val == this.machine_usize_max() {
throw_unsup_format!("SRWLock read-acquired too many times");
}
this.write_scalar(Scalar::from_machine_usize(new_val, this), lock.into())?;
this.rwlock_reader_lock(id, active_thread);
Ok(1)
}
}
Expand All @@ -140,20 +122,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
lock_op: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
assert_eq!(this.get_total_thread_count(), 1, "concurrency on Windows is not supported");

let lock = deref_lock(this, lock_op)?;
let lock_val = this.read_scalar(lock.into())?.to_machine_usize(this)?;
if lock_val == this.machine_usize_max() {
// Currently write locked. This is a UB.
throw_ub_format!("calling ReleaseSRWLockShared on write-locked SRWLock");
} else if lock_val == 0 {
// Currently not locked at all.
throw_ub_format!("calling ReleaseSRWLockShared on unlocked SRWLock");
} else {
// Decrement read counter (cannot overflow as we just checkd against 0);
let new_val = lock_val-1;
this.write_scalar(Scalar::from_machine_usize(new_val, this), lock.into())?;
let id = srwlock_get_or_create_id(this, lock_op)?;
let active_thread = this.get_active_thread();

if !this.rwlock_reader_unlock(id, active_thread) {
// The docs do not say anything about this case, but it seems better to not allow it.
throw_ub_format!("calling ReleaseSRWLockShared on an SRWLock that is not locked by the current thread");
}

Ok(())
Expand Down
Loading

0 comments on commit 3a5bcb9

Please sign in to comment.