Skip to content

Commit

Permalink
make release_clock always work on the current thread
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed May 26, 2024
1 parent 331bb3f commit 5fa30f7
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 106 deletions.
10 changes: 3 additions & 7 deletions src/tools/miri/src/alloc_addresses/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,8 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
memory_kind,
ecx.get_active_thread(),
) {
if let Some(clock) = clock
&& let Some(data_race) = &ecx.machine.data_race
{
data_race.acquire_clock(&clock, ecx.get_active_thread());
if let Some(clock) = clock {
ecx.acquire_clock(&clock);
}
reuse_addr
} else {
Expand Down Expand Up @@ -372,9 +370,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
let thread = self.threads.get_active_thread_id();
global_state.reuse.add_addr(rng, addr, size, align, kind, thread, || {
if let Some(data_race) = &self.data_race {
data_race
.release_clock(thread, self.threads.active_thread_ref().current_span())
.clone()
data_race.release_clock(&self.threads).clone()
} else {
VClock::default()
}
Expand Down
35 changes: 30 additions & 5 deletions src/tools/miri/src/concurrency/data_race.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
assert!(!old, "cannot nest allow_data_races");
}
}

/// Returns the `release` clock of the current thread.
/// Other threads can acquire this clock in the future to establish synchronization
/// with this program point.
fn release_clock<'a>(&'a self) -> Option<Ref<'a, VClock>>
where
'mir: 'a,
{
let this = self.eval_context_ref();
Some(this.machine.data_race.as_ref()?.release_clock(&this.machine.threads))
}

/// Acquire the given clock into the current thread, establishing synchronization with
/// the moment when that clock snapshot was taken via `release_clock`.
fn acquire_clock(&self, clock: &VClock) {
let this = self.eval_context_ref();
if let Some(data_race) = &this.machine.data_race {
data_race.acquire_clock(clock, this.get_active_thread());
}
}
}

/// Vector clock metadata for a logical memory allocation.
Expand Down Expand Up @@ -1706,19 +1726,24 @@ impl GlobalState {
/// the moment when that clock snapshot was taken via `release_clock`.
/// As this is an acquire operation, the thread timestamp is not
/// incremented.
pub fn acquire_clock(&self, lock: &VClock, thread: ThreadId) {
pub fn acquire_clock(&self, clock: &VClock, thread: ThreadId) {
let (_, mut clocks) = self.thread_state_mut(thread);
clocks.clock.join(lock);
clocks.clock.join(clock);
}

/// Returns the `release` clock of the given thread.
/// Returns the `release` clock of the current thread.
/// Other threads can acquire this clock in the future to establish synchronization
/// with this program point.
pub fn release_clock(&self, thread: ThreadId, current_span: Span) -> Ref<'_, VClock> {
pub fn release_clock<'mir, 'tcx>(
&self,
threads: &ThreadManager<'mir, 'tcx>,
) -> Ref<'_, VClock> {
let thread = threads.get_active_thread_id();
let span = threads.active_thread_ref().current_span();
// We increment the clock each time this happens, to ensure no two releases
// can be confused with each other.
let (index, mut clocks) = self.thread_state_mut(thread);
clocks.increment_clock(index, current_span);
clocks.increment_clock(index, span);
drop(clocks);
// To return a read-only view, we need to release the RefCell
// and borrow it again.
Expand Down
25 changes: 10 additions & 15 deletions src/tools/miri/src/concurrency/init_once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let current_thread = this.get_active_thread();

if let Some(data_race) = &this.machine.data_race {
data_race
.acquire_clock(&this.machine.threads.sync.init_onces[id].clock, current_thread);
data_race.acquire_clock(&this.machine.sync.init_onces[id].clock, current_thread);
}
}

Expand Down Expand Up @@ -112,11 +111,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx, Option<InitOnceId>>,
{
let this = self.eval_context_mut();
let next_index = this.machine.threads.sync.init_onces.next_index();
let next_index = this.machine.sync.init_onces.next_index();
if let Some(old) = existing(this, next_index)? {
Ok(old)
} else {
let new_index = this.machine.threads.sync.init_onces.push(Default::default());
let new_index = this.machine.sync.init_onces.push(Default::default());
assert_eq!(next_index, new_index);
Ok(new_index)
}
Expand All @@ -125,7 +124,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
#[inline]
fn init_once_status(&mut self, id: InitOnceId) -> InitOnceStatus {
let this = self.eval_context_ref();
this.machine.threads.sync.init_onces[id].status
this.machine.sync.init_onces[id].status
}

/// Put the thread into the queue waiting for the initialization.
Expand All @@ -137,7 +136,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
callback: Box<dyn MachineCallback<'mir, 'tcx> + 'tcx>,
) {
let this = self.eval_context_mut();
let init_once = &mut this.machine.threads.sync.init_onces[id];
let init_once = &mut this.machine.sync.init_onces[id];
assert_ne!(init_once.status, InitOnceStatus::Complete, "queueing on complete init once");
init_once.waiters.push_back(InitOnceWaiter { thread, callback });
this.block_thread(thread, BlockReason::InitOnce(id));
Expand All @@ -148,7 +147,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
#[inline]
fn init_once_begin(&mut self, id: InitOnceId) {
let this = self.eval_context_mut();
let init_once = &mut this.machine.threads.sync.init_onces[id];
let init_once = &mut this.machine.sync.init_onces[id];
assert_eq!(
init_once.status,
InitOnceStatus::Uninitialized,
Expand All @@ -160,9 +159,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
#[inline]
fn init_once_complete(&mut self, id: InitOnceId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let current_thread = this.get_active_thread();
let current_span = this.machine.current_span();
let init_once = &mut this.machine.threads.sync.init_onces[id];
let init_once = &mut this.machine.sync.init_onces[id];

assert_eq!(
init_once.status,
Expand All @@ -174,7 +171,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// Each complete happens-before the end of the wait
if let Some(data_race) = &this.machine.data_race {
init_once.clock.clone_from(&data_race.release_clock(current_thread, current_span));
init_once.clock.clone_from(&data_race.release_clock(&this.machine.threads));
}

// Wake up everyone.
Expand All @@ -189,9 +186,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
#[inline]
fn init_once_fail(&mut self, id: InitOnceId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let current_thread = this.get_active_thread();
let current_span = this.machine.current_span();
let init_once = &mut this.machine.threads.sync.init_onces[id];
let init_once = &mut this.machine.sync.init_onces[id];
assert_eq!(
init_once.status,
InitOnceStatus::Begun,
Expand All @@ -200,7 +195,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// Each complete happens-before the end of the wait
if let Some(data_race) = &this.machine.data_race {
init_once.clock.clone_from(&data_race.release_clock(current_thread, current_span));
init_once.clock.clone_from(&data_race.release_clock(&this.machine.threads));
}

// Wake up one waiting thread, so they can go ahead and try to init this.
Expand Down
Loading

0 comments on commit 5fa30f7

Please sign in to comment.