avoid thread-local var indirection for non-halting diagnostics

Author: RalfJung

src/concurrency/weak_memory.rs

@@ -544,7 +544,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
                     validate,
                 )?;
                 if global.track_outdated_loads && recency == LoadRecency::Outdated {
-                    register_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad);
+                    this.emit_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad);
                 }
 
                 return Ok(loaded);

src/diagnostics.rs

@@ -190,11 +190,6 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         Evaluator::new(config, layout_cx),
     );
 
-    // Capture the current interpreter stack state (which should be empty) so that we can emit
-    // allocation-tracking and tag-tracking diagnostics for allocations which are part of the
-    // early runtime setup.
-    let info = ecx.preprocess_diagnostics();
-
     // Some parts of initialization require a full `InterpCx`.
     Evaluator::late_init(&mut ecx, config)?;
 
@@ -324,10 +319,6 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         }
     }
 
-    // Emit any diagnostics related to the setup process for the runtime, so that when the
-    // interpreter loop starts there are no unprocessed diagnostics.
-    ecx.process_diagnostics(info);
-
     Ok((ecx, ret_place))
 }
 
@@ -356,7 +347,6 @@ pub fn eval_entry<'tcx>(
     let res: thread::Result<InterpResult<'_, i64>> = panic::catch_unwind(AssertUnwindSafe(|| {
         // Main loop.
         loop {
-            let info = ecx.preprocess_diagnostics();
             match ecx.schedule()? {
                 SchedulingAction::ExecuteStep => {
                     assert!(ecx.step()?, "a terminated thread was scheduled for execution");
@@ -374,7 +364,6 @@ pub fn eval_entry<'tcx>(
                     break;
                 }
             }
-            ecx.process_diagnostics(info);
         }
         let return_code = ecx.read_scalar(&ret_place.into())?.to_machine_isize(&ecx)?;
         Ok(return_code)

src/eval.rs

@@ -508,7 +508,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 Ok(())
             }
             RejectOpWith::Warning => {
-                register_diagnostic(NonHaltingDiagnostic::RejectedIsolatedOp(op_name.to_string()));
+                this.emit_diagnostic(NonHaltingDiagnostic::RejectedIsolatedOp(op_name.to_string()));
                 Ok(())
             }
             RejectOpWith::NoWarning => Ok(()), // no warning
@@ -881,6 +881,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             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> {
@@ -901,6 +906,12 @@ pub struct CurrentSpan<'a, '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);
+    }
+
     /// Get the current span, skipping non-local frames.
     /// This function is backed by a cache, and can be assumed to be very fast.
     pub fn get(&mut self) -> Span {

src/helpers.rs

@@ -142,7 +142,7 @@ impl<'mir, 'tcx> GlobalStateInner {
                     let first = past_warnings.is_empty();
                     if past_warnings.insert(ecx.cur_span()) {
                         // Newly inserted, so first time we see this span.
-                        register_diagnostic(NonHaltingDiagnostic::Int2Ptr { details: first });
+                        ecx.emit_diagnostic(NonHaltingDiagnostic::Int2Ptr { details: first });
                     }
                 });
             }

src/intptrcast.rs

@@ -94,7 +94,7 @@ pub use crate::concurrency::{
     },
 };
 pub use crate::diagnostics::{
-    register_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt,
+    emit_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt,
     NonHaltingDiagnostic, TerminationInfo,
 };
 pub use crate::eval::{

src/lib.rs

@@ -755,7 +755,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
     ) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra>>> {
         let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
         if ecx.machine.tracked_alloc_ids.contains(&id) {
-            register_diagnostic(NonHaltingDiagnostic::CreatedAlloc(
+            ecx.emit_diagnostic(NonHaltingDiagnostic::CreatedAlloc(
                 id,
                 alloc.size(),
                 alloc.align,
@@ -813,7 +813,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'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)
+            stacked_borrows.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.current_span())
         } else {
             // Value does not matter, SB is disabled
             SbTag::default()
@@ -937,7 +937,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         range: AllocRange,
     ) -> InterpResult<'tcx> {
         if machine.tracked_alloc_ids.contains(&alloc_id) {
-            register_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id));
+            emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id), machine, tcx);
         }
         if let Some(data_race) = &mut alloc_extra.data_race {
             data_race.deallocate(
@@ -993,7 +993,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         let stacked_borrows = ecx.machine.stacked_borrows.as_ref();
 
         let extra = FrameData {
-            stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame()),
+            stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame(&ecx.current_span())),
             catch_unwind: None,
             timing,
         };
@@ -1018,7 +1018,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         // Possibly report our progress.
         if let Some(report_progress) = ecx.machine.report_progress {
             if ecx.machine.basic_block_count % u64::from(report_progress) == 0 {
-                register_diagnostic(NonHaltingDiagnostic::ProgressReport {
+                ecx.emit_diagnostic(NonHaltingDiagnostic::ProgressReport {
                     block_count: ecx.machine.basic_block_count,
                 });
             }

src/machine.rs

@@ -471,7 +471,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
                 Some((orig_tag, kind))
             }
         };
-        register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary));
+        self.current_span.emit_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary));
     }
 }
 

src/stacked_borrows/diagnostics.rs

@@ -178,11 +178,11 @@ impl GlobalStateInner {
         id
     }
 
-    pub fn new_frame(&mut self) -> FrameExtra {
+    pub fn new_frame(&mut self, current_span: &CurrentSpan<'_, '_, '_>) -> FrameExtra {
         let call_id = self.next_call_id;
         trace!("new_frame: Assigning call ID {}", call_id);
         if self.tracked_call_ids.contains(&call_id) {
-            register_diagnostic(NonHaltingDiagnostic::CreatedCallId(call_id));
+            current_span.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 @@ impl GlobalStateInner {
         }
     }
 
-    pub fn base_ptr_tag(&mut self, id: AllocId) -> SbTag {
+    pub fn base_ptr_tag(&mut self, id: AllocId, current_span: &CurrentSpan<'_, '_, '_>) -> SbTag {
         self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| {
             let tag = self.new_ptr();
             if self.tracked_pointer_tags.contains(&tag) {
-                register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(tag.0, None));
+                current_span.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 @@ impl Stacks {
             // 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), Permission::Unique),
+            MemoryKind::Stack => (extra.base_ptr_tag(id, &current_span), Permission::Unique),
             // Everything else is shared by default.
-            _ => (extra.base_ptr_tag(id), Permission::SharedReadWrite),
+            _ => (extra.base_ptr_tag(id, &current_span), Permission::SharedReadWrite),
         };
         Stacks::new(size, perm, base_tag, id, &mut current_span)
     }
@@ -674,7 +674,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriEvalContextEx
          -> InterpResult<'tcx> {
             let global = this.machine.stacked_borrows.as_ref().unwrap().borrow();
             if global.tracked_pointer_tags.contains(&new_tag) {
-                register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
+                this.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
                     new_tag.0,
                     loc.map(|(alloc_id, base_offset, _)| (alloc_id, alloc_range(base_offset, size))),
                 ));