diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 69f9bd09f26..10e6e252e94 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -55,9 +55,9 @@ impl fmt::Debug for BorTag { } } -/// Per-frame data for borrow tracking +/// Per-call-stack-frame data for borrow tracking #[derive(Debug)] -pub struct FrameExtra { +pub struct FrameState { /// The ID of the call this frame corresponds to. pub call_id: CallId, @@ -72,7 +72,7 @@ pub struct FrameExtra { pub protected_tags: SmallVec<[BorTag; 2]>, } -impl VisitTags for FrameExtra { +impl VisitTags for FrameState { fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { // `protected_tags` are fine to GC. } @@ -190,17 +190,17 @@ impl GlobalStateInner { id } - pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameExtra { + pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameState { let call_id = self.next_call_id; trace!("new_frame: Assigning call ID {}", call_id); if self.tracked_call_ids.contains(&call_id) { machine.emit_diagnostic(NonHaltingDiagnostic::CreatedCallId(call_id)); } self.next_call_id = NonZeroU64::new(call_id.get() + 1).unwrap(); - FrameExtra { call_id, protected_tags: SmallVec::new() } + FrameState { call_id, protected_tags: SmallVec::new() } } - pub fn end_call(&mut self, frame: &machine::FrameData<'_>) { + pub fn end_call(&mut self, frame: &machine::FrameExtra<'_>) { for tag in &frame .borrow_tracker .as_ref() @@ -253,10 +253,10 @@ impl GlobalStateInner { alloc_size: Size, kind: MemoryKind, machine: &MiriMachine<'_, '_>, - ) -> AllocExtra { + ) -> AllocState { match self.borrow_tracker_method { BorrowTrackerMethod::StackedBorrows => - AllocExtra::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation( + AllocState::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation( id, alloc_size, self, kind, machine, )))), } @@ -292,24 +292,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Extra per-allocation data for borrow tracking #[derive(Debug, Clone)] -pub enum AllocExtra { +pub enum AllocState { /// Data corresponding to Stacked Borrows - StackedBorrows(Box>), + StackedBorrows(Box>), } -impl AllocExtra { - pub fn assert_sb(&self) -> &RefCell { - match self { - AllocExtra::StackedBorrows(ref sb) => sb, +impl machine::AllocExtra { + #[track_caller] + pub fn borrow_tracker_sb(&self) -> &RefCell { + match self.borrow_tracker { + Some(AllocState::StackedBorrows(ref sb)) => sb, + _ => panic!("expected Stacked Borrows borrow tracking, got something else"), } } - pub fn assert_sb_mut(&mut self) -> &mut RefCell { - match self { - AllocExtra::StackedBorrows(ref mut sb) => sb, + #[track_caller] + pub fn borrow_tracker_sb_mut(&mut self) -> &mut RefCell { + match self.borrow_tracker { + Some(AllocState::StackedBorrows(ref mut sb)) => sb, + _ => panic!("expected Stacked Borrows borrow tracking, got something else"), } } +} +impl AllocState { pub fn before_memory_read<'tcx>( &self, alloc_id: AllocId, @@ -318,7 +324,7 @@ impl AllocExtra { machine: &MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { match self { - AllocExtra::StackedBorrows(sb) => + AllocState::StackedBorrows(sb) => sb.borrow_mut().before_memory_read(alloc_id, prov_extra, range, machine), } } @@ -331,7 +337,7 @@ impl AllocExtra { machine: &mut MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { match self { - AllocExtra::StackedBorrows(sb) => + AllocState::StackedBorrows(sb) => sb.get_mut().before_memory_write(alloc_id, prov_extra, range, machine), } } @@ -344,22 +350,22 @@ impl AllocExtra { machine: &mut MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { match self { - AllocExtra::StackedBorrows(sb) => + AllocState::StackedBorrows(sb) => sb.get_mut().before_memory_deallocation(alloc_id, prov_extra, range, machine), } } pub fn remove_unreachable_tags(&self, tags: &FxHashSet) { match self { - AllocExtra::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags), + AllocState::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags), } } } -impl VisitTags for AllocExtra { +impl VisitTags for AllocState { fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { - AllocExtra::StackedBorrows(sb) => sb.visit_tags(visit), + AllocState::StackedBorrows(sb) => sb.visit_tags(visit), } } } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index ec3be398a2c..50c2ad75ca7 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -25,7 +25,7 @@ mod stack; pub use stack::Stack; pub mod diagnostics; -pub type AllocExtra = Stacks; +pub type AllocState = Stacks; /// Extra per-allocation state. #[derive(Clone, Debug)] @@ -500,10 +500,6 @@ impl Stacks { })?; Ok(()) } - - fn expose_tag(&mut self, tag: BorTag) { - self.exposed_tags.insert(tag); - } } /// Retagging/reborrowing. There is some policy in here, such as which permissions @@ -567,10 +563,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // uncovers a non-supported `extern static`. let extra = this.get_alloc_extra(alloc_id)?; let mut stacked_borrows = extra - .borrow_tracker - .as_ref() - .expect("We should have borrow tracking data") - .assert_sb() + .borrow_tracker_sb() .borrow_mut(); // Note that we create a *second* `DiagnosticCxBuilder` below for the actual retag. // FIXME: can this be done cleaner? @@ -681,12 +674,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // We have to use shared references to alloc/memory_extra here since // `visit_freeze_sensitive` needs to access the global state. let alloc_extra = this.get_alloc_extra(alloc_id)?; - let mut stacked_borrows = alloc_extra - .borrow_tracker - .as_ref() - .expect("We should have borrow tracking data") - .assert_sb() - .borrow_mut(); + let mut stacked_borrows = alloc_extra.borrow_tracker_sb().borrow_mut(); this.visit_freeze_sensitive(place, size, |mut range, frozen| { // Adjust range. range.start += base_offset; @@ -736,12 +724,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Note that this asserts that the allocation is mutable -- but since we are creating a // mutable pointer, that seems reasonable. let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; - let stacked_borrows = alloc_extra - .borrow_tracker - .as_mut() - .expect("We should have borrow tracking data") - .assert_sb_mut() - .get_mut(); + let stacked_borrows = alloc_extra.borrow_tracker_sb_mut().get_mut(); let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); let global = machine.borrow_tracker.as_ref().unwrap().borrow(); @@ -993,13 +976,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // uncovers a non-supported `extern static`. let alloc_extra = this.get_alloc_extra(alloc_id)?; trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id:?}"); - alloc_extra - .borrow_tracker - .as_ref() - .expect("We should have borrow tracking data") - .assert_sb() - .borrow_mut() - .expose_tag(tag); + alloc_extra.borrow_tracker_sb().borrow_mut().exposed_tags.insert(tag); } AllocKind::Function | AllocKind::VTable | AllocKind::Dead => { // No stacked borrows on these allocations. @@ -1011,12 +988,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn print_stacks(&mut self, alloc_id: AllocId) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let alloc_extra = this.get_alloc_extra(alloc_id)?; - let stacks = alloc_extra - .borrow_tracker - .as_ref() - .expect("We should have borrow tracking data") - .assert_sb() - .borrow(); + let stacks = alloc_extra.borrow_tracker_sb().borrow(); for (range, stack) in stacks.stacks.iter_all() { print!("{range:?}: ["); if let Some(bottom) = stack.unknown_bottom() { diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 99288480386..dd2102043d2 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -59,7 +59,7 @@ use super::{ weak_memory::EvalContextExt as _, }; -pub type AllocExtra = VClockAlloc; +pub type AllocState = VClockAlloc; /// Valid atomic read-write orderings, alias of atomic::Ordering (not non-exhaustive). #[derive(Copy, Clone, PartialEq, Eq, Debug)] diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 6ba93a13aaf..03f9ed351fb 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -113,7 +113,7 @@ pub struct Thread<'mir, 'tcx> { thread_name: Option>, /// The virtual call stack. - stack: Vec>>, + stack: Vec>>, /// The function to call when the stack ran empty, to figure out what to do next. /// Conceptually, this is the interpreter implementation of the things that happen 'after' the @@ -232,7 +232,7 @@ impl VisitTags for Thread<'_, '_> { } } -impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> { +impl VisitTags for Frame<'_, '_, Provenance, FrameExtra<'_>> { fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Frame { return_place, @@ -385,20 +385,20 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { } /// Borrow the stack of the active thread. - pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] { + pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] { &self.threads[self.active_thread].stack } /// Mutably borrow the stack of the active thread. fn active_thread_stack_mut( &mut self, - ) -> &mut Vec>> { + ) -> &mut Vec>> { &mut self.threads[self.active_thread].stack } pub fn all_stacks( &self, - ) -> impl Iterator>]> { + ) -> impl Iterator>]> { self.threads.iter().map(|t| &t.stack[..]) } @@ -921,7 +921,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } #[inline] - fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] { + fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] { let this = self.eval_context_ref(); this.machine.threads.active_thread_stack() } @@ -929,7 +929,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { #[inline] fn active_thread_stack_mut( &mut self, - ) -> &mut Vec>> { + ) -> &mut Vec>> { let this = self.eval_context_mut(); this.machine.threads.active_thread_stack_mut() } diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index 4c32efcfa35..391681444d9 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -93,7 +93,7 @@ use super::{ vector_clock::{VClock, VTimestamp, VectorIdx}, }; -pub type AllocExtra = StoreBufferAlloc; +pub type AllocState = StoreBufferAlloc; // Each store buffer must be bounded otherwise it will grow indefinitely. // However, bounding the store buffer means restricting the amount of weak diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index f0d8b676881..8f6a5fbc1f0 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -988,7 +988,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { self.stack()[frame_idx].current_span() } - fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] { + fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameExtra<'tcx>>] { self.threads.active_thread_stack() } diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 2c427c166c1..42519797976 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -106,7 +106,7 @@ pub use crate::eval::{ pub use crate::helpers::EvalContextExt as _; pub use crate::intptrcast::ProvenanceMode; pub use crate::machine::{ - AllocExtra, FrameData, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, + AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, PrimitiveLayouts, Provenance, ProvenanceExtra, PAGE_SIZE, STACK_ADDR, STACK_SIZE, }; pub use crate::mono_hash_map::MonoHashMap; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 920ee3e1ef1..c110229c985 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -37,9 +37,9 @@ pub const STACK_ADDR: u64 = 32 * PAGE_SIZE; // not really about the "stack", but pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever /// Extra data stored with each stack frame -pub struct FrameData<'tcx> { +pub struct FrameExtra<'tcx> { /// Extra data for Stacked Borrows. - pub borrow_tracker: Option, + pub borrow_tracker: Option, /// If this is Some(), then this is a special "catch unwind" frame (the frame of `try_fn` /// called by `try`). When this frame is popped during unwinding a panic, @@ -58,10 +58,10 @@ pub struct FrameData<'tcx> { pub is_user_relevant: bool, } -impl<'tcx> std::fmt::Debug for FrameData<'tcx> { +impl<'tcx> std::fmt::Debug for FrameExtra<'tcx> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Omitting `timing`, it does not support `Debug`. - let FrameData { borrow_tracker, catch_unwind, timing: _, is_user_relevant: _ } = self; + let FrameExtra { borrow_tracker, catch_unwind, timing: _, is_user_relevant: _ } = self; f.debug_struct("FrameData") .field("borrow_tracker", borrow_tracker) .field("catch_unwind", catch_unwind) @@ -69,9 +69,9 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> { } } -impl VisitTags for FrameData<'_> { +impl VisitTags for FrameExtra<'_> { fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { - let FrameData { catch_unwind, borrow_tracker, timing: _, is_user_relevant: _ } = self; + let FrameExtra { catch_unwind, borrow_tracker, timing: _, is_user_relevant: _ } = self; catch_unwind.visit_tags(visit); borrow_tracker.visit_tags(visit); @@ -255,13 +255,13 @@ impl ProvenanceExtra { #[derive(Debug, Clone)] pub struct AllocExtra { /// Global state of the borrow tracker, if enabled. - pub borrow_tracker: Option, + pub borrow_tracker: Option, /// Data race detection via the use of a vector-clock, /// this is only added if it is enabled. - pub data_race: Option, + pub data_race: Option, /// Weak memory emulation via the use of store buffers, /// this is only added if it is enabled. - pub weak_memory: Option, + pub weak_memory: Option, } impl VisitTags for AllocExtra { @@ -736,7 +736,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { type MemoryKind = MiriMemoryKind; type ExtraFnVal = Dlsym; - type FrameExtra = FrameData<'tcx>; + type FrameExtra = FrameExtra<'tcx>; type AllocExtra = AllocExtra; type Provenance = Provenance; @@ -908,14 +908,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { .map(|bt| bt.borrow_mut().new_allocation(id, alloc.size(), kind, &ecx.machine)); let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| { - data_race::AllocExtra::new_allocation( + data_race::AllocState::new_allocation( data_race, &ecx.machine.threads, alloc.size(), kind, ) }); - let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocExtra::new_allocation); + let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocState::new_allocation); let alloc: Allocation = alloc.adjust_from_tcx( &ecx.tcx, AllocExtra { borrow_tracker, data_race: race_alloc, weak_memory: buffer_alloc }, @@ -1070,7 +1070,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { fn init_frame_extra( ecx: &mut InterpCx<'mir, 'tcx, Self>, frame: Frame<'mir, 'tcx, Provenance>, - ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> { + ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> { // Start recording our event before doing anything else let timing = if let Some(profiler) = ecx.machine.profiler.as_ref() { let fn_name = frame.instance.to_string(); @@ -1088,7 +1088,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let borrow_tracker = ecx.machine.borrow_tracker.as_ref(); - let extra = FrameData { + let extra = FrameExtra { borrow_tracker: borrow_tracker.map(|bt| bt.borrow_mut().new_frame(&ecx.machine)), catch_unwind: None, timing, @@ -1157,7 +1157,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { #[inline(always)] fn after_stack_pop( ecx: &mut InterpCx<'mir, 'tcx, Self>, - mut frame: Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>, + mut frame: Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>, unwinding: bool, ) -> InterpResult<'tcx, StackPopJump> { if frame.extra.is_user_relevant { diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 7c30845dc67..db3e42facad 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -125,7 +125,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn handle_stack_pop_unwind( &mut self, - mut extra: FrameData<'tcx>, + mut extra: FrameExtra<'tcx>, unwinding: bool, ) -> InterpResult<'tcx, StackPopJump> { let this = self.eval_context_mut();