diff --git a/README.md b/README.md index f16751cd7478..f18a5d668c39 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,8 @@ for example: * Not sufficiently aligned memory accesses and references * Violation of *some* basic type invariants (a `bool` that is not 0 or 1, for example, or an invalid enum discriminant) -* **Experimental**: Violations of the rules governing aliasing for reference types +* **Experimental**: Violations of the [Stacked Borrows] rules governing aliasing + for reference types Miri has already discovered some [real-world bugs](#bugs-found-by-miri). If you found a bug with Miri, we'd appreciate if you tell us and we'll add it to the @@ -47,6 +48,7 @@ program, and cannot run all programs: [mir]: https://github.com/rust-lang/rfcs/blob/master/text/1211-mir.md [`unreachable_unchecked`]: https://doc.rust-lang.org/stable/std/hint/fn.unreachable_unchecked.html [`copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/ptr/fn.copy_nonoverlapping.html +[Stacked Borrows]: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md ## Using Miri @@ -152,10 +154,13 @@ Several `-Z` flags are relevant for Miri: **NOTE**: This entropy is not good enough for cryptographic use! Do not generate secret keys in Miri or perform other kinds of cryptographic operations that rely on proper random numbers. -* `-Zmiri-disable-validation` disables enforcing validity invariants and - reference aliasing rules, which are enforced by default. This is mostly - useful for debugging. It means Miri will miss bugs in your program. However, - this can also help to make Miri run faster. +* `-Zmiri-disable-validation` disables enforcing validity invariants, which are + enforced by default. This is mostly useful for debugging. It means Miri will + miss bugs in your program. However, this can also help to make Miri run + faster. +* `-Zmiri-disable-stacked-borrows` disables checking the experimental + [Stacked Borrows] aliasing rules. This can make Miri run faster, but it also + means no aliasing violations will be detected. * `-Zmiri-disable-isolation` disables host host isolation. As a consequence, the program has access to host resources such as environment variables, file systems, and randomness. @@ -234,7 +239,7 @@ Definite bugs found: * [The Unix allocator calling `posix_memalign` in an invalid way](https://github.com/rust-lang/rust/issues/62251) * [`getrandom` calling the `getrandom` syscall in an invalid way](https://github.com/rust-random/getrandom/pull/73) -Violations of Stacked Borrows found that are likely bugs (but Stacked Borrows is currently just an experiment): +Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment): * [`VecDeque` creating overlapping mutable references](https://github.com/rust-lang/rust/pull/56161) * [`BTreeMap` creating mutable references that overlap with shared references](https://github.com/rust-lang/rust/pull/58431) diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 228c50e4a57b..9523609889cd 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -130,6 +130,7 @@ fn main() { // Parse our arguments and split them across `rustc` and `miri`. let mut validate = true; + let mut stacked_borrows = true; let mut communicate = false; let mut ignore_leaks = false; let mut seed: Option = None; @@ -150,6 +151,9 @@ fn main() { "-Zmiri-disable-validation" => { validate = false; } + "-Zmiri-disable-stacked-borrows" => { + stacked_borrows = false; + } "-Zmiri-disable-isolation" => { communicate = true; } @@ -229,6 +233,7 @@ fn main() { debug!("miri arguments: {:?}", miri_args); let miri_config = miri::MiriConfig { validate, + stacked_borrows, communicate, ignore_leaks, excluded_env_vars, diff --git a/src/eval.rs b/src/eval.rs index 9ce3d2b08c23..5a0c766b5ae1 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -14,8 +14,10 @@ use crate::*; /// Configuration needed to spawn a Miri instance. #[derive(Clone)] pub struct MiriConfig { - /// Determine if validity checking and Stacked Borrows are enabled. + /// Determine if validity checking is enabled. pub validate: bool, + /// Determines if Stacked Borrows is enabled. + pub stacked_borrows: bool, /// Determines if communication with the host environment is enabled. pub communicate: bool, /// Determines if memory leaks should be ignored. @@ -52,6 +54,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( MemoryExtra::new( StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate, + config.stacked_borrows, config.tracked_pointer_tag, ), ); diff --git a/src/lib.rs b/src/lib.rs index 85ee98aa3a12..7cea203d5e76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,7 +56,7 @@ pub use crate::mono_hash_map::MonoHashMap; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; pub use crate::range_map::RangeMap; pub use crate::stacked_borrows::{ - EvalContextExt as StackedBorEvalContextExt, GlobalState, Item, Permission, PtrId, Stack, + EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, Stack, Stacks, Tag, }; diff --git a/src/machine.rs b/src/machine.rs index 5d933fe8a7f6..7aaf97cc1236 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -4,6 +4,7 @@ use std::borrow::Cow; use std::cell::RefCell; use std::rc::Rc; +use std::num::NonZeroU64; use rand::rngs::StdRng; @@ -63,14 +64,14 @@ impl Into> for MiriMemoryKind { /// Extra per-allocation data #[derive(Debug, Clone)] pub struct AllocExtra { - /// Stacked Borrows state is only added if validation is enabled. + /// Stacked Borrows state is only added if it is enabled. pub stacked_borrows: Option, } /// Extra global memory data #[derive(Clone, Debug)] pub struct MemoryExtra { - pub stacked_borrows: stacked_borrows::MemoryExtra, + pub stacked_borrows: Option, pub intptrcast: intptrcast::MemoryExtra, /// The random number generator used for resolving non-determinism. @@ -81,9 +82,14 @@ pub struct MemoryExtra { } impl MemoryExtra { - pub fn new(rng: StdRng, validate: bool, tracked_pointer_tag: Option) -> Self { + pub fn new(rng: StdRng, validate: bool, stacked_borrows: bool, tracked_pointer_tag: Option) -> Self { + let stacked_borrows = if stacked_borrows { + Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag)))) + } else { + None + }; MemoryExtra { - stacked_borrows: Rc::new(RefCell::new(GlobalState::new(tracked_pointer_tag))), + stacked_borrows, intptrcast: Default::default(), rng: RefCell::new(rng), validate, @@ -299,11 +305,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { ) -> (Cow<'b, Allocation>, Self::PointerTag) { let kind = kind.expect("we set our STATIC_KIND so this cannot be None"); let alloc = alloc.into_owned(); - let (stacks, base_tag) = if memory_extra.validate { + let (stacks, base_tag) = if let Some(stacked_borrows) = memory_extra.stacked_borrows.as_ref() { let (stacks, base_tag) = Stacks::new_allocation( id, alloc.size, - Rc::clone(&memory_extra.stacked_borrows), + Rc::clone(stacked_borrows), kind, ); (Some(stacks), base_tag) @@ -311,15 +317,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { // No stacks, no tag. (None, Tag::Untagged) }; - let mut stacked_borrows = memory_extra.stacked_borrows.borrow_mut(); + let mut stacked_borrows = memory_extra.stacked_borrows.as_ref().map(|sb| sb.borrow_mut()); let alloc: Allocation = alloc.with_tags_and_extra( |alloc| { - if !memory_extra.validate { - Tag::Untagged - } else { + if let Some(stacked_borrows) = stacked_borrows.as_mut() { // Only statics may already contain pointers at this point assert_eq!(kind, MiriMemoryKind::Static.into()); stacked_borrows.static_base_ptr(alloc) + } else { + Tag::Untagged } }, AllocExtra { stacked_borrows: stacks }, @@ -329,10 +335,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { #[inline(always)] fn tag_static_base_pointer(memory_extra: &MemoryExtra, id: AllocId) -> Self::PointerTag { - if !memory_extra.validate { - Tag::Untagged + if let Some(stacked_borrows) = memory_extra.stacked_borrows.as_ref() { + stacked_borrows.borrow_mut().static_base_ptr(id) } else { - memory_extra.stacked_borrows.borrow_mut().static_base_ptr(id) + Tag::Untagged } } @@ -342,7 +348,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { kind: mir::RetagKind, place: PlaceTy<'tcx, Tag>, ) -> InterpResult<'tcx> { - if !Self::enforce_validity(ecx) { + if ecx.memory.extra.stacked_borrows.is_none() { // No tracking. Ok(()) } else { @@ -352,8 +358,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { #[inline(always)] fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, FrameData<'tcx>> { + let call_id = ecx.memory.extra.stacked_borrows.as_ref().map_or( + NonZeroU64::new(1).unwrap(), + |stacked_borrows| stacked_borrows.borrow_mut().new_call(), + ); Ok(FrameData { - call_id: ecx.memory.extra.stacked_borrows.borrow_mut().new_call(), + call_id, catch_panic: None, }) } diff --git a/src/shims/panic.rs b/src/shims/panic.rs index 880932ae0472..e930af7f46c5 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -146,7 +146,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } else { StackPopInfo::Normal }; - this.memory.extra.stacked_borrows.borrow_mut().end_call(extra.call_id); + if let Some(stacked_borrows) = this.memory.extra.stacked_borrows.as_ref() { + stacked_borrows.borrow_mut().end_call(extra.call_id); + } Ok(res) } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 1b7a118e637d..9a674830c8ec 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -575,7 +575,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // breaking `Rc::from_raw`. RefKind::Raw { .. } => Tag::Untagged, // All other pointesr are properly tracked. - _ => Tag::Tagged(this.memory.extra.stacked_borrows.borrow_mut().new_ptr()), + _ => Tag::Tagged(this.memory.extra.stacked_borrows.as_ref().unwrap().borrow_mut().new_ptr()), }; // Reborrow. diff --git a/tests/compile-fail/modifying_constants.rs b/tests/compile-fail/modifying_constants.rs index 4546e8a4d7c9..9770917b629b 100644 --- a/tests/compile-fail/modifying_constants.rs +++ b/tests/compile-fail/modifying_constants.rs @@ -1,5 +1,5 @@ -// This should fail even without validation -// compile-flags: -Zmiri-disable-validation +// This should fail even without validation/SB +// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows fn main() { let x = &1; // the `&1` is promoted to a constant, but it used to be that only the pointer is marked static, not the pointee diff --git a/tests/compile-fail/reference_to_packed.rs b/tests/compile-fail/reference_to_packed.rs index befe96f2b35d..030353c2cedb 100644 --- a/tests/compile-fail/reference_to_packed.rs +++ b/tests/compile-fail/reference_to_packed.rs @@ -1,5 +1,5 @@ -// This should fail even without validation -// compile-flags: -Zmiri-disable-validation +// This should fail even without validation/SB +// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows #![allow(dead_code, unused_variables)] diff --git a/tests/compile-fail/stack_free.rs b/tests/compile-fail/stack_free.rs index 43cc17308c06..b8ed2e3f1f35 100644 --- a/tests/compile-fail/stack_free.rs +++ b/tests/compile-fail/stack_free.rs @@ -1,5 +1,5 @@ -// Validation changes why we fail -// compile-flags: -Zmiri-disable-validation +// Validation/SB changes why we fail +// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows // error-pattern: tried to deallocate `Stack` memory but gave `Machine(Rust)` as the kind diff --git a/tests/compile-fail/static_memory_modification1.rs b/tests/compile-fail/static_memory_modification1.rs index 07a277a16f3a..a7bb33431e72 100644 --- a/tests/compile-fail/static_memory_modification1.rs +++ b/tests/compile-fail/static_memory_modification1.rs @@ -1,5 +1,5 @@ -// Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmiri-disable-validation +// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail +// compile-flags: -Zmiri-disable-stacked-borrows static X: usize = 5; diff --git a/tests/compile-fail/static_memory_modification2.rs b/tests/compile-fail/static_memory_modification2.rs index 73928f533cb7..065206a60dbf 100644 --- a/tests/compile-fail/static_memory_modification2.rs +++ b/tests/compile-fail/static_memory_modification2.rs @@ -1,5 +1,5 @@ -// Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmiri-disable-validation +// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail +// compile-flags: -Zmiri-disable-stacked-borrows use std::mem::transmute; diff --git a/tests/compile-fail/static_memory_modification3.rs b/tests/compile-fail/static_memory_modification3.rs index 280f34a5a021..94f88205073e 100644 --- a/tests/compile-fail/static_memory_modification3.rs +++ b/tests/compile-fail/static_memory_modification3.rs @@ -1,5 +1,5 @@ -// Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmiri-disable-validation +// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail +// compile-flags: -Zmiri-disable-stacked-borrows use std::mem::transmute; diff --git a/tests/run-pass/observed_local_mut.rs b/tests/run-pass/observed_local_mut.rs index c58e8c84bb2e..888b6f85e3fd 100644 --- a/tests/run-pass/observed_local_mut.rs +++ b/tests/run-pass/observed_local_mut.rs @@ -1,5 +1,5 @@ -// Validation catches this (correctly) as UB. -// compile-flags: -Zmiri-disable-validation +// Stacked Borrows catches this (correctly) as UB. +// compile-flags: -Zmiri-disable-stacked-borrows // This test is intended to guard against the problem described in commit // 39bb1254d1eaf74f45a4e741097e33fc942168d5. diff --git a/tests/run-pass/transmute_fat.rs b/tests/run-pass/transmute_fat.rs index 27da44935b10..238122de8d4c 100644 --- a/tests/run-pass/transmute_fat.rs +++ b/tests/run-pass/transmute_fat.rs @@ -1,5 +1,5 @@ -// Validation disallows this becuase the reference is never cast to a raw pointer. -// compile-flags: -Zmiri-disable-validation +// Stacked Borrows disallows this becuase the reference is never cast to a raw pointer. +// compile-flags: -Zmiri-disable-stacked-borrows fn main() { // If we are careful, we can exploit data layout...