Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure RET assignments do not get propagated on unwinding #3114

Merged
merged 3 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions tests/fail/function_calls/return_pointer_aliasing2.rs
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
}
40 changes: 40 additions & 0 deletions tests/fail/function_calls/return_pointer_aliasing2.stack.stderr
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

55 changes: 55 additions & 0 deletions tests/fail/function_calls/return_pointer_on_unwind.rs
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
}
19 changes: 19 additions & 0 deletions tests/fail/function_calls/return_pointer_on_unwind.stderr
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 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();
}