Skip to content

Commit

Permalink
Auto merge of rust-lang#3114 - RalfJung:fn-call-tests, r=RalfJung
Browse files Browse the repository at this point in the history
ensure RET assignments do not get propagated on unwinding

Mostly this adds a test for rust-lang/unsafe-code-guidelines#468, and then also some other related tests I thought of while writing that test.
  • Loading branch information
bors committed Oct 9, 2023
2 parents a04b7a3 + e6637cb commit ec1db97
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//@compile-flags: -Zmiri-tree-borrows
// This does need an aliasing model.
//@revisions: stack tree
//@[tree]compile-flags: -Zmiri-tree-borrows
#![feature(raw_ref_op)]
#![feature(core_intrinsics)]
#![feature(custom_mir)]
Expand All @@ -25,6 +27,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) };
//~^ ERROR: /write access .* forbidden/
//~[stack]^ ERROR: tag does not exist in the borrow stack
//~[tree]| ERROR: /write access .* forbidden/
13
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
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/return_pointer_aliasing2.rs:LL:CC
|
LL | unsafe { ptr.write(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 SharedReadWrite retag at offsets [0x0..0x4]
--> $DIR/return_pointer_aliasing2.rs:LL:CC
|
LL | / mir! {
LL | | {
LL | | let _x = 0;
LL | | let ptr = &raw mut _x;
... |
LL | | }
LL | | }
| |_____^
help: <TAG> was later invalidated at offsets [0x0..0x4] by a Unique in-place function argument/return passing protection
--> $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 = myfun(ptr), after_call)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Doesn't need an aliasing model.
//@compile-flags: -Zmiri-disable-stacked-borrows
#![feature(raw_ref_op)]
#![feature(core_intrinsics)]
#![feature(custom_mir)]

use std::intrinsics::mir::*;
use std::panic;

#[repr(C)]
struct S(i32, [u8; 128]);

#[custom_mir(dialect = "runtime", phase = "optimized")]
fn docall(out: &mut S) {
mir! {
{
Call(*out = callee(), after_call)
}

after_call = {
Return()
}
}
}

fn startpanic() -> () {
panic!()
}

#[custom_mir(dialect = "runtime", phase = "optimized")]
fn callee() -> S {
mir! {
type RET = S;
let _unit: ();
{
// We test whether changes done to RET before unwinding
// become visible to the outside. In codegen we can see them
// but Miri should detect this as UB!
RET.0 = 42;
Call(_unit = startpanic(), after_call)
}

after_call = {
Return()
}
}
}

fn main() {
let mut x = S(0, [0; 128]);
panic::catch_unwind(panic::AssertUnwindSafe(|| docall(&mut x))).unwrap_err();
// The return place got de-initialized before the call and assigning to RET
// does not propagate if we do not reach the `Return`.
dbg!(x.0); //~ERROR: uninitialized
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
thread 'main' panicked at $DIR/return_pointer_on_unwind.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
--> $DIR/return_pointer_on_unwind.rs:LL:CC
|
LL | dbg!(x.0);
| ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
|
= 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 RUSTLIB/std/src/macros.rs:LL:CC
= note: this error originates in the macro `dbg` (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

16 changes: 16 additions & 0 deletions src/tools/miri/tests/pass/calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,26 @@ fn const_fn_call() -> i64 {
x
}

fn call_return_into_passed_reference() {
pub fn func<T>(v: &mut T, f: fn(&T) -> T) {
// MIR building will introduce a temporary, so this becomes
// `let temp = f(v); *v = temp;`.
// If this got optimized to `*v = f(v)` on the MIR level we'd have UB
// since the return place may not be observed while the function runs!
*v = f(v);
}

let mut x = 0;
func(&mut x, |v| v + 1);
assert_eq!(x, 1);
}

fn main() {
assert_eq!(call(), 2);
assert_eq!(factorial_recursive(), 3628800);
assert_eq!(call_generic(), (42, true));
assert_eq!(cross_crate_fn_call(), 1);
assert_eq!(const_fn_call(), 11);

call_return_into_passed_reference();
}

0 comments on commit ec1db97

Please sign in to comment.