]> git.lizzy.rs Git - rust.git/blobdiff - src/stacked_borrows.rs
Tidy up comments and function layout, should fix most of the review notes.
[rust.git] / src / stacked_borrows.rs
index 37f2acf4c3bce3b308d5ca823703b02512606818..616950eb0a0a44a5ca923ae78a7a68c3c2c6e9e3 100644 (file)
@@ -6,9 +6,12 @@
 use std::num::NonZeroU64;
 use std::rc::Rc;
 
+use log::trace;
+
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc::mir::RetagKind;
-use rustc::ty::{self, layout::Size};
+use rustc_middle::mir::RetagKind;
+use rustc_middle::ty;
+use rustc_target::abi::{Align, LayoutOf, Size};
 use rustc_hir::Mutability;
 
 use crate::*;
@@ -101,8 +104,12 @@ pub struct GlobalState {
     next_call_id: CallId,
     /// Those call IDs corresponding to functions that are still running.
     active_calls: FxHashSet<CallId>,
-    /// The id to trace in this execution run
+    /// The pointer id to trace
     tracked_pointer_tag: Option<PtrId>,
+    /// The call id to trace
+    tracked_call_id: Option<CallId>,
+    /// Whether to track raw pointers.
+    track_raw: bool,
 }
 /// Memory extra state gives us interior mutable access to the global state.
 pub type MemoryExtra = Rc<RefCell<GlobalState>>;
@@ -150,18 +157,23 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 
 /// Utilities for initialization and ID generation
 impl GlobalState {
-    pub fn new(tracked_pointer_tag: Option<PtrId>) -> Self {
+    pub fn new(tracked_pointer_tag: Option<PtrId>, tracked_call_id: Option<CallId>, track_raw: bool) -> Self {
         GlobalState {
             next_ptr_id: NonZeroU64::new(1).unwrap(),
             base_ptr_ids: FxHashMap::default(),
             next_call_id: NonZeroU64::new(1).unwrap(),
             active_calls: FxHashSet::default(),
             tracked_pointer_tag,
+            tracked_call_id,
+            track_raw,
         }
     }
 
     fn new_ptr(&mut self) -> PtrId {
         let id = self.next_ptr_id;
+        if Some(id) == self.tracked_pointer_tag {
+            register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(id));
+        }
         self.next_ptr_id = NonZeroU64::new(id.get() + 1).unwrap();
         id
     }
@@ -169,6 +181,9 @@ fn new_ptr(&mut self) -> PtrId {
     pub fn new_call(&mut self) -> CallId {
         let id = self.next_call_id;
         trace!("new_call: Assigning ID {}", id);
+        if Some(id) == self.tracked_call_id {
+            register_diagnostic(NonHaltingDiagnostic::CreatedCallId(id));
+        }
         assert!(self.active_calls.insert(id));
         self.next_call_id = NonZeroU64::new(id.get() + 1).unwrap();
         id
@@ -182,7 +197,7 @@ fn is_active(&self, id: CallId) -> bool {
         self.active_calls.contains(&id)
     }
 
-    pub fn static_base_ptr(&mut self, id: AllocId) -> Tag {
+    pub fn global_base_ptr(&mut self, id: AllocId) -> Tag {
         self.base_ptr_ids.get(&id).copied().unwrap_or_else(|| {
             let tag = Tag::Tagged(self.new_ptr());
             trace!("New allocation {:?} has base tag {:?}", id, tag);
@@ -274,7 +289,7 @@ fn find_first_write_incompatible(&self, granting: usize) -> usize {
     fn check_protector(item: &Item, tag: Option<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
         if let Tag::Tagged(id) = item.tag {
             if Some(id) == global.tracked_pointer_tag {
-                register_diagnostic(NonHaltingDiagnostic::PoppedTrackedPointerTag(item.clone()));
+                register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(item.clone()));
             }
         }
         if let Some(call) = item.protector {
@@ -297,14 +312,14 @@ fn check_protector(item: &Item, tag: Option<Tag>, global: &GlobalState) -> Inter
 
     /// Test if a memory `access` using pointer tagged `tag` is granted.
     /// If yes, return the index of the item that granted it.
-    fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
+    fn access(&mut self, access: AccessKind, ptr: Pointer<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
         // Two main steps: Find granting item, remove incompatible items above.
 
         // Step 1: Find granting item.
-        let granting_idx = self.find_granting(access, tag).ok_or_else(|| {
+        let granting_idx = self.find_granting(access, ptr.tag).ok_or_else(|| {
             err_sb_ub(format!(
-                "no item granting {} to tag {:?} found in borrow stack.",
-                access, tag
+                "no item granting {} to tag {:?} at {} found in borrow stack.",
+                access, ptr.tag, ptr.erase_tag(),
             ))
         })?;
 
@@ -316,7 +331,7 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte
             let first_incompatible_idx = self.find_first_write_incompatible(granting_idx);
             for item in self.borrows.drain(first_incompatible_idx..).rev() {
                 trace!("access: popping item {:?}", item);
-                Stack::check_protector(&item, Some(tag), global)?;
+                Stack::check_protector(&item, Some(ptr.tag), global)?;
             }
         } else {
             // On a read, *disable* all `Unique` above the granting item.  This ensures U2 for read accesses.
@@ -331,7 +346,7 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte
                 let item = &mut self.borrows[idx];
                 if item.perm == Permission::Unique {
                     trace!("access: disabling item {:?}", item);
-                    Stack::check_protector(item, Some(tag), global)?;
+                    Stack::check_protector(item, Some(ptr.tag), global)?;
                     item.perm = Permission::Disabled;
                 }
             }
@@ -343,12 +358,12 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte
 
     /// Deallocate a location: Like a write access, but also there must be no
     /// active protectors at all because we will remove all items.
-    fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
+    fn dealloc(&mut self, ptr: Pointer<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
         // Step 1: Find granting item.
-        self.find_granting(AccessKind::Write, tag).ok_or_else(|| {
+        self.find_granting(AccessKind::Write, ptr.tag).ok_or_else(|| {
             err_sb_ub(format!(
-                "no item granting write access for deallocation to tag {:?} found in borrow stack",
-                tag,
+                "no item granting write access for deallocation to tag {:?} at {} found in borrow stack",
+                ptr.tag, ptr.erase_tag(),
             ))
         })?;
 
@@ -360,20 +375,20 @@ fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
         Ok(())
     }
 
-    /// Derived a new pointer from one with the given tag.
+    /// Derive a new pointer from one with the given tag.
     /// `weak` controls whether this operation is weak or strong: weak granting does not act as
     /// an access, and they add the new item directly on top of the one it is derived
     /// from instead of all the way at the top of the stack.
-    fn grant(&mut self, derived_from: Tag, new: Item, global: &GlobalState) -> InterpResult<'tcx> {
+    fn grant(&mut self, derived_from: Pointer<Tag>, new: Item, global: &GlobalState) -> InterpResult<'tcx> {
         // Figure out which access `perm` corresponds to.
         let access =
             if new.perm.grants(AccessKind::Write) { AccessKind::Write } else { AccessKind::Read };
         // Now we figure out which item grants our parent (`derived_from`) this kind of access.
         // We use that to determine where to put the new item.
-        let granting_idx = self.find_granting(access, derived_from)
+        let granting_idx = self.find_granting(access, derived_from.tag)
             .ok_or_else(|| err_sb_ub(format!(
-                "trying to reborrow for {:?}, but parent tag {:?} does not have an appropriate item in the borrow stack",
-                new.perm, derived_from,
+                "trying to reborrow for {:?} at {}, but parent tag {:?} does not have an appropriate item in the borrow stack",
+                new.perm, derived_from.erase_tag(), derived_from.tag,
             )))?;
 
         // Compute where to put the new item.
@@ -431,12 +446,14 @@ fn for_each(
         &self,
         ptr: Pointer<Tag>,
         size: Size,
-        f: impl Fn(&mut Stack, &GlobalState) -> InterpResult<'tcx>,
+        f: impl Fn(Pointer<Tag>, &mut Stack, &GlobalState) -> InterpResult<'tcx>,
     ) -> InterpResult<'tcx> {
         let global = self.global.borrow();
         let mut stacks = self.stacks.borrow_mut();
-        for stack in stacks.iter_mut(ptr.offset, size) {
-            f(stack, &*global)?;
+        for (offset, stack) in stacks.iter_mut(ptr.offset, size) {
+            let mut cur_ptr = ptr;
+            cur_ptr.offset = offset;
+            f(cur_ptr, stack, &*global)?;
         }
         Ok(())
     }
@@ -457,15 +474,20 @@ pub fn new_allocation(
             // everything else off the stack, invalidating all previous pointers,
             // and in particular, *all* raw pointers.
             MemoryKind::Stack => (Tag::Tagged(extra.borrow_mut().new_ptr()), Permission::Unique),
-            // Static memory can be referenced by "global" pointers from `tcx`.
-            // Thus we call `static_base_ptr` such that the global pointers get the same tag
+            // `Global` memory can be referenced by global pointers from `tcx`.
+            // Thus we call `global_base_ptr` such that the global pointers get the same tag
             // as what we use here.
+            // `ExternStatic` is used for extern statics, and thus must also be listed here.
+            // `Env` we list because we can get away with precise tracking there.
             // The base pointer is not unique, so the base permission is `SharedReadWrite`.
-            MemoryKind::Machine(MiriMemoryKind::Static) | MemoryKind::Machine(MiriMemoryKind::Machine) =>
-                (extra.borrow_mut().static_base_ptr(id), Permission::SharedReadWrite),
-            // Everything else we handle entirely untagged for now.
-            // FIXME: experiment with more precise tracking.
-            _ => (Tag::Untagged, Permission::SharedReadWrite),
+            MemoryKind::Machine(MiriMemoryKind::Global | MiriMemoryKind::ExternStatic | MiriMemoryKind::Tls | MiriMemoryKind::Env) =>
+                (extra.borrow_mut().global_base_ptr(id), Permission::SharedReadWrite),
+            // Everything else we handle like raw pointers for now.
+            _ => {
+                let mut extra = extra.borrow_mut();
+                let tag = if extra.track_raw { Tag::Tagged(extra.new_ptr()) } else { Tag::Untagged };
+                (tag, Permission::SharedReadWrite)
+            }
         };
         (Stacks::new(size, perm, tag, extra), tag)
     }
@@ -473,19 +495,13 @@ pub fn new_allocation(
     #[inline(always)]
     pub fn memory_read<'tcx>(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
         trace!("read access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |stack, global| {
-            stack.access(AccessKind::Read, ptr.tag, global)?;
-            Ok(())
-        })
+        self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Read, ptr, global))
     }
 
     #[inline(always)]
     pub fn memory_written<'tcx>(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
         trace!("write access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |stack, global| {
-            stack.access(AccessKind::Write, ptr.tag, global)?;
-            Ok(())
-        })
+        self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Write, ptr, global))
     }
 
     #[inline(always)]
@@ -495,13 +511,13 @@ pub fn memory_deallocated<'tcx>(
         size: Size,
     ) -> InterpResult<'tcx> {
         trace!("deallocation with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |stack, global| stack.dealloc(ptr.tag, global))
+        self.for_each(ptr, size, |ptr, stack, global| stack.dealloc(ptr, global))
     }
 }
 
 /// Retagging/reborrowing.  There is some policy in here, such as which permissions
 /// to grant for which references, and when to add protectors.
-impl<'mir, 'tcx> EvalContextPrivExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
+impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn reborrow(
         &mut self,
@@ -547,14 +563,14 @@ fn reborrow(
                         Permission::SharedReadWrite
                     };
                     let item = Item { perm, tag: new_tag, protector };
-                    stacked_borrows.for_each(cur_ptr, size, |stack, global| {
-                        stack.grant(cur_ptr.tag, item, global)
+                    stacked_borrows.for_each(cur_ptr, size, |cur_ptr, stack, global| {
+                        stack.grant(cur_ptr, item, global)
                     })
                 });
             }
         };
         let item = Item { perm, tag: new_tag, protector };
-        stacked_borrows.for_each(ptr, size, |stack, global| stack.grant(ptr.tag, item, global))
+        stacked_borrows.for_each(ptr, size, |ptr, stack, global| stack.grant(ptr, item, global))
     }
 
     /// Retags an indidual pointer, returning the retagged version.
@@ -564,7 +580,7 @@ fn retag_reference(
         val: ImmTy<'tcx, Tag>,
         kind: RefKind,
         protect: bool,
-    ) -> InterpResult<'tcx, Immediate<Tag>> {
+    ) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> {
         let this = self.eval_context_mut();
         // We want a place for where the ptr *points to*, so we get one.
         let place = this.ref_to_mplace(val)?;
@@ -572,25 +588,25 @@ fn retag_reference(
             .size_and_align_of_mplace(place)?
             .map(|(size, _)| size)
             .unwrap_or_else(|| place.layout.size);
+        // `reborrow` relies on getting a `Pointer` and everything being in-bounds,
+        // so let's ensure that. However, we do not care about alignment.
         // We can see dangling ptrs in here e.g. after a Box's `Unique` was
-        // updated using "self.0 = ..." (can happen in Box::from_raw); see miri#1050.
-        let place = this.mplace_access_checked(place)?;
+        // updated using "self.0 = ..." (can happen in Box::from_raw) so we cannot ICE; see miri#1050.
+        let place = this.mplace_access_checked(place, Some(Align::from_bytes(1).unwrap()))?;
+        // Nothing to do for ZSTs.
         if size == Size::ZERO {
-            // Nothing to do for ZSTs.
-            return Ok(*val);
+            return Ok(val);
         }
 
         // Compute new borrow.
-        let new_tag = match kind {
-            // Give up tracking for raw pointers.
-            // FIXME: Experiment with more precise tracking. Blocked on `&raw`
-            // because `Rc::into_raw` currently creates intermediate references,
-            // breaking `Rc::from_raw`.
-            RefKind::Raw { .. } => Tag::Untagged,
-            // All other pointesr are properly tracked.
-            _ => Tag::Tagged(
-                this.memory.extra.stacked_borrows.as_ref().unwrap().borrow_mut().new_ptr(),
-            ),
+        let new_tag = {
+            let mut mem_extra = this.memory.extra.stacked_borrows.as_ref().unwrap().borrow_mut();
+            match kind {
+                // Give up tracking for raw pointers.
+                RefKind::Raw { .. } if !mem_extra.track_raw => Tag::Untagged,
+                // All other pointers are properly tracked.
+                _ => Tag::Tagged(mem_extra.new_ptr()),
+            }
         };
 
         // Reborrow.
@@ -598,11 +614,11 @@ fn retag_reference(
         let new_place = place.replace_tag(new_tag);
 
         // Return new pointer.
-        Ok(new_place.to_ref())
+        Ok(ImmTy::from_immediate(new_place.to_ref(), val.layout))
     }
 }
 
-impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
+impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn retag(&mut self, kind: RetagKind, place: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
@@ -610,7 +626,7 @@ fn retag(&mut self, kind: RetagKind, place: PlaceTy<'tcx, Tag>) -> InterpResult<
         // Cannot use `builtin_deref` because that reports *immutable* for `Box`,
         // making it useless.
         fn qualify(ty: ty::Ty<'_>, kind: RetagKind) -> Option<(RefKind, bool)> {
-            match ty.kind {
+            match ty.kind() {
                 // References are simple.
                 ty::Ref(_, _, Mutability::Mut) => Some((
                     RefKind::Unique { two_phase: kind == RetagKind::TwoPhase },
@@ -635,8 +651,41 @@ fn qualify(ty: ty::Ty<'_>, kind: RetagKind) -> Option<(RefKind, bool)> {
             // Fast path.
             let val = this.read_immediate(this.place_to_op(place)?)?;
             let val = this.retag_reference(val, mutbl, protector)?;
-            this.write_immediate(val, place)?;
+            this.write_immediate(*val, place)?;
+        }
+
+        Ok(())
+    }
+
+    /// After a stack frame got pushed, retag the return place so that we are sure
+    /// it does not alias with anything.
+    /// 
+    /// This is a HACK because there is nothing in MIR that would make the retag
+    /// explicit. Also see https://github.com/rust-lang/rust/issues/71117.
+    fn retag_return_place(&mut self) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+        let return_place = if let Some(return_place) = this.frame_mut().return_place {
+            return_place
+        } else {
+            // No return place, nothing to do.
+            return Ok(());
+        };
+        if return_place.layout.is_zst() {
+            // There may not be any memory here, nothing to do.
+            return Ok(());
         }
+        // We need this to be in-memory to use tagged pointers.
+        let return_place = this.force_allocation(return_place)?;
+
+        // We have to turn the place into a pointer to use the existing code.
+        // (The pointer type does not matter, so we use a raw pointer.)
+        let ptr_layout = this.layout_of(this.tcx.mk_mut_ptr(return_place.layout.ty))?;
+        let val = ImmTy::from_immediate(return_place.to_ref(), ptr_layout);
+        // Reborrow it.
+        let val = this.retag_reference(val, RefKind::Unique { two_phase: false }, /*protector*/ true)?;
+        // And use reborrowed pointer for return place.
+        let return_place = this.ref_to_mplace(val)?;
+        this.frame_mut().return_place = Some(return_place.into());
 
         Ok(())
     }