diff options
5 files changed, 111 insertions, 7 deletions
diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 084d45cf2cb..23d362de308 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -2,6 +2,8 @@ //! //! The main entry point is the `step` method. +use std::iter; + use either::Either; use rustc_abi::{FIRST_VARIANT, FieldIdx}; use rustc_data_structures::fx::FxHashSet; @@ -426,6 +428,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { terminator: &mir::Terminator<'tcx>, func: &mir::Operand<'tcx>, args: &[Spanned<mir::Operand<'tcx>>], + dest: &mir::Place<'tcx>, ) -> InterpResult<'tcx, EvaluatedCalleeAndArgs<'tcx, M>> { let func = self.eval_operand(func, None)?; @@ -435,14 +438,20 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // protection, but then we'd force *a lot* of arguments into memory. So we do some syntactic // pre-processing here where if all `move` arguments are syntactically distinct local // variables (and none is indirect), we can skip the in-memory forcing. + // We have to include `dest` in that list so that we can detect aliasing of an in-place + // argument with the return place. let move_definitely_disjoint = 'move_definitely_disjoint: { let mut previous_locals = FxHashSet::<mir::Local>::default(); - for arg in args { - let mir::Operand::Move(place) = arg.node else { - continue; // we can skip non-`Move` arguments. - }; + for place in args + .iter() + .filter_map(|a| { + // We only have to care about `Move` arguments. + if let mir::Operand::Move(place) = &a.node { Some(place) } else { None } + }) + .chain(iter::once(dest)) + { if place.is_indirect_first_projection() { - // An indirect `Move` argument could alias with anything else... + // An indirect in-place argument could alias with anything else... break 'move_definitely_disjoint false; } if !previous_locals.insert(place.local) { @@ -544,7 +553,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let old_loc = self.frame().loc; let EvaluatedCalleeAndArgs { callee, args, fn_sig, fn_abi, with_caller_location } = - self.eval_callee_and_args(terminator, func, args)?; + self.eval_callee_and_args(terminator, func, args, &destination)?; let destination = self.eval_place(destination)?; self.init_fn_call( @@ -567,7 +576,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let old_frame_idx = self.frame_idx(); let EvaluatedCalleeAndArgs { callee, args, fn_sig, fn_abi, with_caller_location } = - self.eval_callee_and_args(terminator, func, args)?; + self.eval_callee_and_args(terminator, func, args, &mir::Place::return_place())?; self.init_fn_tail_call(callee, (fn_sig.abi, fn_abi), &args, with_caller_location)?; diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.rs index b91a41d7650..744d64b9b1e 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.rs +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.rs @@ -1,3 +1,5 @@ +//! Ensure we detect aliasing of two in-place arguments for the tricky case where they do not +//! live in memory. //@revisions: stack tree //@[tree]compile-flags: -Zmiri-tree-borrows // Validation forces more things into memory, which we can't have here. diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.rs new file mode 100644 index 00000000000..dff724f8d96 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.rs @@ -0,0 +1,34 @@ +//! Ensure we detect aliasing of a in-place argument with the return place for the tricky case where +//! they do not live in memory. +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +// Validation forces more things into memory, which we can't have here. +//@compile-flags: -Zmiri-disable-validation +#![feature(custom_mir, core_intrinsics)] +use std::intrinsics::mir::*; + +#[allow(unused)] +pub struct S(i32); + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn main() { + mir! { + let _unit: (); + { + let staging = S(42); // This forces `staging` into memory... + let _non_copy = staging; // ... so we move it to a non-inmemory local here. + // This specifically uses a type with scalar representation to tempt Miri to use the + // efficient way of storing local variables (outside adressable memory). + Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), UnwindContinue()) + //~[stack]^ ERROR: not granting access + //~[tree]| ERROR: /reborrow .* forbidden/ + } + after_call = { + Return() + } + } +} + +pub fn callee(x: S) -> S { + x +} diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.stack.stderr new file mode 100644 index 00000000000..fcd5b8752e7 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.stack.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 here, as the root tag for ALLOC + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: <TAG> is this argument + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | x + | ^ + = note: BACKTRACE (of the first span): + = note: inside `main` at tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr new file mode 100644 index 00000000000..b7f514de0af --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr @@ -0,0 +1,34 @@ +error: Undefined Behavior: reborrow through <TAG> (root of the allocation) at ALLOC[0x0] is forbidden + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information + = 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 reborrow (acting as a foreign read 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 + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the protected tag <TAG> was created here, in the initial state Reserved + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | x + | ^ +help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] + --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + | +LL | x + | ^ + = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference + = note: BACKTRACE (of the first span): + = note: inside `main` at tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + |