From 1bbd6e609cb1fef17e7454bb6ced66a37f743ded Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 May 2021 14:47:14 +0200 Subject: [PATCH] get rid of Rc in data_race --- src/data_race.rs | 37 ++++++++++++++++--------------------- src/machine.rs | 9 ++++----- src/thread.rs | 7 +++---- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/data_race.rs b/src/data_race.rs index bcc2e1765431..ff6c720740a1 100644 --- a/src/data_race.rs +++ b/src/data_race.rs @@ -65,7 +65,6 @@ cell::{Cell, Ref, RefCell, RefMut}, fmt::Debug, mem, - rc::Rc, }; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -80,7 +79,7 @@ }; pub type AllocExtra = VClockAlloc; -pub type MemoryExtra = Rc; +pub type MemoryExtra = GlobalState; /// Valid atomic read-write operations, alias of atomic::Ordering (not non-exhaustive). #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -488,7 +487,7 @@ fn read_scalar_atomic( ) -> InterpResult<'tcx, ScalarMaybeUninit> { 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) } @@ -501,7 +500,7 @@ fn write_scalar_atomic( ) -> 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. @@ -733,9 +732,6 @@ fn reset_vector_clocks(&mut self, ptr: Pointer, size: Size) -> InterpResult pub struct VClockAlloc { /// Assigning each byte a MemoryCellClocks. alloc_ranges: RefCell>, - - /// Pointer to global state. - global: MemoryExtra, } impl VClockAlloc { @@ -767,7 +763,6 @@ pub fn new_allocation( | MemoryKind::Vtable => (0, VectorIdx::MAX_INDEX), }; VClockAlloc { - global: Rc::clone(global), alloc_ranges: RefCell::new(RangeMap::new( len, MemoryCellClocks::new(alloc_timestamp, alloc_index), @@ -888,15 +883,15 @@ fn report_data_race<'tcx>( /// 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, 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, 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, @@ -917,14 +912,15 @@ fn unique_access<'tcx>( pointer: Pointer, 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, @@ -943,16 +939,16 @@ fn unique_access<'tcx>( /// 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, len: Size) -> InterpResult<'tcx> { - self.unique_access(pointer, len, WriteType::Write) + pub fn write<'tcx>(&mut self, pointer: Pointer, 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, len: Size) -> InterpResult<'tcx> { - self.unique_access(pointer, len, WriteType::Deallocate) + pub fn deallocate<'tcx>(&mut self, pointer: Pointer, len: Size, global: &mut GlobalState) -> InterpResult<'tcx> { + self.unique_access(pointer, len, WriteType::Deallocate, global) } } @@ -1035,7 +1031,6 @@ fn validate_atomic_op( ); // 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) @@ -1043,7 +1038,7 @@ fn validate_atomic_op( 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, diff --git a/src/machine.rs b/src/machine.rs index 0e0f3ad1568b..ce4b96ad4a46 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -5,7 +5,6 @@ use std::cell::RefCell; use std::fmt; use std::num::NonZeroU64; -use std::rc::Rc; use std::time::Instant; use log::trace; @@ -153,7 +152,7 @@ pub fn new(config: &MiriConfig) -> Self { None }; let data_race = if config.data_race_detector { - Some(Rc::new(data_race::GlobalState::new())) + Some(data_race::GlobalState::new()) } else { None }; @@ -513,7 +512,7 @@ fn memory_read( 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()) @@ -530,7 +529,7 @@ fn memory_written( 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()) @@ -550,7 +549,7 @@ fn memory_deallocated( 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()) diff --git a/src/thread.rs b/src/thread.rs index 7d6fe8041e98..8aaeb7e349c6 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -4,7 +4,6 @@ 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; @@ -333,7 +332,7 @@ fn detach_thread(&mut self, id: ThreadId) -> InterpResult<'tcx> { fn join_thread( &mut self, joined_thread_id: ThreadId, - data_race: &Option>, + data_race: &Option, ) -> InterpResult<'tcx> { if self.threads[joined_thread_id].join_status != ThreadJoinStatus::Joinable { throw_ub_format!("trying to join a detached or already joined thread"); @@ -439,7 +438,7 @@ fn get_ready_callback(&mut self) -> Option<(ThreadId, TimeoutCallback<'mir, 'tcx /// The `AllocId` that can now be freed is returned. fn thread_terminated( &mut self, - data_race: &Option>, + data_race: &Option, ) -> Vec { let mut free_tls_statics = Vec::new(); { @@ -481,7 +480,7 @@ fn thread_terminated( /// blocked, terminated, or has explicitly asked to be preempted). fn schedule( &mut self, - data_race: &Option>, + data_race: &Option, ) -> 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