From cc149c76912dd7219cf4e3eef43e69390b5689e3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 19 Sep 2022 14:43:02 +0200 Subject: [PATCH] put a tcx into the Machine so that we have to pass around fewer things --- src/diagnostics.rs | 176 ++++++++++++++--------------- src/helpers.rs | 22 ++-- src/lib.rs | 2 +- src/machine.rs | 29 +++-- src/stacked_borrows/diagnostics.rs | 2 +- src/stacked_borrows/mod.rs | 19 ++-- 6 files changed, 124 insertions(+), 126 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 0118f9f421e..5e0da17ade8 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -3,7 +3,6 @@ use log::trace; -use rustc_middle::ty::TyCtxt; use rustc_span::{source_map::DUMMY_SP, SpanData, Symbol}; use rustc_target::abi::{Align, Size}; @@ -91,13 +90,12 @@ enum DiagLevel { fn prune_stacktrace<'tcx>( mut stacktrace: Vec>, machine: &Evaluator<'_, 'tcx>, - tcx: TyCtxt<'tcx>, ) -> (Vec>, bool) { match machine.backtrace_style { BacktraceStyle::Off => { // Remove all frames marked with `caller_location` -- that attribute indicates we // usually want to point at the caller, not them. - stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(tcx)); + stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(machine.tcx)); // Retain one frame so that we can print a span for the error itself stacktrace.truncate(1); (stacktrace, false) @@ -111,7 +109,7 @@ fn prune_stacktrace<'tcx>( if has_local_frame { // Remove all frames marked with `caller_location` -- that attribute indicates we // usually want to point at the caller, not them. - stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(tcx)); + stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(machine.tcx)); // This is part of the logic that `std` uses to select the relevant part of a // backtrace. But here, we only look for __rust_begin_short_backtrace, not @@ -121,7 +119,7 @@ fn prune_stacktrace<'tcx>( .into_iter() .take_while(|frame| { let def_id = frame.instance.def_id(); - let path = tcx.def_path_str(def_id); + let path = machine.tcx.def_path_str(def_id); !path.contains("__rust_begin_short_backtrace") }) .collect::>(); @@ -256,7 +254,7 @@ pub fn report_error<'tcx, 'mir>( }; let stacktrace = ecx.generate_stacktrace(); - let (stacktrace, was_pruned) = prune_stacktrace(stacktrace, &ecx.machine, *ecx.tcx); + let (stacktrace, was_pruned) = prune_stacktrace(stacktrace, &ecx.machine); e.print_backtrace(); msg.insert(0, e.to_string()); report_msg( @@ -267,7 +265,6 @@ pub fn report_error<'tcx, 'mir>( helps, &stacktrace, &ecx.machine, - *ecx.tcx, ); // Include a note like `std` does when we omit frames from a backtrace @@ -315,13 +312,13 @@ fn report_msg<'tcx>( helps: Vec<(Option, String)>, stacktrace: &[FrameInfo<'tcx>], machine: &Evaluator<'_, 'tcx>, - tcx: TyCtxt<'tcx>, ) { let span = stacktrace.first().map_or(DUMMY_SP, |fi| fi.span); + let sess = machine.tcx.sess; let mut err = match diag_level { - DiagLevel::Error => tcx.sess.struct_span_err(span, title).forget_guarantee(), - DiagLevel::Warning => tcx.sess.struct_span_warn(span, title), - DiagLevel::Note => tcx.sess.diagnostic().span_note_diag(span, title), + DiagLevel::Error => sess.struct_span_err(span, title).forget_guarantee(), + DiagLevel::Warning => sess.struct_span_warn(span, title), + DiagLevel::Note => sess.diagnostic().span_note_diag(span, title), }; // Show main message. @@ -370,95 +367,97 @@ fn report_msg<'tcx>( err.emit(); } -pub fn emit_diagnostic<'tcx>(e: NonHaltingDiagnostic, machine: &Evaluator<'_, 'tcx>, tcx: TyCtxt<'tcx>) { - use NonHaltingDiagnostic::*; +impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { + pub fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { + use NonHaltingDiagnostic::*; - let stacktrace = MiriEvalContext::generate_stacktrace_from_stack(machine.threads.active_thread_stack()); - let (stacktrace, _was_pruned) = prune_stacktrace(stacktrace, machine, tcx); + let stacktrace = MiriEvalContext::generate_stacktrace_from_stack(self.threads.active_thread_stack()); + let (stacktrace, _was_pruned) = prune_stacktrace(stacktrace, self); - let (title, diag_level) = match e { - RejectedIsolatedOp(_) => - ("operation rejected by isolation", DiagLevel::Warning), - Int2Ptr { .. } => ("integer-to-pointer cast", DiagLevel::Warning), - CreatedPointerTag(..) - | PoppedPointerTag(..) - | CreatedCallId(..) - | CreatedAlloc(..) - | FreedAlloc(..) - | ProgressReport { .. } - | WeakMemoryOutdatedLoad => - ("tracking was triggered", DiagLevel::Note), - }; + let (title, diag_level) = match e { + RejectedIsolatedOp(_) => + ("operation rejected by isolation", DiagLevel::Warning), + Int2Ptr { .. } => ("integer-to-pointer cast", DiagLevel::Warning), + CreatedPointerTag(..) + | PoppedPointerTag(..) + | CreatedCallId(..) + | CreatedAlloc(..) + | FreedAlloc(..) + | ProgressReport { .. } + | WeakMemoryOutdatedLoad => + ("tracking was triggered", DiagLevel::Note), + }; - let msg = match e { - CreatedPointerTag(tag, None) => - format!("created tag {tag:?}"), - CreatedPointerTag(tag, Some((alloc_id, range))) => - format!("created tag {tag:?} at {alloc_id:?}{range:?}"), - PoppedPointerTag(item, tag) => - match tag { - None => - format!( - "popped tracked tag for item {item:?} due to deallocation", - ), - Some((tag, access)) => { - format!( - "popped tracked tag for item {item:?} due to {access:?} access for {tag:?}", - ) - } - }, - CreatedCallId(id) => - format!("function call with id {id}"), - CreatedAlloc(AllocId(id), size, align, kind) => - format!( - "created {kind} allocation of {size} bytes (alignment {align} bytes) with id {id}", - size = size.bytes(), - align = align.bytes(), - ), - FreedAlloc(AllocId(id)) => - format!("freed allocation with id {id}"), - RejectedIsolatedOp(ref op) => - format!("{op} was made to return an error due to isolation"), - ProgressReport { .. } => - format!("progress report: current operation being executed is here"), - Int2Ptr { .. } => - format!("integer-to-pointer cast"), - WeakMemoryOutdatedLoad => - format!("weak memory emulation: outdated value returned from load"), - }; + let msg = match e { + CreatedPointerTag(tag, None) => + format!("created tag {tag:?}"), + CreatedPointerTag(tag, Some((alloc_id, range))) => + format!("created tag {tag:?} at {alloc_id:?}{range:?}"), + PoppedPointerTag(item, tag) => + match tag { + None => + format!( + "popped tracked tag for item {item:?} due to deallocation", + ), + Some((tag, access)) => { + format!( + "popped tracked tag for item {item:?} due to {access:?} access for {tag:?}", + ) + } + }, + CreatedCallId(id) => + format!("function call with id {id}"), + CreatedAlloc(AllocId(id), size, align, kind) => + format!( + "created {kind} allocation of {size} bytes (alignment {align} bytes) with id {id}", + size = size.bytes(), + align = align.bytes(), + ), + FreedAlloc(AllocId(id)) => + format!("freed allocation with id {id}"), + RejectedIsolatedOp(ref op) => + format!("{op} was made to return an error due to isolation"), + ProgressReport { .. } => + format!("progress report: current operation being executed is here"), + Int2Ptr { .. } => + format!("integer-to-pointer cast"), + WeakMemoryOutdatedLoad => + format!("weak memory emulation: outdated value returned from load"), + }; - let notes = match e { - ProgressReport { block_count } => { - // It is important that each progress report is slightly different, since - // identical diagnostics are being deduplicated. - vec![ - (None, format!("so far, {block_count} basic blocks have been executed")), - ] - } - _ => vec![], - }; + let notes = match e { + ProgressReport { block_count } => { + // It is important that each progress report is slightly different, since + // identical diagnostics are being deduplicated. + vec![ + (None, format!("so far, {block_count} basic blocks have been executed")), + ] + } + _ => vec![], + }; - let helps = match e { - Int2Ptr { details: true } => - vec![ - (None, format!("This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,")), - (None, format!("which means that Miri might miss pointer bugs in this program.")), - (None, format!("See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.")), - (None, format!("To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead.")), - (None, format!("You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.")), - (None, format!("Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.")), - ], - _ => vec![], - }; + let helps = match e { + Int2Ptr { details: true } => + vec![ + (None, format!("This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,")), + (None, format!("which means that Miri might miss pointer bugs in this program.")), + (None, format!("See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.")), + (None, format!("To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead.")), + (None, format!("You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.")), + (None, format!("Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.")), + ], + _ => vec![], + }; - report_msg(diag_level, title, vec![msg], notes, helps, &stacktrace, machine, tcx); + report_msg(diag_level, title, vec![msg], notes, helps, &stacktrace, self); + } } impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { let this = self.eval_context_ref(); - emit_diagnostic(e, &this.machine, *this.tcx); + this.machine.emit_diagnostic(e); } /// We had a panic in Miri itself, try to print something useful. @@ -477,7 +476,6 @@ fn handle_ice(&self) { vec![], &stacktrace, &this.machine, - *this.tcx, ); } } diff --git a/src/helpers.rs b/src/helpers.rs index 84815ee7691..ee972bf6858 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -881,16 +881,11 @@ fn item_link_name(&self, def_id: DefId) -> Symbol { None => tcx.item_name(def_id), } } - - fn current_span(&self) -> CurrentSpan<'_, 'mir, 'tcx> { - let this = self.eval_context_ref(); - CurrentSpan { current_frame_idx: None, machine: &this.machine, tcx: *this.tcx } - } } impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { - pub fn current_span(&self, tcx: TyCtxt<'tcx>) -> CurrentSpan<'_, 'mir, 'tcx> { - CurrentSpan { current_frame_idx: None, machine: self, tcx } + pub fn current_span(&self) -> CurrentSpan<'_, 'mir, 'tcx> { + CurrentSpan { current_frame_idx: None, machine: self } } } @@ -901,15 +896,12 @@ pub fn current_span(&self, tcx: TyCtxt<'tcx>) -> CurrentSpan<'_, 'mir, 'tcx> { #[derive(Clone)] pub struct CurrentSpan<'a, 'mir, 'tcx> { current_frame_idx: Option, - tcx: TyCtxt<'tcx>, machine: &'a Evaluator<'mir, 'tcx>, } impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> { - /// Not really about the `CurrentSpan`, but we just happen to have all the things needed to emit - /// diagnostics like that. - pub fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { - emit_diagnostic(e, self.machine, self.tcx); + pub fn machine(&self) -> &'a Evaluator<'mir, 'tcx> { + self.machine } /// Get the current span, skipping non-local frames. @@ -939,13 +931,13 @@ fn frame_span(machine: &Evaluator<'_, '_>, idx: usize) -> Span { fn current_frame_idx(&mut self) -> usize { *self .current_frame_idx - .get_or_insert_with(|| Self::compute_current_frame_index(self.tcx, self.machine)) + .get_or_insert_with(|| Self::compute_current_frame_index(self.machine)) } // Find the position of the inner-most frame which is part of the crate being // compiled/executed, part of the Cargo workspace, and is also not #[track_caller]. #[inline(never)] - fn compute_current_frame_index(tcx: TyCtxt<'_>, machine: &Evaluator<'_, '_>) -> usize { + fn compute_current_frame_index(machine: &Evaluator<'_, '_>) -> usize { machine .threads .active_thread_stack() @@ -955,7 +947,7 @@ fn compute_current_frame_index(tcx: TyCtxt<'_>, machine: &Evaluator<'_, '_>) -> .find_map(|(idx, frame)| { let def_id = frame.instance.def_id(); if (def_id.is_local() || machine.local_crates.contains(&def_id.krate)) - && !frame.instance.def.requires_caller_location(tcx) + && !frame.instance.def.requires_caller_location(machine.tcx) { Some(idx) } else { diff --git a/src/lib.rs b/src/lib.rs index 900daa88d27..7cea752b10e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -94,7 +94,7 @@ }, }; pub use crate::diagnostics::{ - emit_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt, + report_error, EvalContextExt as DiagnosticsEvalContextExt, NonHaltingDiagnostic, TerminationInfo, }; pub use crate::eval::{ diff --git a/src/machine.rs b/src/machine.rs index 19978e550c3..a7fdf8c35ab 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -292,8 +292,16 @@ fn new(layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Result { + // 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, + + /// Data race detector global data. pub data_race: Option, + + /// Ptr-int-cast module global data. pub intptrcast: intptrcast::GlobalState, /// Environment variables set by `setenv`. @@ -419,6 +427,7 @@ pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) }); let data_race = config.data_race_detector.then(|| data_race::GlobalState::new(config)); Evaluator { + tcx: layout_cx.tcx, stacked_borrows, data_race, intptrcast: RefCell::new(intptrcast::GlobalStateInner::new(config)), @@ -770,7 +779,7 @@ fn adjust_allocation<'b>( alloc.size(), stacked_borrows, kind, - ecx.machine.current_span(*ecx.tcx), + ecx.machine.current_span(), ) }); let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| { @@ -813,7 +822,7 @@ fn adjust_alloc_base_pointer( } 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.current_span()) + stacked_borrows.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine) } else { // Value does not matter, SB is disabled SbTag::default() @@ -866,7 +875,7 @@ fn ptr_get_alloc( #[inline(always)] fn before_memory_read( - tcx: TyCtxt<'tcx>, + _tcx: TyCtxt<'tcx>, machine: &Self, alloc_extra: &AllocExtra, (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra), @@ -886,7 +895,7 @@ fn before_memory_read( prov_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(tcx), + machine.current_span(), &machine.threads, )?; } @@ -898,7 +907,7 @@ fn before_memory_read( #[inline(always)] fn before_memory_write( - tcx: TyCtxt<'tcx>, + _tcx: TyCtxt<'tcx>, machine: &mut Self, alloc_extra: &mut AllocExtra, (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra), @@ -918,7 +927,7 @@ fn before_memory_write( prov_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(tcx), + machine.current_span(), &machine.threads, )?; } @@ -930,14 +939,14 @@ fn before_memory_write( #[inline(always)] fn before_memory_deallocation( - tcx: TyCtxt<'tcx>, + _tcx: TyCtxt<'tcx>, machine: &mut Self, alloc_extra: &mut AllocExtra, (alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra), range: AllocRange, ) -> InterpResult<'tcx> { if machine.tracked_alloc_ids.contains(&alloc_id) { - emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id), machine, tcx); + machine.emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id)); } if let Some(data_race) = &mut alloc_extra.data_race { data_race.deallocate( @@ -953,7 +962,7 @@ fn before_memory_deallocation( prove_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(tcx), + machine.current_span(), &machine.threads, ) } else { @@ -993,7 +1002,7 @@ fn init_frame_extra( let stacked_borrows = ecx.machine.stacked_borrows.as_ref(); let extra = FrameData { - stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame(&ecx.current_span())), + stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame(&ecx.machine)), catch_unwind: None, timing, }; diff --git a/src/stacked_borrows/diagnostics.rs b/src/stacked_borrows/diagnostics.rs index 278058c9f5f..7148c489090 100644 --- a/src/stacked_borrows/diagnostics.rs +++ b/src/stacked_borrows/diagnostics.rs @@ -471,7 +471,7 @@ pub fn check_tracked_tag_popped(&self, item: &Item, global: &GlobalStateInner) { Some((orig_tag, kind)) } }; - self.current_span.emit_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary)); + self.current_span.machine().emit_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary)); } } diff --git a/src/stacked_borrows/mod.rs b/src/stacked_borrows/mod.rs index b4edf278568..de9ca280838 100644 --- a/src/stacked_borrows/mod.rs +++ b/src/stacked_borrows/mod.rs @@ -178,11 +178,11 @@ fn new_ptr(&mut self) -> SbTag { id } - pub fn new_frame(&mut self, current_span: &CurrentSpan<'_, '_, '_>) -> FrameExtra { + pub fn new_frame(&mut self, machine: &Evaluator<'_, '_>) -> FrameExtra { let call_id = self.next_call_id; trace!("new_frame: Assigning call ID {}", call_id); if self.tracked_call_ids.contains(&call_id) { - current_span.emit_diagnostic(NonHaltingDiagnostic::CreatedCallId(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() } @@ -199,11 +199,11 @@ pub fn end_call(&mut self, frame: &machine::FrameData<'_>) { } } - pub fn base_ptr_tag(&mut self, id: AllocId, current_span: &CurrentSpan<'_, '_, '_>) -> SbTag { + pub fn base_ptr_tag(&mut self, id: AllocId, machine: &Evaluator<'_, '_>) -> SbTag { self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| { let tag = self.new_ptr(); if self.tracked_pointer_tags.contains(&tag) { - current_span.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(tag.0, None)); + machine.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(tag.0, None)); } trace!("New allocation {:?} has base tag {:?}", id, tag); self.base_ptr_tags.try_insert(id, tag).unwrap(); @@ -572,9 +572,9 @@ pub fn new_allocation( // 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, ¤t_span), Permission::Unique), + MemoryKind::Stack => (extra.base_ptr_tag(id, current_span.machine()), Permission::Unique), // Everything else is shared by default. - _ => (extra.base_ptr_tag(id, ¤t_span), Permission::SharedReadWrite), + _ => (extra.base_ptr_tag(id, current_span.machine()), Permission::SharedReadWrite), }; Stacks::new(size, perm, base_tag, id, &mut current_span) } @@ -688,7 +688,7 @@ fn reborrow( let (_size, _align, alloc_kind) = this.get_alloc_info(alloc_id); match alloc_kind { AllocKind::LiveData => { - let current_span = &mut this.machine.current_span(*this.tcx); + let current_span = &mut this.machine.current_span(); // This should have alloc_extra data, but `get_alloc_extra` can still fail // if converting this alloc_id from a global to a local one // uncovers a non-supported `extern static`. @@ -805,7 +805,7 @@ fn reborrow( .expect("we should have Stacked Borrows data") .borrow_mut(); // FIXME: can't share this with the current_span inside log_creation - let mut current_span = this.machine.current_span(*this.tcx); + let mut current_span = this.machine.current_span(); this.visit_freeze_sensitive(place, size, |mut range, frozen| { // Adjust range. range.start += base_offset; @@ -843,7 +843,6 @@ fn reborrow( // Here we can avoid `borrow()` calls because we have mutable references. // Note that this asserts that the allocation is mutable -- but since we are creating a // mutable pointer, that seems reasonable. - let tcx = *this.tcx; let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; let mut stacked_borrows = alloc_extra .stacked_borrows @@ -854,7 +853,7 @@ fn reborrow( let range = alloc_range(base_offset, size); let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut(); // FIXME: can't share this with the current_span inside log_creation - let current_span = &mut machine.current_span(tcx); + let current_span = &mut machine.current_span(); let dcx = DiagnosticCxBuilder::retag( current_span, &machine.threads,