Auto merge of #2524 - RalfJung:progress-report, r=RalfJung

fix progress report being deduplicated

Fixes #2522

Author: bors

src/diagnostics.rs

@@ -70,7 +70,9 @@ pub enum NonHaltingDiagnostic {
     CreatedAlloc(AllocId, Size, Align, MemoryKind<MiriMemoryKind>),
     FreedAlloc(AllocId),
     RejectedIsolatedOp(String),
-    ProgressReport,
+    ProgressReport {
+        block_count: u64, // how many basic blocks have been run so far
+    },
     Int2Ptr {
         details: bool,
     },
@@ -261,6 +263,7 @@ pub fn report_error<'tcx, 'mir>(
         DiagLevel::Error,
         &if let Some(title) = title { format!("{}: {}", title, msg[0]) } else { msg[0].clone() },
         msg,
+        vec![],
         helps,
         &stacktrace,
     );
@@ -307,6 +310,7 @@ fn report_msg<'mir, 'tcx>(
     diag_level: DiagLevel,
     title: &str,
     span_msg: Vec<String>,
+    notes: Vec<(Option<SpanData>, String)>,
     helps: Vec<(Option<SpanData>, String)>,
     stacktrace: &[FrameInfo<'tcx>],
 ) {
@@ -331,17 +335,24 @@ fn report_msg<'mir, 'tcx>(
         err.note("(no span available)");
     }
 
-    // Show help messages.
-    if !helps.is_empty() {
-        for (span_data, help) in helps {
-            if let Some(span_data) = span_data {
-                err.span_help(span_data.span(), &help);
-            } else {
-                err.help(&help);
-            }
+    // Show note and help messages.
+    for (span_data, note) in &notes {
+        if let Some(span_data) = span_data {
+            err.span_note(span_data.span(), note);
+        } else {
+            err.note(note);
         }
+    }
+    for (span_data, help) in &helps {
+        if let Some(span_data) = span_data {
+            err.span_help(span_data.span(), help);
+        } else {
+            err.help(help);
+        }
+    }
+    if notes.len() + helps.len() > 0 {
         // Add visual separator before backtrace.
-        err.note("backtrace:");
+        err.note("BACKTRACE:");
     }
     // Add backtrace
     for (idx, frame_info) in stacktrace.iter().enumerate() {
@@ -436,6 +447,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             // Show diagnostics.
             for e in diagnostics.drain(..) {
                 use NonHaltingDiagnostic::*;
+
+                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:?}"),
@@ -465,26 +491,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         format!("freed allocation with id {id}"),
                     RejectedIsolatedOp(ref op) =>
                         format!("{op} was made to return an error due to isolation"),
-                    ProgressReport =>
+                    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 (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 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 {
@@ -500,7 +523,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     _ => vec![],
                 };
 
-                report_msg(this, diag_level, title, vec![msg], helps, &stacktrace);
+                report_msg(this, diag_level, title, vec![msg], notes, helps, &stacktrace);
             }
         });
     }
@@ -519,6 +542,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             "the place in the program where the ICE was triggered",
             vec![],
             vec![],
+            vec![],
             &stacktrace,
         );
     }

src/machine.rs

@@ -374,8 +374,8 @@ pub struct Evaluator<'mir, 'tcx> {
 
     /// If `Some`, we will report the current stack every N basic blocks.
     pub(crate) report_progress: Option<u32>,
-    /// The number of blocks that passed since the last progress report.
-    pub(crate) since_progress_report: u32,
+    // The total number of blocks that have been executed.
+    pub(crate) basic_block_count: u64,
 
     /// Handle of the optional shared object file for external functions.
     pub external_so_lib: Option<(libloading::Library, std::path::PathBuf)>,
@@ -433,7 +433,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             weak_memory: config.weak_memory_emulation,
             preemption_rate: config.preemption_rate,
             report_progress: config.report_progress,
-            since_progress_report: 0,
+            basic_block_count: 0,
             external_so_lib: config.external_so_file.as_ref().map(|lib_file_path| {
                 // Check if host target == the session target.
                 if env!("TARGET") != target_triple {
@@ -992,14 +992,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
     }
 
     fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
+        ecx.machine.basic_block_count += 1u64; // a u64 that is only incremented by 1 will "never" overflow
         // Possibly report our progress.
         if let Some(report_progress) = ecx.machine.report_progress {
-            if ecx.machine.since_progress_report >= report_progress {
-                register_diagnostic(NonHaltingDiagnostic::ProgressReport);
-                ecx.machine.since_progress_report = 0;
+            if ecx.machine.basic_block_count % u64::from(report_progress) == 0 {
+                register_diagnostic(NonHaltingDiagnostic::ProgressReport {
+                    block_count: ecx.machine.basic_block_count,
+                });
             }
-            // Cannot overflow, since it is strictly less than `report_progress`.
-            ecx.machine.since_progress_report += 1;
         }
         // These are our preemption points.
         ecx.maybe_preempt_active_thread();

tests/extern-so/fail/function_not_in_so.stderr

@@ -5,7 +5,7 @@ LL |         foo();
    |         ^^^^^ can't call foreign function: foo
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `main` at $DIR/function_not_in_so.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/alloc/deallocate-bad-alignment.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-bad-alignment.rs:LL:CC
   --> $DIR/deallocate-bad-alignment.rs:LL:CC

tests/fail/alloc/deallocate-bad-size.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-bad-size.rs:LL:CC
   --> $DIR/deallocate-bad-size.rs:LL:CC

tests/fail/alloc/deallocate-twice.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-twice.rs:LL:CC
   --> $DIR/deallocate-twice.rs:LL:CC

tests/fail/alloc/global_system_mixup.stderr

@@ -6,7 +6,7 @@ LL |         FREE();
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `std::sys::PLATFORM::alloc::<impl std::alloc::GlobalAlloc for std::alloc::System>::dealloc` at RUSTLIB/std/src/sys/PLATFORM/alloc.rs:LL:CC
    = note: inside `<std::alloc::System as std::alloc::Allocator>::deallocate` at RUSTLIB/std/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/global_system_mixup.rs:LL:CC

tests/fail/alloc/no_global_allocator.stderr

@@ -5,7 +5,7 @@ LL |         __rust_alloc(1, 1);
    |         ^^^^^^^^^^^^^^^^^^ can't call foreign function: __rust_alloc
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `start` at $DIR/no_global_allocator.rs:LL:CC
 
 error: aborting due to previous error

tests/fail/alloc/reallocate-bad-size.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `std::alloc::realloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/reallocate-bad-size.rs:LL:CC
   --> $DIR/reallocate-bad-size.rs:LL:CC

tests/fail/alloc/reallocate-change-alloc.stderr

@@ -6,7 +6,7 @@ LL |         let _z = *x;
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-   = note: backtrace:
+   = note: BACKTRACE:
    = note: inside `main` at $DIR/reallocate-change-alloc.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace