From fe545d62db72b632364ba9cc29bbe7259ddad385 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 16:39:23 +0100 Subject: [PATCH] add option to track all read/write accesses to tracked allocations --- src/tools/miri/src/bin/miri.rs | 2 ++ src/tools/miri/src/borrow_tracker/mod.rs | 7 ------- .../borrow_tracker/stacked_borrows/diagnostics.rs | 2 +- .../miri/src/borrow_tracker/stacked_borrows/mod.rs | 2 +- .../src/borrow_tracker/tree_borrows/diagnostics.rs | 2 +- .../miri/src/borrow_tracker/tree_borrows/mod.rs | 2 +- .../miri/src/borrow_tracker/tree_borrows/perms.rs | 2 +- .../miri/src/borrow_tracker/tree_borrows/tree.rs | 2 +- src/tools/miri/src/diagnostics.rs | 4 ++++ src/tools/miri/src/eval.rs | 3 +++ src/tools/miri/src/helpers.rs | 7 +++++++ src/tools/miri/src/lib.rs | 2 +- src/tools/miri/src/machine.rs | 12 ++++++++++++ .../fail-dep/concurrency/windows_join_main.stderr | 1 + .../fail-dep/concurrency/windows_join_self.stderr | 1 + src/tools/miri/tests/fail/memleak_rc.32bit.stderr | 1 + 16 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 281a32b77c5..2f37a64576e 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -534,6 +534,8 @@ fn main() { ), }; miri_config.tracked_alloc_ids.extend(ids); + } else if arg == "-Zmiri-track-alloc-accesses" { + miri_config.track_alloc_accesses = true; } else if let Some(param) = arg.strip_prefix("-Zmiri-compare-exchange-weak-failure-rate=") { let rate = match param.parse::() { Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate, diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 711323b51c2..2e784e74195 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -122,13 +122,6 @@ fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { /// We need interior mutable access to the global state. pub type GlobalState = RefCell; -/// Indicates which kind of access is being performed. -#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] -pub enum AccessKind { - Read, - Write, -} - impl fmt::Display for AccessKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index b964b1f9ec2..aa99a14b18e 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -5,7 +5,7 @@ use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; -use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind}; +use crate::borrow_tracker::{GlobalStateInner, ProtectorKind}; use crate::*; /// Error reporting diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 0fe422180f7..86d22229714 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -16,7 +16,7 @@ use crate::borrow_tracker::{ stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder}, - AccessKind, GlobalStateInner, ProtectorKind, + GlobalStateInner, ProtectorKind, }; use crate::*; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 43e6616e34a..0663417ca41 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -9,7 +9,7 @@ tree::LocationState, unimap::UniIndex, }; -use crate::borrow_tracker::{AccessKind, ProtectorKind}; +use crate::borrow_tracker::ProtectorKind; use crate::*; /// Cause of an access: either a real access or one diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index cc982865341..8618ab42e91 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -1,6 +1,6 @@ use rustc_target::abi::{Abi, Size}; -use crate::borrow_tracker::{AccessKind, GlobalState, GlobalStateInner, ProtectorKind}; +use crate::borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind}; use rustc_middle::{ mir::{Mutability, RetagKind}, ty::{ diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index bf72e902993..5c7f5ea46ba 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -3,7 +3,7 @@ use crate::borrow_tracker::tree_borrows::diagnostics::TransitionError; use crate::borrow_tracker::tree_borrows::tree::AccessRelatedness; -use crate::borrow_tracker::AccessKind; +use crate::AccessKind; /// The activation states of a pointer. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 4b47cc0cb82..c7c15150fbd 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -24,7 +24,7 @@ unimap::{UniEntry, UniIndex, UniKeyMap, UniValMap}, Permission, }; -use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind}; +use crate::borrow_tracker::{GlobalState, ProtectorKind}; use crate::*; mod tests; diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index c7d0f9a823b..7afcf919b91 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -116,6 +116,7 @@ pub enum NonHaltingDiagnostic { CreatedCallId(CallId), CreatedAlloc(AllocId, Size, Align, MemoryKind), FreedAlloc(AllocId), + AccessedAlloc(AllocId, AccessKind), RejectedIsolatedOp(String), ProgressReport { block_count: u64, // how many basic blocks have been run so far @@ -538,6 +539,7 @@ pub fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { | PoppedPointerTag(..) | CreatedCallId(..) | CreatedAlloc(..) + | AccessedAlloc(..) | FreedAlloc(..) | ProgressReport { .. } | WeakMemoryOutdatedLoad => ("tracking was triggered".to_string(), DiagLevel::Note), @@ -559,6 +561,8 @@ pub fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { size = size.bytes(), align = align.bytes(), ), + AccessedAlloc(AllocId(id), access_kind) => + format!("{access_kind} access to allocation with id {id}"), FreedAlloc(AllocId(id)) => format!("freed allocation with id {id}"), RejectedIsolatedOp(ref op) => format!("{op} was made to return an error due to isolation"), diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 9bab9488e37..ef50bd43de3 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -112,6 +112,8 @@ pub struct MiriConfig { pub tracked_call_ids: FxHashSet, /// The allocation ids to report about. pub tracked_alloc_ids: FxHashSet, + /// For the tracked alloc ids, also report read/write accesses. + pub track_alloc_accesses: bool, /// Determine if data race detection should be enabled pub data_race_detector: bool, /// Determine if weak memory emulation should be enabled. Requires data race detection to be enabled @@ -169,6 +171,7 @@ fn default() -> MiriConfig { tracked_pointer_tags: FxHashSet::default(), tracked_call_ids: FxHashSet::default(), tracked_alloc_ids: FxHashSet::default(), + track_alloc_accesses: false, data_race_detector: true, weak_memory_emulation: true, track_outdated_loads: false, diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index d9b4363d604..65260254ae2 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -22,6 +22,13 @@ use crate::*; +/// Indicates which kind of access is being performed. +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum AccessKind { + Read, + Write, +} + // This mapping should match `decode_error_kind` in // . const UNIX_IO_ERROR_TABLE: &[(&str, std::io::ErrorKind)] = { diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index c567949102f..e1d0bc1c183 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -120,7 +120,7 @@ pub use crate::eval::{ create_ecx, eval_entry, AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, RejectOpWith, }; -pub use crate::helpers::EvalContextExt as _; +pub use crate::helpers::{AccessKind, EvalContextExt as _}; pub use crate::intptrcast::{EvalContextExt as _, ProvenanceMode}; pub use crate::machine::{ AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 40d041c8fdb..c411962fcf2 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -512,6 +512,8 @@ pub struct MiriMachine<'mir, 'tcx> { /// The allocation IDs to report when they are being allocated /// (helps for debugging memory leaks and use after free bugs). tracked_alloc_ids: FxHashSet, + /// For the tracked alloc ids, also report read/write accesses. + track_alloc_accesses: bool, /// Controls whether alignment of memory accesses is being checked. pub(crate) check_alignment: AlignmentCheck, @@ -654,6 +656,7 @@ pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) extern_statics: FxHashMap::default(), rng: RefCell::new(rng), tracked_alloc_ids: config.tracked_alloc_ids.clone(), + track_alloc_accesses: config.track_alloc_accesses, check_alignment: config.check_alignment, cmpxchg_weak_failure_rate: config.cmpxchg_weak_failure_rate, mute_stdout_stderr: config.mute_stdout_stderr, @@ -793,6 +796,7 @@ fn visit_provenance(&self, visit: &mut VisitWith<'_>) { local_crates: _, rng: _, tracked_alloc_ids: _, + track_alloc_accesses: _, check_alignment: _, cmpxchg_weak_failure_rate: _, mute_stdout_stderr: _, @@ -1235,6 +1239,10 @@ fn before_memory_read( (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra), range: AllocRange, ) -> InterpResult<'tcx> { + if machine.track_alloc_accesses && machine.tracked_alloc_ids.contains(&alloc_id) { + machine + .emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc(alloc_id, AccessKind::Read)); + } if let Some(data_race) = &alloc_extra.data_race { data_race.read(alloc_id, range, machine)?; } @@ -1255,6 +1263,10 @@ fn before_memory_write( (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra), range: AllocRange, ) -> InterpResult<'tcx> { + if machine.track_alloc_accesses && machine.tracked_alloc_ids.contains(&alloc_id) { + machine + .emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc(alloc_id, AccessKind::Write)); + } if let Some(data_race) = &mut alloc_extra.data_race { data_race.write(alloc_id, range, machine)?; } diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.stderr b/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.stderr index cb51c7e0bd9..d9137ee7437 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.stderr @@ -4,6 +4,7 @@ error: deadlock: the evaluated program deadlocked LL | assert_eq!(WaitForSingleObject(MAIN_THREAD, INFINITE), WAIT_OBJECT_0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program deadlocked | + = note: BACKTRACE on thread `unnamed-ID`: = note: inside closure at RUSTLIB/core/src/macros/mod.rs:LL:CC = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_join_self.stderr b/src/tools/miri/tests/fail-dep/concurrency/windows_join_self.stderr index 8b76e124215..74699a0317f 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/windows_join_self.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_join_self.stderr @@ -4,6 +4,7 @@ error: deadlock: the evaluated program deadlocked LL | assert_eq!(WaitForSingleObject(native, INFINITE), WAIT_OBJECT_0); | ^ the evaluated program deadlocked | + = note: BACKTRACE on thread `unnamed-ID`: = note: inside closure at $DIR/windows_join_self.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/memleak_rc.32bit.stderr b/src/tools/miri/tests/fail/memleak_rc.32bit.stderr index 3f8b7a3e819..781e1458db9 100644 --- a/src/tools/miri/tests/fail/memleak_rc.32bit.stderr +++ b/src/tools/miri/tests/fail/memleak_rc.32bit.stderr @@ -4,6 +4,7 @@ error: memory leaked: ALLOC (Rust heap, size: 16, align: 4), allocated here: LL | __rust_alloc(layout.size(), layout.align()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: BACKTRACE: = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC