Auto merge of rust-lang#2196 - carbotaniuman:permissive-stacked-borrows, r=RalfJung

Handle wildcard pointers in SB

This uses an permissive `Unknown` implementation, where a wildcard pointer (and any SRW derived from a wildcard pointer) can access any previously-exposed SB tag. This is missing any meaningful test-cases, and all of the edge-cases have not yet been worked through.

I think there's also some bugs here with differing Unknowns in different ranges and having things behave really weirdly too, alongside some issues with retagging to `SRO` or `Unique`.

Author: bors

README.md

@@ -358,9 +358,7 @@ to Miri failing to detect cases of undefined behavior in a program.
   for pointer-to-int and int-to-pointer casts, respectively. This will
   necessarily miss some bugs as those semantics are not efficiently
   implementable in a sanitizer, but it will only miss bugs that concerns
-  memory/pointers which is subject to these operations. Also note that this flag
-  is currently incompatible with Stacked Borrows, so you will have to also pass
-  `-Zmiri-disable-stacked-borrows` to use this.
+  memory/pointers which is subject to these operations.
 * `-Zmiri-symbolic-alignment-check` makes the alignment check more strict.  By
   default, alignment is checked by casting the pointer to an integer, and making
   sure that is a multiple of the alignment.  This can lead to cases where a

src/diagnostics.rs

@@ -8,7 +8,7 @@ use rustc_middle::ty;
 use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol};
 
 use crate::helpers::HexRange;
-use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind, SbTag};
+use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind};
 use crate::*;
 
 /// Details of premature program termination.
@@ -61,9 +61,9 @@ impl MachineStopType for TerminationInfo {}
 /// Miri specific diagnostics
 pub enum NonHaltingDiagnostic {
     CreatedPointerTag(NonZeroU64),
-    /// This `Item` was popped from the borrow stack, either due to a grant of
-    /// `AccessKind` to `SbTag` or a deallocation when the second argument is `None`.
-    PoppedPointerTag(Item, Option<(SbTag, AccessKind)>),
+    /// This `Item` was popped from the borrow stack, either due to an access with the given tag or
+    /// a deallocation when the second argument is `None`.
+    PoppedPointerTag(Item, Option<(SbTagExtra, AccessKind)>),
     CreatedCallId(CallId),
     CreatedAlloc(AllocId),
     FreedAlloc(AllocId),

src/intptrcast.rs

@@ -101,14 +101,16 @@ impl<'mir, 'tcx> GlobalStateInner {
         }
     }
 
-    pub fn expose_addr(ecx: &MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId) {
-        trace!("Exposing allocation id {:?}", alloc_id);
-
-        let mut global_state = ecx.machine.intptrcast.borrow_mut();
+    pub fn expose_ptr(ecx: &mut MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId, sb: SbTag) {
+        let global_state = ecx.machine.intptrcast.get_mut();
         // In legacy and strict mode, we don't need this, so we can save some cycles
         // by not tracking it.
         if global_state.provenance_mode == ProvenanceMode::Permissive {
+            trace!("Exposing allocation id {alloc_id:?}");
             global_state.exposed.insert(alloc_id);
+            if ecx.machine.stacked_borrows.is_some() {
+                ecx.expose_tag(alloc_id, sb);
+            }
         }
     }
 
@@ -140,9 +142,7 @@ impl<'mir, 'tcx> GlobalStateInner {
                 // Determine the allocation this points to at cast time.
                 let alloc_id = Self::alloc_id_from_addr(ecx, addr);
                 Pointer::new(
-                    alloc_id.map(|alloc_id| {
-                        Tag::Concrete(ConcreteTag { alloc_id, sb: SbTag::Untagged })
-                    }),
+                    alloc_id.map(|alloc_id| Tag::Concrete { alloc_id, sb: SbTag::Untagged }),
                     Size::from_bytes(addr),
                 )
             }
@@ -220,8 +220,8 @@ impl<'mir, 'tcx> GlobalStateInner {
     ) -> Option<(AllocId, Size)> {
         let (tag, addr) = ptr.into_parts(); // addr is absolute (Tag provenance)
 
-        let alloc_id = if let Tag::Concrete(concrete) = tag {
-            concrete.alloc_id
+        let alloc_id = if let Tag::Concrete { alloc_id, .. } = tag {
+            alloc_id
         } else {
             // A wildcard pointer.
             assert_eq!(ecx.machine.intptrcast.borrow().provenance_mode, ProvenanceMode::Permissive);

src/lib.rs

@@ -6,6 +6,8 @@
 #![feature(let_else)]
 #![feature(io_error_more)]
 #![feature(yeet_expr)]
+#![feature(is_some_with)]
+#![feature(nonzero_ops)]
 #![warn(rust_2018_idioms)]
 #![allow(
     clippy::collapsible_else_if,
@@ -80,15 +82,15 @@ pub use crate::eval::{
 pub use crate::helpers::{CurrentSpan, EvalContextExt as HelpersEvalContextExt};
 pub use crate::intptrcast::ProvenanceMode;
 pub use crate::machine::{
-    AllocExtra, ConcreteTag, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt,
-    MiriMemoryKind, Tag, NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
+    AllocExtra, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, Tag,
+    NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
 };
 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::{
-    CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, Stack,
-    Stacks,
+    CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, SbTagExtra,
+    Stack, Stacks,
 };
 pub use crate::sync::{CondvarId, EvalContextExt as SyncEvalContextExt, MutexId, RwLockId};
 pub use crate::thread::{

src/machine.rs

@@ -130,17 +130,14 @@ impl fmt::Display for MiriMemoryKind {
 /// Pointer provenance (tag).
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum Tag {
-    Concrete(ConcreteTag),
+    Concrete {
+        alloc_id: AllocId,
+        /// Stacked Borrows tag.
+        sb: SbTag,
+    },
     Wildcard,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct ConcreteTag {
-    pub alloc_id: AllocId,
-    /// Stacked Borrows tag.
-    pub sb: SbTag,
-}
-
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 static_assert_size!(Pointer<Tag>, 24);
 // #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
@@ -160,15 +157,15 @@ impl Provenance for Tag {
         write!(f, "0x{:x}", addr.bytes())?;
 
         match tag {
-            Tag::Concrete(tag) => {
+            Tag::Concrete { alloc_id, sb } => {
                 // Forward `alternate` flag to `alloc_id` printing.
                 if f.alternate() {
-                    write!(f, "[{:#?}]", tag.alloc_id)?;
+                    write!(f, "[{:#?}]", alloc_id)?;
                 } else {
-                    write!(f, "[{:?}]", tag.alloc_id)?;
+                    write!(f, "[{:?}]", alloc_id)?;
                 }
                 // Print Stacked Borrows tag.
-                write!(f, "{:?}", tag.sb)?;
+                write!(f, "{:?}", sb)?;
             }
             Tag::Wildcard => {
                 write!(f, "[Wildcard]")?;
@@ -180,7 +177,7 @@ impl Provenance for Tag {
 
     fn get_alloc_id(self) -> Option<AllocId> {
         match self {
-            Tag::Concrete(concrete) => Some(concrete.alloc_id),
+            Tag::Concrete { alloc_id, .. } => Some(alloc_id),
             Tag::Wildcard => None,
         }
     }
@@ -489,7 +486,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
     type AllocExtra = AllocExtra;
 
     type PointerTag = Tag;
-    type TagExtra = SbTag;
+    type TagExtra = SbTagExtra;
 
     type MemoryMap =
         MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<Tag, Self::AllocExtra>)>;
@@ -649,7 +646,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         let alloc: Allocation<Tag, Self::AllocExtra> = alloc.convert_tag_add_extra(
             &ecx.tcx,
             AllocExtra {
-                stacked_borrows: stacks,
+                stacked_borrows: stacks.map(RefCell::new),
                 data_race: race_alloc,
                 weak_memory: buffer_alloc,
             },
@@ -682,7 +679,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
             SbTag::Untagged
         };
         Pointer::new(
-            Tag::Concrete(ConcreteTag { alloc_id: ptr.provenance, sb: sb_tag }),
+            Tag::Concrete { alloc_id: ptr.provenance, sb: sb_tag },
             Size::from_bytes(absolute_addr),
         )
     }
@@ -708,8 +705,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         ptr: Pointer<Self::PointerTag>,
     ) -> InterpResult<'tcx> {
         match ptr.provenance {
-            Tag::Concrete(concrete) =>
-                intptrcast::GlobalStateInner::expose_addr(ecx, concrete.alloc_id),
+            Tag::Concrete { alloc_id, sb } => {
+                intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb);
+            }
             Tag::Wildcard => {
                 // No need to do anything for wildcard pointers as
                 // their provenances have already been previously exposed.
@@ -728,8 +726,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
 
         rel.map(|(alloc_id, size)| {
             let sb = match ptr.provenance {
-                Tag::Concrete(ConcreteTag { sb, .. }) => sb,
-                Tag::Wildcard => SbTag::Untagged,
+                Tag::Concrete { sb, .. } => SbTagExtra::Concrete(sb),
+                Tag::Wildcard => SbTagExtra::Wildcard,
             };
             (alloc_id, size, sb)
         })
@@ -747,7 +745,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
             data_race.read(alloc_id, range, machine.data_race.as_ref().unwrap())?;
         }
         if let Some(stacked_borrows) = &alloc_extra.stacked_borrows {
-            stacked_borrows.memory_read(
+            stacked_borrows.borrow_mut().memory_read(
                 alloc_id,
                 tag,
                 range,
@@ -773,7 +771,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
             data_race.write(alloc_id, range, machine.data_race.as_mut().unwrap())?;
         }
         if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
-            stacked_borrows.memory_written(
+            stacked_borrows.get_mut().memory_written(
                 alloc_id,
                 tag,
                 range,
@@ -802,7 +800,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
             data_race.deallocate(alloc_id, range, machine.data_race.as_mut().unwrap())?;
         }
         if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
-            stacked_borrows.memory_deallocated(
+            stacked_borrows.get_mut().memory_deallocated(
                 alloc_id,
                 tag,
                 range,