Skip to content

Commit 136dab6

Browse files
committedJul 12, 2023
Auto merge of #113569 - RalfJung:miri, r=oli-obk
miri: protect Move() function arguments during the call This gives `Move` operands a meaning specific to function calls: - for the duration of the call, the place the operand comes from is protected, making all read and write accesses insta-UB. - the contents of that place are reset to `Uninit`, so looking at them again after the function returns, we cannot observe their contents Turns out we can replace the existing "retag return place" hack with the exact same sort of protection on the return place, which is nicely symmetric. Fixes #112564 Fixes rust-lang/miri#2927 This starts with a Miri rustc-push, since we'd otherwise conflict with a PR that recently landed in Miri. (The "miri tree borrows" commit is an unrelated cleanup I noticed while doing the PR. I can remove it if you prefer.) r? `@oli-obk`
2 parents 910be1b + e7c6db7 commit 136dab6

37 files changed

+747
-161
lines changed
 

‎compiler/rustc_const_eval/src/const_eval/machine.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_target::spec::abi::Abi as CallAbi;
2222

2323
use crate::errors::{LongRunning, LongRunningWarn};
2424
use crate::interpret::{
25-
self, compile_time_machine, AllocId, ConstAllocation, FnVal, Frame, ImmTy, InterpCx,
25+
self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
2626
InterpResult, OpTy, PlaceTy, Pointer, Scalar,
2727
};
2828
use crate::{errors, fluent_generated as fluent};
@@ -201,7 +201,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
201201
fn hook_special_const_fn(
202202
&mut self,
203203
instance: ty::Instance<'tcx>,
204-
args: &[OpTy<'tcx>],
204+
args: &[FnArg<'tcx>],
205205
dest: &PlaceTy<'tcx>,
206206
ret: Option<mir::BasicBlock>,
207207
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
@@ -210,6 +210,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
210210
if Some(def_id) == self.tcx.lang_items().panic_display()
211211
|| Some(def_id) == self.tcx.lang_items().begin_panic_fn()
212212
{
213+
let args = self.copy_fn_args(args)?;
213214
// &str or &&str
214215
assert!(args.len() == 1);
215216

@@ -236,8 +237,9 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
236237

237238
return Ok(Some(new_instance));
238239
} else if Some(def_id) == self.tcx.lang_items().align_offset_fn() {
240+
let args = self.copy_fn_args(args)?;
239241
// For align_offset, we replace the function call if the pointer has no address.
240-
match self.align_offset(instance, args, dest, ret)? {
242+
match self.align_offset(instance, &args, dest, ret)? {
241243
ControlFlow::Continue(()) => return Ok(Some(instance)),
242244
ControlFlow::Break(()) => return Ok(None),
243245
}
@@ -293,7 +295,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
293295
self.eval_fn_call(
294296
FnVal::Instance(instance),
295297
(CallAbi::Rust, fn_abi),
296-
&[addr, align],
298+
&[FnArg::Copy(addr), FnArg::Copy(align)],
297299
/* with_caller_location = */ false,
298300
dest,
299301
ret,
@@ -427,7 +429,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
427429
ecx: &mut InterpCx<'mir, 'tcx, Self>,
428430
instance: ty::Instance<'tcx>,
429431
_abi: CallAbi,
430-
args: &[OpTy<'tcx>],
432+
args: &[FnArg<'tcx>],
431433
dest: &PlaceTy<'tcx>,
432434
ret: Option<mir::BasicBlock>,
433435
_unwind: mir::UnwindAction, // unwinding is not supported in consts

‎compiler/rustc_const_eval/src/interpret/eval_context.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -682,11 +682,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
682682
return_to_block: StackPopCleanup,
683683
) -> InterpResult<'tcx> {
684684
trace!("body: {:#?}", body);
685-
// Clobber previous return place contents, nobody is supposed to be able to see them any more
686-
// This also checks dereferenceable, but not align. We rely on all constructed places being
687-
// sufficiently aligned (in particular we rely on `deref_operand` checking alignment).
688-
self.write_uninit(return_place)?;
689-
// first push a stack frame so we have access to the local substs
685+
// First push a stack frame so we have access to the local substs
690686
let pre_frame = Frame {
691687
body,
692688
loc: Right(body.span), // Span used for errors caused during preamble.
@@ -805,6 +801,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
805801
throw_ub_custom!(fluent::const_eval_unwind_past_top);
806802
}
807803

804+
M::before_stack_pop(self, self.frame())?;
805+
808806
// Copy return value. Must of course happen *before* we deallocate the locals.
809807
let copy_ret_result = if !unwinding {
810808
let op = self

‎compiler/rustc_const_eval/src/interpret/intern.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use super::{
3030
use crate::const_eval;
3131
use crate::errors::{DanglingPtrInFinal, UnsupportedUntypedPointer};
3232

33-
pub trait CompileTimeMachine<'mir, 'tcx, T> = Machine<
33+
pub trait CompileTimeMachine<'mir, 'tcx: 'mir, T> = Machine<
3434
'mir,
3535
'tcx,
3636
MemoryKind = T,

‎compiler/rustc_const_eval/src/interpret/machine.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi as CallAbi;
1717
use crate::const_eval::CheckAlignment;
1818

1919
use super::{
20-
AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, Frame, ImmTy, InterpCx,
20+
AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
2121
InterpResult, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
2222
};
2323

@@ -84,7 +84,7 @@ pub trait AllocMap<K: Hash + Eq, V> {
8484

8585
/// Methods of this trait signifies a point where CTFE evaluation would fail
8686
/// and some use case dependent behaviour can instead be applied.
87-
pub trait Machine<'mir, 'tcx>: Sized {
87+
pub trait Machine<'mir, 'tcx: 'mir>: Sized {
8888
/// Additional memory kinds a machine wishes to distinguish from the builtin ones
8989
type MemoryKind: Debug + std::fmt::Display + MayLeak + Eq + 'static;
9090

@@ -182,7 +182,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
182182
ecx: &mut InterpCx<'mir, 'tcx, Self>,
183183
instance: ty::Instance<'tcx>,
184184
abi: CallAbi,
185-
args: &[OpTy<'tcx, Self::Provenance>],
185+
args: &[FnArg<'tcx, Self::Provenance>],
186186
destination: &PlaceTy<'tcx, Self::Provenance>,
187187
target: Option<mir::BasicBlock>,
188188
unwind: mir::UnwindAction,
@@ -194,7 +194,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
194194
ecx: &mut InterpCx<'mir, 'tcx, Self>,
195195
fn_val: Self::ExtraFnVal,
196196
abi: CallAbi,
197-
args: &[OpTy<'tcx, Self::Provenance>],
197+
args: &[FnArg<'tcx, Self::Provenance>],
198198
destination: &PlaceTy<'tcx, Self::Provenance>,
199199
target: Option<mir::BasicBlock>,
200200
unwind: mir::UnwindAction,
@@ -418,6 +418,18 @@ pub trait Machine<'mir, 'tcx>: Sized {
418418
Ok(())
419419
}
420420

421+
/// Called on places used for in-place function argument and return value handling.
422+
///
423+
/// These places need to be protected to make sure the program cannot tell whether the
424+
/// argument/return value was actually copied or passed in-place..
425+
fn protect_in_place_function_argument(
426+
ecx: &mut InterpCx<'mir, 'tcx, Self>,
427+
place: &PlaceTy<'tcx, Self::Provenance>,
428+
) -> InterpResult<'tcx> {
429+
// Without an aliasing model, all we can do is put `Uninit` into the place.
430+
ecx.write_uninit(place)
431+
}
432+
421433
/// Called immediately before a new stack frame gets pushed.
422434
fn init_frame_extra(
423435
ecx: &mut InterpCx<'mir, 'tcx, Self>,
@@ -439,6 +451,14 @@ pub trait Machine<'mir, 'tcx>: Sized {
439451
Ok(())
440452
}
441453

454+
/// Called just before the return value is copied to the caller-provided return place.
455+
fn before_stack_pop(
456+
_ecx: &InterpCx<'mir, 'tcx, Self>,
457+
_frame: &Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
458+
) -> InterpResult<'tcx> {
459+
Ok(())
460+
}
461+
442462
/// Called immediately after a stack frame got popped, but before jumping back to the caller.
443463
/// The `locals` have already been destroyed!
444464
fn after_stack_pop(
@@ -484,7 +504,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
484504
_ecx: &mut InterpCx<$mir, $tcx, Self>,
485505
fn_val: !,
486506
_abi: CallAbi,
487-
_args: &[OpTy<$tcx>],
507+
_args: &[FnArg<$tcx>],
488508
_destination: &PlaceTy<$tcx, Self::Provenance>,
489509
_target: Option<mir::BasicBlock>,
490510
_unwind: mir::UnwindAction,

‎compiler/rustc_const_eval/src/interpret/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackP
2626
pub use self::memory::{AllocKind, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind};
2727
pub use self::operand::{ImmTy, Immediate, OpTy, Operand};
2828
pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy};
29+
pub use self::terminator::FnArg;
2930
pub use self::validity::{CtfeValidationMode, RefTracking};
3031
pub use self::visitor::{MutValueVisitor, Value, ValueVisitor};
3132

‎compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -575,14 +575,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
575575
Ok(op)
576576
}
577577

578-
/// Evaluate a bunch of operands at once
579-
pub(super) fn eval_operands(
580-
&self,
581-
ops: &[mir::Operand<'tcx>],
582-
) -> InterpResult<'tcx, Vec<OpTy<'tcx, M::Provenance>>> {
583-
ops.iter().map(|op| self.eval_operand(op, None)).collect()
584-
}
585-
586578
fn eval_ty_constant(
587579
&self,
588580
val: ty::Const<'tcx>,

‎compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,8 @@ where
328328
};
329329

330330
let mplace = MemPlace { ptr: ptr.to_pointer(self)?, meta };
331-
// When deref'ing a pointer, the *static* alignment given by the type is what matters.
331+
// `ref_to_mplace` is called on raw pointers even if they don't actually get dereferenced;
332+
// we hence can't call `size_and_align_of` since that asserts more validity than we want.
332333
let align = layout.align.abi;
333334
Ok(MPlaceTy { mplace, layout, align })
334335
}
@@ -354,34 +355,37 @@ where
354355
#[inline]
355356
pub(super) fn get_place_alloc(
356357
&self,
357-
place: &MPlaceTy<'tcx, M::Provenance>,
358+
mplace: &MPlaceTy<'tcx, M::Provenance>,
358359
) -> InterpResult<'tcx, Option<AllocRef<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
359360
{
360-
assert!(place.layout.is_sized());
361-
assert!(!place.meta.has_meta());
362-
let size = place.layout.size;
363-
self.get_ptr_alloc(place.ptr, size, place.align)
361+
let (size, _align) = self
362+
.size_and_align_of_mplace(&mplace)?
363+
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
364+
// Due to packed places, only `mplace.align` matters.
365+
self.get_ptr_alloc(mplace.ptr, size, mplace.align)
364366
}
365367

366368
#[inline]
367369
pub(super) fn get_place_alloc_mut(
368370
&mut self,
369-
place: &MPlaceTy<'tcx, M::Provenance>,
371+
mplace: &MPlaceTy<'tcx, M::Provenance>,
370372
) -> InterpResult<'tcx, Option<AllocRefMut<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
371373
{
372-
assert!(place.layout.is_sized());
373-
assert!(!place.meta.has_meta());
374-
let size = place.layout.size;
375-
self.get_ptr_alloc_mut(place.ptr, size, place.align)
374+
let (size, _align) = self
375+
.size_and_align_of_mplace(&mplace)?
376+
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
377+
// Due to packed places, only `mplace.align` matters.
378+
self.get_ptr_alloc_mut(mplace.ptr, size, mplace.align)
376379
}
377380

378381
/// Check if this mplace is dereferenceable and sufficiently aligned.
379382
pub fn check_mplace(&self, mplace: MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
380-
let (size, align) = self
383+
let (size, _align) = self
381384
.size_and_align_of_mplace(&mplace)?
382385
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
383-
assert!(mplace.align <= align, "dynamic alignment less strict than static one?");
384-
let align = if M::enforce_alignment(self).should_check() { align } else { Align::ONE };
386+
// Due to packed places, only `mplace.align` matters.
387+
let align =
388+
if M::enforce_alignment(self).should_check() { mplace.align } else { Align::ONE };
385389
self.check_ptr_access_align(mplace.ptr, size, align, CheckInAllocMsg::DerefTest)?;
386390
Ok(())
387391
}

‎compiler/rustc_const_eval/src/interpret/terminator.rs

Lines changed: 129 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use std::borrow::Cow;
22

3+
use either::Either;
34
use rustc_ast::ast::InlineAsmOptions;
4-
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf};
5+
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout};
56
use rustc_middle::ty::Instance;
67
use rustc_middle::{
78
mir,
@@ -12,12 +13,63 @@ use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMo
1213
use rustc_target::spec::abi::Abi;
1314

1415
use super::{
15-
FnVal, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine, MemoryKind, OpTy, Operand,
16-
PlaceTy, Scalar, StackPopCleanup,
16+
AllocId, FnVal, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine, MemoryKind, OpTy,
17+
Operand, PlaceTy, Provenance, Scalar, StackPopCleanup,
1718
};
1819
use crate::fluent_generated as fluent;
1920

21+
/// An argment passed to a function.
22+
#[derive(Clone, Debug)]
23+
pub enum FnArg<'tcx, Prov: Provenance = AllocId> {
24+
/// Pass a copy of the given operand.
25+
Copy(OpTy<'tcx, Prov>),
26+
/// Allow for the argument to be passed in-place: destroy the value originally stored at that place and
27+
/// make the place inaccessible for the duration of the function call.
28+
InPlace(PlaceTy<'tcx, Prov>),
29+
}
30+
31+
impl<'tcx, Prov: Provenance> FnArg<'tcx, Prov> {
32+
pub fn layout(&self) -> &TyAndLayout<'tcx> {
33+
match self {
34+
FnArg::Copy(op) => &op.layout,
35+
FnArg::InPlace(place) => &place.layout,
36+
}
37+
}
38+
}
39+
2040
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
41+
/// Make a copy of the given fn_arg. Any `InPlace` are degenerated to copies, no protection of the
42+
/// original memory occurs.
43+
pub fn copy_fn_arg(
44+
&self,
45+
arg: &FnArg<'tcx, M::Provenance>,
46+
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
47+
match arg {
48+
FnArg::Copy(op) => Ok(op.clone()),
49+
FnArg::InPlace(place) => self.place_to_op(&place),
50+
}
51+
}
52+
53+
/// Make a copy of the given fn_args. Any `InPlace` are degenerated to copies, no protection of the
54+
/// original memory occurs.
55+
pub fn copy_fn_args(
56+
&self,
57+
args: &[FnArg<'tcx, M::Provenance>],
58+
) -> InterpResult<'tcx, Vec<OpTy<'tcx, M::Provenance>>> {
59+
args.iter().map(|fn_arg| self.copy_fn_arg(fn_arg)).collect()
60+
}
61+
62+
pub fn fn_arg_field(
63+
&mut self,
64+
arg: &FnArg<'tcx, M::Provenance>,
65+
field: usize,
66+
) -> InterpResult<'tcx, FnArg<'tcx, M::Provenance>> {
67+
Ok(match arg {
68+
FnArg::Copy(op) => FnArg::Copy(self.operand_field(op, field)?),
69+
FnArg::InPlace(place) => FnArg::InPlace(self.place_field(place, field)?),
70+
})
71+
}
72+
2173
pub(super) fn eval_terminator(
2274
&mut self,
2375
terminator: &mir::Terminator<'tcx>,
@@ -68,14 +120,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
68120
let old_stack = self.frame_idx();
69121
let old_loc = self.frame().loc;
70122
let func = self.eval_operand(func, None)?;
71-
let args = self.eval_operands(args)?;
123+
let args = self.eval_fn_call_arguments(args)?;
72124

73125
let fn_sig_binder = func.layout.ty.fn_sig(*self.tcx);
74126
let fn_sig =
75127
self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig_binder);
76128
let extra_args = &args[fn_sig.inputs().len()..];
77129
let extra_args =
78-
self.tcx.mk_type_list_from_iter(extra_args.iter().map(|arg| arg.layout.ty));
130+
self.tcx.mk_type_list_from_iter(extra_args.iter().map(|arg| arg.layout().ty));
79131

80132
let (fn_val, fn_abi, with_caller_location) = match *func.layout.ty.kind() {
81133
ty::FnPtr(_sig) => {
@@ -185,6 +237,21 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
185237
Ok(())
186238
}
187239

240+
/// Evaluate the arguments of a function call
241+
pub(super) fn eval_fn_call_arguments(
242+
&mut self,
243+
ops: &[mir::Operand<'tcx>],
244+
) -> InterpResult<'tcx, Vec<FnArg<'tcx, M::Provenance>>> {
245+
ops.iter()
246+
.map(|op| {
247+
Ok(match op {
248+
mir::Operand::Move(place) => FnArg::InPlace(self.eval_place(*place)?),
249+
_ => FnArg::Copy(self.eval_operand(op, None)?),
250+
})
251+
})
252+
.collect()
253+
}
254+
188255
fn check_argument_compat(
189256
caller_abi: &ArgAbi<'tcx, Ty<'tcx>>,
190257
callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
@@ -275,7 +342,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
275342
fn pass_argument<'x, 'y>(
276343
&mut self,
277344
caller_args: &mut impl Iterator<
278-
Item = (&'x OpTy<'tcx, M::Provenance>, &'y ArgAbi<'tcx, Ty<'tcx>>),
345+
Item = (&'x FnArg<'tcx, M::Provenance>, &'y ArgAbi<'tcx, Ty<'tcx>>),
279346
>,
280347
callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
281348
callee_arg: &PlaceTy<'tcx, M::Provenance>,
@@ -295,21 +362,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
295362
// Now, check
296363
if !Self::check_argument_compat(caller_abi, callee_abi) {
297364
let callee_ty = format!("{}", callee_arg.layout.ty);
298-
let caller_ty = format!("{}", caller_arg.layout.ty);
365+
let caller_ty = format!("{}", caller_arg.layout().ty);
299366
throw_ub_custom!(
300367
fluent::const_eval_incompatible_types,
301368
callee_ty = callee_ty,
302369
caller_ty = caller_ty,
303370
)
304371
}
372+
// We work with a copy of the argument for now; if this is in-place argument passing, we
373+
// will later protect the source it comes from. This means the callee cannot observe if we
374+
// did in-place of by-copy argument passing, except for pointer equality tests.
375+
let caller_arg_copy = self.copy_fn_arg(&caller_arg)?;
305376
// Special handling for unsized parameters.
306-
if caller_arg.layout.is_unsized() {
377+
if caller_arg_copy.layout.is_unsized() {
307378
// `check_argument_compat` ensures that both have the same type, so we know they will use the metadata the same way.
308-
assert_eq!(caller_arg.layout.ty, callee_arg.layout.ty);
379+
assert_eq!(caller_arg_copy.layout.ty, callee_arg.layout.ty);
309380
// We have to properly pre-allocate the memory for the callee.
310-
// So let's tear down some wrappers.
381+
// So let's tear down some abstractions.
311382
// This all has to be in memory, there are no immediate unsized values.
312-
let src = caller_arg.assert_mem_place();
383+
let src = caller_arg_copy.assert_mem_place();
313384
// The destination cannot be one of these "spread args".
314385
let (dest_frame, dest_local) = callee_arg.assert_local();
315386
// We are just initializing things, so there can't be anything here yet.
@@ -331,7 +402,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
331402
// FIXME: Depending on the PassMode, this should reset some padding to uninitialized. (This
332403
// is true for all `copy_op`, but there are a lot of special cases for argument passing
333404
// specifically.)
334-
self.copy_op(&caller_arg, callee_arg, /*allow_transmute*/ true)
405+
self.copy_op(&caller_arg_copy, callee_arg, /*allow_transmute*/ true)?;
406+
// If this was an in-place pass, protect the place it comes from for the duration of the call.
407+
if let FnArg::InPlace(place) = caller_arg {
408+
M::protect_in_place_function_argument(self, place)?;
409+
}
410+
Ok(())
335411
}
336412

337413
/// Call this function -- pushing the stack frame and initializing the arguments.
@@ -346,7 +422,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
346422
&mut self,
347423
fn_val: FnVal<'tcx, M::ExtraFnVal>,
348424
(caller_abi, caller_fn_abi): (Abi, &FnAbi<'tcx, Ty<'tcx>>),
349-
args: &[OpTy<'tcx, M::Provenance>],
425+
args: &[FnArg<'tcx, M::Provenance>],
350426
with_caller_location: bool,
351427
destination: &PlaceTy<'tcx, M::Provenance>,
352428
target: Option<mir::BasicBlock>,
@@ -372,8 +448,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
372448
match instance.def {
373449
ty::InstanceDef::Intrinsic(def_id) => {
374450
assert!(self.tcx.is_intrinsic(def_id));
375-
// caller_fn_abi is not relevant here, we interpret the arguments directly for each intrinsic.
376-
M::call_intrinsic(self, instance, args, destination, target, unwind)
451+
// FIXME: Should `InPlace` arguments be reset to uninit?
452+
M::call_intrinsic(
453+
self,
454+
instance,
455+
&self.copy_fn_args(args)?,
456+
destination,
457+
target,
458+
unwind,
459+
)
377460
}
378461
ty::InstanceDef::VTableShim(..)
379462
| ty::InstanceDef::ReifyShim(..)
@@ -428,7 +511,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
428511
"caller ABI: {:?}, args: {:#?}",
429512
caller_abi,
430513
args.iter()
431-
.map(|arg| (arg.layout.ty, format!("{:?}", **arg)))
514+
.map(|arg| (
515+
arg.layout().ty,
516+
match arg {
517+
FnArg::Copy(op) => format!("copy({:?})", *op),
518+
FnArg::InPlace(place) => format!("in-place({:?})", *place),
519+
}
520+
))
432521
.collect::<Vec<_>>()
433522
);
434523
trace!(
@@ -449,7 +538,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
449538
// last incoming argument. These two iterators do not have the same type,
450539
// so to keep the code paths uniform we accept an allocation
451540
// (for RustCall ABI only).
452-
let caller_args: Cow<'_, [OpTy<'tcx, M::Provenance>]> =
541+
let caller_args: Cow<'_, [FnArg<'tcx, M::Provenance>]> =
453542
if caller_abi == Abi::RustCall && !args.is_empty() {
454543
// Untuple
455544
let (untuple_arg, args) = args.split_last().unwrap();
@@ -458,11 +547,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
458547
args.iter()
459548
.map(|a| Ok(a.clone()))
460549
.chain(
461-
(0..untuple_arg.layout.fields.count())
462-
.map(|i| self.operand_field(untuple_arg, i)),
550+
(0..untuple_arg.layout().fields.count())
551+
.map(|i| self.fn_arg_field(untuple_arg, i)),
463552
)
464-
.collect::<InterpResult<'_, Vec<OpTy<'tcx, M::Provenance>>>>(
465-
)?,
553+
.collect::<InterpResult<'_, Vec<_>>>()?,
466554
)
467555
} else {
468556
// Plain arg passing
@@ -523,6 +611,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
523611
caller_ty = caller_ty,
524612
)
525613
}
614+
// Ensure the return place is aligned and dereferenceable, and protect it for
615+
// in-place return value passing.
616+
if let Either::Left(mplace) = destination.as_mplace_or_local() {
617+
self.check_mplace(mplace)?;
618+
} else {
619+
// Nothing to do for locals, they are always properly allocated and aligned.
620+
}
621+
M::protect_in_place_function_argument(self, destination)?;
526622
};
527623
match res {
528624
Err(err) => {
@@ -538,7 +634,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
538634
// We have to implement all "object safe receivers". So we have to go search for a
539635
// pointer or `dyn Trait` type, but it could be wrapped in newtypes. So recursively
540636
// unwrap those newtypes until we are there.
541-
let mut receiver = args[0].clone();
637+
// An `InPlace` does nothing here, we keep the original receiver intact. We can't
638+
// really pass the argument in-place anyway, and we are constructing a new
639+
// `Immediate` receiver.
640+
let mut receiver = self.copy_fn_arg(&args[0])?;
542641
let receiver_place = loop {
543642
match receiver.layout.ty.kind() {
544643
ty::Ref(..) | ty::RawPtr(..) => {
@@ -648,11 +747,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
648747
}
649748

650749
// Adjust receiver argument. Layout can be any (thin) ptr.
651-
args[0] = ImmTy::from_immediate(
652-
Scalar::from_maybe_pointer(adjusted_receiver, self).into(),
653-
self.layout_of(Ty::new_mut_ptr(self.tcx.tcx, dyn_ty))?,
654-
)
655-
.into();
750+
args[0] = FnArg::Copy(
751+
ImmTy::from_immediate(
752+
Scalar::from_maybe_pointer(adjusted_receiver, self).into(),
753+
self.layout_of(Ty::new_mut_ptr(self.tcx.tcx, dyn_ty))?,
754+
)
755+
.into(),
756+
);
656757
trace!("Patched receiver operand to {:#?}", args[0]);
657758
// recurse with concrete function
658759
self.eval_fn_call(
@@ -710,7 +811,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
710811
self.eval_fn_call(
711812
FnVal::Instance(instance),
712813
(Abi::Rust, fn_abi),
713-
&[arg.into()],
814+
&[FnArg::Copy(arg.into())],
714815
false,
715816
&ret.into(),
716817
Some(target),

‎compiler/rustc_const_eval/src/interpret/visitor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use super::{InterpCx, MPlaceTy, Machine, OpTy, PlaceTy};
1313
/// A thing that we can project into, and that has a layout.
1414
/// This wouldn't have to depend on `Machine` but with the current type inference,
1515
/// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
16-
pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
16+
pub trait Value<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized {
1717
/// Gets this value's layout.
1818
fn layout(&self) -> TyAndLayout<'tcx>;
1919

@@ -54,7 +54,7 @@ pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
5454
/// A thing that we can project into given *mutable* access to `ecx`, and that has a layout.
5555
/// This wouldn't have to depend on `Machine` but with the current type inference,
5656
/// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
57-
pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
57+
pub trait ValueMut<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized {
5858
/// Gets this value's layout.
5959
fn layout(&self) -> TyAndLayout<'tcx>;
6060

‎compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,10 +1050,6 @@ pub type PlaceElem<'tcx> = ProjectionElem<Local, Ty<'tcx>>;
10501050
/// there may be other effects: if the type has a validity constraint loading the place might be UB
10511051
/// if the validity constraint is not met.
10521052
///
1053-
/// **Needs clarification:** Ralf proposes that loading a place not have side-effects.
1054-
/// This is what is implemented in miri today. Are these the semantics we want for MIR? Is this
1055-
/// something we can even decide without knowing more about Rust's memory model?
1056-
///
10571053
/// **Needs clarification:** Is loading a place that has its variant index set well-formed? Miri
10581054
/// currently implements it, but it seems like this may be something to check against in the
10591055
/// validator.
@@ -1071,6 +1067,16 @@ pub enum Operand<'tcx> {
10711067
/// in [UCG#188]. You should not emit MIR that may attempt a subsequent second load of this
10721068
/// place without first re-initializing it.
10731069
///
1070+
/// **Needs clarification:** The operational impact of `Move` is unclear. Currently (both in
1071+
/// Miri and codegen) it has no effect at all unless it appears in an argument to `Call`; for
1072+
/// `Call` it allows the argument to be passed to the callee "in-place", i.e. the callee might
1073+
/// just get a reference to this place instead of a full copy. Miri implements this with a
1074+
/// combination of aliasing model "protectors" and putting `uninit` into the place. Ralf
1075+
/// proposes that we don't want these semantics for `Move` in regular assignments, because
1076+
/// loading a place should not have side-effects, and the aliasing model "protectors" are
1077+
/// inherently tied to a function call. Are these the semantics we want for MIR? Is this
1078+
/// something we can even decide without knowing more about Rust's memory model?
1079+
///
10741080
/// [UCG#188]: https://github.com/rust-lang/unsafe-code-guidelines/issues/188
10751081
Move(Place<'tcx>),
10761082

‎compiler/rustc_mir_transform/src/const_prop.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ use rustc_target::spec::abi::Abi as CallAbi;
2222

2323
use crate::MirPass;
2424
use rustc_const_eval::interpret::{
25-
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, Frame, ImmTy, Immediate,
26-
InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, Scalar,
25+
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, FnArg, Frame, ImmTy,
26+
Immediate, InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, Scalar,
2727
StackPopCleanup,
2828
};
2929

@@ -185,7 +185,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
185185
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
186186
_instance: ty::Instance<'tcx>,
187187
_abi: CallAbi,
188-
_args: &[OpTy<'tcx>],
188+
_args: &[FnArg<'tcx>],
189189
_destination: &PlaceTy<'tcx>,
190190
_target: Option<BasicBlock>,
191191
_unwind: UnwindAction,

‎compiler/rustc_mir_transform/src/dataflow_const_prop.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ impl<'tcx, 'map, 'a> Visitor<'tcx> for OperandCollector<'tcx, 'map, 'a> {
532532

533533
struct DummyMachine;
534534

535-
impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachine {
535+
impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachine {
536536
rustc_const_eval::interpret::compile_time_machine!(<'mir, 'tcx>);
537537
type MemoryKind = !;
538538
const PANIC_ON_ALLOC_FAIL: bool = true;
@@ -557,7 +557,7 @@ impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachi
557557
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
558558
_instance: ty::Instance<'tcx>,
559559
_abi: rustc_target::spec::abi::Abi,
560-
_args: &[rustc_const_eval::interpret::OpTy<'tcx, Self::Provenance>],
560+
_args: &[rustc_const_eval::interpret::FnArg<'tcx, Self::Provenance>],
561561
_destination: &rustc_const_eval::interpret::PlaceTy<'tcx, Self::Provenance>,
562562
_target: Option<BasicBlock>,
563563
_unwind: UnwindAction,

‎src/tools/miri/rust-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
d4096e0412ac5de785d739a0aa2b1c1c7b9d3b7d
1+
743333f3dd90721461c09387ec73d09c080d5f5f

‎src/tools/miri/src/borrow_tracker/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
302302
}
303303
}
304304

305-
fn retag_return_place(&mut self) -> InterpResult<'tcx> {
305+
fn protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
306306
let this = self.eval_context_mut();
307307
let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method;
308308
match method {
309-
BorrowTrackerMethod::StackedBorrows => this.sb_retag_return_place(),
310-
BorrowTrackerMethod::TreeBorrows => this.tb_retag_return_place(),
309+
BorrowTrackerMethod::StackedBorrows => this.sb_protect_place(place),
310+
BorrowTrackerMethod::TreeBorrows => this.tb_protect_place(place),
311311
}
312312
}
313313

‎src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ struct RetagOp {
189189
#[derive(Debug, Clone, Copy, PartialEq)]
190190
pub enum RetagCause {
191191
Normal,
192-
FnReturnPlace,
192+
InPlaceFnPassing,
193193
FnEntry,
194194
TwoPhase,
195195
}
@@ -501,7 +501,7 @@ impl RetagCause {
501501
match self {
502502
RetagCause::Normal => "retag",
503503
RetagCause::FnEntry => "function-entry retag",
504-
RetagCause::FnReturnPlace => "return-place retag",
504+
RetagCause::InPlaceFnPassing => "in-place function argument/return passing protection",
505505
RetagCause::TwoPhase => "two-phase retag",
506506
}
507507
.to_string()

‎src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -994,35 +994,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
994994
}
995995
}
996996

997-
/// After a stack frame got pushed, retag the return place so that we are sure
998-
/// it does not alias with anything.
999-
///
1000-
/// This is a HACK because there is nothing in MIR that would make the retag
1001-
/// explicit. Also see <https://github.com/rust-lang/rust/issues/71117>.
1002-
fn sb_retag_return_place(&mut self) -> InterpResult<'tcx> {
997+
/// Protect a place so that it cannot be used any more for the duration of the current function
998+
/// call.
999+
///
1000+
/// This is used to ensure soundness of in-place function argument/return passing.
1001+
fn sb_protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
10031002
let this = self.eval_context_mut();
1004-
let return_place = &this.frame().return_place;
1005-
if return_place.layout.is_zst() {
1006-
// There may not be any memory here, nothing to do.
1007-
return Ok(());
1008-
}
1009-
// We need this to be in-memory to use tagged pointers.
1010-
let return_place = this.force_allocation(&return_place.clone())?;
10111003

1012-
// We have to turn the place into a pointer to use the existing code.
1004+
// We have to turn the place into a pointer to use the usual retagging logic.
10131005
// (The pointer type does not matter, so we use a raw pointer.)
1014-
let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?;
1015-
let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout);
1016-
// Reborrow it. With protection! That is part of the point.
1006+
let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, place.layout.ty))?;
1007+
let ptr = ImmTy::from_immediate(place.to_ref(this), ptr_layout);
1008+
// Reborrow it. With protection! That is the entire point.
10171009
let new_perm = NewPermission::Uniform {
10181010
perm: Permission::Unique,
10191011
access: Some(AccessKind::Write),
10201012
protector: Some(ProtectorKind::StrongProtector),
10211013
};
1022-
let val = this.sb_retag_reference(&val, new_perm, RetagCause::FnReturnPlace)?;
1023-
// And use reborrowed pointer for return place.
1024-
let return_place = this.ref_to_mplace(&val)?;
1025-
this.frame_mut().return_place = return_place.into();
1014+
let _new_ptr = this.sb_retag_reference(&ptr, new_perm, RetagCause::InPlaceFnPassing)?;
1015+
// We just throw away `new_ptr`, so nobody can access this memory while it is protected.
10261016

10271017
Ok(())
10281018
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
178178
&mut self,
179179
place: &MPlaceTy<'tcx, Provenance>, // parent tag extracted from here
180180
ptr_size: Size,
181-
new_perm: Option<NewPermission>,
181+
new_perm: NewPermission,
182182
new_tag: BorTag,
183183
) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> {
184184
let this = self.eval_context_mut();
@@ -256,10 +256,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
256256
ptr_size.bytes()
257257
);
258258

259-
let Some(new_perm) = new_perm else {
260-
return Ok(Some((alloc_id, orig_tag)));
261-
};
262-
263259
if let Some(protect) = new_perm.protector {
264260
// We register the protection in two different places.
265261
// This makes creating a protector slower, but checking whether a tag
@@ -305,7 +301,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
305301
fn tb_retag_reference(
306302
&mut self,
307303
val: &ImmTy<'tcx, Provenance>,
308-
new_perm: Option<NewPermission>,
304+
new_perm: NewPermission,
309305
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
310306
let this = self.eval_context_mut();
311307
// We want a place for where the ptr *points to*, so we get one.
@@ -317,7 +313,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
317313
// - if the pointer is not reborrowed (raw pointer) or if `zero_size` is set
318314
// then we override the size to do a zero-length reborrow.
319315
let reborrow_size = match new_perm {
320-
Some(NewPermission { zero_size: false, .. }) =>
316+
NewPermission { zero_size: false, .. } =>
321317
this.size_and_align_of_mplace(&place)?
322318
.map(|(size, _)| size)
323319
.unwrap_or(place.layout.size),
@@ -374,7 +370,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
374370
NewPermission::from_ref_ty(pointee, mutability, kind, this),
375371
_ => None,
376372
};
377-
this.tb_retag_reference(val, new_perm)
373+
if let Some(new_perm) = new_perm {
374+
this.tb_retag_reference(val, new_perm)
375+
} else {
376+
Ok(val.clone())
377+
}
378378
}
379379

380380
/// Retag all pointers that are stored in this place.
@@ -405,9 +405,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
405405
place: &PlaceTy<'tcx, Provenance>,
406406
new_perm: Option<NewPermission>,
407407
) -> InterpResult<'tcx> {
408-
let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?;
409-
let val = self.ecx.tb_retag_reference(&val, new_perm)?;
410-
self.ecx.write_immediate(*val, place)?;
408+
if let Some(new_perm) = new_perm {
409+
let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?;
410+
let val = self.ecx.tb_retag_reference(&val, new_perm)?;
411+
self.ecx.write_immediate(*val, place)?;
412+
}
411413
Ok(())
412414
}
413415
}
@@ -493,37 +495,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
493495
}
494496
}
495497

496-
/// After a stack frame got pushed, retag the return place so that we are sure
497-
/// it does not alias with anything.
498-
///
499-
/// This is a HACK because there is nothing in MIR that would make the retag
500-
/// explicit. Also see <https://github.com/rust-lang/rust/issues/71117>.
501-
fn tb_retag_return_place(&mut self) -> InterpResult<'tcx> {
498+
/// Protect a place so that it cannot be used any more for the duration of the current function
499+
/// call.
500+
///
501+
/// This is used to ensure soundness of in-place function argument/return passing.
502+
fn tb_protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
502503
let this = self.eval_context_mut();
503-
//this.debug_hint_location();
504-
let return_place = &this.frame().return_place;
505-
if return_place.layout.is_zst() {
506-
// There may not be any memory here, nothing to do.
507-
return Ok(());
508-
}
509-
// We need this to be in-memory to use tagged pointers.
510-
let return_place = this.force_allocation(&return_place.clone())?;
511504

512-
// We have to turn the place into a pointer to use the existing code.
505+
// We have to turn the place into a pointer to use the usual retagging logic.
513506
// (The pointer type does not matter, so we use a raw pointer.)
514-
let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?;
515-
let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout);
516-
// Reborrow it. With protection! That is part of the point.
517-
// FIXME: do we truly want a 2phase borrow here?
518-
let new_perm = Some(NewPermission {
519-
initial_state: Permission::new_unique_2phase(/*freeze*/ false),
507+
let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, place.layout.ty))?;
508+
let ptr = ImmTy::from_immediate(place.to_ref(this), ptr_layout);
509+
// Reborrow it. With protection! That is the entire point.
510+
let new_perm = NewPermission {
511+
initial_state: Permission::new_active(),
520512
zero_size: false,
521513
protector: Some(ProtectorKind::StrongProtector),
522-
});
523-
let val = this.tb_retag_reference(&val, new_perm)?;
524-
// And use reborrowed pointer for return place.
525-
let return_place = this.ref_to_mplace(&val)?;
526-
this.frame_mut().return_place = return_place.into();
514+
};
515+
let _new_ptr = this.tb_retag_reference(&ptr, new_perm)?;
516+
// We just throw away `new_ptr`, so nobody can access this memory while it is protected.
527517

528518
Ok(())
529519
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ impl Permission {
138138
Self { inner: Reserved { ty_is_freeze } }
139139
}
140140

141+
/// Default initial permission for return place.
142+
pub fn new_active() -> Self {
143+
Self { inner: Active }
144+
}
145+
141146
/// Default initial permission of a reborrowed shared reference
142147
pub fn new_frozen() -> Self {
143148
Self { inner: Frozen }

‎src/tools/miri/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ extern crate rustc_index;
5555
extern crate rustc_session;
5656
extern crate rustc_span;
5757
extern crate rustc_target;
58+
extern crate either; // the one from rustc
5859

5960
// Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta
6061
// files.

‎src/tools/miri/src/machine.rs

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::fmt;
77
use std::path::Path;
88
use std::process;
99

10+
use either::Either;
1011
use rand::rngs::StdRng;
1112
use rand::SeedableRng;
1213

@@ -533,15 +534,16 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
533534
let target = &tcx.sess.target;
534535
match target.arch.as_ref() {
535536
"wasm32" | "wasm64" => 64 * 1024, // https://webassembly.github.io/spec/core/exec/runtime.html#memory-instances
536-
"aarch64" =>
537+
"aarch64" => {
537538
if target.options.vendor.as_ref() == "apple" {
538539
// No "definitive" source, but see:
539540
// https://www.wwdcnotes.com/notes/wwdc20/10214/
540541
// https://github.com/ziglang/zig/issues/11308 etc.
541542
16 * 1024
542543
} else {
543544
4 * 1024
544-
},
545+
}
546+
}
545547
_ => 4 * 1024,
546548
}
547549
};
@@ -892,7 +894,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
892894
ecx: &mut MiriInterpCx<'mir, 'tcx>,
893895
instance: ty::Instance<'tcx>,
894896
abi: Abi,
895-
args: &[OpTy<'tcx, Provenance>],
897+
args: &[FnArg<'tcx, Provenance>],
896898
dest: &PlaceTy<'tcx, Provenance>,
897899
ret: Option<mir::BasicBlock>,
898900
unwind: mir::UnwindAction,
@@ -905,12 +907,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
905907
ecx: &mut MiriInterpCx<'mir, 'tcx>,
906908
fn_val: Dlsym,
907909
abi: Abi,
908-
args: &[OpTy<'tcx, Provenance>],
910+
args: &[FnArg<'tcx, Provenance>],
909911
dest: &PlaceTy<'tcx, Provenance>,
910912
ret: Option<mir::BasicBlock>,
911913
_unwind: mir::UnwindAction,
912914
) -> InterpResult<'tcx> {
913-
ecx.call_dlsym(fn_val, abi, args, dest, ret)
915+
let args = ecx.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit?
916+
ecx.call_dlsym(fn_val, abi, &args, dest, ret)
914917
}
915918

916919
#[inline(always)]
@@ -1094,8 +1097,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
10941097
ptr: Pointer<Self::Provenance>,
10951098
) -> InterpResult<'tcx> {
10961099
match ptr.provenance {
1097-
Provenance::Concrete { alloc_id, tag } =>
1098-
intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag),
1100+
Provenance::Concrete { alloc_id, tag } => {
1101+
intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag)
1102+
}
10991103
Provenance::Wildcard => {
11001104
// No need to do anything for wildcard pointers as
11011105
// their provenances have already been previously exposed.
@@ -1206,6 +1210,25 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
12061210
Ok(())
12071211
}
12081212

1213+
fn protect_in_place_function_argument(
1214+
ecx: &mut InterpCx<'mir, 'tcx, Self>,
1215+
place: &PlaceTy<'tcx, Provenance>,
1216+
) -> InterpResult<'tcx> {
1217+
// We do need to write `uninit` so that even after the call ends, the former contents of
1218+
// this place cannot be observed any more.
1219+
ecx.write_uninit(place)?;
1220+
// If we have a borrow tracker, we also have it set up protection so that all reads *and
1221+
// writes* during this call are insta-UB.
1222+
if ecx.machine.borrow_tracker.is_some() {
1223+
if let Either::Left(place) = place.as_mplace_or_local() {
1224+
ecx.protect_place(&place)?;
1225+
} else {
1226+
// Locals that don't have their address taken are as protected as they can ever be.
1227+
}
1228+
}
1229+
Ok(())
1230+
}
1231+
12091232
#[inline(always)]
12101233
fn init_frame_extra(
12111234
ecx: &mut InterpCx<'mir, 'tcx, Self>,
@@ -1288,8 +1311,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
12881311
let stack_len = ecx.active_thread_stack().len();
12891312
ecx.active_thread_mut().set_top_user_relevant_frame(stack_len - 1);
12901313
}
1291-
if ecx.machine.borrow_tracker.is_some() {
1292-
ecx.retag_return_place()?;
1314+
Ok(())
1315+
}
1316+
1317+
fn before_stack_pop(
1318+
ecx: &InterpCx<'mir, 'tcx, Self>,
1319+
frame: &Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
1320+
) -> InterpResult<'tcx> {
1321+
// We want this *before* the return value copy, because the return place itself is protected
1322+
// until we do `end_call` here.
1323+
if let Some(borrow_tracker) = &ecx.machine.borrow_tracker {
1324+
borrow_tracker.borrow_mut().end_call(&frame.extra);
12931325
}
12941326
Ok(())
12951327
}
@@ -1308,9 +1340,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
13081340
ecx.active_thread_mut().recompute_top_user_relevant_frame();
13091341
}
13101342
let timing = frame.extra.timing.take();
1311-
if let Some(borrow_tracker) = &ecx.machine.borrow_tracker {
1312-
borrow_tracker.borrow_mut().end_call(&frame.extra);
1313-
}
13141343
let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding);
13151344
if let Some(profiler) = ecx.machine.profiler.as_ref() {
13161345
profiler.finish_recording_interval_event(timing.unwrap());

‎src/tools/miri/src/shims/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
3131
&mut self,
3232
instance: ty::Instance<'tcx>,
3333
abi: Abi,
34-
args: &[OpTy<'tcx, Provenance>],
34+
args: &[FnArg<'tcx, Provenance>],
3535
dest: &PlaceTy<'tcx, Provenance>,
3636
ret: Option<mir::BasicBlock>,
3737
unwind: mir::UnwindAction,
@@ -41,7 +41,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
4141

4242
// There are some more lang items we want to hook that CTFE does not hook (yet).
4343
if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) {
44-
let [ptr, align] = check_arg_count(args)?;
44+
let args = this.copy_fn_args(args)?;
45+
let [ptr, align] = check_arg_count(&args)?;
4546
if this.align_offset(ptr, align, dest, ret, unwind)? {
4647
return Ok(None);
4748
}
@@ -55,7 +56,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
5556
// to run extra MIR), and Ok(Some(body)) if we found MIR to run for the
5657
// foreign function
5758
// Any needed call to `goto_block` will be performed by `emulate_foreign_item`.
58-
return this.emulate_foreign_item(instance.def_id(), abi, args, dest, ret, unwind);
59+
let args = this.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit?
60+
return this.emulate_foreign_item(instance.def_id(), abi, &args, dest, ret, unwind);
5961
}
6062

6163
// Otherwise, load the MIR.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//@revisions: stack tree
2+
//@[tree]compile-flags: -Zmiri-tree-borrows
3+
#![feature(custom_mir, core_intrinsics)]
4+
use std::intrinsics::mir::*;
5+
6+
pub struct S(i32);
7+
8+
#[custom_mir(dialect = "runtime", phase = "optimized")]
9+
fn main() {
10+
mir! {
11+
let unit: ();
12+
{
13+
let non_copy = S(42);
14+
let ptr = std::ptr::addr_of_mut!(non_copy);
15+
// Inside `callee`, the first argument and `*ptr` are basically
16+
// aliasing places!
17+
Call(unit, after_call, callee(Move(*ptr), ptr))
18+
}
19+
after_call = {
20+
Return()
21+
}
22+
23+
}
24+
}
25+
26+
pub fn callee(x: S, ptr: *mut S) {
27+
// With the setup above, if `x` is indeed moved in
28+
// (i.e. we actually just get a pointer to the underlying storage),
29+
// then writing to `ptr` will change the value stored in `x`!
30+
unsafe { ptr.write(S(0)) };
31+
//~[stack]^ ERROR: not granting access
32+
//~[tree]| ERROR: /write access .* forbidden/
33+
assert_eq!(x.0, 42);
34+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
2+
--> $DIR/arg_inplace_mutate.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.write(S(0)) };
5+
| ^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
8+
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9+
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
10+
--> $DIR/arg_inplace_mutate.rs:LL:CC
11+
|
12+
LL | / mir! {
13+
LL | | let unit: ();
14+
LL | | {
15+
LL | | let non_copy = S(42);
16+
... |
17+
LL | |
18+
LL | | }
19+
| |_____^
20+
help: <TAG> is this argument
21+
--> $DIR/arg_inplace_mutate.rs:LL:CC
22+
|
23+
LL | unsafe { ptr.write(S(0)) };
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
= note: BACKTRACE (of the first span):
26+
= note: inside `callee` at $DIR/arg_inplace_mutate.rs:LL:CC
27+
note: inside `main`
28+
--> $DIR/arg_inplace_mutate.rs:LL:CC
29+
|
30+
LL | Call(unit, after_call, callee(Move(*ptr), ptr))
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
= 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)
33+
34+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
35+
36+
error: aborting due to previous error
37+
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: Undefined Behavior: write access through <TAG> (root of the allocation) is forbidden
2+
--> $DIR/arg_inplace_mutate.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.write(S(0)) };
5+
| ^^^^^^^^^^^^^^^ write access through <TAG> (root of the allocation) is forbidden
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
8+
= help: the accessed tag <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child)
9+
= help: this foreign write access would cause the protected tag <TAG> to transition from Active to Disabled
10+
= help: this transition would be a loss of read and write permissions, which is not allowed for protected tags
11+
help: the accessed tag <TAG> was created here
12+
--> $DIR/arg_inplace_mutate.rs:LL:CC
13+
|
14+
LL | / mir! {
15+
LL | | let unit: ();
16+
LL | | {
17+
LL | | let non_copy = S(42);
18+
... |
19+
LL | |
20+
LL | | }
21+
| |_____^
22+
help: the protected tag <TAG> was created here, in the initial state Active
23+
--> $DIR/arg_inplace_mutate.rs:LL:CC
24+
|
25+
LL | unsafe { ptr.write(S(0)) };
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
27+
= note: BACKTRACE (of the first span):
28+
= note: inside `callee` at $DIR/arg_inplace_mutate.rs:LL:CC
29+
note: inside `main`
30+
--> $DIR/arg_inplace_mutate.rs:LL:CC
31+
|
32+
LL | Call(unit, after_call, callee(Move(*ptr), ptr))
33+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
34+
= 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)
35+
36+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
37+
38+
error: aborting due to previous error
39+
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![feature(custom_mir, core_intrinsics)]
2+
use std::intrinsics::mir::*;
3+
4+
pub struct S(i32);
5+
6+
#[custom_mir(dialect = "runtime", phase = "optimized")]
7+
fn main() {
8+
// FIXME: the span is not great (probably caused by custom MIR)
9+
mir! { //~ERROR: uninitialized
10+
let unit: ();
11+
{
12+
let non_copy = S(42);
13+
// This could change `non_copy` in-place
14+
Call(unit, after_call, change_arg(Move(non_copy)))
15+
}
16+
after_call = {
17+
// So now we must not be allowed to observe non-copy again.
18+
let _observe = non_copy.0;
19+
Return()
20+
}
21+
22+
}
23+
}
24+
25+
pub fn change_arg(mut x: S) {
26+
x.0 = 0;
27+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> $DIR/arg_inplace_observe_after.rs:LL:CC
3+
|
4+
LL | / mir! {
5+
LL | | let unit: ();
6+
LL | | {
7+
LL | | let non_copy = S(42);
8+
... |
9+
LL | |
10+
LL | | }
11+
| |_____^ using uninitialized data, but this operation requires initialized memory
12+
|
13+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
14+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
15+
= note: BACKTRACE:
16+
= note: inside `main` at RUSTLIB/core/src/intrinsics/mir.rs:LL:CC
17+
= 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)
18+
19+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
20+
21+
error: aborting due to previous error
22+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
13+
|
14+
LL | Call(unit, after_call, change_arg(Move(*ptr), ptr))
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to previous error
20+
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//@revisions: stack tree none
2+
//@[tree]compile-flags: -Zmiri-tree-borrows
3+
//@[none]compile-flags: -Zmiri-disable-stacked-borrows
4+
#![feature(custom_mir, core_intrinsics)]
5+
use std::intrinsics::mir::*;
6+
7+
pub struct S(i32);
8+
9+
#[custom_mir(dialect = "runtime", phase = "optimized")]
10+
fn main() {
11+
mir! {
12+
let unit: ();
13+
{
14+
let non_copy = S(42);
15+
let ptr = std::ptr::addr_of_mut!(non_copy);
16+
// This could change `non_copy` in-place
17+
Call(unit, after_call, change_arg(Move(*ptr), ptr))
18+
}
19+
after_call = {
20+
Return()
21+
}
22+
23+
}
24+
}
25+
26+
pub fn change_arg(mut x: S, ptr: *mut S) {
27+
x.0 = 0;
28+
// If `x` got passed in-place, we'd see the write through `ptr`!
29+
// Make sure we are not allowed to do that read.
30+
unsafe { ptr.read() };
31+
//~[stack]^ ERROR: not granting access
32+
//~[tree]| ERROR: /read access .* forbidden/
33+
//~[none]| ERROR: uninitialized
34+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
2+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
8+
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9+
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
10+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
11+
|
12+
LL | / mir! {
13+
LL | | let unit: ();
14+
LL | | {
15+
LL | | let non_copy = S(42);
16+
... |
17+
LL | |
18+
LL | | }
19+
| |_____^
20+
help: <TAG> is this argument
21+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
22+
|
23+
LL | x.0 = 0;
24+
| ^^^^^^^
25+
= note: BACKTRACE (of the first span):
26+
= note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC
27+
note: inside `main`
28+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
29+
|
30+
LL | Call(unit, after_call, change_arg(Move(*ptr), ptr))
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
= 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)
33+
34+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
35+
36+
error: aborting due to previous error
37+
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: Undefined Behavior: read access through <TAG> (root of the allocation) is forbidden
2+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ read access through <TAG> (root of the allocation) is forbidden
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
8+
= help: the accessed tag <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child)
9+
= help: this foreign read access would cause the protected tag <TAG> to transition from Active to Frozen
10+
= help: this transition would be a loss of write permissions, which is not allowed for protected tags
11+
help: the accessed tag <TAG> was created here
12+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
13+
|
14+
LL | / mir! {
15+
LL | | let unit: ();
16+
LL | | {
17+
LL | | let non_copy = S(42);
18+
... |
19+
LL | |
20+
LL | | }
21+
| |_____^
22+
help: the protected tag <TAG> was created here, in the initial state Active
23+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
24+
|
25+
LL | x.0 = 0;
26+
| ^^^^^^^
27+
= note: BACKTRACE (of the first span):
28+
= note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC
29+
note: inside `main`
30+
--> $DIR/arg_inplace_observe_during.rs:LL:CC
31+
|
32+
LL | Call(unit, after_call, change_arg(Move(*ptr), ptr))
33+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
34+
= 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)
35+
36+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
37+
38+
error: aborting due to previous error
39+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> $DIR/return_pointer_aliasing.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/return_pointer_aliasing.rs:LL:CC
13+
|
14+
LL | Call(*ptr, after_call, myfun(ptr))
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to previous error
20+
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//@revisions: stack tree none
2+
//@[tree]compile-flags: -Zmiri-tree-borrows
3+
//@[none]compile-flags: -Zmiri-disable-stacked-borrows
4+
#![feature(raw_ref_op)]
5+
#![feature(core_intrinsics)]
6+
#![feature(custom_mir)]
7+
8+
use std::intrinsics::mir::*;
9+
10+
#[custom_mir(dialect = "runtime", phase = "optimized")]
11+
pub fn main() {
12+
mir! {
13+
{
14+
let x = 0;
15+
let ptr = &raw mut x;
16+
// We arrange for `myfun` to have a pointer that aliases
17+
// its return place. Even just reading from that pointer is UB.
18+
Call(*ptr, after_call, myfun(ptr))
19+
}
20+
21+
after_call = {
22+
Return()
23+
}
24+
}
25+
}
26+
27+
fn myfun(ptr: *mut i32) -> i32 {
28+
unsafe { ptr.read() };
29+
//~[stack]^ ERROR: not granting access
30+
//~[tree]| ERROR: /read access .* forbidden/
31+
//~[none]| ERROR: uninitialized
32+
// Without an aliasing model, reads are "fine" but at least they return uninit data.
33+
13
34+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
2+
--> $DIR/return_pointer_aliasing.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
8+
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9+
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
10+
--> $DIR/return_pointer_aliasing.rs:LL:CC
11+
|
12+
LL | / mir! {
13+
LL | | {
14+
LL | | let x = 0;
15+
LL | | let ptr = &raw mut x;
16+
... |
17+
LL | | }
18+
LL | | }
19+
| |_____^
20+
help: <TAG> is this argument
21+
--> $DIR/return_pointer_aliasing.rs:LL:CC
22+
|
23+
LL | unsafe { ptr.read() };
24+
| ^^^^^^^^^^^^^^^^^^^^^
25+
= note: BACKTRACE (of the first span):
26+
= note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC
27+
note: inside `main`
28+
--> $DIR/return_pointer_aliasing.rs:LL:CC
29+
|
30+
LL | Call(*ptr, after_call, myfun(ptr))
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
= 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)
33+
34+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
35+
36+
error: aborting due to previous error
37+
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: Undefined Behavior: read access through <TAG> (root of the allocation) is forbidden
2+
--> $DIR/return_pointer_aliasing.rs:LL:CC
3+
|
4+
LL | unsafe { ptr.read() };
5+
| ^^^^^^^^^^ read access through <TAG> (root of the allocation) is forbidden
6+
|
7+
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
8+
= help: the accessed tag <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child)
9+
= help: this foreign read access would cause the protected tag <TAG> to transition from Active to Frozen
10+
= help: this transition would be a loss of write permissions, which is not allowed for protected tags
11+
help: the accessed tag <TAG> was created here
12+
--> $DIR/return_pointer_aliasing.rs:LL:CC
13+
|
14+
LL | / mir! {
15+
LL | | {
16+
LL | | let x = 0;
17+
LL | | let ptr = &raw mut x;
18+
... |
19+
LL | | }
20+
LL | | }
21+
| |_____^
22+
help: the protected tag <TAG> was created here, in the initial state Active
23+
--> $DIR/return_pointer_aliasing.rs:LL:CC
24+
|
25+
LL | unsafe { ptr.read() };
26+
| ^^^^^^^^^^^^^^^^^^^^^
27+
= note: BACKTRACE (of the first span):
28+
= note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC
29+
note: inside `main`
30+
--> $DIR/return_pointer_aliasing.rs:LL:CC
31+
|
32+
LL | Call(*ptr, after_call, myfun(ptr))
33+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
34+
= 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)
35+
36+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
37+
38+
error: aborting due to previous error
39+

‎src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
// should find the bug even without validation and stacked borrows, but gets masked by optimizations
2-
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 -Cdebug-assertions=no
1+
// should find the bug even without, but gets masked by optimizations
2+
//@compile-flags: -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 -Cdebug-assertions=no
3+
//@normalize-stderr-test: "but found [0-9]+" -> "but found $$ALIGN"
34

45
#[repr(align(256))]
56
#[derive(Debug)]
@@ -19,6 +20,6 @@ fn main() {
1920
(&mut ptr as *mut _ as *mut *const u8).write(&buf as *const _ as *const u8);
2021
}
2122
// Re-borrow that. This should be UB.
22-
let _ptr = &*ptr; //~ERROR: alignment 256 is required
23+
let _ptr = &*ptr; //~ERROR: required 256 byte alignment
2324
}
2425
}

‎src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
1+
error: Undefined Behavior: constructing invalid value: encountered an unaligned reference (required 256 byte alignment but found $ALIGN)
22
--> $DIR/dyn_alignment.rs:LL:CC
33
|
44
LL | let _ptr = &*ptr;
5-
| ^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required
5+
| ^^^^^ constructing invalid value: encountered an unaligned reference (required 256 byte alignment but found $ALIGN)
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![feature(raw_ref_op)]
2+
#![feature(core_intrinsics)]
3+
#![feature(custom_mir)]
4+
5+
use std::intrinsics::mir::*;
6+
7+
// Make sure calls with the return place "on the heap" work.
8+
#[custom_mir(dialect = "runtime", phase = "optimized")]
9+
pub fn main() {
10+
mir! {
11+
{
12+
let x = 0;
13+
let ptr = &raw mut x;
14+
Call(*ptr, after_call, myfun())
15+
}
16+
17+
after_call = {
18+
Return()
19+
}
20+
}
21+
}
22+
23+
fn myfun() -> i32 {
24+
13
25+
}

0 commit comments

Comments
 (0)
Please sign in to comment.