add an option to disable Stacked Borrows

Author: RalfJung

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)

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<u64> = 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,

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,
         ),
     );

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,
 };
 

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<MemoryKind<MiriMemoryKind>> 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<stacked_borrows::AllocExtra>,
 }
 
 /// Extra global memory data
 #[derive(Clone, Debug)]
 pub struct MemoryExtra {
-    pub stacked_borrows: stacked_borrows::MemoryExtra,
+    pub stacked_borrows: Option<stacked_borrows::MemoryExtra>,
     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<PtrId>) -> Self {
+    pub fn new(rng: StdRng, validate: bool, stacked_borrows: bool, tracked_pointer_tag: Option<PtrId>) -> 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,27 +305,27 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
     ) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, 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)
         } else {
             // 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<Tag, Self::AllocExtra> = 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,
         })
     }

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)
     }
 

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.

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

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)]
 

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