]> git.lizzy.rs Git - rust.git/blobdiff - src/stacked_borrows.rs
better approach to skip ZST reborrows
[rust.git] / src / stacked_borrows.rs
index 88f42efd13cf0cfa4915a00db1ea4e71e7b7a45a..3e176d94b9902e29206eb784f3eb3d42041a0173 100644 (file)
@@ -1,12 +1,10 @@
 //! Implements "Stacked Borrows".  See <https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md>
 //! for further information.
 
+use log::trace;
 use std::cell::RefCell;
 use std::fmt;
 use std::num::NonZeroU64;
-use std::rc::Rc;
-
-use log::trace;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::Mutability;
@@ -87,8 +85,6 @@ pub struct Stack {
 pub struct Stacks {
     // Even reading memory can have effects on the stack, so we need a `RefCell` here.
     stacks: RefCell<RangeMap<Stack>>,
-    // Pointer to global state
-    global: MemoryExtra,
 }
 
 /// Extra global state, available to the memory access hooks.
@@ -112,7 +108,7 @@ pub struct GlobalState {
     track_raw: bool,
 }
 /// Memory extra state gives us interior mutable access to the global state.
-pub type MemoryExtra = Rc<RefCell<GlobalState>>;
+pub type MemoryExtra = RefCell<GlobalState>;
 
 /// Indicates which kind of access is being performed.
 #[derive(Copy, Clone, Hash, PartialEq, Eq)]
@@ -449,11 +445,11 @@ fn grant(
 /// Map per-stack operations to higher-level per-location-range operations.
 impl<'tcx> Stacks {
     /// Creates new stack with initial tag.
-    fn new(size: Size, perm: Permission, tag: Tag, extra: MemoryExtra) -> Self {
+    fn new(size: Size, perm: Permission, tag: Tag) -> Self {
         let item = Item { perm, tag, protector: None };
         let stack = Stack { borrows: vec![item] };
 
-        Stacks { stacks: RefCell::new(RangeMap::new(size, stack)), global: extra }
+        Stacks { stacks: RefCell::new(RangeMap::new(size, stack)) }
     }
 
     /// Call `f` on every stack in the range.
@@ -461,14 +457,29 @@ fn for_each(
         &self,
         ptr: Pointer<Tag>,
         size: Size,
-        f: impl Fn(Pointer<Tag>, &mut Stack, &GlobalState) -> InterpResult<'tcx>,
+        f: impl Fn(Pointer<Tag>, &mut Stack) -> InterpResult<'tcx>,
     ) -> InterpResult<'tcx> {
-        let global = self.global.borrow();
         let mut stacks = self.stacks.borrow_mut();
         for (offset, stack) in stacks.iter_mut(ptr.offset, size) {
             let mut cur_ptr = ptr;
             cur_ptr.offset = offset;
-            f(cur_ptr, stack, &*global)?;
+            f(cur_ptr, stack)?;
+        }
+        Ok(())
+    }
+
+    /// Call `f` on every stack in the range.
+    fn for_each_mut(
+        &mut self,
+        ptr: Pointer<Tag>,
+        size: Size,
+        f: impl Fn(Pointer<Tag>, &mut Stack) -> InterpResult<'tcx>,
+    ) -> InterpResult<'tcx> {
+        let stacks = self.stacks.get_mut();
+        for (offset, stack) in stacks.iter_mut(ptr.offset, size) {
+            let mut cur_ptr = ptr;
+            cur_ptr.offset = offset;
+            f(cur_ptr, stack)?;
         }
         Ok(())
     }
@@ -479,16 +490,17 @@ impl Stacks {
     pub fn new_allocation(
         id: AllocId,
         size: Size,
-        extra: MemoryExtra,
+        extra: &MemoryExtra,
         kind: MemoryKind<MiriMemoryKind>,
     ) -> (Self, Tag) {
+        let mut extra = extra.borrow_mut();
         let (tag, perm) = match kind {
             // New unique borrow. This tag is not accessible by the program,
             // so it will only ever be used when using the local directly (i.e.,
             // not through a pointer). That is, whenever we directly write to a local, this will pop
             // 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),
+            MemoryKind::Stack => (Tag::Tagged(extra.new_ptr()), Permission::Unique),
             // `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.
@@ -500,28 +512,39 @@ pub fn new_allocation(
                 | MiriMemoryKind::ExternStatic
                 | MiriMemoryKind::Tls
                 | MiriMemoryKind::Env,
-            ) => (extra.borrow_mut().global_base_ptr(id), Permission::SharedReadWrite),
+            ) => (extra.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)
+        (Stacks::new(size, perm, tag), tag)
     }
 
     #[inline(always)]
-    pub fn memory_read<'tcx>(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
+    pub fn memory_read<'tcx>(
+        &self,
+        ptr: Pointer<Tag>,
+        size: Size,
+        extra: &MemoryExtra,
+    ) -> InterpResult<'tcx> {
         trace!("read access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Read, ptr, global))
+        let global = &*extra.borrow();
+        self.for_each(ptr, size, move |ptr, stack| stack.access(AccessKind::Read, ptr, global))
     }
 
     #[inline(always)]
-    pub fn memory_written<'tcx>(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
+    pub fn memory_written<'tcx>(
+        &mut self,
+        ptr: Pointer<Tag>,
+        size: Size,
+        extra: &mut MemoryExtra,
+    ) -> InterpResult<'tcx> {
         trace!("write access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Write, ptr, global))
+        let global = extra.get_mut();
+        self.for_each_mut(ptr, size, move |ptr, stack| stack.access(AccessKind::Write, ptr, global))
     }
 
     #[inline(always)]
@@ -529,9 +552,11 @@ pub fn memory_deallocated<'tcx>(
         &mut self,
         ptr: Pointer<Tag>,
         size: Size,
+        extra: &mut MemoryExtra,
     ) -> InterpResult<'tcx> {
         trace!("deallocation with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
-        self.for_each(ptr, size, |ptr, stack, global| stack.dealloc(ptr, global))
+        let global = extra.get_mut();
+        self.for_each_mut(ptr, size, move |ptr, stack| stack.dealloc(ptr, global))
     }
 }
 
@@ -547,6 +572,18 @@ fn reborrow(
         new_tag: Tag,
         protect: bool,
     ) -> InterpResult<'tcx> {
+        // Nothing to do for ZSTs.
+        if size == Size::ZERO {
+            trace!(
+                "reborrow of size 0: {} reference {:?} derived from {:?} (pointee {})",
+                kind,
+                new_tag,
+                place.ptr,
+                place.layout.ty,
+            );
+            return Ok(());
+        }
+
         let this = self.eval_context_mut();
         let protector = if protect { Some(this.frame().extra.call_id) } else { None };
         let ptr = place.ptr.assert_ptr();
@@ -560,10 +597,6 @@ fn reborrow(
             size.bytes()
         );
 
-        // Get the allocation. It might not be mutable, so we cannot use `get_mut`.
-        let extra = &this.memory.get_raw(ptr.alloc_id)?.extra;
-        let stacked_borrows =
-            extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data");
         // Update the stacks.
         // Make sure that raw pointers and mutable shared references are reborrowed "weak":
         // There could be existing unique pointers reborrowed from them that should remain valid!
@@ -575,6 +608,12 @@ fn reborrow(
                 // Shared references and *const are a whole different kind of game, the
                 // permission is not uniform across the entire range!
                 // We need a frozen-sensitive reborrow.
+                // We have to use shared references to alloc/memory_extra here since
+                // `visit_freeze_sensitive` needs to access the global state.
+                let extra = this.memory.get_alloc_extra(ptr.alloc_id)?;
+                let stacked_borrows =
+                    extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data");
+                let global = this.memory.extra.stacked_borrows.as_ref().unwrap().borrow();
                 return this.visit_freeze_sensitive(place, size, |cur_ptr, size, frozen| {
                     // We are only ever `SharedReadOnly` inside the frozen bits.
                     let perm = if frozen {
@@ -583,14 +622,21 @@ fn reborrow(
                         Permission::SharedReadWrite
                     };
                     let item = Item { perm, tag: new_tag, protector };
-                    stacked_borrows.for_each(cur_ptr, size, |cur_ptr, stack, global| {
-                        stack.grant(cur_ptr, item, global)
+                    stacked_borrows.for_each(cur_ptr, size, |cur_ptr, stack| {
+                        stack.grant(cur_ptr, item, &*global)
                     })
                 });
             }
         };
+        // Here we can avoid `borrow()` calls because we have mutable references.
+        // Note that this asserts that the allocation is mutable -- but since we are creating a
+        // mutable pointer, that seems reasonable.
+        let (alloc_extra, memory_extra) = this.memory.get_alloc_extra_mut(ptr.alloc_id)?;
+        let stacked_borrows =
+            alloc_extra.stacked_borrows.as_mut().expect("we should have Stacked Borrows data");
+        let global = memory_extra.stacked_borrows.as_mut().unwrap().get_mut();
         let item = Item { perm, tag: new_tag, protector };
-        stacked_borrows.for_each(ptr, size, |ptr, stack, global| stack.grant(ptr, item, global))
+        stacked_borrows.for_each_mut(ptr, size, |ptr, stack| stack.grant(ptr, item, global))
     }
 
     /// Retags an indidual pointer, returning the retagged version.
@@ -617,16 +663,10 @@ fn retag_reference(
         // 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) 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. We use `is_bits` here because we *do* need to retag even ZSTs
-        // when there actually is a tag (to avoid inheriting a tag that would let us access more
-        // than 0 bytes).
-        if size == Size::ZERO && place.ptr.is_bits() {
-            return Ok(*val);
-        }
 
         // Compute new borrow.
         let new_tag = {
-            let mut mem_extra = this.memory.extra.stacked_borrows.as_ref().unwrap().borrow_mut();
+            let mem_extra = this.memory.extra.stacked_borrows.as_mut().unwrap().get_mut();
             match kind {
                 // Give up tracking for raw pointers.
                 RefKind::Raw { .. } if !mem_extra.track_raw => Tag::Untagged,