From b20c6cfd81b8df20e0714e3bc8a6054be682d5f1 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 7 Dec 2021 22:05:13 -0500 Subject: [PATCH] Factor current-span logic into a lazy caching handle --- src/helpers.rs | 44 ++++++++++++++++++++++++++--- src/lib.rs | 2 +- src/machine.rs | 10 +++---- src/stacked_borrows.rs | 45 ++++++++++++++---------------- src/stacked_borrows/diagnostics.rs | 40 +++++++++----------------- 5 files changed, 79 insertions(+), 62 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 6beb3f8c3b..2d1fffc6a1 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -2,7 +2,6 @@ pub mod convert; use std::mem; use std::num::NonZeroUsize; -use std::rc::Rc; use std::time::Duration; use log::trace; @@ -14,7 +13,7 @@ use rustc_middle::ty::{ layout::{LayoutOf, TyAndLayout}, List, TyCtxt, }; -use rustc_span::{def_id::CrateNum, sym, Symbol}; +use rustc_span::{def_id::CrateNum, sym, Span, Symbol}; use rustc_target::abi::{Align, FieldsShape, Size, Variants}; use rustc_target::spec::abi::Abi; @@ -800,6 +799,43 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } +impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { + pub fn current_span(&self) -> CurrentSpan<'_, 'mir, 'tcx> { + CurrentSpan { span: None, machine: self } + } +} + +/// A `CurrentSpan` should be created infrequently (ideally once) per interpreter step. It does +/// nothing on creation, but when `CurrentSpan::get` is called, searches the current stack for the +/// topmost frame which corresponds to a local crate, and returns the current span in that frame. +/// The result of that search is cached so that later calls are approximately free. +#[derive(Clone)] +pub struct CurrentSpan<'a, 'tcx, 'mir> { + span: Option, + machine: &'a Evaluator<'tcx, 'mir>, +} + +impl<'a, 'tcx, 'mir> CurrentSpan<'a, 'tcx, 'mir> { + pub fn get(&mut self) -> Span { + *self.span.get_or_insert_with(|| Self::current_span(&self.machine)) + } + + #[inline(never)] + fn current_span(machine: &Evaluator<'_, '_>) -> Span { + machine + .threads + .active_thread_stack() + .into_iter() + .rev() + .find(|frame| { + let def_id = frame.instance.def_id(); + def_id.is_local() || machine.local_crates.contains(&def_id.krate) + }) + .map(|frame| frame.current_span()) + .unwrap_or(rustc_span::DUMMY_SP) + } +} + /// Check that the number of args is what we expect. pub fn check_arg_count<'a, 'tcx, const N: usize>( args: &'a [OpTy<'tcx, Tag>], @@ -822,7 +858,7 @@ pub fn isolation_abort_error(name: &str) -> InterpResult<'static> { /// Retrieve the list of local crates that should have been passed by cargo-miri in /// MIRI_LOCAL_CRATES and turn them into `CrateNum`s. -pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Rc<[CrateNum]> { +pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Vec { // Convert the local crate names from the passed-in config into CrateNums so that they can // be looked up quickly during execution let local_crate_names = std::env::var("MIRI_LOCAL_CRATES") @@ -836,7 +872,7 @@ pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Rc<[CrateNum]> { local_crates.push(crate_num); } } - Rc::from(local_crates.as_slice()) + local_crates } /// Formats an AllocRange like [0x1..0x3], for use in diagnostics. diff --git a/src/lib.rs b/src/lib.rs index ee7a3bcdc5..9fa2c61fd8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -77,7 +77,7 @@ pub use crate::diagnostics::{ pub use crate::eval::{ create_ecx, eval_entry, AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, RejectOpWith, }; -pub use crate::helpers::EvalContextExt as HelpersEvalContextExt; +pub use crate::helpers::{CurrentSpan, EvalContextExt as HelpersEvalContextExt}; pub use crate::machine::{ AllocExtra, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, Tag, NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE, diff --git a/src/machine.rs b/src/machine.rs index 9e95c632c7..d78b0135e9 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -6,7 +6,6 @@ use std::cell::RefCell; use std::collections::HashSet; use std::fmt; use std::num::NonZeroU64; -use std::rc::Rc; use std::time::Instant; use rand::rngs::StdRng; @@ -278,7 +277,7 @@ pub struct Evaluator<'mir, 'tcx> { pub(crate) backtrace_style: BacktraceStyle, /// Crates which are considered local for the purposes of error reporting. - pub(crate) local_crates: Rc<[CrateNum]>, + pub(crate) local_crates: Vec, /// Mapping extern static names to their base pointer. extern_statics: FxHashMap>, @@ -584,8 +583,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { alloc.size(), stacked_borrows, kind, - &ecx.machine.threads, - ecx.machine.local_crates.clone(), + ecx.machine.current_span(), )) } else { None @@ -667,7 +665,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { tag, range, machine.stacked_borrows.as_ref().unwrap(), - &machine.threads, + machine.current_span(), ) } else { Ok(()) @@ -691,7 +689,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { tag, range, machine.stacked_borrows.as_ref().unwrap(), - &machine.threads, + machine.current_span(), ) } else { Ok(()) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index d7c2139323..625ffb2c5d 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -5,7 +5,6 @@ use log::trace; use std::cell::RefCell; use std::fmt; use std::num::NonZeroU64; -use std::rc::Rc; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::Mutability; @@ -14,7 +13,6 @@ use rustc_middle::ty::{ self, layout::{HasParamEnv, LayoutOf}, }; -use rustc_span::def_id::CrateNum; use rustc_span::DUMMY_SP; use rustc_target::abi::Size; use std::collections::HashSet; @@ -22,9 +20,7 @@ use std::collections::HashSet; use crate::*; pub mod diagnostics; -use diagnostics::AllocHistory; - -use diagnostics::TagHistory; +use diagnostics::{AllocHistory, TagHistory}; pub type PtrId = NonZeroU64; pub type CallId = NonZeroU64; @@ -376,7 +372,7 @@ impl<'tcx> Stack { tag: SbTag, (alloc_id, alloc_range, offset): (AllocId, AllocRange, Size), // just for debug printing and error messages global: &mut GlobalStateInner, - threads: &ThreadManager<'_, 'tcx>, + current_span: &mut CurrentSpan<'_, '_, 'tcx>, alloc_history: &mut AllocHistory, ) -> InterpResult<'tcx> { // Two main steps: Find granting item, remove incompatible items above. @@ -400,7 +396,7 @@ impl<'tcx> Stack { global, alloc_history, )?; - alloc_history.log_invalidation(item.tag, alloc_range, threads); + alloc_history.log_invalidation(item.tag, alloc_range, current_span); } } else { // On a read, *disable* all `Unique` above the granting item. This ensures U2 for read accesses. @@ -422,7 +418,7 @@ impl<'tcx> Stack { alloc_history, )?; item.perm = Permission::Disabled; - alloc_history.log_invalidation(item.tag, alloc_range, threads); + alloc_history.log_invalidation(item.tag, alloc_range, current_span); } } } @@ -471,7 +467,7 @@ impl<'tcx> Stack { new: Item, (alloc_id, alloc_range, offset): (AllocId, AllocRange, Size), // just for debug printing and error messages global: &mut GlobalStateInner, - threads: &ThreadManager<'_, 'tcx>, + current_span: &mut CurrentSpan<'_, '_, 'tcx>, alloc_history: &mut AllocHistory, ) -> InterpResult<'tcx> { // Figure out which access `perm` corresponds to. @@ -505,7 +501,7 @@ impl<'tcx> Stack { derived_from, (alloc_id, alloc_range, offset), global, - threads, + current_span, alloc_history, )?; @@ -533,13 +529,13 @@ impl<'tcx> Stack { /// Map per-stack operations to higher-level per-location-range operations. impl<'tcx> Stacks { /// Creates new stack with initial tag. - fn new(size: Size, perm: Permission, tag: SbTag, local_crates: Rc<[CrateNum]>) -> Self { + fn new(size: Size, perm: Permission, tag: SbTag) -> Self { let item = Item { perm, tag, protector: None }; let stack = Stack { borrows: vec![item] }; Stacks { stacks: RefCell::new(RangeMap::new(size, stack)), - history: RefCell::new(AllocHistory::new(local_crates)), + history: RefCell::new(AllocHistory::new()), } } @@ -579,8 +575,7 @@ impl Stacks { size: Size, state: &GlobalState, kind: MemoryKind, - threads: &ThreadManager<'_, '_>, - local_crates: Rc<[CrateNum]>, + mut current_span: CurrentSpan<'_, '_, '_>, ) -> Self { let mut extra = state.borrow_mut(); let (base_tag, perm) = match kind { @@ -614,12 +609,12 @@ impl Stacks { (tag, Permission::SharedReadWrite) } }; - let stacks = Stacks::new(size, perm, base_tag, local_crates); + let stacks = Stacks::new(size, perm, base_tag); stacks.history.borrow_mut().log_creation( None, base_tag, alloc_range(Size::ZERO, size), - threads, + &mut current_span, ); stacks } @@ -631,7 +626,7 @@ impl Stacks { tag: SbTag, range: AllocRange, state: &GlobalState, - threads: &ThreadManager<'_, 'tcx>, + mut current_span: CurrentSpan<'_, '_, 'tcx>, ) -> InterpResult<'tcx> { trace!( "read access with tag {:?}: {:?}, size {}", @@ -646,7 +641,7 @@ impl Stacks { tag, (alloc_id, range, offset), &mut state, - threads, + &mut current_span, history, ) }) @@ -659,7 +654,7 @@ impl Stacks { tag: SbTag, range: AllocRange, state: &GlobalState, - threads: &ThreadManager<'_, 'tcx>, + mut current_span: CurrentSpan<'_, '_, 'tcx>, ) -> InterpResult<'tcx> { trace!( "write access with tag {:?}: {:?}, size {}", @@ -674,7 +669,7 @@ impl Stacks { tag, (alloc_id, range, offset), &mut state, - threads, + &mut current_span, history, ) }) @@ -723,6 +718,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?; + let mut current_span = this.machine.current_span(); { let extra = this.get_alloc_extra(alloc_id)?; let stacked_borrows = @@ -732,10 +728,10 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Some(orig_tag), new_tag, alloc_range(base_offset, size), - &this.machine.threads, + &mut current_span, ); if protect { - alloc_history.log_protector(orig_tag, new_tag, &this.machine.threads); + alloc_history.log_protector(orig_tag, new_tag, &mut current_span); } } @@ -804,7 +800,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx item, (alloc_id, range, offset), &mut *global, - &this.machine.threads, + &mut current_span, history, ) }) @@ -821,13 +817,14 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let item = Item { perm, tag: new_tag, protector }; let range = alloc_range(base_offset, size); let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut(); + let mut current_span = machine.current_span(); stacked_borrows.for_each_mut(range, |offset, stack, history| { stack.grant( orig_tag, item, (alloc_id, range, offset), &mut global, - &machine.threads, + &mut current_span, history, ) })?; diff --git a/src/stacked_borrows/diagnostics.rs b/src/stacked_borrows/diagnostics.rs index 734c3a14e3..f3692cdeeb 100644 --- a/src/stacked_borrows/diagnostics.rs +++ b/src/stacked_borrows/diagnostics.rs @@ -1,17 +1,14 @@ use smallvec::SmallVec; -use std::rc::Rc; use rustc_middle::mir::interpret::{AllocId, AllocRange}; -use rustc_span::def_id::CrateNum; use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; -use crate::helpers::HexRange; +use crate::helpers::{CurrentSpan, HexRange}; use crate::stacked_borrows::{err_sb_ub, AccessKind, Permission}; use crate::Item; use crate::SbTag; use crate::Stack; -use crate::ThreadManager; use rustc_middle::mir::interpret::InterpError; @@ -23,8 +20,6 @@ pub struct AllocHistory { creations: smallvec::SmallVec<[Event; 2]>, invalidations: smallvec::SmallVec<[Event; 1]>, protectors: smallvec::SmallVec<[Protection; 1]>, - /// This field is a clone of the `local_crates` field on `Evaluator`. - local_crates: Rc<[CrateNum]>, } #[derive(Clone, Debug)] @@ -59,37 +54,23 @@ pub enum TagHistory { } impl AllocHistory { - pub fn new(local_crates: Rc<[CrateNum]>) -> Self { + pub fn new() -> Self { Self { current_time: 0, creations: SmallVec::new(), invalidations: SmallVec::new(), protectors: SmallVec::new(), - local_crates, } } - fn current_span(&self, threads: &ThreadManager<'_, '_>) -> Span { - threads - .active_thread_stack() - .into_iter() - .rev() - .find(|frame| { - let def_id = frame.instance.def_id(); - def_id.is_local() || self.local_crates.contains(&def_id.krate) - }) - .map(|frame| frame.current_span()) - .unwrap_or(rustc_span::DUMMY_SP) - } - pub fn log_creation( &mut self, parent: Option, tag: SbTag, range: AllocRange, - threads: &ThreadManager<'_, '_>, + current_span: &mut CurrentSpan<'_, '_, '_>, ) { - let span = self.current_span(threads); + let span = current_span.get(); self.creations.push(Event { parent, tag, range, span, time: self.current_time }); self.current_time += 1; } @@ -98,15 +79,20 @@ impl AllocHistory { &mut self, tag: SbTag, range: AllocRange, - threads: &ThreadManager<'_, '_>, + current_span: &mut CurrentSpan<'_, '_, '_>, ) { - let span = self.current_span(threads); + let span = current_span.get(); self.invalidations.push(Event { parent: None, tag, range, span, time: self.current_time }); self.current_time += 1; } - pub fn log_protector(&mut self, orig_tag: SbTag, tag: SbTag, threads: &ThreadManager<'_, '_>) { - let span = self.current_span(threads); + pub fn log_protector( + &mut self, + orig_tag: SbTag, + tag: SbTag, + current_span: &mut CurrentSpan<'_, '_, '_>, + ) { + let span = current_span.get(); self.protectors.push(Protection { orig_tag, tag, span }); self.current_time += 1; }