interpret: fix in-place return place semantics when the return place expression is a local variable

13 files changed, 53 insertions, 40 deletions

diff --git a/compiler/rustc_const_eval/src/interpret/call.rs b/compiler/rustc_const_eval/src/interpret/call.rs
index b1cc0cc2878..7b3c80538ca 100644
--- a/compiler/rustc_const_eval/src/interpret/call.rs
+++ b/compiler/rustc_const_eval/src/interpret/call.rs
@@ -27,8 +27,9 @@ use crate::{enter_trace_span, fluent_generated as fluent};
 pub enum FnArg<'tcx, Prov: Provenance = CtfeProvenance> {
     /// Pass a copy of the given operand.
     Copy(OpTy<'tcx, Prov>),
-    /// Allow for the argument to be passed in-place: destroy the value originally stored at that place and
-    /// make the place inaccessible for the duration of the function call.
+    /// Allow for the argument to be passed in-place: destroy the value originally stored at that
+    /// place and make the place inaccessible for the duration of the function call. This *must* be
+    /// an in-memory place so that we can do the proper alias checks.
     InPlace(MPlaceTy<'tcx, Prov>),
 }
 
@@ -379,6 +380,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             }
         }
 
+        // *Before* pushing the new frame, determine whether the return destination is in memory.
+        // Need to use `place_to_op` to be *sure* we get the mplace if there is one.
+        let destination_mplace = self.place_to_op(destination)?.as_mplace_or_imm().left();
+
+        // Push the "raw" frame -- this leaves locals uninitialized.
         self.push_stack_frame_raw(instance, body, destination, cont)?;
 
         // If an error is raised here, pop the frame again to get an accurate backtrace.
@@ -496,7 +502,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
             // Protect return place for in-place return value passing.
             // We only need to protect anything if this is actually an in-memory place.
-            if let Left(mplace) = destination.as_mplace_or_local() {
+            if let Some(mplace) = destination_mplace {
                 M::protect_in_place_function_argument(self, &mplace)?;
             }
 
diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs
index 6ff50dc700f..921fa8677fe 100644
--- a/compiler/rustc_const_eval/src/interpret/place.rs
+++ b/compiler/rustc_const_eval/src/interpret/place.rs
@@ -234,6 +234,12 @@ impl<'tcx, Prov: Provenance> PlaceTy<'tcx, Prov> {
     }
 
     /// A place is either an mplace or some local.
+    ///
+    /// Note that the return value can be different even for logically identical places!
+    /// Specifically, if a local is stored in-memory, this may return `Local` or `MPlaceTy`
+    /// depending on how the place was constructed. In other words, seeing `Local` here does *not*
+    /// imply that this place does not point to memory. Every caller must therefore always handle
+    /// both cases.
     #[inline(always)]
     pub fn as_mplace_or_local(
         &self,
diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs
index 76e470b69dc..36251f774c6 100644
--- a/compiler/rustc_const_eval/src/interpret/step.rs
+++ b/compiler/rustc_const_eval/src/interpret/step.rs
@@ -415,6 +415,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                         // caller directly access this local!
                         // This is also crucial for tail calls, where we want the `FnArg` to
                         // stay valid when the old stack frame gets popped.
+                        // FIXME: How can this be right for aliasing arguments?
                         FnArg::Copy(op)
                     }
                 }
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr
index 24091547258..d478568ceae 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr
@@ -11,8 +11,8 @@ LL |     unsafe { ptr.read() };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Uninitialized memory occurred at ALLOC[0x0..0x4], in this allocation:
 ALLOC (stack variable, size: 4, align: 4) {
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs
index a6e0134bd17..dc22e129e18 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs
@@ -10,11 +10,11 @@ use std::intrinsics::mir::*;
 pub fn main() {
     mir! {
         {
-            let x = 0;
-            let ptr = &raw mut x;
+            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 = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+            Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
         }
 
         after_call = {
@@ -25,7 +25,7 @@ pub fn main() {
 
 fn myfun(ptr: *mut i32) -> i32 {
     unsafe { ptr.read() };
-    //~[stack]^ ERROR: not granting access
+    //~[stack]^ ERROR: does not exist in the borrow stack
     //~[tree]| ERROR: /read access .* forbidden/
     //~[none]| ERROR: uninitialized
     // Without an aliasing model, reads are "fine" but at least they return uninit data.
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr
index 77cc0332a1c..86adbab353b 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected
+error: Undefined Behavior: attempting a read access using <TAG> at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
   --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC
    |
 LL |     unsafe { ptr.read() };
-   |              ^^^^^^^^^^ Undefined Behavior occurred here
+   |              ^^^^^^^^^^ 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
@@ -11,12 +11,12 @@ help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
    |
 LL | /     mir! {
 LL | |         {
-LL | |             let x = 0;
-LL | |             let ptr = &raw mut x;
+LL | |             let _x = 0;
+LL | |             let ptr = &raw mut _x;
 ...  |
 LL | |     }
    | |_____^
-help: <TAG> is this argument
+help: <TAG> was later invalidated at offsets [0x0..0x4] by a Unique in-place function argument/return passing protection
   --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC
    |
 LL |     unsafe { ptr.read() };
@@ -26,8 +26,8 @@ LL |     unsafe { ptr.read() };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr
index bae3819c979..a1cf0b730eb 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr
@@ -13,8 +13,8 @@ help: the accessed tag <TAG> was created here
    |
 LL | /     mir! {
 LL | |         {
-LL | |             let x = 0;
-LL | |             let ptr = &raw mut x;
+LL | |             let _x = 0;
+LL | |             let ptr = &raw mut _x;
 ...  |
 LL | |     }
    | |_____^
@@ -34,8 +34,8 @@ LL |     unsafe { ptr.read() };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs
index 6155e925c4b..2fddaf37235 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs
@@ -14,7 +14,7 @@ pub fn main() {
             let ptr = &raw mut _x;
             // We arrange for `myfun` to have a pointer that aliases
             // its return place. Writing to that pointer is UB.
-            Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+            Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
         }
 
         after_call = {
@@ -26,7 +26,7 @@ pub fn main() {
 fn myfun(ptr: *mut i32) -> i32 {
     // This overwrites the return place, which shouldn't be possible through another pointer.
     unsafe { ptr.write(0) };
-    //~[stack]^ ERROR: strongly protected
+    //~[stack]^ ERROR: does not exist in the borrow stack
     //~[tree]| ERROR: /write access .* forbidden/
     13
 }
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr
index 828b2339f8c..faae6172d75 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected
+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
   --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC
    |
 LL |     unsafe { ptr.write(0) };
-   |              ^^^^^^^^^^^^ Undefined Behavior occurred here
+   |              ^^^^^^^^^^^^ 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
@@ -16,7 +16,7 @@ LL | |             let ptr = &raw mut _x;
 ...  |
 LL | |     }
    | |_____^
-help: <TAG> is this argument
+help: <TAG> was later invalidated at offsets [0x0..0x4] by a Unique in-place function argument/return passing protection
   --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC
    |
 LL |     unsafe { ptr.write(0) };
@@ -26,8 +26,8 @@ LL |     unsafe { ptr.write(0) };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr
index e0df9370fee..01d15f38af6 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr
@@ -34,8 +34,8 @@ LL |     unsafe { ptr.write(0) };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs
index 37ee7ae1b0d..5f3ecb65022 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs
@@ -16,7 +16,7 @@ pub fn main() {
             let ptr = &raw mut _x;
             // We arrange for `myfun` to have a pointer that aliases
             // its return place. Writing to that pointer is UB.
-            Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+            Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
         }
 
         after_call = {
@@ -32,7 +32,7 @@ fn myfun(ptr: *mut i32) -> i32 {
 fn myfun2(ptr: *mut i32) -> i32 {
     // This overwrites the return place, which shouldn't be possible through another pointer.
     unsafe { ptr.write(0) };
-    //~[stack]^ ERROR: strongly protected
+    //~[stack]^ ERROR: does not exist in the borrow stack
     //~[tree]| ERROR: /write access .* forbidden/
     13
 }
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr
index f5183cfaf5b..1a18857bb17 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected
+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
   --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC
    |
 LL |     unsafe { ptr.write(0) };
-   |              ^^^^^^^^^^^^ Undefined Behavior occurred here
+   |              ^^^^^^^^^^^^ 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
@@ -16,18 +16,18 @@ LL | |             let ptr = &raw mut _x;
 ...  |
 LL | |     }
    | |_____^
-help: <TAG> is this argument
+help: <TAG> was later invalidated at offsets [0x0..0x4] by a Unique in-place function argument/return passing protection
   --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC
    |
-LL |     unsafe { ptr.write(0) };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^
+LL |     become myfun2(ptr)
+   |     ^^^^^^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `myfun2` at tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr
index fa03ed6869b..812ddb94a73 100644
--- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr
@@ -34,8 +34,8 @@ LL |     unsafe { ptr.write(0) };
 note: inside `main`
   --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC
    |
-LL |             Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue())
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |             Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue())
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = 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