Auto merge of #115608 - RalfJung:fn-arg-validity, r=oli-obk

miri: catch function calls where the argument is caller-invalid / the return value callee-invalid

When doing a type-changing copy, we must validate the data both at the old and new type.

Fixes https://github.com/rust-lang/miri/issues/3017

compiler/rustc_const_eval/src/interpret/place.rs

@@ -796,6 +796,13 @@ pub fn copy_op(
         dest: &impl Writeable<'tcx, M::Provenance>,
         allow_transmute: bool,
     ) -> InterpResult<'tcx> {
+        // Generally for transmutation, data must be valid both at the old and new type.
+        // But if the types are the same, the 2nd validation below suffices.
+        if src.layout().ty != dest.layout().ty && M::enforce_validity(self, src.layout()) {
+            self.validate_operand(&src.to_op(self)?)?;
+        }
+
+        // Do the actual copy.
         self.copy_op_no_validate(src, dest, allow_transmute)?;
 
         if M::enforce_validity(self, dest.layout()) {

src/tools/miri/tests/fail/validity/cast_fn_ptr_invalid_callee_arg.stderr

@@ -1,5 +1,5 @@
 error: Undefined Behavior: constructing invalid value: encountered a null reference
-  --> $DIR/cast_fn_ptr1.rs:LL:CC
+  --> $DIR/cast_fn_ptr_invalid_callee_arg.rs:LL:CC
    |
 LL |     g(0usize as *const i32)
    |     ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a null reference
@@ -7,7 +7,7 @@ LL |     g(0usize as *const i32)
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
-   = note: inside `main` at $DIR/cast_fn_ptr1.rs:LL:CC
+   = note: inside `main` at $DIR/cast_fn_ptr_invalid_callee_arg.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail/validity/cast_fn_ptr_invalid_callee_ret.rs

@@ -0,0 +1,28 @@
+#![allow(internal_features)]
+#![feature(core_intrinsics, custom_mir)]
+
+use std::intrinsics::mir::*;
+use std::num::NonZeroU32;
+use std::ptr;
+
+// This function supposedly returns a NonZeroU32, but actually returns something invalid in a way that
+// never materializes a bad NonZeroU32 value: we take a pointer to the return place and cast the pointer
+// type. That way we never get an "invalid value constructed" error inside the function, it can
+// only possibly be detected when the return value is passed to the caller.
+#[custom_mir(dialect = "runtime", phase = "optimized")]
+fn f() -> NonZeroU32 {
+    mir! {
+        {
+            let tmp = ptr::addr_of_mut!(RET);
+            let ptr = tmp as *mut u32;
+            *ptr = 0;
+            Return()
+        }
+    }
+}
+
+fn main() {
+    let f: fn() -> u32 = unsafe { std::mem::transmute(f as fn() -> NonZeroU32) };
+    // There's a NonZeroU32-to-u32 transmute happening here
+    f(); //~ERROR: expected something greater or equal to 1
+}

src/tools/miri/tests/fail/validity/cast_fn_ptr_invalid_callee_ret.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: constructing invalid value: encountered 0, but expected something greater or equal to 1
+  --> $DIR/cast_fn_ptr_invalid_callee_ret.rs:LL:CC
+   |
+LL |     f();
+   |     ^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/cast_fn_ptr_invalid_callee_ret.rs:LL:CC
+
+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/validity/cast_fn_ptr_invalid_caller_arg.rs

@@ -0,0 +1,34 @@
+#![allow(internal_features)]
+#![feature(core_intrinsics, custom_mir)]
+
+use std::intrinsics::mir::*;
+use std::num::NonZeroU32;
+use std::ptr;
+
+fn f(c: u32) {
+    println!("{c}");
+}
+
+// Call that function in a bad way, with an invalid NonZeroU32, but without
+// ever materializing this as a NonZeroU32 value outside the call itself.
+#[custom_mir(dialect = "runtime", phase = "optimized")]
+fn call(f: fn(NonZeroU32)) {
+    mir! {
+        let _res: ();
+        {
+            let c = 0;
+            let tmp = ptr::addr_of!(c);
+            let ptr = tmp as *const NonZeroU32;
+            // The call site now is a NonZeroU32-to-u32 transmute.
+            Call(_res = f(*ptr), retblock) //~ERROR: expected something greater or equal to 1
+        }
+        retblock = {
+            Return()
+        }
+    }
+}
+
+fn main() {
+    let f: fn(NonZeroU32) = unsafe { std::mem::transmute(f as fn(u32)) };
+    call(f);
+}

src/tools/miri/tests/fail/validity/cast_fn_ptr_invalid_caller_arg.stderr

@@ -0,0 +1,20 @@
+error: Undefined Behavior: constructing invalid value: encountered 0, but expected something greater or equal to 1
+  --> $DIR/cast_fn_ptr_invalid_caller_arg.rs:LL:CC
+   |
+LL |             Call(_res = f(*ptr), retblock)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `call` at $DIR/cast_fn_ptr_invalid_caller_arg.rs:LL:CC
+note: inside `main`
+  --> $DIR/cast_fn_ptr_invalid_caller_arg.rs:LL:CC
+   |
+LL |     call(f);
+   |     ^^^^^^^
+
+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/validity/cast_fn_ptr_invalid_caller_ret.stderr

@@ -1,5 +1,5 @@
 error: Undefined Behavior: constructing invalid value: encountered a null reference
-  --> $DIR/cast_fn_ptr2.rs:LL:CC
+  --> $DIR/cast_fn_ptr_invalid_caller_ret.rs:LL:CC
    |
 LL |     let _x = g();
    |              ^^^ constructing invalid value: encountered a null reference
@@ -7,7 +7,7 @@ LL |     let _x = g();
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
-   = note: inside `main` at $DIR/cast_fn_ptr2.rs:LL:CC
+   = note: inside `main` at $DIR/cast_fn_ptr_invalid_caller_ret.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace