Auto merge of #2713 - RalfJung:not-unpin-fake-read, r=RalfJung

for now, do not do fake reads on non-Unpin mutable references

Work-around for https://github.com/rust-lang/unsafe-code-guidelines/issues/381, needed to make the new test pass. Undoes parts of https://github.com/rust-lang/miri/pull/2694.

4 files changed, 116 insertions, 54 deletions

diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
index 50c2ad75ca7..bcac873251f 100644
--- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
+++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
@@ -45,7 +45,9 @@ pub struct Stacks {
 /// new pointer.
 #[derive(Copy, Clone, Hash, PartialEq, Eq)]
 enum RefKind {
-    /// `&mut` and `Box`.
+    /// `Box`.
+    Box,
+    /// `&mut`.
     Unique { two_phase: bool },
     /// `&` with or without interior mutability.
     Shared,
@@ -56,6 +58,7 @@ enum RefKind {
 impl fmt::Display for RefKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
+            RefKind::Box => write!(f, "Box"),
             RefKind::Unique { two_phase: false } => write!(f, "unique reference"),
             RefKind::Unique { two_phase: true } => write!(f, "unique reference (two-phase)"),
             RefKind::Shared => write!(f, "shared reference"),
@@ -654,15 +657,17 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
         let (perm, access) = match kind {
             RefKind::Unique { two_phase } => {
                 // Permission is Unique only if the type is `Unpin` and this is not twophase
-                let perm = if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) {
-                    Permission::Unique
+                if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) {
+                    (Permission::Unique, Some(AccessKind::Write))
                 } else {
-                    Permission::SharedReadWrite
-                };
-                // We do an access for all full borrows, even if `!Unpin`.
-                let access = if !two_phase { Some(AccessKind::Write) } else { None };
-                (perm, access)
+                    // FIXME: We emit `dereferenceable` for `!Unpin` mutable references, so we
+                    // should do fake accesses here. But then we run into
+                    // <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>, so for now
+                    // we don't do that.
+                    (Permission::SharedReadWrite, None)
+                }
             }
+            RefKind::Box => (Permission::Unique, Some(AccessKind::Write)),
             RefKind::Raw { mutable: true } => {
                 // Creating a raw ptr does not count as an access
                 (Permission::SharedReadWrite, None)
@@ -853,7 +858,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // Boxes get a weak protectors, since they may be deallocated.
                 self.retag_place(
                     place,
-                    RefKind::Unique { two_phase: false },
+                    RefKind::Box,
                     self.retag_cause,
                     /*protector*/
                     (self.kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector),
diff --git a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs
deleted file mode 100644
index d660921bfe6..00000000000
--- a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs
+++ /dev/null
@@ -1,17 +0,0 @@
-//! Reborrowing a `&mut !Unpin` must still act like a (fake) read.
-use std::marker::PhantomPinned;
-
-struct NotUnpin(i32, PhantomPinned);
-
-fn main() {
-    unsafe {
-        let mut x = NotUnpin(0, PhantomPinned);
-        // Mutable borrow of `Unpin` field (with lifetime laundering)
-        let fieldref = &mut *(&mut x.0 as *mut i32);
-        // Mutable reborrow of the entire `x`, which is `!Unpin` but should
-        // still count as a read since we would add `dereferenceable`.
-        let _xref = &mut x;
-        // That read should have invalidated `fieldref`.
-        *fieldref = 0; //~ ERROR: /write access .* tag does not exist in the borrow stack/
-    }
-}
diff --git a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr
deleted file mode 100644
index 3ef8a8e0e9c..00000000000
--- a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr
+++ /dev/null
@@ -1,28 +0,0 @@
-error: Undefined Behavior: attempting a write access using <TAG> at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
-  --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC
-   |
-LL |         *fieldref = 0;
-   |         ^^^^^^^^^^^^^
-   |         |
-   |         attempting a write access using <TAG> at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
-   |         this error occurs as part of an access at ALLOC[0x0..0x4]
-   |
-   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
-   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
-help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
-  --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC
-   |
-LL |         let fieldref = &mut *(&mut x.0 as *mut i32);
-   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: <TAG> was later invalidated at offsets [0x0..0x4] by a SharedReadWrite retag
-  --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC
-   |
-LL |         let _xref = &mut x;
-   |                     ^^^^^^
-   = note: BACKTRACE:
-   = note: inside `main` at $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC
-
-note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
-
-error: aborting due to previous error
-
diff --git a/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs b/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs
new file mode 100644
index 00000000000..3ba21552fd3
--- /dev/null
+++ b/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs
@@ -0,0 +1,102 @@
+#![feature(pin_macro)]
+
+use std::future::*;
+use std::marker::PhantomPinned;
+use std::pin::*;
+use std::ptr;
+use std::task::*;
+
+struct Delay {
+    delay: usize,
+}
+
+impl Delay {
+    fn new(delay: usize) -> Self {
+        Delay { delay }
+    }
+}
+
+impl Future for Delay {
+    type Output = ();
+    fn poll(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<()> {
+        if self.delay > 0 {
+            self.delay -= 1;
+            Poll::Pending
+        } else {
+            Poll::Ready(())
+        }
+    }
+}
+
+async fn do_stuff() {
+    (&mut Delay::new(1)).await;
+}
+
+// Same thing implemented by hand
+struct DoStuff {
+    state: usize,
+    delay: Delay,
+    delay_ref: *mut Delay,
+    _marker: PhantomPinned,
+}
+
+impl DoStuff {
+    fn new() -> Self {
+        DoStuff {
+            state: 0,
+            delay: Delay::new(1),
+            delay_ref: ptr::null_mut(),
+            _marker: PhantomPinned,
+        }
+    }
+}
+
+impl Future for DoStuff {
+    type Output = ();
+    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> {
+        unsafe {
+            let this = self.get_unchecked_mut();
+            match this.state {
+                0 => {
+                    // Set up self-ref.
+                    this.delay_ref = &mut this.delay;
+                    // Move to next state.
+                    this.state = 1;
+                    Poll::Pending
+                }
+                1 => {
+                    let delay = &mut *this.delay_ref;
+                    Pin::new_unchecked(delay).poll(cx)
+                }
+                _ => unreachable!(),
+            }
+        }
+    }
+}
+
+fn run_fut<T>(fut: impl Future<Output = T>) -> T {
+    use std::sync::Arc;
+
+    struct MyWaker;
+    impl Wake for MyWaker {
+        fn wake(self: Arc<Self>) {
+            unimplemented!()
+        }
+    }
+
+    let waker = Waker::from(Arc::new(MyWaker));
+    let mut context = Context::from_waker(&waker);
+
+    let mut pinned = pin!(fut);
+    loop {
+        match pinned.as_mut().poll(&mut context) {
+            Poll::Pending => continue,
+            Poll::Ready(v) => return v,
+        }
+    }
+}
+
+fn main() {
+    run_fut(do_stuff());
+    run_fut(DoStuff::new());
+}