diff --git a/README.md b/README.md index f6c675839e..b89d664963 100644 --- a/README.md +++ b/README.md @@ -464,11 +464,6 @@ to Miri failing to detect cases of undefined behavior in a program. errors and warnings. * `-Zmiri-recursive-validation` is a *highly experimental* flag that makes validity checking recurse below references. -* `-Zmiri-retag-fields[=]` controls when Stacked Borrows retagging recurses into - fields. `all` means it always recurses (the default, and equivalent to `-Zmiri-retag-fields` - without an explicit value), `none` means it never recurses, `scalar` means it only recurses for - types where we would also emit `noalias` annotations in the generated LLVM IR (types passed as - individual scalars or pairs of scalars). Setting this to `none` is **unsound**. * `-Zmiri-preemption-rate` configures the probability that at the end of a basic block, the active thread will be preempted. The default is `0.01` (i.e., 1%). Setting this to `0` disables preemption. Note that even without preemption, the schedule is still non-deterministic: diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 8b4445e379..5869f461a8 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -36,7 +36,7 @@ use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; use miri::{ BacktraceStyle, BorrowTrackerMethod, GenmcConfig, GenmcCtx, MiriConfig, MiriEntryFnType, - ProvenanceMode, RetagFields, TreeBorrowsParams, ValidationMode, run_genmc_mode, + ProvenanceMode, TreeBorrowsParams, ValidationMode, run_genmc_mode, }; use rustc_abi::ExternAbi; use rustc_data_structures::sync; @@ -535,7 +535,10 @@ fn main() { } else if arg == "-Zmiri-mute-stdout-stderr" { miri_config.mute_stdout_stderr = true; } else if arg == "-Zmiri-retag-fields" { - miri_config.retag_fields = RetagFields::Yes; + eprintln!( + "warning: `-Zmiri-retag-fields` is a NOP and will be removed in a future version of Miri.\n\ + Field retagging has been on-by-default for a long time." + ); } else if arg == "-Zmiri-fixed-schedule" { miri_config.fixed_scheduling = true; } else if arg == "-Zmiri-deterministic-concurrency" { @@ -543,13 +546,6 @@ fn main() { miri_config.address_reuse_cross_thread_rate = 0.0; miri_config.cmpxchg_weak_failure_rate = 0.0; miri_config.weak_memory_emulation = false; - } else if let Some(retag_fields) = arg.strip_prefix("-Zmiri-retag-fields=") { - miri_config.retag_fields = match retag_fields { - "all" => RetagFields::Yes, - "none" => RetagFields::No, - "scalar" => RetagFields::OnlyScalar, - _ => fatal_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), - }; } else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") { let seed = param.parse::().unwrap_or_else(|_| { fatal_error!("-Zmiri-seed must be an integer that fits into u64") diff --git a/src/borrow_tracker/mod.rs b/src/borrow_tracker/mod.rs index ebca7377fd..e83028f024 100644 --- a/src/borrow_tracker/mod.rs +++ b/src/borrow_tracker/mod.rs @@ -116,8 +116,6 @@ pub struct GlobalStateInner { protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, - /// Whether to recurse into datatypes when searching for pointers to retag. - retag_fields: RetagFields, } impl VisitProvenance for GlobalStateInner { @@ -131,18 +129,6 @@ impl VisitProvenance for GlobalStateInner { /// We need interior mutable access to the global state. pub type GlobalState = RefCell; -/// Policy on whether to recurse into fields to retag -#[derive(Copy, Clone, Debug)] -pub enum RetagFields { - /// Don't retag any fields. - No, - /// Retag all fields. - Yes, - /// Only retag fields of types with Scalar and ScalarPair layout, - /// to match the LLVM `noalias` we generate. - OnlyScalar, -} - /// The flavor of the protector. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum ProtectorKind { @@ -168,7 +154,6 @@ impl GlobalStateInner { pub fn new( borrow_tracker_method: BorrowTrackerMethod, tracked_pointer_tags: FxHashSet, - retag_fields: RetagFields, ) -> Self { GlobalStateInner { borrow_tracker_method, @@ -176,7 +161,6 @@ impl GlobalStateInner { root_ptr_tags: FxHashMap::default(), protected_tags: FxHashMap::default(), tracked_pointer_tags, - retag_fields, } } @@ -244,11 +228,7 @@ pub struct TreeBorrowsParams { impl BorrowTrackerMethod { pub fn instantiate_global_state(self, config: &MiriConfig) -> GlobalState { - RefCell::new(GlobalStateInner::new( - self, - config.tracked_pointer_tags.clone(), - config.retag_fields, - )) + RefCell::new(GlobalStateInner::new(self, config.tracked_pointer_tags.clone())) } pub fn get_tree_borrows_params(self) -> TreeBorrowsParams { diff --git a/src/borrow_tracker/stacked_borrows/mod.rs b/src/borrow_tracker/stacked_borrows/mod.rs index fa60f27185..e8d97491ac 100644 --- a/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/borrow_tracker/stacked_borrows/mod.rs @@ -9,7 +9,7 @@ use std::fmt::Write; use std::sync::atomic::AtomicBool; use std::{cmp, mem}; -use rustc_abi::{BackendRepr, Size}; +use rustc_abi::Size; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; use rustc_middle::ty::layout::HasTypingEnv; @@ -887,14 +887,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { place: &PlaceTy<'tcx>, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; let retag_cause = match kind { RetagKind::TwoPhase => unreachable!(), // can only happen in `retag_ptr_value` RetagKind::FnEntry => RetagCause::FnEntry, RetagKind::Default | RetagKind::Raw => RetagCause::Normal, }; - let mut visitor = - RetagVisitor { ecx: this, kind, retag_cause, retag_fields, in_field: false }; + let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, in_field: false }; return visitor.visit_value(place); // The actual visitor. @@ -902,7 +900,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ecx: &'ecx mut MiriInterpCx<'tcx>, kind: RetagKind, retag_cause: RetagCause, - retag_fields: RetagFields, in_field: bool, } impl<'ecx, 'tcx> RetagVisitor<'ecx, 'tcx> { @@ -967,24 +964,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { self.walk_value(place)?; } _ => { - // Not a reference/pointer/box. Only recurse if configured appropriately. - let recurse = match self.retag_fields { - RetagFields::No => false, - RetagFields::Yes => true, - RetagFields::OnlyScalar => { - // Matching `ArgAbi::new` at the time of writing, only fields of - // `Scalar` and `ScalarPair` ABI are considered. - matches!( - place.layout.backend_repr, - BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..) - ) - } - }; - if recurse { - let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value - self.walk_value(place)?; - self.in_field = in_field; - } + // Not a reference/pointer/box. Recurse. + let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value + self.walk_value(place)?; + self.in_field = in_field; } } diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index 720c5b2394..756ccc5322 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -1,4 +1,4 @@ -use rustc_abi::{BackendRepr, Size}; +use rustc_abi::Size; use rustc_middle::mir::{Mutability, RetagKind}; use rustc_middle::ty::layout::HasTypingEnv; use rustc_middle::ty::{self, Ty}; @@ -468,16 +468,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { place: &PlaceTy<'tcx>, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let options = this.machine.borrow_tracker.as_mut().unwrap().get_mut(); - let retag_fields = options.retag_fields; - let mut visitor = RetagVisitor { ecx: this, kind, retag_fields }; + let mut visitor = RetagVisitor { ecx: this, kind }; return visitor.visit_value(place); // The actual visitor. struct RetagVisitor<'ecx, 'tcx> { ecx: &'ecx mut MiriInterpCx<'tcx>, kind: RetagKind, - retag_fields: RetagFields, } impl<'ecx, 'tcx> RetagVisitor<'ecx, 'tcx> { #[inline(always)] // yes this helps in our benchmarks @@ -545,22 +542,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { self.walk_value(place)?; } _ => { - // Not a reference/pointer/box. Only recurse if configured appropriately. - let recurse = match self.retag_fields { - RetagFields::No => false, - RetagFields::Yes => true, - RetagFields::OnlyScalar => { - // Matching `ArgAbi::new` at the time of writing, only fields of - // `Scalar` and `ScalarPair` ABI are considered. - matches!( - place.layout.backend_repr, - BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..) - ) - } - }; - if recurse { - self.walk_value(place)?; - } + // Not a reference/pointer/box. Recurse. + self.walk_value(place)?; } } interp_ok(()) diff --git a/src/eval.rs b/src/eval.rs index 6daee0ba69..cbc93eb05d 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -88,8 +88,6 @@ pub struct MiriConfig { pub preemption_rate: f64, /// Report the current instruction being executed every N basic blocks. pub report_progress: Option, - /// Whether Stacked Borrows and Tree Borrows retagging should recurse into fields of datatypes. - pub retag_fields: RetagFields, /// The location of the shared object files to load when calling external functions pub native_lib: Vec, /// Whether to enable the new native lib tracing system. @@ -147,7 +145,6 @@ impl Default for MiriConfig { mute_stdout_stderr: false, preemption_rate: 0.01, // 1% report_progress: None, - retag_fields: RetagFields::Yes, native_lib: vec![], native_lib_enable_tracing: false, gc_interval: 10_000, diff --git a/src/lib.rs b/src/lib.rs index 4a48672660..351eced83b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -119,7 +119,7 @@ pub use crate::borrow_tracker::stacked_borrows::{ }; pub use crate::borrow_tracker::tree_borrows::{EvalContextExt as _, Tree}; pub use crate::borrow_tracker::{ - BorTag, BorrowTrackerMethod, EvalContextExt as _, RetagFields, TreeBorrowsParams, + BorTag, BorrowTrackerMethod, EvalContextExt as _, TreeBorrowsParams, }; pub use crate::clock::{Instant, MonotonicClock}; pub use crate::concurrency::cpu_affinity::MAX_CPUS; diff --git a/tests/pass/stacked_borrows/no_field_retagging.rs b/tests/pass/stacked_borrows/no_field_retagging.rs deleted file mode 100644 index 507df068a7..0000000000 --- a/tests/pass/stacked_borrows/no_field_retagging.rs +++ /dev/null @@ -1,19 +0,0 @@ -//@compile-flags: -Zmiri-retag-fields=none - -struct Newtype<'a>(#[allow(dead_code)] &'a mut i32); - -fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { - dealloc(); -} - -// Make sure that we do *not* retag the fields of `Newtype`. -fn main() { - let ptr = Box::into_raw(Box::new(0i32)); - #[rustfmt::skip] // I like my newlines - unsafe { - dealloc_while_running( - Newtype(&mut *ptr), - || drop(Box::from_raw(ptr)), - ) - }; -} diff --git a/tests/pass/stacked_borrows/non_scalar_field_retagging.rs b/tests/pass/stacked_borrows/non_scalar_field_retagging.rs deleted file mode 100644 index 92d8f3237d..0000000000 --- a/tests/pass/stacked_borrows/non_scalar_field_retagging.rs +++ /dev/null @@ -1,23 +0,0 @@ -//@compile-flags: -Zmiri-retag-fields=scalar - -struct Newtype<'a>( - #[allow(dead_code)] &'a mut i32, - #[allow(dead_code)] i32, - #[allow(dead_code)] i32, -); - -fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { - dealloc(); -} - -// Make sure that with -Zmiri-retag-fields=scalar, we do *not* retag the fields of `Newtype`. -fn main() { - let ptr = Box::into_raw(Box::new(0i32)); - #[rustfmt::skip] // I like my newlines - unsafe { - dealloc_while_running( - Newtype(&mut *ptr, 0, 0), - || drop(Box::from_raw(ptr)), - ) - }; -}