From 2d42d265eae884cb2ba90df11567257973d7be6f Mon Sep 17 00:00:00 2001 From: Vanille-N Date: Fri, 2 Dec 2022 13:23:37 +0100 Subject: [PATCH 1/5] extract common borrow tracking logic --- src/tools/miri/src/borrow_tracker/mod.rs | 365 +++++++++++++++++++++++ 1 file changed, 365 insertions(+) create mode 100644 src/tools/miri/src/borrow_tracker/mod.rs diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs new file mode 100644 index 000000000000..69f9bd09f26d --- /dev/null +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -0,0 +1,365 @@ +use std::cell::RefCell; +use std::fmt; +use std::num::NonZeroU64; + +use log::trace; +use smallvec::SmallVec; + +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_middle::mir::RetagKind; +use rustc_target::abi::Size; + +use crate::*; +pub mod stacked_borrows; +use stacked_borrows::diagnostics::RetagCause; + +pub type CallId = NonZeroU64; + +/// Tracking pointer provenance +#[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct BorTag(NonZeroU64); + +impl BorTag { + pub fn new(i: u64) -> Option { + NonZeroU64::new(i).map(BorTag) + } + + pub fn get(&self) -> u64 { + self.0.get() + } + + pub fn inner(&self) -> NonZeroU64 { + self.0 + } + + pub fn succ(self) -> Option { + self.0.checked_add(1).map(Self) + } + + /// The minimum representable tag + pub fn one() -> Self { + Self::new(1).unwrap() + } +} + +impl std::default::Default for BorTag { + /// The default to be used when borrow tracking is disabled + fn default() -> Self { + Self::one() + } +} + +impl fmt::Debug for BorTag { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "<{}>", self.0) + } +} + +/// Per-frame data for borrow tracking +#[derive(Debug)] +pub struct FrameExtra { + /// The ID of the call this frame corresponds to. + pub call_id: CallId, + + /// If this frame is protecting any tags, they are listed here. We use this list to do + /// incremental updates of the global list of protected tags stored in the + /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected + /// tag, to identify which call is responsible for protecting the tag. + /// See `Stack::item_popped` for more explanation. + /// + /// This will contain one tag per reference passed to the function, so + /// a size of 2 is enough for the vast majority of functions. + pub protected_tags: SmallVec<[BorTag; 2]>, +} + +impl VisitTags for FrameExtra { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { + // `protected_tags` are fine to GC. + } +} + +/// Extra global state, available to the memory access hooks. +#[derive(Debug)] +pub struct GlobalStateInner { + /// Borrow tracker method currently in use. + pub borrow_tracker_method: BorrowTrackerMethod, + /// Next unused pointer ID (tag). + pub next_ptr_tag: BorTag, + /// Table storing the "base" tag for each allocation. + /// The base tag is the one used for the initial pointer. + /// We need this in a separate table to handle cyclic statics. + pub base_ptr_tags: FxHashMap, + /// Next unused call ID (for protectors). + pub next_call_id: CallId, + /// All currently protected tags. + /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. + /// We add tags to this when they are created with a protector in `reborrow`, and + /// we remove tags from this when the call which is protecting them returns, in + /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details. + pub protected_tags: FxHashMap, + /// The pointer ids to trace + pub tracked_pointer_tags: FxHashSet, + /// The call ids to trace + pub tracked_call_ids: FxHashSet, + /// Whether to recurse into datatypes when searching for pointers to retag. + pub retag_fields: RetagFields, +} + +impl VisitTags for GlobalStateInner { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { + // The only candidate is base_ptr_tags, and that does not need visiting since we don't ever + // GC the bottommost tag. + } +} + +/// We need interior mutable access to the global state. +pub type GlobalState = RefCell; + +/// Indicates which kind of access is being performed. +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum AccessKind { + Read, + Write, +} + +impl fmt::Display for AccessKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AccessKind::Read => write!(f, "read access"), + AccessKind::Write => write!(f, "write access"), + } + } +} + +/// Policy on whether to recurse into fields to retag +#[derive(Copy, Clone, Debug)] +pub enum RetagFields { + /// Don't retag any fields. + No, + /// Retag all fields. + Yes, + /// Only retag fields of types with Scalar and ScalarPair layout, + /// to match the LLVM `noalias` we generate. + OnlyScalar, +} + +/// The flavor of the protector. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum ProtectorKind { + /// Protected against aliasing violations from other pointers. + /// + /// Items protected like this cause UB when they are invalidated, *but* the pointer itself may + /// still be used to issue a deallocation. + /// + /// This is required for LLVM IR pointers that are `noalias` but *not* `dereferenceable`. + WeakProtector, + + /// Protected against any kind of invalidation. + /// + /// Items protected like this cause UB when they are invalidated or the memory is deallocated. + /// This is strictly stronger protection than `WeakProtector`. + /// + /// This is required for LLVM IR pointers that are `dereferenceable` (and also allows `noalias`). + StrongProtector, +} + +/// Utilities for initialization and ID generation +impl GlobalStateInner { + pub fn new( + borrow_tracker_method: BorrowTrackerMethod, + tracked_pointer_tags: FxHashSet, + tracked_call_ids: FxHashSet, + retag_fields: RetagFields, + ) -> Self { + GlobalStateInner { + borrow_tracker_method, + next_ptr_tag: BorTag::one(), + base_ptr_tags: FxHashMap::default(), + next_call_id: NonZeroU64::new(1).unwrap(), + protected_tags: FxHashMap::default(), + tracked_pointer_tags, + tracked_call_ids, + retag_fields, + } + } + + /// Generates a new pointer tag. Remember to also check track_pointer_tags and log its creation! + pub fn new_ptr(&mut self) -> BorTag { + let id = self.next_ptr_tag; + self.next_ptr_tag = id.succ().unwrap(); + id + } + + pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameExtra { + 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() } + } + + pub fn end_call(&mut self, frame: &machine::FrameData<'_>) { + for tag in &frame + .borrow_tracker + .as_ref() + .expect("we should have borrow tracking data") + .protected_tags + { + self.protected_tags.remove(tag); + } + } + + pub fn base_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> BorTag { + self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| { + let tag = self.new_ptr(); + if self.tracked_pointer_tags.contains(&tag) { + machine.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag( + tag.inner(), + None, + None, + )); + } + trace!("New allocation {:?} has base tag {:?}", id, tag); + self.base_ptr_tags.try_insert(id, tag).unwrap(); + tag + }) + } +} + +/// Which borrow tracking method to use +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum BorrowTrackerMethod { + /// Stacked Borrows, as implemented in borrow_tracker/stacked + StackedBorrows, +} + +impl BorrowTrackerMethod { + pub fn instanciate_global_state(self, config: &MiriConfig) -> GlobalState { + RefCell::new(GlobalStateInner::new( + self, + config.tracked_pointer_tags.clone(), + config.tracked_call_ids.clone(), + config.retag_fields, + )) + } +} + +impl GlobalStateInner { + pub fn new_allocation( + &mut self, + id: AllocId, + alloc_size: Size, + kind: MemoryKind, + machine: &MiriMachine<'_, '_>, + ) -> AllocExtra { + match self.borrow_tracker_method { + BorrowTrackerMethod::StackedBorrows => + AllocExtra::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation( + id, alloc_size, self, kind, machine, + )))), + } + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + fn retag(&mut self, kind: RetagKind, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + match method { + BorrowTrackerMethod::StackedBorrows => this.sb_retag(kind, place), + } + } + + fn retag_return_place(&mut self) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + match method { + BorrowTrackerMethod::StackedBorrows => this.sb_retag_return_place(), + } + } + + fn expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + match method { + BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag), + } + } +} + +/// Extra per-allocation data for borrow tracking +#[derive(Debug, Clone)] +pub enum AllocExtra { + /// Data corresponding to Stacked Borrows + StackedBorrows(Box>), +} + +impl AllocExtra { + pub fn assert_sb(&self) -> &RefCell { + match self { + AllocExtra::StackedBorrows(ref sb) => sb, + } + } + + pub fn assert_sb_mut(&mut self) -> &mut RefCell { + match self { + AllocExtra::StackedBorrows(ref mut sb) => sb, + } + } + + pub fn before_memory_read<'tcx>( + &self, + alloc_id: AllocId, + prov_extra: ProvenanceExtra, + range: AllocRange, + machine: &MiriMachine<'_, 'tcx>, + ) -> InterpResult<'tcx> { + match self { + AllocExtra::StackedBorrows(sb) => + sb.borrow_mut().before_memory_read(alloc_id, prov_extra, range, machine), + } + } + + pub fn before_memory_write<'tcx>( + &mut self, + alloc_id: AllocId, + prov_extra: ProvenanceExtra, + range: AllocRange, + machine: &mut MiriMachine<'_, 'tcx>, + ) -> InterpResult<'tcx> { + match self { + AllocExtra::StackedBorrows(sb) => + sb.get_mut().before_memory_write(alloc_id, prov_extra, range, machine), + } + } + + pub fn before_memory_deallocation<'tcx>( + &mut self, + alloc_id: AllocId, + prov_extra: ProvenanceExtra, + range: AllocRange, + machine: &mut MiriMachine<'_, 'tcx>, + ) -> InterpResult<'tcx> { + match self { + AllocExtra::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), + } + } +} + +impl VisitTags for AllocExtra { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { + match self { + AllocExtra::StackedBorrows(sb) => sb.visit_tags(visit), + } + } +} From 2528f4e6684d68b086179735454d95a6950ef46b Mon Sep 17 00:00:00 2001 From: Vanille-N Date: Fri, 2 Dec 2022 13:23:57 +0100 Subject: [PATCH 2/5] move stacked_borrows to borrow_tracker/stacked_borrows --- .../stacked_borrows/diagnostics.rs | 25 +- .../stacked_borrows/item.rs | 16 +- .../stacked_borrows/mod.rs | 343 +++++------------- .../stacked_borrows/stack.rs | 23 +- 4 files changed, 116 insertions(+), 291 deletions(-) rename src/tools/miri/src/{ => borrow_tracker}/stacked_borrows/diagnostics.rs (97%) rename src/tools/miri/src/{ => borrow_tracker}/stacked_borrows/item.rs (89%) rename src/tools/miri/src/{ => borrow_tracker}/stacked_borrows/mod.rs (80%) rename src/tools/miri/src/{ => borrow_tracker}/stacked_borrows/stack.rs (96%) diff --git a/src/tools/miri/src/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs similarity index 97% rename from src/tools/miri/src/stacked_borrows/diagnostics.rs rename to src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 023f6005419a..c5eb2113f9f8 100644 --- a/src/tools/miri/src/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -1,17 +1,16 @@ use smallvec::SmallVec; use std::fmt; -use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange}; +use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange, InterpError}; use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; -use crate::stacked_borrows::{ - err_sb_ub, AccessKind, GlobalStateInner, Permission, ProtectorKind, Stack, +use crate::borrow_tracker::{ + stacked_borrows::{err_sb_ub, Permission}, + AccessKind, GlobalStateInner, ProtectorKind, }; use crate::*; -use rustc_middle::mir::interpret::InterpError; - #[derive(Clone, Debug)] pub struct AllocHistory { id: AllocId, @@ -53,7 +52,7 @@ impl Creation { #[derive(Clone, Debug)] struct Invalidation { - tag: SbTag, + tag: BorTag, range: AllocRange, span: Span, cause: InvalidationCause, @@ -100,7 +99,7 @@ impl fmt::Display for InvalidationCause { #[derive(Clone, Debug)] struct Protection { - tag: SbTag, + tag: BorTag, span: Span, } @@ -135,7 +134,7 @@ impl<'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { pub fn retag( machine: &'ecx MiriMachine<'mir, 'tcx>, cause: RetagCause, - new_tag: SbTag, + new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, ) -> Self { @@ -185,7 +184,7 @@ enum Operation { #[derive(Debug, Clone)] struct RetagOp { cause: RetagCause, - new_tag: SbTag, + new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, permission: Option, @@ -257,7 +256,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { .push(Creation { retag: op.clone(), span: self.machine.current_span() }); } - pub fn log_invalidation(&mut self, tag: SbTag) { + pub fn log_invalidation(&mut self, tag: BorTag) { let mut span = self.machine.current_span(); let (range, cause) = match &self.operation { Operation::Retag(RetagOp { cause, range, permission, .. }) => { @@ -288,8 +287,8 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { pub fn get_logs_relevant_to( &self, - tag: SbTag, - protector_tag: Option, + tag: BorTag, + protector_tag: Option, ) -> Option { let Some(created) = self.history .creations @@ -410,7 +409,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { .all_stacks() .flatten() .map(|frame| { - frame.extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data") + frame.extra.borrow_tracker.as_ref().expect("we should have borrow tracking data") }) .find(|frame| frame.protected_tags.contains(&item.tag())) .map(|frame| frame.call_id) diff --git a/src/tools/miri/src/stacked_borrows/item.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs similarity index 89% rename from src/tools/miri/src/stacked_borrows/item.rs rename to src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs index 709b27d191b2..b9a52e4966cd 100644 --- a/src/tools/miri/src/stacked_borrows/item.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs @@ -1,13 +1,13 @@ -use crate::stacked_borrows::SbTag; use std::fmt; -use std::num::NonZeroU64; + +use crate::borrow_tracker::BorTag; /// An item in the per-location borrow stack. #[derive(Copy, Clone, Hash, PartialEq, Eq)] pub struct Item(u64); // An Item contains 3 bitfields: -// * Bits 0-61 store an SbTag +// * Bits 0-61 store a BorTag // * Bits 61-63 store a Permission // * Bit 64 stores a flag which indicates if we have a protector const TAG_MASK: u64 = u64::MAX >> 3; @@ -18,9 +18,9 @@ const PERM_SHIFT: u64 = 61; const PROTECTED_SHIFT: u64 = 63; impl Item { - pub fn new(tag: SbTag, perm: Permission, protected: bool) -> Self { - assert!(tag.0.get() <= TAG_MASK); - let packed_tag = tag.0.get(); + pub fn new(tag: BorTag, perm: Permission, protected: bool) -> Self { + assert!(tag.get() <= TAG_MASK); + let packed_tag = tag.get(); let packed_perm = perm.to_bits() << PERM_SHIFT; let packed_protected = u64::from(protected) << PROTECTED_SHIFT; @@ -34,8 +34,8 @@ impl Item { } /// The pointers the permission is granted to. - pub fn tag(self) -> SbTag { - SbTag(NonZeroU64::new(self.0 & TAG_MASK).unwrap()) + pub fn tag(self) -> BorTag { + BorTag::new(self.0 & TAG_MASK).unwrap() } /// The permission this item grants. diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs similarity index 80% rename from src/tools/miri/src/stacked_borrows/mod.rs rename to src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 4e369f4291a3..ec3be398a2c2 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -2,81 +2,30 @@ //! for further information. use log::trace; -use std::cell::RefCell; use std::cmp; -use std::fmt; -use std::fmt::Write; -use std::num::NonZeroU64; +use std::fmt::{self, Write}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_hir::Mutability; -use rustc_middle::mir::RetagKind; +use rustc_data_structures::fx::FxHashSet; +use rustc_middle::mir::{Mutability, RetagKind}; use rustc_middle::ty::{ self, layout::{HasParamEnv, LayoutOf}, }; -use rustc_target::abi::Abi; -use rustc_target::abi::Size; -use smallvec::SmallVec; +use rustc_target::abi::{Abi, Size}; +use crate::borrow_tracker::{ + stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder, TagHistory}, + AccessKind, GlobalStateInner, ProtectorKind, RetagCause, RetagFields, +}; use crate::*; -pub mod diagnostics; -use diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder, RetagCause, TagHistory}; - mod item; pub use item::{Item, Permission}; mod stack; pub use stack::Stack; +pub mod diagnostics; -pub type CallId = NonZeroU64; - -// Even reading memory can have effects on the stack, so we need a `RefCell` here. -pub type AllocExtra = RefCell; - -/// Tracking pointer provenance -#[derive(Copy, Clone, Hash, PartialEq, Eq)] -pub struct SbTag(NonZeroU64); - -impl SbTag { - pub fn new(i: u64) -> Option { - NonZeroU64::new(i).map(SbTag) - } - - // The default to be used when SB is disabled - #[allow(clippy::should_implement_trait)] - pub fn default() -> Self { - Self::new(1).unwrap() - } -} - -impl fmt::Debug for SbTag { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "<{}>", self.0) - } -} - -#[derive(Debug)] -pub struct FrameExtra { - /// The ID of the call this frame corresponds to. - call_id: CallId, - - /// If this frame is protecting any tags, they are listed here. We use this list to do - /// incremental updates of the global list of protected tags stored in the - /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected - /// tag, to identify which call is responsible for protecting the tag. - /// See `Stack::item_invalidated` for more explanation. - /// - /// This will contain one tag per reference passed to the function, so - /// a size of 2 is enough for the vast majority of functions. - protected_tags: SmallVec<[SbTag; 2]>, -} - -impl VisitTags for FrameExtra { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { - // `protected_tags` are fine to GC. - } -} +pub type AllocExtra = Stacks; /// Extra per-allocation state. #[derive(Clone, Debug)] @@ -86,98 +35,16 @@ pub struct Stacks { /// Stores past operations on this allocation history: AllocHistory, /// The set of tags that have been exposed inside this allocation. - exposed_tags: FxHashSet, + exposed_tags: FxHashSet, /// Whether this memory has been modified since the last time the tag GC ran modified_since_last_gc: bool, } -/// The flavor of the protector. -#[derive(Copy, Clone, Debug)] -enum ProtectorKind { - /// Protected against aliasing violations from other pointers. - /// - /// Items protected like this cause UB when they are invalidated, *but* the pointer itself may - /// still be used to issue a deallocation. - /// - /// This is required for LLVM IR pointers that are `noalias` but *not* `dereferenceable`. - WeakProtector, - - /// Protected against any kind of invalidation. - /// - /// Items protected like this cause UB when they are invalidated or the memory is deallocated. - /// This is strictly stronger protection than `WeakProtector`. - /// - /// This is required for LLVM IR pointers that are `dereferenceable` (and also allows `noalias`). - StrongProtector, -} - -/// Extra global state, available to the memory access hooks. -#[derive(Debug)] -pub struct GlobalStateInner { - /// Next unused pointer ID (tag). - next_ptr_tag: SbTag, - /// Table storing the "base" tag for each allocation. - /// The base tag is the one used for the initial pointer. - /// We need this in a separate table to handle cyclic statics. - base_ptr_tags: FxHashMap, - /// Next unused call ID (for protectors). - next_call_id: CallId, - /// All currently protected tags, and the status of their protection. - /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. - /// We add tags to this when they are created with a protector in `reborrow`, and - /// we remove tags from this when the call which is protecting them returns, in - /// `GlobalStateInner::end_call`. See `Stack::item_invalidated` for more details. - protected_tags: FxHashMap, - /// The pointer ids to trace - tracked_pointer_tags: FxHashSet, - /// The call ids to trace - tracked_call_ids: FxHashSet, - /// Whether to recurse into datatypes when searching for pointers to retag. - retag_fields: RetagFields, -} - -#[derive(Copy, Clone, Debug)] -pub enum RetagFields { - /// Don't retag any fields. - No, - /// Retag all fields. - Yes, - /// Only retag fields of types with Scalar and ScalarPair layout, - /// to match the LLVM `noalias` we generate. - OnlyScalar, -} - -impl VisitTags for GlobalStateInner { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { - // The only candidate is base_ptr_tags, and that does not need visiting since we don't ever - // GC the bottommost tag. - } -} - -/// We need interior mutable access to the global state. -pub type GlobalState = RefCell; - -/// Indicates which kind of access is being performed. -#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] -pub enum AccessKind { - Read, - Write, -} - -impl fmt::Display for AccessKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - AccessKind::Read => write!(f, "read access"), - AccessKind::Write => write!(f, "write access"), - } - } -} - /// Indicates which kind of reference is being created. /// Used by high-level `reborrow` to compute which permissions to grant to the /// new pointer. #[derive(Copy, Clone, Hash, PartialEq, Eq)] -pub enum RefKind { +enum RefKind { /// `&mut` and `Box`. Unique { two_phase: bool }, /// `&` with or without interior mutability. @@ -198,65 +65,6 @@ impl fmt::Display for RefKind { } } -/// Utilities for initialization and ID generation -impl GlobalStateInner { - pub fn new( - tracked_pointer_tags: FxHashSet, - tracked_call_ids: FxHashSet, - retag_fields: RetagFields, - ) -> Self { - GlobalStateInner { - next_ptr_tag: SbTag(NonZeroU64::new(1).unwrap()), - base_ptr_tags: FxHashMap::default(), - next_call_id: NonZeroU64::new(1).unwrap(), - protected_tags: FxHashMap::default(), - tracked_pointer_tags, - tracked_call_ids, - retag_fields, - } - } - - /// Generates a new pointer tag. Remember to also check track_pointer_tags and log its creation! - fn new_ptr(&mut self) -> SbTag { - let id = self.next_ptr_tag; - self.next_ptr_tag = SbTag(NonZeroU64::new(id.0.get() + 1).unwrap()); - id - } - - pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameExtra { - 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() } - } - - pub fn end_call(&mut self, frame: &machine::FrameData<'_>) { - for tag in &frame - .stacked_borrows - .as_ref() - .expect("we should have Stacked Borrows data") - .protected_tags - { - self.protected_tags.remove(tag); - } - } - - pub fn base_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> SbTag { - self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| { - let tag = self.new_ptr(); - if self.tracked_pointer_tags.contains(&tag) { - machine.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(tag.0, None, None)); - } - trace!("New allocation {:?} has base tag {:?}", id, tag); - self.base_ptr_tags.try_insert(id, tag).unwrap(); - tag - }) - } -} - /// Error reporting pub fn err_sb_ub<'tcx>( msg: String, @@ -329,14 +137,7 @@ impl<'tcx> Stack { } } - /// Check if the given item is protected. - /// - /// The `provoking_access` argument is only used to produce diagnostics. - /// It is `Some` when we are granting the contained access for said tag, and it is - /// `None` during a deallocation. - /// Within `provoking_access, the `AllocRange` refers the entire operation, and - /// the `Size` refers to the specific location in the `AllocRange` that we are - /// currently checking. + /// The given item was invalidated -- check its protectors for whether that will cause UB. fn item_invalidated( item: &Item, global: &GlobalStateInner, @@ -386,7 +187,7 @@ impl<'tcx> Stack { tag: ProvenanceExtra, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, - exposed_tags: &FxHashSet, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Two main steps: Find granting item, remove incompatible items above. @@ -442,23 +243,24 @@ impl<'tcx> Stack { if granting_idx.is_none() || matches!(tag, ProvenanceExtra::Wildcard) { // Compute the upper bound of the items that remain. // (This is why we did all the work above: to reduce the items we have to consider here.) - let mut max = NonZeroU64::new(1).unwrap(); + let mut max = BorTag::one(); for i in 0..self.len() { let item = self.get(i).unwrap(); // Skip disabled items, they cannot be matched anyway. if !matches!(item.perm(), Permission::Disabled) { // We are looking for a strict upper bound, so add 1 to this tag. - max = cmp::max(item.tag().0.checked_add(1).unwrap(), max); + max = cmp::max(item.tag().succ().unwrap(), max); } } if let Some(unk) = self.unknown_bottom() { - max = cmp::max(unk.0, max); + max = cmp::max(unk, max); } // Use `max` as new strict upper bound for everything. trace!( - "access: forgetting stack to upper bound {max} due to wildcard or unknown access" + "access: forgetting stack to upper bound {max} due to wildcard or unknown access", + max = max.get(), ); - self.set_unknown_bottom(SbTag(max)); + self.set_unknown_bottom(max); } // Done. @@ -472,7 +274,7 @@ impl<'tcx> Stack { tag: ProvenanceExtra, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, - exposed_tags: &FxHashSet, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Step 1: Make a write access. // As part of this we do regular protector checking, i.e. even weakly protected items cause UB when popped. @@ -497,7 +299,7 @@ impl<'tcx> Stack { access: Option, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, - exposed_tags: &FxHashSet, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { dcx.start_grant(new.perm()); @@ -550,9 +352,9 @@ impl<'tcx> Stack { } // # Stacked Borrows Core End -/// Integration with the SbTag garbage collector +/// Integration with the BorTag garbage collector impl Stacks { - pub fn remove_unreachable_tags(&mut self, live_tags: &FxHashSet) { + pub fn remove_unreachable_tags(&mut self, live_tags: &FxHashSet) { if self.modified_since_last_gc { for stack in self.stacks.iter_mut_all() { if stack.len() > 64 { @@ -565,7 +367,7 @@ impl Stacks { } impl VisitTags for Stacks { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { for tag in self.exposed_tags.iter().copied() { visit(tag); } @@ -579,7 +381,7 @@ impl<'tcx> Stacks { fn new( size: Size, perm: Permission, - tag: SbTag, + tag: BorTag, id: AllocId, machine: &MiriMachine<'_, '_>, ) -> Self { @@ -602,7 +404,7 @@ impl<'tcx> Stacks { mut f: impl FnMut( &mut Stack, &mut DiagnosticCx<'_, '_, '_, 'tcx>, - &mut FxHashSet, + &mut FxHashSet, ) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { self.modified_since_last_gc = true; @@ -620,20 +422,19 @@ impl Stacks { pub fn new_allocation( id: AllocId, size: Size, - state: &GlobalState, + state: &mut GlobalStateInner, kind: MemoryKind, machine: &MiriMachine<'_, '_>, ) -> Self { - let mut extra = state.borrow_mut(); let (base_tag, perm) = match kind { // New unique borrow. This tag is not accessible by the program, // so it will only ever be used when using the local directly (i.e., // not through a pointer). That is, whenever we directly write to a local, this will pop // everything else off the stack, invalidating all previous pointers, // and in particular, *all* raw pointers. - MemoryKind::Stack => (extra.base_ptr_tag(id, machine), Permission::Unique), + MemoryKind::Stack => (state.base_ptr_tag(id, machine), Permission::Unique), // Everything else is shared by default. - _ => (extra.base_ptr_tag(id, machine), Permission::SharedReadWrite), + _ => (state.base_ptr_tag(id, machine), Permission::SharedReadWrite), }; Stacks::new(size, perm, base_tag, id, machine) } @@ -656,7 +457,7 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::read(machine, tag, range); - let state = machine.stacked_borrows.as_ref().unwrap().borrow(); + let state = machine.borrow_tracker.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Read, tag, &state, dcx, exposed_tags) }) @@ -677,7 +478,7 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::write(machine, tag, range); - let state = machine.stacked_borrows.as_ref().unwrap().borrow(); + let state = machine.borrow_tracker.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Write, tag, &state, dcx, exposed_tags) }) @@ -693,12 +494,16 @@ impl Stacks { ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", tag, alloc_id, range.size.bytes()); let dcx = DiagnosticCxBuilder::dealloc(machine, tag); - let state = machine.stacked_borrows.as_ref().unwrap().borrow(); + let state = machine.borrow_tracker.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.dealloc(tag, &state, dcx, exposed_tags) })?; 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 @@ -710,13 +515,13 @@ impl<'mir: 'ecx, 'tcx: 'mir, 'ecx> EvalContextPrivExt<'mir, 'tcx, 'ecx> trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Returns the `AllocId` the reborrow was done in, if some actual borrow stack manipulation /// happened. - fn reborrow( + fn sb_reborrow( &mut self, place: &MPlaceTy<'tcx, Provenance>, size: Size, kind: RefKind, retag_cause: RetagCause, // What caused this retag, for diagnostics only - new_tag: SbTag, + new_tag: BorTag, protect: Option, ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); @@ -725,7 +530,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let log_creation = |this: &MiriInterpCx<'mir, 'tcx>, loc: Option<(AllocId, Size, ProvenanceExtra)>| // alloc_id, base_offset, orig_tag -> InterpResult<'tcx> { - let global = this.machine.stacked_borrows.as_ref().unwrap().borrow(); + let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let ty = place.layout.ty; if global.tracked_pointer_tags.contains(&new_tag) { let mut kind_str = format!("{kind}"); @@ -743,7 +548,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' _ => write!(kind_str, " (pointee type {ty})").unwrap(), }; this.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag( - new_tag.0, + new_tag.inner(), Some(kind_str), loc.map(|(alloc_id, base_offset, orig_tag)| (alloc_id, alloc_range(base_offset, size), orig_tag)), )); @@ -762,9 +567,10 @@ 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 - .stacked_borrows + .borrow_tracker .as_ref() - .expect("we should have Stacked Borrows data") + .expect("We should have borrow tracking data") + .assert_sb() .borrow_mut(); // Note that we create a *second* `DiagnosticCxBuilder` below for the actual retag. // FIXME: can this be done cleaner? @@ -780,7 +586,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' if protect.is_some() { dcx.log_protector(); } - } + }, AllocKind::Function | AllocKind::VTable | AllocKind::Dead => { // No stacked borrows on these allocations. } @@ -839,9 +645,9 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' if let Some(protect) = protect { // See comment in `Stack::item_invalidated` for why we store the tag twice. - this.frame_mut().extra.stacked_borrows.as_mut().unwrap().protected_tags.push(new_tag); + this.frame_mut().extra.borrow_tracker.as_mut().unwrap().protected_tags.push(new_tag); this.machine - .stacked_borrows + .borrow_tracker .as_mut() .unwrap() .get_mut() @@ -876,9 +682,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // `visit_freeze_sensitive` needs to access the global state. let alloc_extra = this.get_alloc_extra(alloc_id)?; let mut stacked_borrows = alloc_extra - .stacked_borrows + .borrow_tracker .as_ref() - .expect("we should have Stacked Borrows data") + .expect("We should have borrow tracking data") + .assert_sb() .borrow_mut(); this.visit_freeze_sensitive(place, size, |mut range, frozen| { // Adjust range. @@ -900,7 +707,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' false }; let item = Item::new(new_tag, perm, protected); - let global = this.machine.stacked_borrows.as_ref().unwrap().borrow(); + let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( &this.machine, retag_cause, @@ -930,13 +737,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // mutable pointer, that seems reasonable. let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; let stacked_borrows = alloc_extra - .stacked_borrows + .borrow_tracker .as_mut() - .expect("we should have Stacked Borrows data") + .expect("We should have borrow tracking data") + .assert_sb_mut() .get_mut(); let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); - let global = machine.stacked_borrows.as_ref().unwrap().borrow(); + let global = machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( machine, retag_cause, @@ -960,8 +768,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' } /// Retags an indidual pointer, returning the retagged version. - /// `mutbl` can be `None` to make this a raw pointer. - fn retag_reference( + /// `kind` indicates what kind of reference is being created. + fn sb_retag_reference( &mut self, val: &ImmTy<'tcx, Provenance>, kind: RefKind, @@ -981,10 +789,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' }; // Compute new borrow. - let new_tag = this.machine.stacked_borrows.as_mut().unwrap().get_mut().new_ptr(); + let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.reborrow(&place, size, kind, retag_cause, new_tag, protect)?; + let alloc_id = this.sb_reborrow(&place, size, kind, retag_cause, new_tag, protect)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -993,7 +801,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' Some(alloc_id) => { // If `reborrow` could figure out the AllocId of this ptr, hard-code it into the new one. // Even if we started out with a wildcard, this newly retagged pointer is tied to that allocation. - Provenance::Concrete { alloc_id, sb: new_tag } + Provenance::Concrete { alloc_id, tag: new_tag } } None => { // Looks like this has to stay a wildcard pointer. @@ -1011,9 +819,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { - fn retag(&mut self, kind: RetagKind, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { + fn sb_retag( + &mut self, + kind: RetagKind, + place: &PlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let retag_fields = this.machine.stacked_borrows.as_mut().unwrap().get_mut().retag_fields; + let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; let retag_cause = match kind { RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, RetagKind::FnEntry => RetagCause::FnEntry, @@ -1039,7 +851,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { protector: Option, ) -> InterpResult<'tcx> { let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; - let val = self.ecx.retag_reference(&val, ref_kind, retag_cause, protector)?; + let val = self.ecx.sb_retag_reference(&val, ref_kind, retag_cause, protector)?; self.ecx.write_immediate(*val, place)?; Ok(()) } @@ -1138,7 +950,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// /// This is a HACK because there is nothing in MIR that would make the retag /// explicit. Also see . - fn retag_return_place(&mut self) -> InterpResult<'tcx> { + fn sb_retag_return_place(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let return_place = &this.frame().return_place; if return_place.layout.is_zst() { @@ -1153,7 +965,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr_layout = this.layout_of(this.tcx.mk_mut_ptr(return_place.layout.ty))?; let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); // Reborrow it. With protection! That is part of the point. - let val = this.retag_reference( + let val = this.sb_retag_reference( &val, RefKind::Unique { two_phase: false }, RetagCause::FnReturn, @@ -1167,7 +979,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. - fn expose_tag(&mut self, alloc_id: AllocId, tag: SbTag) -> InterpResult<'tcx> { + fn sb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { let this = self.eval_context_mut(); // Function pointers and dead objects don't have an alloc_extra so we ignore them. @@ -1181,7 +993,13 @@ 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.stacked_borrows.as_ref().unwrap().borrow_mut().exposed_tags.insert(tag); + alloc_extra + .borrow_tracker + .as_ref() + .expect("We should have borrow tracking data") + .assert_sb() + .borrow_mut() + .expose_tag(tag); } AllocKind::Function | AllocKind::VTable | AllocKind::Dead => { // No stacked borrows on these allocations. @@ -1193,7 +1011,12 @@ 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.stacked_borrows.as_ref().unwrap().borrow(); + let stacks = alloc_extra + .borrow_tracker + .as_ref() + .expect("We should have borrow tracking data") + .assert_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/stacked_borrows/stack.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs similarity index 96% rename from src/tools/miri/src/stacked_borrows/stack.rs rename to src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs index 51a6fba6df01..1d5cfec3500a 100644 --- a/src/tools/miri/src/stacked_borrows/stack.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs @@ -3,11 +3,14 @@ use std::ops::Range; use rustc_data_structures::fx::FxHashSet; -use crate::stacked_borrows::{AccessKind, Item, Permission, SbTag}; +use crate::borrow_tracker::{ + stacked_borrows::{Item, Permission}, + AccessKind, BorTag, +}; use crate::ProvenanceExtra; /// Exactly what cache size we should use is a difficult tradeoff. There will always be some -/// workload which has a `SbTag` working set which exceeds the size of the cache, and ends up +/// workload which has a `BorTag` working set which exceeds the size of the cache, and ends up /// falling back to linear searches of the borrow stack very often. /// The cost of making this value too large is that the loop in `Stack::insert` which ensures the /// entries in the cache stay correct after an insert becomes expensive. @@ -28,7 +31,7 @@ pub struct Stack { /// than `id`. /// When the bottom is unknown, `borrows` always has a `SharedReadOnly` or `Unique` at the bottom; /// we never have the unknown-to-known boundary in an SRW group. - unknown_bottom: Option, + unknown_bottom: Option, /// A small LRU cache of searches of the borrow stack. #[cfg(feature = "stack-cache")] @@ -40,7 +43,7 @@ pub struct Stack { } impl Stack { - pub fn retain(&mut self, tags: &FxHashSet) { + pub fn retain(&mut self, tags: &FxHashSet) { let mut first_removed = None; // We never consider removing the bottom-most tag. For stacks without an unknown @@ -185,7 +188,7 @@ impl<'tcx> Stack { &mut self, access: AccessKind, tag: ProvenanceExtra, - exposed_tags: &FxHashSet, + exposed_tags: &FxHashSet, ) -> Result, ()> { #[cfg(all(feature = "stack-cache", debug_assertions))] self.verify_cache_consistency(); @@ -219,12 +222,12 @@ impl<'tcx> Stack { // Couldn't find it in the stack; but if there is an unknown bottom it might be there. let found = self.unknown_bottom.is_some_and(|unknown_limit| { - tag.0 < unknown_limit.0 // unknown_limit is an upper bound for what can be in the unknown bottom. + tag < unknown_limit // unknown_limit is an upper bound for what can be in the unknown bottom. }); if found { Ok(None) } else { Err(()) } } - fn find_granting_tagged(&mut self, access: AccessKind, tag: SbTag) -> Option { + fn find_granting_tagged(&mut self, access: AccessKind, tag: BorTag) -> Option { #[cfg(feature = "stack-cache")] if let Some(idx) = self.find_granting_cache(access, tag) { return Some(idx); @@ -243,7 +246,7 @@ impl<'tcx> Stack { } #[cfg(feature = "stack-cache")] - fn find_granting_cache(&mut self, access: AccessKind, tag: SbTag) -> Option { + fn find_granting_cache(&mut self, access: AccessKind, tag: BorTag) -> Option { // This looks like a common-sense optimization; we're going to do a linear search of the // cache or the borrow stack to scan the shorter of the two. This optimization is miniscule // and this check actually ensures we do not access an invalid cache. @@ -349,11 +352,11 @@ impl<'tcx> Stack { self.borrows.len() } - pub fn unknown_bottom(&self) -> Option { + pub fn unknown_bottom(&self) -> Option { self.unknown_bottom } - pub fn set_unknown_bottom(&mut self, tag: SbTag) { + pub fn set_unknown_bottom(&mut self, tag: BorTag) { // We clear the borrow stack but the lookup cache doesn't support clearing per se. Instead, // there is a check explained in `find_granting_cache` which protects against accessing the // cache when it has been cleared and not yet refilled. From 3a0149343326b5002a2a5623d8f5360324bbe302 Mon Sep 17 00:00:00 2001 From: Vanille-N Date: Fri, 2 Dec 2022 13:27:41 +0100 Subject: [PATCH 3/5] SbTag -> BorTag everywhere --- src/tools/miri/src/bin/miri.rs | 2 +- src/tools/miri/src/concurrency/data_race.rs | 4 +- src/tools/miri/src/concurrency/init_once.rs | 2 +- src/tools/miri/src/concurrency/sync.rs | 2 +- src/tools/miri/src/concurrency/thread.rs | 6 +-- src/tools/miri/src/concurrency/weak_memory.rs | 2 +- src/tools/miri/src/eval.rs | 4 +- src/tools/miri/src/intptrcast.rs | 4 +- src/tools/miri/src/machine.rs | 36 ++++++++--------- src/tools/miri/src/shims/env.rs | 2 +- src/tools/miri/src/shims/panic.rs | 2 +- src/tools/miri/src/shims/time.rs | 2 +- src/tools/miri/src/shims/tls.rs | 2 +- src/tools/miri/src/shims/unix/fs.rs | 4 +- src/tools/miri/src/shims/unix/linux/sync.rs | 2 +- src/tools/miri/src/shims/unix/sync.rs | 2 +- src/tools/miri/src/shims/windows/sync.rs | 6 +-- src/tools/miri/src/tag_gc.rs | 40 +++++++++---------- 18 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index ffe89921d986..8d3c53590122 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -413,7 +413,7 @@ fn main() { err ), }; - for id in ids.into_iter().map(miri::SbTag::new) { + for id in ids.into_iter().map(miri::BorTag::new) { if let Some(id) = id { miri_config.tracked_pointer_tags.insert(id); } else { diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index d669cc1362a9..99288480386e 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -670,7 +670,7 @@ pub struct VClockAlloc { } impl VisitTags for VClockAlloc { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { // No tags here. } } @@ -1220,7 +1220,7 @@ pub struct GlobalState { } impl VisitTags for GlobalState { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { // We don't have any tags. } } diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index eb42cdf80abb..9c9d505297c2 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -45,7 +45,7 @@ pub(super) struct InitOnce<'mir, 'tcx> { } impl<'mir, 'tcx> VisitTags for InitOnce<'mir, 'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { for waiter in self.waiters.iter() { waiter.callback.visit_tags(visit); } diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index dc4b435b7101..402c9ce6fc9a 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -181,7 +181,7 @@ pub(crate) struct SynchronizationState<'mir, 'tcx> { } impl<'mir, 'tcx> VisitTags for SynchronizationState<'mir, 'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { for init_once in self.init_onces.iter() { init_once.visit_tags(visit); } diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 900d24443cc9..6ba93a13aaf2 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -212,7 +212,7 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> { } impl VisitTags for Thread<'_, '_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Thread { panic_payload, last_error, @@ -233,7 +233,7 @@ impl VisitTags for Thread<'_, '_> { } impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Frame { return_place, locals, @@ -316,7 +316,7 @@ pub struct ThreadManager<'mir, 'tcx> { } impl VisitTags for ThreadManager<'_, '_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let ThreadManager { threads, thread_local_alloc_ids, diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index f2a365729549..4c32efcfa351 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -109,7 +109,7 @@ pub struct StoreBufferAlloc { } impl VisitTags for StoreBufferAlloc { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Self { store_buffers } = self; for val in store_buffers .borrow() diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index b8578b1277f5..91354d35b233 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -103,7 +103,7 @@ pub struct MiriConfig { /// The seed to use when non-determinism or randomness are required (e.g. ptr-to-int cast, `getrandom()`). pub seed: Option, /// The stacked borrows pointer ids to report about - pub tracked_pointer_tags: FxHashSet, + pub tracked_pointer_tags: FxHashSet, /// The stacked borrows call IDs to report about pub tracked_call_ids: FxHashSet, /// The allocation ids to report about. @@ -138,7 +138,7 @@ pub struct MiriConfig { /// The location of a shared object file to load when calling external functions /// FIXME! consider allowing users to specify paths to multiple SO files, or to a directory pub external_so_file: Option, - /// Run a garbage collector for SbTags every N basic blocks. + /// Run a garbage collector for BorTags every N basic blocks. pub gc_interval: u32, /// The number of CPUs to be reported by miri. pub num_cpus: u32, diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs index 9722b7643e42..0f4f489409a0 100644 --- a/src/tools/miri/src/intptrcast.rs +++ b/src/tools/miri/src/intptrcast.rs @@ -45,7 +45,7 @@ pub struct GlobalStateInner { } impl VisitTags for GlobalStateInner { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { // Nothing to visit here. } } @@ -105,7 +105,7 @@ impl<'mir, 'tcx> GlobalStateInner { pub fn expose_ptr( ecx: &mut MiriInterpCx<'mir, 'tcx>, alloc_id: AllocId, - sb: SbTag, + tag: BorTag, ) -> InterpResult<'tcx> { let global_state = ecx.machine.intptrcast.get_mut(); // In strict mode, we don't need this, so we can save some cycles by not tracking it. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 4d3444bc39cd..770284372873 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -147,7 +147,7 @@ pub enum Provenance { Concrete { alloc_id: AllocId, /// Stacked Borrows tag. - sb: SbTag, + tag: BorTag, }, Wildcard, } @@ -173,7 +173,7 @@ impl std::hash::Hash for Provenance { /// The "extra" information a pointer has over a regular AllocId. #[derive(Copy, Clone, PartialEq)] pub enum ProvenanceExtra { - Concrete(SbTag), + Concrete(BorTag), Wildcard, } @@ -188,7 +188,7 @@ static_assert_size!(Scalar, 32); impl fmt::Debug for Provenance { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Provenance::Concrete { alloc_id, sb } => { + Provenance::Concrete { alloc_id, tag } => { // Forward `alternate` flag to `alloc_id` printing. if f.alternate() { write!(f, "[{alloc_id:#?}]")?; @@ -196,7 +196,7 @@ impl fmt::Debug for Provenance { write!(f, "[{alloc_id:?}]")?; } // Print Stacked Borrows tag. - write!(f, "{sb:?}")?; + write!(f, "{tag:?}")?; } Provenance::Wildcard => { write!(f, "[wildcard]")?; @@ -221,9 +221,9 @@ impl interpret::Provenance for Provenance { match (left, right) { // If both are the *same* concrete tag, that is the result. ( - Some(Provenance::Concrete { alloc_id: left_alloc, sb: left_sb }), - Some(Provenance::Concrete { alloc_id: right_alloc, sb: right_sb }), - ) if left_alloc == right_alloc && left_sb == right_sb => left, + Some(Provenance::Concrete { alloc_id: left_alloc, tag: left_tag }), + Some(Provenance::Concrete { alloc_id: right_alloc, tag: right_tag }), + ) if left_alloc == right_alloc && left_tag == right_tag => left, // If one side is a wildcard, the best possible outcome is that it is equal to the other // one, and we use that. (Some(Provenance::Wildcard), o) | (o, Some(Provenance::Wildcard)) => o, @@ -243,7 +243,7 @@ impl fmt::Debug for ProvenanceExtra { } impl ProvenanceExtra { - pub fn and_then(self, f: impl FnOnce(SbTag) -> Option) -> Option { + pub fn and_then(self, f: impl FnOnce(BorTag) -> Option) -> Option { match self { ProvenanceExtra::Concrete(pid) => f(pid), ProvenanceExtra::Wildcard => None, @@ -463,9 +463,9 @@ pub struct MiriMachine<'mir, 'tcx> { #[cfg(not(target_os = "linux"))] pub external_so_lib: Option, - /// Run a garbage collector for SbTags every N basic blocks. + /// Run a garbage collector for BorTags every N basic blocks. pub(crate) gc_interval: u32, - /// The number of blocks that passed since the last SbTag GC pass. + /// The number of blocks that passed since the last BorTag GC pass. pub(crate) since_gc: u32, /// The number of CPUs to be reported by miri. pub(crate) num_cpus: u32, @@ -656,7 +656,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { } impl VisitTags for MiriMachine<'_, '_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { #[rustfmt::skip] let MiriMachine { threads, @@ -959,10 +959,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { stacked_borrows.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine) } else { // Value does not matter, SB is disabled - SbTag::default() + BorTag::default() }; Pointer::new( - Provenance::Concrete { alloc_id: ptr.provenance, sb: sb_tag }, + Provenance::Concrete { alloc_id: ptr.provenance, tag }, Size::from_bytes(absolute_addr), ) } @@ -980,8 +980,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ptr: Pointer, ) -> InterpResult<'tcx> { match ptr.provenance { - Provenance::Concrete { alloc_id, sb } => - intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb), + Provenance::Concrete { alloc_id, tag } => + intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag), Provenance::Wildcard => { // No need to do anything for wildcard pointers as // their provenances have already been previously exposed. @@ -999,11 +999,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let rel = intptrcast::GlobalStateInner::abs_ptr_to_rel(ecx, ptr); rel.map(|(alloc_id, size)| { - let sb = match ptr.provenance { - Provenance::Concrete { sb, .. } => ProvenanceExtra::Concrete(sb), + let tag = match ptr.provenance { + Provenance::Concrete { tag, .. } => ProvenanceExtra::Concrete(tag), Provenance::Wildcard => ProvenanceExtra::Wildcard, }; - (alloc_id, size, sb) + (alloc_id, size, tag) }) } diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index bf6c1f875629..80fb4ff2fe98 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -37,7 +37,7 @@ pub struct EnvVars<'tcx> { } impl VisitTags for EnvVars<'_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let EnvVars { map, environ } = self; environ.visit_tags(visit); diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 698e025961da..7c30845dc675 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -36,7 +36,7 @@ pub struct CatchUnwindData<'tcx> { } impl VisitTags for CatchUnwindData<'_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let CatchUnwindData { catch_fn, data, dest, ret: _ } = self; catch_fn.visit_tags(visit); data.visit_tags(visit); diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index bc0b71fbc209..d263aab351b1 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -278,7 +278,7 @@ struct UnblockCallback { } impl VisitTags for UnblockCallback { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {} + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {} } impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for UnblockCallback { diff --git a/src/tools/miri/src/shims/tls.rs b/src/tools/miri/src/shims/tls.rs index fe278ff717f0..54fdf2872ab4 100644 --- a/src/tools/miri/src/shims/tls.rs +++ b/src/tools/miri/src/shims/tls.rs @@ -208,7 +208,7 @@ impl<'tcx> TlsData<'tcx> { } impl VisitTags for TlsData<'_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let TlsData { keys, macos_thread_dtors, next_key: _ } = self; for scalar in keys.values().flat_map(|v| v.data.values()) { diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index e048d53a17e0..988627db5611 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -278,7 +278,7 @@ pub struct FileHandler { } impl VisitTags for FileHandler { - fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) { // All our FileDescriptor do not have any tags. } } @@ -490,7 +490,7 @@ impl Default for DirHandler { } impl VisitTags for DirHandler { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let DirHandler { streams, next_id: _ } = self; for dir in streams.values() { diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs index 292b9d2e7a17..343232c4bbb2 100644 --- a/src/tools/miri/src/shims/unix/linux/sync.rs +++ b/src/tools/miri/src/shims/unix/linux/sync.rs @@ -183,7 +183,7 @@ pub fn futex<'tcx>( } impl<'tcx> VisitTags for Callback<'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Callback { thread: _, addr_usize: _, dest } = self; dest.visit_tags(visit); } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index e0afb500cb18..f9b5774f0090 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -747,7 +747,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'tcx> VisitTags for Callback<'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Callback { active_thread: _, mutex_id: _, id: _, dest } = self; dest.visit_tags(visit); } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 8f414d98dba5..6b043c6d2c9e 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -182,7 +182,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'tcx> VisitTags for Callback<'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Callback { init_once_id: _, pending_place } = self; pending_place.visit_tags(visit); } @@ -315,7 +315,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'tcx> VisitTags for Callback<'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Callback { thread: _, addr: _, dest } = self; dest.visit_tags(visit); } @@ -419,7 +419,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'tcx> VisitTags for Callback<'tcx> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let Callback { thread: _, condvar_id: _, lock_id: _, mode: _, dest } = self; dest.visit_tags(visit); } diff --git a/src/tools/miri/src/tag_gc.rs b/src/tools/miri/src/tag_gc.rs index 73712348f0d5..b460122196a5 100644 --- a/src/tools/miri/src/tag_gc.rs +++ b/src/tools/miri/src/tag_gc.rs @@ -3,11 +3,11 @@ use rustc_data_structures::fx::FxHashSet; use crate::*; pub trait VisitTags { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)); + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)); } impl VisitTags for Option { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { if let Some(x) = self { x.visit_tags(visit); } @@ -15,41 +15,41 @@ impl VisitTags for Option { } impl VisitTags for std::cell::RefCell { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { self.borrow().visit_tags(visit) } } -impl VisitTags for SbTag { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { +impl VisitTags for BorTag { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { visit(*self) } } impl VisitTags for Provenance { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - if let Provenance::Concrete { sb, .. } = self { - visit(*sb); + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { + if let Provenance::Concrete { tag, .. } = self { + visit(*tag); } } } impl VisitTags for Pointer { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let (prov, _offset) = self.into_parts(); prov.visit_tags(visit); } } impl VisitTags for Pointer> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let (prov, _offset) = self.into_parts(); prov.visit_tags(visit); } } impl VisitTags for Scalar { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { Scalar::Ptr(ptr, _) => ptr.visit_tags(visit), Scalar::Int(_) => (), @@ -58,7 +58,7 @@ impl VisitTags for Scalar { } impl VisitTags for Immediate { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { Immediate::Scalar(s) => { s.visit_tags(visit); @@ -73,7 +73,7 @@ impl VisitTags for Immediate { } impl VisitTags for MemPlaceMeta { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { MemPlaceMeta::Meta(m) => m.visit_tags(visit), MemPlaceMeta::None => {} @@ -82,7 +82,7 @@ impl VisitTags for MemPlaceMeta { } impl VisitTags for MemPlace { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { let MemPlace { ptr, meta } = self; ptr.visit_tags(visit); meta.visit_tags(visit); @@ -90,13 +90,13 @@ impl VisitTags for MemPlace { } impl VisitTags for MPlaceTy<'_, Provenance> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { (**self).visit_tags(visit) } } impl VisitTags for Place { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { Place::Ptr(p) => p.visit_tags(visit), Place::Local { .. } => { @@ -107,13 +107,13 @@ impl VisitTags for Place { } impl VisitTags for PlaceTy<'_, Provenance> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { (**self).visit_tags(visit) } } impl VisitTags for Operand { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { match self { Operand::Immediate(imm) => { imm.visit_tags(visit); @@ -126,7 +126,7 @@ impl VisitTags for Operand { } impl VisitTags for Allocation { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { for prov in self.provenance().provenances() { prov.visit_tags(visit); } @@ -136,7 +136,7 @@ impl VisitTags for Allocation { } impl VisitTags for crate::MiriInterpCx<'_, '_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { // Memory. self.memory.alloc_map().iter(|it| { for (_id, (_kind, alloc)) in it { From 8bb3d9e94a74dec560e520659c45c82ab3ba0566 Mon Sep 17 00:00:00 2001 From: Vanille-N Date: Fri, 2 Dec 2022 13:29:48 +0100 Subject: [PATCH 4/5] other renames, introduction of BorrowTrackerMethod and AllocExtra --- src/tools/miri/src/bin/miri.rs | 2 +- src/tools/miri/src/eval.rs | 4 +- src/tools/miri/src/intptrcast.rs | 4 +- src/tools/miri/src/machine.rs | 108 +++++++++++++------------------ src/tools/miri/src/tag_gc.rs | 14 ++-- 5 files changed, 56 insertions(+), 76 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 8d3c53590122..9ac04c4930f2 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -317,7 +317,7 @@ fn main() { } else if arg == "-Zmiri-disable-validation" { miri_config.validate = false; } else if arg == "-Zmiri-disable-stacked-borrows" { - miri_config.stacked_borrows = false; + miri_config.borrow_tracker = None; } else if arg == "-Zmiri-disable-data-race-detector" { miri_config.data_race_detector = false; miri_config.weak_memory_emulation = false; diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 91354d35b233..6c9c96eef623 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -87,7 +87,7 @@ pub struct MiriConfig { /// Determine if validity checking is enabled. pub validate: bool, /// Determines if Stacked Borrows is enabled. - pub stacked_borrows: bool, + pub borrow_tracker: Option, /// Controls alignment checking. pub check_alignment: AlignmentCheck, /// Controls function [ABI](Abi) checking. @@ -149,7 +149,7 @@ impl Default for MiriConfig { MiriConfig { env: vec![], validate: true, - stacked_borrows: true, + borrow_tracker: Some(BorrowTrackerMethod::StackedBorrows), check_alignment: AlignmentCheck::Int, check_abi: true, isolated_op: IsolatedOp::Reject(RejectOpWith::Abort), diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs index 0f4f489409a0..c26828b11e0e 100644 --- a/src/tools/miri/src/intptrcast.rs +++ b/src/tools/miri/src/intptrcast.rs @@ -112,8 +112,8 @@ impl<'mir, 'tcx> GlobalStateInner { if global_state.provenance_mode != ProvenanceMode::Strict { trace!("Exposing allocation id {alloc_id:?}"); global_state.exposed.insert(alloc_id); - if ecx.machine.stacked_borrows.is_some() { - ecx.expose_tag(alloc_id, sb)?; + if ecx.machine.borrow_tracker.is_some() { + ecx.expose_tag(alloc_id, tag)?; } } Ok(()) diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 770284372873..920ee3e1ef1f 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -39,7 +39,7 @@ pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever /// Extra data stored with each stack frame pub struct FrameData<'tcx> { /// Extra data for Stacked Borrows. - pub stacked_borrows: 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, @@ -61,20 +61,20 @@ pub struct FrameData<'tcx> { impl<'tcx> std::fmt::Debug for FrameData<'tcx> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Omitting `timing`, it does not support `Debug`. - let FrameData { stacked_borrows, catch_unwind, timing: _, is_user_relevant: _ } = self; + let FrameData { borrow_tracker, catch_unwind, timing: _, is_user_relevant: _ } = self; f.debug_struct("FrameData") - .field("stacked_borrows", stacked_borrows) + .field("borrow_tracker", borrow_tracker) .field("catch_unwind", catch_unwind) .finish() } } impl VisitTags for FrameData<'_> { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - let FrameData { catch_unwind, stacked_borrows, timing: _, is_user_relevant: _ } = self; + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { + let FrameData { catch_unwind, borrow_tracker, timing: _, is_user_relevant: _ } = self; catch_unwind.visit_tags(visit); - stacked_borrows.visit_tags(visit); + borrow_tracker.visit_tags(visit); } } @@ -254,8 +254,8 @@ impl ProvenanceExtra { /// Extra per-allocation data #[derive(Debug, Clone)] pub struct AllocExtra { - /// Stacked Borrows state is only added if it is enabled. - pub stacked_borrows: Option, + /// Global state of the borrow tracker, if enabled. + 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, @@ -265,10 +265,10 @@ pub struct AllocExtra { } impl VisitTags for AllocExtra { - fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - let AllocExtra { stacked_borrows, data_race, weak_memory } = self; + fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { + let AllocExtra { borrow_tracker, data_race, weak_memory } = self; - stacked_borrows.visit_tags(visit); + borrow_tracker.visit_tags(visit); data_race.visit_tags(visit); weak_memory.visit_tags(visit); } @@ -350,8 +350,8 @@ pub struct MiriMachine<'mir, 'tcx> { // We carry a copy of the global `TyCtxt` for convenience, so methods taking just `&Evaluator` have `tcx` access. pub tcx: TyCtxt<'tcx>, - /// Stacked Borrows global data. - pub stacked_borrows: Option, + /// Global data for borrow tracking. + pub borrow_tracker: Option, /// Data race detector global data. pub data_race: Option, @@ -480,17 +480,11 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { measureme::Profiler::new(out).expect("Couldn't create `measureme` profiler") }); let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0)); - let stacked_borrows = config.stacked_borrows.then(|| { - RefCell::new(stacked_borrows::GlobalStateInner::new( - config.tracked_pointer_tags.clone(), - config.tracked_call_ids.clone(), - config.retag_fields, - )) - }); + let borrow_tracker = config.borrow_tracker.map(|bt| bt.instanciate_global_state(config)); let data_race = config.data_race_detector.then(|| data_race::GlobalState::new(config)); MiriMachine { tcx: layout_cx.tcx, - stacked_borrows, + borrow_tracker, data_race, intptrcast: RefCell::new(intptrcast::GlobalStateInner::new(config)), // `env_vars` depends on a full interpreter so we cannot properly initialize it yet. @@ -668,7 +662,7 @@ impl VisitTags for MiriMachine<'_, '_> { cmd_line, extern_statics, dir_handler, - stacked_borrows, + borrow_tracker, data_race, intptrcast, file_handler, @@ -706,7 +700,7 @@ impl VisitTags for MiriMachine<'_, '_> { dir_handler.visit_tags(visit); file_handler.visit_tags(visit); data_race.visit_tags(visit); - stacked_borrows.visit_tags(visit); + borrow_tracker.visit_tags(visit); intptrcast.visit_tags(visit); main_fn_ret_place.visit_tags(visit); argc.visit_tags(visit); @@ -907,15 +901,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } let alloc = alloc.into_owned(); - let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| { - stacked_borrows::Stacks::new_allocation( - id, - alloc.size(), - stacked_borrows, - kind, - &ecx.machine, - ) - }); + let borrow_tracker = ecx + .machine + .borrow_tracker + .as_ref() + .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, @@ -927,11 +918,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocExtra::new_allocation); let alloc: Allocation = alloc.adjust_from_tcx( &ecx.tcx, - AllocExtra { - stacked_borrows: stacks.map(RefCell::new), - data_race: race_alloc, - weak_memory: buffer_alloc, - }, + AllocExtra { borrow_tracker, data_race: race_alloc, weak_memory: buffer_alloc }, |ptr| ecx.global_base_pointer(ptr), )?; Ok(Cow::Owned(alloc)) @@ -955,8 +942,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } } let absolute_addr = intptrcast::GlobalStateInner::rel_ptr_to_addr(ecx, ptr); - let sb_tag = if let Some(stacked_borrows) = &ecx.machine.stacked_borrows { - stacked_borrows.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine) + let tag = if let Some(borrow_tracker) = &ecx.machine.borrow_tracker { + borrow_tracker.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine) } else { // Value does not matter, SB is disabled BorTag::default() @@ -1018,10 +1005,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { if let Some(data_race) = &alloc_extra.data_race { data_race.read(alloc_id, range, machine)?; } - if let Some(stacked_borrows) = &alloc_extra.stacked_borrows { - stacked_borrows - .borrow_mut() - .before_memory_read(alloc_id, prov_extra, range, machine)?; + if let Some(borrow_tracker) = &alloc_extra.borrow_tracker { + borrow_tracker.before_memory_read(alloc_id, prov_extra, range, machine)?; } if let Some(weak_memory) = &alloc_extra.weak_memory { weak_memory.memory_accessed(range, machine.data_race.as_ref().unwrap()); @@ -1040,8 +1025,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { if let Some(data_race) = &mut alloc_extra.data_race { data_race.write(alloc_id, range, machine)?; } - if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { - stacked_borrows.get_mut().before_memory_write(alloc_id, prov_extra, range, machine)?; + if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { + borrow_tracker.before_memory_write(alloc_id, prov_extra, range, machine)?; } if let Some(weak_memory) = &alloc_extra.weak_memory { weak_memory.memory_accessed(range, machine.data_race.as_ref().unwrap()); @@ -1063,16 +1048,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { if let Some(data_race) = &mut alloc_extra.data_race { data_race.deallocate(alloc_id, range, machine)?; } - if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { - stacked_borrows.get_mut().before_memory_deallocation( - alloc_id, - prove_extra, - range, - machine, - ) - } else { - Ok(()) + if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { + borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, range, machine)?; } + Ok(()) } #[inline(always)] @@ -1081,7 +1060,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { kind: mir::RetagKind, place: &PlaceTy<'tcx, Provenance>, ) -> InterpResult<'tcx> { - if ecx.machine.stacked_borrows.is_some() { ecx.retag(kind, place) } else { Ok(()) } + if ecx.machine.borrow_tracker.is_some() { + ecx.retag(kind, place)?; + } + Ok(()) } #[inline(always)] @@ -1104,10 +1086,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { None }; - let stacked_borrows = ecx.machine.stacked_borrows.as_ref(); + let borrow_tracker = ecx.machine.borrow_tracker.as_ref(); let extra = FrameData { - stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame(&ecx.machine)), + borrow_tracker: borrow_tracker.map(|bt| bt.borrow_mut().new_frame(&ecx.machine)), catch_unwind: None, timing, is_user_relevant: ecx.machine.is_user_relevant(&frame), @@ -1140,7 +1122,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } } - // Search for SbTags to find all live pointers, then remove all other tags from borrow + // Search for BorTags to find all live pointers, then remove all other tags from borrow // stacks. // When debug assertions are enabled, run the GC as often as possible so that any cases // where it mistakenly removes an important tag become visible. @@ -1166,8 +1148,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let stack_len = ecx.active_thread_stack().len(); ecx.active_thread_mut().set_top_user_relevant_frame(stack_len - 1); } - - if ecx.machine.stacked_borrows.is_some() { ecx.retag_return_place() } else { Ok(()) } + if ecx.machine.borrow_tracker.is_some() { + ecx.retag_return_place()?; + } + Ok(()) } #[inline(always)] @@ -1184,8 +1168,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ecx.active_thread_mut().recompute_top_user_relevant_frame(); } let timing = frame.extra.timing.take(); - if let Some(stacked_borrows) = &ecx.machine.stacked_borrows { - stacked_borrows.borrow_mut().end_call(&frame.extra); + if let Some(borrow_tracker) = &ecx.machine.borrow_tracker { + borrow_tracker.borrow_mut().end_call(&frame.extra); } let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding); if let Some(profiler) = ecx.machine.profiler.as_ref() { diff --git a/src/tools/miri/src/tag_gc.rs b/src/tools/miri/src/tag_gc.rs index b460122196a5..c1194fe22163 100644 --- a/src/tools/miri/src/tag_gc.rs +++ b/src/tools/miri/src/tag_gc.rs @@ -154,7 +154,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { fn garbage_collect_tags(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); // No reason to do anything at all if stacked borrows is off. - if this.machine.stacked_borrows.is_none() { + if this.machine.borrow_tracker.is_none() { return Ok(()); } @@ -167,17 +167,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { Ok(()) } - fn remove_unreachable_tags(&mut self, tags: FxHashSet) { + fn remove_unreachable_tags(&mut self, tags: FxHashSet) { let this = self.eval_context_mut(); this.memory.alloc_map().iter(|it| { for (_id, (_kind, alloc)) in it { - alloc - .extra - .stacked_borrows - .as_ref() - .unwrap() - .borrow_mut() - .remove_unreachable_tags(&tags); + if let Some(bt) = &alloc.extra.borrow_tracker { + bt.remove_unreachable_tags(&tags); + } } }); } From ab08f2a81301d535dfbaafe2d846ecfad1ed8688 Mon Sep 17 00:00:00 2001 From: Vanille-N Date: Fri, 2 Dec 2022 13:30:06 +0100 Subject: [PATCH 5/5] fix imports --- src/tools/miri/src/diagnostics.rs | 2 +- src/tools/miri/src/eval.rs | 1 + src/tools/miri/src/lib.rs | 11 +++++++---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 5cd0a0eeb58c..bc771ca057f0 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -6,7 +6,7 @@ use log::trace; use rustc_span::{source_map::DUMMY_SP, SpanData, Symbol}; use rustc_target::abi::{Align, Size}; -use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind}; +use crate::borrow_tracker::{stacked_borrows::diagnostics::TagHistory, AccessKind}; use crate::*; /// Details of premature program termination. diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 6c9c96eef623..7b4973f3b9da 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -9,6 +9,7 @@ use std::thread; use log::info; +use crate::borrow_tracker::RetagFields; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::Namespace; use rustc_hir::def_id::DefId; diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 6f483cf2cc48..2c427c166c16 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -53,6 +53,7 @@ extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; +mod borrow_tracker; mod clock; mod concurrency; mod diagnostics; @@ -64,7 +65,6 @@ mod mono_hash_map; mod operator; mod range_map; mod shims; -mod stacked_borrows; mod tag_gc; // Establish a "crate-wide prelude": we often import `crate::*`. @@ -84,6 +84,12 @@ pub use crate::shims::time::EvalContextExt as _; pub use crate::shims::tls::TlsData; pub use crate::shims::EvalContextExt as _; +pub use crate::borrow_tracker::stacked_borrows::{ + EvalContextExt as _, Item, Permission, Stack, Stacks, +}; +pub use crate::borrow_tracker::{ + BorTag, BorrowTrackerMethod, CallId, EvalContextExt as _, RetagFields, +}; pub use crate::clock::{Clock, Instant}; pub use crate::concurrency::{ data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _}, @@ -106,9 +112,6 @@ pub use crate::machine::{ pub use crate::mono_hash_map::MonoHashMap; pub use crate::operator::EvalContextExt as _; pub use crate::range_map::RangeMap; -pub use crate::stacked_borrows::{ - CallId, EvalContextExt as _, Item, Permission, RetagFields, SbTag, -}; pub use crate::tag_gc::{EvalContextExt as _, VisitTags}; /// Insert rustc arguments at the beginning of the argument list that Miri wants to be