Skip to content

Commit

Permalink
get rid of Rc in data_race
Browse files Browse the repository at this point in the history
  • Loading branch information
@RalfJung
RalfJung committed May 22, 2021
1 parent ca7283d commit 1bbd6e6
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 30 deletions.
37 changes: 16 additions & 21 deletions src/data_race.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ use std::{
cell::{Cell, Ref, RefCell, RefMut},
fmt::Debug,
mem,
rc::Rc,
};

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
Expand All @@ -80,7 +79,7 @@ use crate::{
};

pub type AllocExtra = VClockAlloc;
pub type MemoryExtra = Rc<GlobalState>;
pub type MemoryExtra = GlobalState;

/// Valid atomic read-write operations, alias of atomic::Ordering (not non-exhaustive).
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -488,7 +487,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
) -> InterpResult<'tcx, ScalarMaybeUninit<Tag>> {
let this = self.eval_context_ref();
let scalar = this.allow_data_races_ref(move |this| this.read_scalar(&place.into()))?;
self.validate_atomic_load(place, atomic)?;
this.validate_atomic_load(place, atomic)?;
Ok(scalar)
}

Expand All @@ -501,7 +500,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
this.allow_data_races_mut(move |this| this.write_scalar(val, &(*dest).into()))?;
self.validate_atomic_store(dest, atomic)
this.validate_atomic_store(dest, atomic)
}

/// Perform a atomic operation on a memory location.
Expand Down Expand Up @@ -733,9 +732,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
pub struct VClockAlloc {
/// Assigning each byte a MemoryCellClocks.
alloc_ranges: RefCell<RangeMap<MemoryCellClocks>>,

/// Pointer to global state.
global: MemoryExtra,
}

impl VClockAlloc {
Expand Down Expand Up @@ -767,7 +763,6 @@ impl VClockAlloc {
| MemoryKind::Vtable => (0, VectorIdx::MAX_INDEX),
};
VClockAlloc {
global: Rc::clone(global),
alloc_ranges: RefCell::new(RangeMap::new(
len,
MemoryCellClocks::new(alloc_timestamp, alloc_index),
Expand Down Expand Up @@ -888,15 +883,15 @@ impl VClockAlloc {
/// being created or if it is temporarily disabled during a racy read or write
/// operation for which data-race detection is handled separately, for example
/// atomic read operations.
pub fn read<'tcx>(&self, pointer: Pointer<Tag>, len: Size) -> InterpResult<'tcx> {
if self.global.multi_threaded.get() {
let (index, clocks) = self.global.current_thread_state();
pub fn read<'tcx>(&self, pointer: Pointer<Tag>, len: Size, global: &GlobalState) -> InterpResult<'tcx> {
if global.multi_threaded.get() {
let (index, clocks) = global.current_thread_state();
let mut alloc_ranges = self.alloc_ranges.borrow_mut();
for (_, range) in alloc_ranges.iter_mut(pointer.offset, len) {
if let Err(DataRace) = range.read_race_detect(&*clocks, index) {
// Report data-race.
return Self::report_data_race(
&self.global,
global,
range,
"Read",
false,
Expand All @@ -917,14 +912,15 @@ impl VClockAlloc {
pointer: Pointer<Tag>,
len: Size,
write_type: WriteType,
global: &mut GlobalState,
) -> InterpResult<'tcx> {
if self.global.multi_threaded.get() {
let (index, clocks) = self.global.current_thread_state();
if global.multi_threaded.get() {
let (index, clocks) = global.current_thread_state();
for (_, range) in self.alloc_ranges.get_mut().iter_mut(pointer.offset, len) {
if let Err(DataRace) = range.write_race_detect(&*clocks, index, write_type) {
// Report data-race
return Self::report_data_race(
&self.global,
global,
range,
write_type.get_descriptor(),
false,
Expand All @@ -943,16 +939,16 @@ impl VClockAlloc {
/// data-race threads if `multi-threaded` is false, either due to no threads
/// being created or if it is temporarily disabled during a racy read or write
/// operation
pub fn write<'tcx>(&mut self, pointer: Pointer<Tag>, len: Size) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Write)
pub fn write<'tcx>(&mut self, pointer: Pointer<Tag>, len: Size, global: &mut GlobalState) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Write, global)
}

/// Detect data-races for an unsynchronized deallocate operation, will not perform
/// data-race threads if `multi-threaded` is false, either due to no threads
/// being created or if it is temporarily disabled during a racy read or write
/// operation
pub fn deallocate<'tcx>(&mut self, pointer: Pointer<Tag>, len: Size) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Deallocate)
pub fn deallocate<'tcx>(&mut self, pointer: Pointer<Tag>, len: Size, global: &mut GlobalState) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Deallocate, global)
}
}

Expand Down Expand Up @@ -1035,15 +1031,14 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
);

// Perform the atomic operation.
let data_race = &alloc_meta.global;
data_race.maybe_perform_sync_operation(|index, mut clocks| {
for (_, range) in
alloc_meta.alloc_ranges.borrow_mut().iter_mut(place_ptr.offset, size)
{
if let Err(DataRace) = op(range, &mut *clocks, index, atomic) {
mem::drop(clocks);
return VClockAlloc::report_data_race(
&alloc_meta.global,
data_race,
range,
description,
true,
Expand Down
9 changes: 4 additions & 5 deletions src/machine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::borrow::Cow;
use std::cell::RefCell;
use std::fmt;
use std::num::NonZeroU64;
use std::rc::Rc;
use std::time::Instant;

use log::trace;
Expand Down Expand Up @@ -153,7 +152,7 @@ impl MemoryExtra {
None
};
let data_race = if config.data_race_detector {
Some(Rc::new(data_race::GlobalState::new()))
Some(data_race::GlobalState::new())
} else {
None
};
Expand Down Expand Up @@ -513,7 +512,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
size: Size,
) -> InterpResult<'tcx> {
if let Some(data_race) = &alloc_extra.data_race {
data_race.read(ptr, size)?;
data_race.read(ptr, size, memory_extra.data_race.as_ref().unwrap())?;
}
if let Some(stacked_borrows) = &alloc_extra.stacked_borrows {
stacked_borrows.memory_read(ptr, size, memory_extra.stacked_borrows.as_ref().unwrap())
Expand All @@ -530,7 +529,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
size: Size,
) -> InterpResult<'tcx> {
if let Some(data_race) = &mut alloc_extra.data_race {
data_race.write(ptr, size)?;
data_race.write(ptr, size, memory_extra.data_race.as_mut().unwrap())?;
}
if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
stacked_borrows.memory_written(ptr, size, memory_extra.stacked_borrows.as_mut().unwrap())
Expand All @@ -550,7 +549,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
register_diagnostic(NonHaltingDiagnostic::FreedAlloc(ptr.alloc_id));
}
if let Some(data_race) = &mut alloc_extra.data_race {
data_race.deallocate(ptr, size)?;
data_race.deallocate(ptr, size, memory_extra.data_race.as_mut().unwrap())?;
}
if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
stacked_borrows.memory_deallocated(ptr, size, memory_extra.stacked_borrows.as_mut().unwrap())
Expand Down
7 changes: 3 additions & 4 deletions src/thread.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::cell::RefCell;
use std::collections::hash_map::Entry;
use std::convert::TryFrom;
use std::num::TryFromIntError;
use std::rc::Rc;
use std::time::{Duration, Instant, SystemTime};

use log::trace;
Expand Down Expand Up @@ -333,7 +332,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
fn join_thread(
&mut self,
joined_thread_id: ThreadId,
data_race: &Option<Rc<data_race::GlobalState>>,
data_race: &Option<data_race::GlobalState>,
) -> InterpResult<'tcx> {
if self.threads[joined_thread_id].join_status != ThreadJoinStatus::Joinable {
throw_ub_format!("trying to join a detached or already joined thread");
Expand Down Expand Up @@ -439,7 +438,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
/// The `AllocId` that can now be freed is returned.
fn thread_terminated(
&mut self,
data_race: &Option<Rc<data_race::GlobalState>>,
data_race: &Option<data_race::GlobalState>,
) -> Vec<AllocId> {
let mut free_tls_statics = Vec::new();
{
Expand Down Expand Up @@ -481,7 +480,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
/// blocked, terminated, or has explicitly asked to be preempted).
fn schedule(
&mut self,
data_race: &Option<Rc<data_race::GlobalState>>,
data_race: &Option<data_race::GlobalState>,
) -> InterpResult<'tcx, SchedulingAction> {
// Check whether the thread has **just** terminated (`check_terminated`
// checks whether the thread has popped all its stack and if yes, sets
Expand Down

0 comments on commit 1bbd6e6

Please sign in to comment.