Skip to content

Use atomic RMW for {mutex, rwlock, cond, srwlock}_get_or_create_id functions #2114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 13, 2022
Merged

Use atomic RMW for {mutex, rwlock, cond, srwlock}_get_or_create_id functions #2114

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a7f6d5
Use proper atomic rmw for {mutex, rwlock, cond, srwlock}_get_or_creat…
cbeuw May 7, 2022
a5db2c3
Refactor to hide *_next_id functions
cbeuw May 12, 2022
10d978c
Inline _create() calls and add assertions
cbeuw May 12, 2022
9e38dc4
Move and rename offset_and_layout_to_place to deref_operand_and_offset
cbeuw May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 62 additions & 51 deletions src/shims/posix/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,27 @@ fn mutex_get_or_create_id<'mir, 'tcx: 'mir>(
mutex_op: &OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, MutexId> {
let value_place = ecx.offset_and_layout_to_place(mutex_op, 4, ecx.machine.layouts.u32)?;
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
ecx.mutex_next_id().to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()?;

if success.to_bool().expect("compare_exchange's second return value is a bool") {
let id = ecx.mutex_create();
Ok(id)
} else {
Ok(MutexId::from_u32(old.to_u32().expect("layout is u32")))
}

ecx.mutex_get_or_create(|ecx, next_id| {
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
next_id.to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()
.expect("compare_exchange returns a scalar pair");

Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
// Caller of the closure needs to allocate next_id
None
} else {
Some(MutexId::from_u32(old.to_u32().expect("layout is u32")))
})
})
}

// pthread_rwlock_t is between 32 and 56 bytes, depending on the platform.
Expand Down Expand Up @@ -165,23 +169,27 @@ fn rwlock_get_or_create_id<'mir, 'tcx: 'mir>(
rwlock_op: &OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, RwLockId> {
let value_place = ecx.offset_and_layout_to_place(rwlock_op, 4, ecx.machine.layouts.u32)?;
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
ecx.rwlock_next_id().to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()?;

if success.to_bool().expect("compare_exchange's second return value is a bool") {
let id = ecx.rwlock_create();
Ok(id)
} else {
Ok(RwLockId::from_u32(old.to_u32().expect("layout is u32")))
}

ecx.rwlock_get_or_create(|ecx, next_id| {
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
next_id.to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()
.expect("compare_exchange returns a scalar pair");

Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
// Caller of the closure needs to allocate next_id
None
} else {
Some(RwLockId::from_u32(old.to_u32().expect("layout is u32")))
})
})
}

// pthread_condattr_t
Expand Down Expand Up @@ -246,23 +254,26 @@ fn cond_get_or_create_id<'mir, 'tcx: 'mir>(
) -> InterpResult<'tcx, CondvarId> {
let value_place = ecx.offset_and_layout_to_place(cond_op, 4, ecx.machine.layouts.u32)?;

let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
ecx.condvar_next_id().to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()?;

if success.to_bool().expect("compare_exchange's second return value is a bool") {
let id = ecx.condvar_create();
Ok(id)
} else {
Ok(CondvarId::from_u32(old.to_u32().expect("layout is u32")))
}
ecx.condvar_get_or_create(|ecx, next_id| {
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
next_id.to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()
.expect("compare_exchange returns a scalar pair");

Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
// Caller of the closure needs to allocate next_id
None
} else {
Some(CondvarId::from_u32(old.to_u32().expect("layout is u32")))
})
})
}

fn cond_get_clock_id<'mir, 'tcx: 'mir>(
Expand Down
37 changes: 20 additions & 17 deletions src/shims/windows/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,26 @@ fn srwlock_get_or_create_id<'mir, 'tcx: 'mir>(
) -> InterpResult<'tcx, RwLockId> {
let value_place = ecx.offset_and_layout_to_place(lock_op, 0, ecx.machine.layouts.u32)?;

let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
ecx.rwlock_next_id().to_u32_scalar().into(),
AtomicRwOp::AcqRel,
AtomicReadOp::Acquire,
false,
)?
.to_scalar_pair()?;

if success.to_bool().expect("compare_exchange's second return value is a bool") {
let id = ecx.rwlock_create();
Ok(id)
} else {
Ok(RwLockId::from_u32(old.to_u32().expect("layout is u32")))
}
ecx.rwlock_get_or_create(|ecx, next_id| {
let (old, success) = ecx
.atomic_compare_exchange_scalar(
&value_place,
&ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
next_id.to_u32_scalar().into(),
AtomicRwOp::Relaxed,
AtomicReadOp::Relaxed,
false,
)?
.to_scalar_pair()
.expect("compare_exchange returns a scalar pair");

Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
// Caller of the closure needs to allocate next_id
None
} else {
Some(RwLockId::from_u32(old.to_u32().expect("layout is u32")))
})
})
}

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
Expand Down
72 changes: 51 additions & 21 deletions src/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,20 +208,28 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// situations.
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
#[inline]
/// Peek the id of the next mutex
fn mutex_next_id(&self) -> MutexId {
let this = self.eval_context_ref();
this.machine.threads.sync.mutexes.next_index()
}

#[inline]
/// Create state for a new mutex.
fn mutex_create(&mut self) -> MutexId {
let this = self.eval_context_mut();
this.machine.threads.sync.mutexes.push(Default::default())
}

#[inline]
/// Provides the closure with the next MutexId. Creates that mutex if the closure returns None,
/// otherwise returns the value from the closure
fn mutex_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, MutexId>
where
F: FnOnce(&mut MiriEvalContext<'mir, 'tcx>, MutexId) -> InterpResult<'tcx, Option<MutexId>>,
{
let this = self.eval_context_mut();
if let Some(old) = existing(this, this.machine.threads.sync.mutexes.next_index())? {
Ok(old)
} else {
Ok(self.mutex_create())
}
}

#[inline]
/// Get the id of the thread that currently owns this lock.
fn mutex_get_owner(&mut self, id: MutexId) -> ThreadId {
Expand Down Expand Up @@ -297,20 +305,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.block_thread(thread);
}

#[inline]
/// Peek the id of the next read write lock
fn rwlock_next_id(&self) -> RwLockId {
let this = self.eval_context_ref();
this.machine.threads.sync.rwlocks.next_index()
}

#[inline]
/// Create state for a new read write lock.
fn rwlock_create(&mut self) -> RwLockId {
let this = self.eval_context_mut();
this.machine.threads.sync.rwlocks.push(Default::default())
}

#[inline]
/// Provides the closure with the next RwLockId. Creates that RwLock if the closure returns None,
/// otherwise returns the value from the closure
fn rwlock_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, RwLockId>
where
F: FnOnce(
&mut MiriEvalContext<'mir, 'tcx>,
RwLockId,
) -> InterpResult<'tcx, Option<RwLockId>>,
{
let this = self.eval_context_mut();
if let Some(old) = existing(this, this.machine.threads.sync.rwlocks.next_index())? {
Ok(old)
} else {
Ok(self.rwlock_create())
}
}

#[inline]
/// Check if locked.
fn rwlock_is_locked(&self, id: RwLockId) -> bool {
Expand Down Expand Up @@ -452,20 +471,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.block_thread(writer);
}

#[inline]
/// Peek the id of the next Condvar
fn condvar_next_id(&self) -> CondvarId {
let this = self.eval_context_ref();
this.machine.threads.sync.condvars.next_index()
}

#[inline]
/// Create state for a new conditional variable.
fn condvar_create(&mut self) -> CondvarId {
let this = self.eval_context_mut();
this.machine.threads.sync.condvars.push(Default::default())
}

#[inline]
/// Provides the closure with the next CondvarId. Creates that Condvar if the closure returns None,
/// otherwise returns the value from the closure
fn condvar_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, CondvarId>
where
F: FnOnce(
&mut MiriEvalContext<'mir, 'tcx>,
CondvarId,
) -> InterpResult<'tcx, Option<CondvarId>>,
{
let this = self.eval_context_mut();
if let Some(old) = existing(this, this.machine.threads.sync.condvars.next_index())? {
Ok(old)
} else {
Ok(self.condvar_create())
}
}

#[inline]
/// Is the conditional variable awaited?
fn condvar_is_awaited(&mut self, id: CondvarId) -> bool {
Expand Down