From 03651013f8edd9019f9dcafe296c5d6c07bc3e85 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Jul 2023 16:43:50 +0200 Subject: test and fix return place alias restrictions --- .../miri/src/borrow_tracker/tree_borrows/mod.rs | 3 +- .../miri/src/borrow_tracker/tree_borrows/perms.rs | 5 +++ .../fail/both_borrows/return_pointer_aliasing.rs | 32 ++++++++++++++++ .../return_pointer_aliasing.stack.stderr | 42 +++++++++++++++++++++ .../return_pointer_aliasing.tree.stderr | 44 ++++++++++++++++++++++ 5 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs create mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr create mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index e134b739888..3bb38a249ff 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -514,9 +514,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?; let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); // Reborrow it. With protection! That is part of the point. - // FIXME: do we truly want a 2phase borrow here? let new_perm = Some(NewPermission { - initial_state: Permission::new_unique_2phase(/*freeze*/ false), + initial_state: Permission::new_active(), zero_size: false, protector: Some(ProtectorKind::StrongProtector), }); diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 6b1e722b65e..362070f1857 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -138,6 +138,11 @@ impl Permission { Self { inner: Reserved { ty_is_freeze } } } + /// Default initial permission for return place. + pub fn new_active() -> Self { + Self { inner: Active } + } + /// Default initial permission of a reborrowed shared reference pub fn new_frozen() -> Self { Self { inner: Frozen } diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs new file mode 100644 index 00000000000..03886e7874d --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs @@ -0,0 +1,32 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; +use std::mem::MaybeUninit; + +#[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(*ptr, after_call, myfun(ptr)) + } + + after_call = { + Return() + } + } +} + +fn myfun(ptr: *mut i32) -> i32 { + unsafe { ptr.cast::>().read() }; + //~[stack]^ ERROR: /not granting access/ + //~[tree]| ERROR: /read access .* forbidden/ + 13 +} diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr new file mode 100644 index 00000000000..5bdddf5e49c --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr @@ -0,0 +1,42 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.cast::>().read() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | + = 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: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: is this argument + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / fn myfun(ptr: *mut i32) -> i32 { +LL | | unsafe { ptr.cast::>().read() }; +LL | | +LL | | +LL | | 13 +LL | | } + | |_^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | Call(*ptr, 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 + diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr new file mode 100644 index 00000000000..dd76d65c7ca --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr @@ -0,0 +1,44 @@ +error: Undefined Behavior: read access through (root of the allocation) is forbidden + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.cast::>().read() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ read access through (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 (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign read access would cause the protected tag to transition from Active to Frozen + = help: this transition would be a loss of write permissions, which is not allowed for protected tags +help: the accessed tag was created here + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Active + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / fn myfun(ptr: *mut i32) -> i32 { +LL | | unsafe { ptr.cast::>().read() }; +LL | | +LL | | +LL | | 13 +LL | | } + | |_^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | Call(*ptr, 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 + -- cgit 1.4.1-3-g733a5