Skip to content

get rid of some Rc #1811

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 3 commits into from
May 22, 2021
Merged

get rid of some Rc #1811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca7283d
get rid of Rc in Stacked Borrows
RalfJung May 22, 2021
1bbd6e6
get rid of Rc in data_race
RalfJung May 22, 2021
c73f8b1
fmt
RalfJung May 22, 2021
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
59 changes: 31 additions & 28 deletions src/data_race.rs
View file
Open in desktop
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,21 +883,19 @@ 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,
range,
"Read",
false,
pointer,
len,
);
return Self::report_data_race(global, range, "Read", false, pointer, len);
}
}
Ok(())
Expand All @@ -917,14 +910,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 +937,26 @@ 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 +1039,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
40 changes: 22 additions & 18 deletions src/machine.rs
View file
Open in desktop
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 @@ -116,7 +115,7 @@ pub struct AllocExtra {
}

/// Extra global memory data
#[derive(Clone, Debug)]
#[derive(Debug)]
pub struct MemoryExtra {
pub stacked_borrows: Option<stacked_borrows::MemoryExtra>,
pub data_race: Option<data_race::MemoryExtra>,
Expand Down Expand Up @@ -144,19 +143,16 @@ impl MemoryExtra {
pub fn new(config: &MiriConfig) -> Self {
let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0));
let stacked_borrows = if config.stacked_borrows {
Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(
Some(RefCell::new(stacked_borrows::GlobalState::new(
config.tracked_pointer_tag,
config.tracked_call_id,
config.track_raw,
))))
} else {
None
};
let data_race = if config.data_race_detector {
Some(Rc::new(data_race::GlobalState::new()))
)))
} else {
None
};
let data_race =
if config.data_race_detector { Some(data_race::GlobalState::new()) } else { None };
MemoryExtra {
stacked_borrows,
data_race,
Expand Down Expand Up @@ -478,7 +474,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
let alloc = alloc.into_owned();
let (stacks, base_tag) = if let Some(stacked_borrows) = &memory_extra.stacked_borrows {
let (stacks, base_tag) =
Stacks::new_allocation(id, alloc.size(), Rc::clone(stacked_borrows), kind);
Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind);
(Some(stacks), base_tag)
} else {
// No stacks, no tag.
Expand Down Expand Up @@ -507,33 +503,37 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {

#[inline(always)]
fn memory_read(
_memory_extra: &Self::MemoryExtra,
memory_extra: &Self::MemoryExtra,
alloc_extra: &AllocExtra,
ptr: Pointer<Tag>,
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)
stacked_borrows.memory_read(ptr, size, memory_extra.stacked_borrows.as_ref().unwrap())
} else {
Ok(())
}
}

#[inline(always)]
fn memory_written(
_memory_extra: &mut Self::MemoryExtra,
memory_extra: &mut Self::MemoryExtra,
alloc_extra: &mut AllocExtra,
ptr: Pointer<Tag>,
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)
stacked_borrows.memory_written(
ptr,
size,
memory_extra.stacked_borrows.as_mut().unwrap(),
)
} else {
Ok(())
}
Expand All @@ -550,10 +550,14 @@ 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)
stacked_borrows.memory_deallocated(
ptr,
size,
memory_extra.stacked_borrows.as_mut().unwrap(),
)
} else {
Ok(())
}
Expand Down
Loading