From a73c86d8a595a1b511392d54d5e2de9edfa2dc02 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 4 Aug 2023 13:59:44 +0200
Subject: [PATCH] fix return place protection when the place is given as a
 local

---
 src/tools/miri/src/machine.rs                 |  3 +-
 .../return_pointer_aliasing2.rs               | 30 ++++++++++++++
 .../return_pointer_aliasing2.stderr           | 39 +++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs
 create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr

diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
index 73035b01a1b..e19be417b22 100644
--- a/src/tools/miri/src/machine.rs
+++ b/src/tools/miri/src/machine.rs
@@ -1219,7 +1219,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         // If we have a borrow tracker, we also have it set up protection so that all reads *and
         // writes* during this call are insta-UB.
         if ecx.machine.borrow_tracker.is_some() {
-            if let Either::Left(place) = place.as_mplace_or_local() {
+            // Have to do `to_op` first because a `Place::Local` doesn't imply the local doesn't have an address.
+            if let Either::Left(place) = ecx.place_to_op(place)?.as_mplace_or_imm() {
                 ecx.protect_place(&place)?;
             } else {
                 // Locals that don't have their address taken are as protected as they can ever be.
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs
new file mode 100644
index 00000000000..7e9a6320026
--- /dev/null
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs
@@ -0,0 +1,30 @@
+//@compile-flags: -Zmiri-tree-borrows
+#![feature(raw_ref_op)]
+#![feature(core_intrinsics)]
+#![feature(custom_mir)]
+
+use std::intrinsics::mir::*;
+
+#[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(x, after_call, myfun(ptr))
+        }
+
+        after_call = {
+            Return()
+        }
+    }
+}
+
+fn myfun(ptr: *mut i32) -> i32 {
+    // This overwrites the return place, which shouldn't be possible through another pointer.
+    unsafe { ptr.write(0) };
+    //~^ ERROR: /write access .* forbidden/
+    13
+}
diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr
new file mode 100644
index 00000000000..33a8a4b46bd
--- /dev/null
+++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr
@@ -0,0 +1,39 @@
+error: Undefined Behavior: write access through <TAG> (root of the allocation) is forbidden
+  --> $DIR/return_pointer_aliasing2.rs:LL:CC
+   |
+LL |     unsafe { ptr.write(0) };
+   |              ^^^^^^^^^^^^ write 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 write access would cause the protected tag <TAG> (currently Active) to become Disabled
+   = help: protected tags must never be Disabled
+help: the accessed tag <TAG> was created here
+  --> $DIR/return_pointer_aliasing2.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_aliasing2.rs:LL:CC
+   |
+LL |     unsafe { ptr.write(0) };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
+   = note: inside `myfun` at $DIR/return_pointer_aliasing2.rs:LL:CC
+note: inside `main`
+  --> $DIR/return_pointer_aliasing2.rs:LL:CC
+   |
+LL |             Call(x, 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
+