Retagging: Recurse into compound values

src/helpers.rs

@@ -158,8 +158,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             unsafe_cell_action: F,
         }
 
-        impl<'ecx, 'a, 'mir, 'tcx, F> ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
-        for UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
+        impl<'ecx, 'a, 'mir, 'tcx, F>
+            ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
+        for
+            UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
         where
             F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
         {
@@ -230,7 +232,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             }
 
             // We should never get to a primitive, but always short-circuit somewhere above
-            fn visit_primitive(&mut self, _val: ImmTy<'tcx, Borrow>) -> EvalResult<'tcx>
+            fn visit_primitive(&mut self, _v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
             {
                 bug!("We should always short-circit before coming to a primitive")
             }

src/stacked_borrows.rs

@@ -4,7 +4,7 @@ use rustc::ty::{self, layout::Size};
 use rustc::hir::{Mutability, MutMutable, MutImmutable};
 
 use crate::{
-    EvalResult, EvalErrorKind, MiriEvalContext, HelpersEvalContextExt,
+    EvalResult, EvalErrorKind, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor,
     MemoryKind, MiriMemoryKind, RangeMap, AllocId, Allocation, AllocationExtra,
     Pointer, MemPlace, Scalar, Immediate, ImmTy, PlaceTy, MPlaceTy,
 };
@@ -303,6 +303,9 @@ impl<'tcx> Stacks {
         trace!("{} access of tag {:?}: {:?}, size {}",
             if is_write { "read" } else { "write" },
             ptr.tag, ptr, size.bytes());
+        // Even reads can have a side-effect, by invalidating other references.
+        // This is fundamentally necessary since `&mut` asserts that there
+        // are no accesses through other references, not even reads.
         let mut stacks = self.stacks.borrow_mut();
         for stack in stacks.iter_mut(ptr.offset, size) {
             stack.access(ptr.tag, is_write)?;
@@ -311,6 +314,7 @@ impl<'tcx> Stacks {
     }
 
     /// Reborrow the given pointer to the new tag for the given kind of reference.
+    /// This works on `&self` because we might encounter references to constant memory.
     fn reborrow(
         &self,
         ptr: Pointer<Borrow>,
@@ -414,8 +418,16 @@ pub trait EvalContextExt<'tcx> {
         kind: MemoryKind<MiriMemoryKind>,
     ) -> Borrow;
 
-    /// Retag an indidual pointer, returning the retagged version.
+    /// Reborrow the given place, returning the newly tagged ptr to it.
     fn reborrow(
+        &mut self,
+        place: MPlaceTy<'tcx, Borrow>,
+        size: Size,
+        new_bor: Borrow
+    ) -> EvalResult<'tcx, Pointer<Borrow>>;
+
+    /// Retag an indidual pointer, returning the retagged version.
+    fn retag_reference(
         &mut self,
         ptr: ImmTy<'tcx, Borrow>,
         mutbl: Mutability,
@@ -536,22 +548,46 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> {
     }
 
     /// The given place may henceforth be accessed through raw pointers.
+    #[inline(always)]
     fn escape_to_raw(
         &mut self,
         place: MPlaceTy<'tcx, Borrow>,
         size: Size,
     ) -> EvalResult<'tcx> {
-        trace!("escape_to_raw: {:?} is now accessible by raw pointers", *place);
-        // Get the allocation
+        self.reborrow(place, size, Borrow::default())?;
+        Ok(())
+    }
+
+    fn reborrow(
+        &mut self,
+        place: MPlaceTy<'tcx, Borrow>,
+        size: Size,
+        new_bor: Borrow
+    ) -> EvalResult<'tcx, Pointer<Borrow>> {
         let ptr = place.ptr.to_ptr()?;
-        self.memory().check_bounds(ptr, size, false)?; // `ptr_dereference` wouldn't do any checks if this is a raw ptr
+        let new_ptr = Pointer::new_with_tag(ptr.alloc_id, ptr.offset, new_bor);
+        trace!("reborrow: Creating new reference for {:?} (pointee {}): {:?}",
+            ptr, place.layout.ty, new_bor);
+
+        // Get the allocation.  It might not be mutable, so we cannot use `get_mut`.
+        self.memory().check_bounds(ptr, size, false)?;
         let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!");
-        // Re-borrow to raw.  This is a NOP for shared borrows, but we do not know the borrow
-        // type here and that's also okay.  Freezing does not matter here.
-        alloc.extra.reborrow(ptr, size, Borrow::default(), RefKind::Raw)
+        // Update the stacks.
+        if let Borrow::Shr(Some(_)) = new_bor {
+            // Reference that cares about freezing. We need a frozen-sensitive reborrow.
+            self.visit_freeze_sensitive(place, size, |cur_ptr, size, frozen| {
+                let kind = if frozen { RefKind::Frozen } else { RefKind::Raw };
+                alloc.extra.reborrow(cur_ptr, size, new_bor, kind)
+            })?;
+        } else {
+            // Just treat this as one big chunk.
+            let kind = if new_bor.is_unique() { RefKind::Unique } else { RefKind::Raw };
+            alloc.extra.reborrow(ptr, size, new_bor, kind)?;
+        }
+        Ok(new_ptr)
     }
 
-    fn reborrow(
+    fn retag_reference(
         &mut self,
         val: ImmTy<'tcx, Borrow>,
         mutbl: Mutability,
@@ -566,33 +602,17 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> {
             return Ok(*val);
         }
 
-        // Prepare to re-borrow this place.
-        let ptr = place.ptr.to_ptr()?;
+        // Compute new borrow.
         let time = self.machine.stacked_borrows.increment_clock();
         let new_bor = match mutbl {
             MutMutable => Borrow::Uniq(time),
             MutImmutable => Borrow::Shr(Some(time)),
         };
-        trace!("reborrow: Creating new {:?} reference for {:?} (pointee {}): {:?}",
-            mutbl, ptr, place.layout.ty, new_bor);
 
-        // Get the allocation.  It might not be mutable, so we cannot use `get_mut`.
-        self.memory().check_bounds(ptr, size, false)?;
-        let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!");
-        // Update the stacks.
-        if mutbl == MutImmutable {
-            // Shared reference. We need a frozen-sensitive reborrow.
-            self.visit_freeze_sensitive(place, size, |cur_ptr, size, frozen| {
-                let kind = if frozen { RefKind::Frozen } else { RefKind::Raw };
-                alloc.extra.reborrow(cur_ptr, size, new_bor, kind)
-            })?;
-        } else {
-            // Mutable reference. Just treat this as one big chunk.
-            alloc.extra.reborrow(ptr, size, new_bor, RefKind::Unique)?;
-        }
+        // Reborrow.
+        let new_ptr = self.reborrow(place, size, new_bor)?;
 
         // Return new ptr
-        let new_ptr = Pointer::new_with_tag(ptr.alloc_id, ptr.offset, new_bor);
         let new_place = MemPlace { ptr: Scalar::Ptr(new_ptr), ..*place };
         Ok(new_place.to_ref())
     }
@@ -602,17 +622,57 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> {
         _fn_entry: bool,
         place: PlaceTy<'tcx, Borrow>
     ) -> EvalResult<'tcx> {
-        // For now, we only retag if the toplevel type is a reference.
-        // TODO: Recurse into structs and enums, sharing code with validation.
         // TODO: Honor `fn_entry`.
-        let mutbl = match place.layout.ty.sty {
-            ty::Ref(_, _, mutbl) => mutbl, // go ahead
-            _ => return Ok(()), // do nothing, for now
+
+        // We need a visitor to visit all references.  However, that requires
+        // a `MemPlace`, so we have a fast path for reference types that
+        // avoids allocating.
+        match place.layout.ty.sty {
+            ty::Ref(_, _, mutbl) => {
+                // fast path
+                let val = self.read_immediate(self.place_to_op(place)?)?;
+                let val = self.retag_reference(val, mutbl)?;
+                self.write_immediate(val, place)?;
+                return Ok(());
+            }
+            _ => {}, // handled with the general case below
         };
-        // Retag the pointer and write it back.
-        let val = self.read_immediate(self.place_to_op(place)?)?;
-        let val = self.reborrow(val, mutbl)?;
-        self.write_immediate(val, place)?;
+        let place = self.force_allocation(place)?;
+
+        let mut visitor = RetagVisitor { ecx: self };
+        visitor.visit_value(place)?;
+
+        // The actual visitor
+        struct RetagVisitor<'ecx, 'a, 'mir, 'tcx> {
+            ecx: &'ecx mut MiriEvalContext<'a, 'mir, 'tcx>,
+        }
+        impl<'ecx, 'a, 'mir, 'tcx>
+            MutValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
+        for
+            RetagVisitor<'ecx, 'a, 'mir, 'tcx>
+        {
+            type V = MPlaceTy<'tcx, Borrow>;
+
+            #[inline(always)]
+            fn ecx(&mut self) -> &mut MiriEvalContext<'a, 'mir, 'tcx> {
+                &mut self.ecx
+            }
+
+            // Primitives of reference type, that is the one thing we are interested in.
+            fn visit_primitive(&mut self, place: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
+            {
+                let mutbl = match place.layout.ty.sty {
+                    ty::Ref(_, _, mutbl) => mutbl,
+                    ty::Adt(..) if place.layout.ty.is_box() => MutMutable,
+                    _ => return Ok(()), // nothing to do
+                };
+                let val = self.ecx.read_immediate(place.into())?;
+                let val = self.ecx.retag_reference(val, mutbl)?;
+                self.ecx.write_immediate(val, place.into())?;
+                Ok(())
+            }
+        }
+
         Ok(())
     }
 }

tests/compile-fail-fullmir/stacked_borrows/box_exclusive_violation1.rs

@@ -0,0 +1,29 @@
+fn demo_mut_advanced_unique(mut our: Box<i32>) -> i32 {
+  unknown_code_1(&*our);
+
+  // This "re-asserts" uniqueness of the reference: After writing, we know
+  // our tag is at the top of the stack.
+  *our = 5;
+
+  unknown_code_2();
+
+  // We know this will return 5
+  *our //~ ERROR does not exist on the stack
+}
+
+// Now comes the evil context
+use std::ptr;
+
+static mut LEAK: *mut i32 = ptr::null_mut();
+
+fn unknown_code_1(x: &i32) { unsafe {
+    LEAK = x as *const _ as *mut _;
+} }
+
+fn unknown_code_2() { unsafe {
+    *LEAK = 7;
+} }
+
+fn main() {
+    assert_eq!(demo_mut_advanced_unique(Box::new(0)), 5);
+}

tests/compile-fail-fullmir/stacked_borrows/buggy_split_at_mut.rs

@@ -11,6 +11,7 @@ mod safe {
             assert!(mid <= len);
 
             (from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid"
+            //~^ ERROR does not exist on the stack
             from_raw_parts_mut(ptr.offset(mid as isize), len - mid))
         }
     }
@@ -19,7 +20,6 @@ mod safe {
 fn main() {
     let mut array = [1,2,3,4];
     let (a, b) = safe::split_at_mut(&mut array, 0);
-    //~^ ERROR does not exist on the stack
     a[1] = 5;
     b[1] = 6;
 }

tests/compile-fail-fullmir/stacked_borrows/return_invalid_mut_option.rs

@@ -0,0 +1,11 @@
+// Make sure that we cannot return a `&mut` that got already invalidated, not even in an `Option`.
+fn foo(x: &mut (i32, i32)) -> Option<&mut i32> {
+    let xraw = x as *mut (i32, i32);
+    let ret = Some(unsafe { &mut (*xraw).1 });
+    let _val = unsafe { *xraw }; // invalidate xref
+    ret //~ ERROR does not exist on the stack
+}
+
+fn main() {
+    foo(&mut (1, 2));
+}

tests/compile-fail-fullmir/stacked_borrows/return_invalid_mut_tuple.rs

@@ -0,0 +1,11 @@
+// Make sure that we cannot return a `&mut` that got already invalidated, not even in a tuple.
+fn foo(x: &mut (i32, i32)) -> (&mut i32,) {
+    let xraw = x as *mut (i32, i32);
+    let ret = (unsafe { &mut (*xraw).1 },);
+    let _val = unsafe { *xraw }; // invalidate xref
+    ret //~ ERROR does not exist on the stack
+}
+
+fn main() {
+    foo(&mut (1, 2));
+}

tests/compile-fail-fullmir/stacked_borrows/return_invalid_shr_option.rs

@@ -0,0 +1,11 @@
+// Make sure that we cannot return a `&` that got already invalidated, not even in an `Option`.
+fn foo(x: &mut (i32, i32)) -> Option<&i32> {
+    let xraw = x as *mut (i32, i32);
+    let ret = Some(unsafe { &(*xraw).1 });
+    unsafe { *xraw = (42, 23) }; // unfreeze
+    ret //~ ERROR is not frozen
+}
+
+fn main() {
+    foo(&mut (1, 2));
+}

tests/compile-fail-fullmir/stacked_borrows/return_invalid_shr_tuple.rs

@@ -0,0 +1,11 @@
+// Make sure that we cannot return a `&` that got already invalidated, not even in a tuple.
+fn foo(x: &mut (i32, i32)) -> (&i32,) {
+    let xraw = x as *mut (i32, i32);
+    let ret = (unsafe { &(*xraw).1 },);
+    unsafe { *xraw = (42, 23) }; // unfreeze
+    ret //~ ERROR is not frozen
+}
+
+fn main() {
+    foo(&mut (1, 2));
+}