Auto merge of #2973 - RalfJung:returnplace, r=RalfJung

test and fix return place alias restrictions

Author: bors

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -514,9 +514,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?;
         let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout);
         // Reborrow it. With protection! That is part of the point.
-        // FIXME: do we truly want a 2phase borrow here?
         let new_perm = Some(NewPermission {
-            initial_state: Permission::new_unique_2phase(/*freeze*/ false),
+            initial_state: Permission::new_active(),
             zero_size: false,
             protector: Some(ProtectorKind::StrongProtector),
         });

src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs

@@ -138,6 +138,11 @@ impl Permission {
         Self { inner: Reserved { ty_is_freeze } }
     }
 
+    /// Default initial permission for return place.
+    pub fn new_active() -> Self {
+        Self { inner: Active }
+    }
+
     /// Default initial permission of a reborrowed shared reference
     pub fn new_frozen() -> Self {
         Self { inner: Frozen }

src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs

@@ -0,0 +1,32 @@
+//@revisions: stack tree
+//@[tree]compile-flags: -Zmiri-tree-borrows
+#![feature(raw_ref_op)]
+#![feature(core_intrinsics)]
+#![feature(custom_mir)]
+
+use std::intrinsics::mir::*;
+use std::mem::MaybeUninit;
+
+#[custom_mir(dialect = "runtime", phase = "optimized")]
+pub fn main() {
+    mir! {
+        {
+            let x = 0;
+            let ptr = &raw mut x;
+            // We arrange for `myfun` to have a pointer that aliases
+            // its return place. Even just reading from that pointer is UB.
+            Call(*ptr, after_call, myfun(ptr))
+        }
+
+        after_call = {
+            Return()
+        }
+    }
+}
+
+fn myfun(ptr: *mut i32) -> i32 {
+    unsafe { ptr.cast::<MaybeUninit<i32>>().read() };
+    //~[stack]^ ERROR: /not granting access/
+    //~[tree]| ERROR: /read access .* forbidden/
+    13
+}

src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr

@@ -0,0 +1,42 @@
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL |     unsafe { ptr.cast::<MaybeUninit<i32>>().read() };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
+   |
+   = 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 SharedReadWrite retag at offsets [0x0..0x4]
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL | /     mir! {
+LL | |         {
+LL | |             let x = 0;
+LL | |             let ptr = &raw mut x;
+...  |
+LL | |         }
+LL | |     }
+   | |_____^
+help: <TAG> is this argument
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL | / fn myfun(ptr: *mut i32) -> i32 {
+LL | |     unsafe { ptr.cast::<MaybeUninit<i32>>().read() };
+LL | |
+LL | |
+LL | |     13
+LL | | }
+   | |_^
+   = note: BACKTRACE (of the first span):
+   = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC
+note: inside `main`
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL |             Call(*ptr, after_call, myfun(ptr))
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr

@@ -0,0 +1,44 @@
+error: Undefined Behavior: read access through <TAG> (root of the allocation) is forbidden
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL |     unsafe { ptr.cast::<MaybeUninit<i32>>().read() };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ read access through <TAG> (root of the allocation) is forbidden
+   |
+   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
+   = help: the accessed tag <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child)
+   = help: this foreign read access would cause the protected tag <TAG> to transition from Active to Frozen
+   = help: this transition would be a loss of write permissions, which is not allowed for protected tags
+help: the accessed tag <TAG> was created here
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL | /     mir! {
+LL | |         {
+LL | |             let x = 0;
+LL | |             let ptr = &raw mut x;
+...  |
+LL | |         }
+LL | |     }
+   | |_____^
+help: the protected tag <TAG> was created here, in the initial state Active
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL | / fn myfun(ptr: *mut i32) -> i32 {
+LL | |     unsafe { ptr.cast::<MaybeUninit<i32>>().read() };
+LL | |
+LL | |
+LL | |     13
+LL | | }
+   | |_^
+   = note: BACKTRACE (of the first span):
+   = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC
+note: inside `main`
+  --> $DIR/return_pointer_aliasing.rs:LL:CC
+   |
+LL |             Call(*ptr, after_call, myfun(ptr))
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+