From 0a885b6b7aed5e73cc9519048ee15d203e95f4ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 13 Nov 2023 20:39:17 +0100 Subject: [PATCH] don't expose all the borrow tracker stuff to the entire crate --- src/borrow_tracker/mod.rs | 58 ++++++++++++++++++++++++++++++--------- src/machine.rs | 30 ++------------------ 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/borrow_tracker/mod.rs b/src/borrow_tracker/mod.rs index a95571572d..f9cc3acbd5 100644 --- a/src/borrow_tracker/mod.rs +++ b/src/borrow_tracker/mod.rs @@ -59,7 +59,7 @@ impl fmt::Debug for BorTag { #[derive(Debug)] pub struct FrameState { /// The ID of the call this frame corresponds to. - pub call_id: CallId, + 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 @@ -72,7 +72,7 @@ pub struct FrameState { /// /// 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<[(AllocId, BorTag); 2]>, + protected_tags: SmallVec<[(AllocId, BorTag); 2]>, } impl VisitTags for FrameState { @@ -85,29 +85,29 @@ impl VisitTags for FrameState { #[derive(Debug)] pub struct GlobalStateInner { /// Borrow tracker method currently in use. - pub borrow_tracker_method: BorrowTrackerMethod, + borrow_tracker_method: BorrowTrackerMethod, /// Next unused pointer ID (tag). - pub next_ptr_tag: BorTag, + 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, + base_ptr_tags: FxHashMap, /// Next unused call ID (for protectors). - pub next_call_id: CallId, + 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, + protected_tags: FxHashMap, /// The pointer ids to trace - pub tracked_pointer_tags: FxHashSet, + tracked_pointer_tags: FxHashSet, /// The call ids to trace - pub tracked_call_ids: FxHashSet, + tracked_call_ids: FxHashSet, /// Whether to recurse into datatypes when searching for pointers to retag. - pub retag_fields: RetagFields, + retag_fields: RetagFields, /// Whether `core::ptr::Unique` gets special (`Box`-like) handling. - pub unique_is_unique: bool, + unique_is_unique: bool, } impl VisitTags for GlobalStateInner { @@ -194,7 +194,7 @@ impl GlobalStateInner { } /// Generates a new pointer tag. Remember to also check track_pointer_tags and log its creation! - pub fn new_ptr(&mut self) -> BorTag { + fn new_ptr(&mut self) -> BorTag { let id = self.next_ptr_tag; self.next_ptr_tag = id.succ().unwrap(); id @@ -210,7 +210,7 @@ impl GlobalStateInner { FrameState { call_id, protected_tags: SmallVec::new() } } - pub fn end_call(&mut self, frame: &machine::FrameExtra<'_>) { + fn end_call(&mut self, frame: &machine::FrameExtra<'_>) { for (_, tag) in &frame .borrow_tracker .as_ref() @@ -355,6 +355,38 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { BorrowTrackerMethod::TreeBorrows => this.print_tree(alloc_id, show_unnamed), } } + + fn on_stack_pop( + &self, + frame: &Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>, + ) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap(); + // The body of this loop needs `borrow_tracker` immutably + // so we can't move this code inside the following `end_call`. + for (alloc_id, tag) in &frame + .extra + .borrow_tracker + .as_ref() + .expect("we should have borrow tracking data") + .protected_tags + { + // Just because the tag is protected doesn't guarantee that + // the allocation still exists (weak protectors allow deallocations) + // so we must check that the allocation exists. + // If it does exist, then we have the guarantee that the + // pointer is readable, and the implicit read access inserted + // will never cause UB on the pointer itself. + let (_, _, kind) = this.get_alloc_info(*alloc_id); + if matches!(kind, AllocKind::LiveData) { + let alloc_extra = this.get_alloc_extra(*alloc_id).unwrap(); + let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap(); + alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?; + } + } + borrow_tracker.borrow_mut().end_call(&frame.extra); + Ok(()) + } } /// Extra per-allocation data for borrow tracking diff --git a/src/machine.rs b/src/machine.rs index d5775912ea..2085df7d06 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -1409,34 +1409,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ) -> InterpResult<'tcx> { // We want this *before* the return value copy, because the return place itself is protected // until we do `end_call` here. - if let Some(global_borrow_tracker) = &ecx.machine.borrow_tracker { - // The body of this loop needs `global_borrow_tracker` immutably - // so we can't move this code inside the following `end_call`. - for (alloc_id, tag) in &frame - .extra - .borrow_tracker - .as_ref() - .expect("we should have borrow tracking data") - .protected_tags - { - // Just because the tag is protected doesn't guarantee that - // the allocation still exists (weak protectors allow deallocations) - // so we must check that the allocation exists. - // If it does exist, then we have the guarantee that the - // pointer is readable, and the implicit read access inserted - // will never cause UB on the pointer itself. - let (_, _, kind) = ecx.get_alloc_info(*alloc_id); - if matches!(kind, AllocKind::LiveData) { - let alloc_extra = ecx.get_alloc_extra(*alloc_id).unwrap(); - let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap(); - alloc_borrow_tracker.release_protector( - &ecx.machine, - global_borrow_tracker, - *tag, - )?; - } - } - global_borrow_tracker.borrow_mut().end_call(&frame.extra); + if ecx.machine.borrow_tracker.is_some() { + ecx.on_stack_pop(frame)?; } Ok(()) }