Skip to content

interpret: avoid a long-lived PlaceTy in stack frames #121985

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

Merged
merged 2 commits into from
Mar 7, 2024
Merged
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
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ use crate::errors::{LongRunning, LongRunningWarn};
use crate::fluent_generated as fluent;
use crate::interpret::{
self, compile_time_machine, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, FnVal,
Frame, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, PointerArithmetic, Scalar,
Frame, ImmTy, InterpCx, InterpResult, MPlaceTy, OpTy, Pointer, PointerArithmetic, Scalar,
};

use super::error::*;
@@ -219,7 +219,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
&mut self,
instance: ty::Instance<'tcx>,
args: &[FnArg<'tcx>],
dest: &PlaceTy<'tcx>,
dest: &MPlaceTy<'tcx>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
let def_id = instance.def_id();
@@ -280,7 +280,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
&mut self,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
dest: &PlaceTy<'tcx>,
dest: &MPlaceTy<'tcx>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx, ControlFlow<()>> {
assert_eq!(args.len(), 2);
@@ -410,7 +410,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
orig_instance: ty::Instance<'tcx>,
_abi: CallAbi,
args: &[FnArg<'tcx>],
dest: &PlaceTy<'tcx>,
dest: &MPlaceTy<'tcx>,
ret: Option<mir::BasicBlock>,
_unwind: mir::UnwindAction, // unwinding is not supported in consts
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
@@ -455,7 +455,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
ecx: &mut InterpCx<'mir, 'tcx, Self>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
dest: &PlaceTy<'tcx, Self::Provenance>,
dest: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
_unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = CtfeProvenance, Extra = ()> {

/// The location where the result of the current stack frame should be written to,
/// and its layout in the caller.
pub return_place: PlaceTy<'tcx, Prov>,
pub return_place: MPlaceTy<'tcx, Prov>,

/// The list of locals for this stack frame, stored in order as
/// `[return_ptr, arguments..., variables..., temporaries...]`.
@@ -771,7 +771,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&mut self,
instance: ty::Instance<'tcx>,
body: &'mir mir::Body<'tcx>,
return_place: &PlaceTy<'tcx, M::Provenance>,
return_place: &MPlaceTy<'tcx, M::Provenance>,
return_to_block: StackPopCleanup,
) -> InterpResult<'tcx> {
trace!("body: {:#?}", body);
@@ -912,7 +912,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
self.copy_op_allow_transmute(&op, &dest)
};
trace!("return value: {:?}", self.dump_place(&dest));
trace!("return value: {:?}", self.dump_place(&dest.into()));
// We delay actually short-circuiting on this error until *after* the stack frame is
// popped, since we want this error to be attributed to the caller, whose type defines
// this transmute.
12 changes: 6 additions & 6 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::Size;

use super::{
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, MPlaceTy, Machine, OpTy,
Pointer,
};

@@ -104,7 +104,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&mut self,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, M::Provenance>],
dest: &PlaceTy<'tcx, M::Provenance>,
dest: &MPlaceTy<'tcx, M::Provenance>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx, bool> {
let instance_args = instance.args;
@@ -377,7 +377,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let index = u64::from(self.read_scalar(&args[1])?.to_u32()?);
let elem = &args[2];
let (input, input_len) = self.operand_to_simd(&args[0])?;
let (dest, dest_len) = self.place_to_simd(dest)?;
let (dest, dest_len) = self.mplace_to_simd(dest)?;
assert_eq!(input_len, dest_len, "Return vector length must match input length");
// Bounds are not checked by typeck so we have to do it ourselves.
if index >= input_len {
@@ -430,7 +430,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
_ => return Ok(false),
}

trace!("{:?}", self.dump_place(dest));
trace!("{:?}", self.dump_place(&dest.clone().into()));
self.go_to_block(ret);
Ok(true)
}
@@ -488,7 +488,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&mut self,
a: &ImmTy<'tcx, M::Provenance>,
b: &ImmTy<'tcx, M::Provenance>,
dest: &PlaceTy<'tcx, M::Provenance>,
dest: &MPlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
assert_eq!(a.layout.ty, b.layout.ty);
assert!(matches!(a.layout.ty.kind(), ty::Int(..) | ty::Uint(..)));
@@ -506,7 +506,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
)
}
// `Rem` says this is all right, so we can let `Div` do its job.
self.binop_ignore_overflow(BinOp::Div, a, b, dest)
self.binop_ignore_overflow(BinOp::Div, a, b, &dest.clone().into())
}

pub fn saturating_arith(
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
@@ -196,7 +196,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
instance: ty::Instance<'tcx>,
abi: CallAbi,
args: &[FnArg<'tcx, Self::Provenance>],
destination: &PlaceTy<'tcx, Self::Provenance>,
destination: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>>;
@@ -208,7 +208,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
fn_val: Self::ExtraFnVal,
abi: CallAbi,
args: &[FnArg<'tcx, Self::Provenance>],
destination: &PlaceTy<'tcx, Self::Provenance>,
destination: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx>;
@@ -219,7 +219,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
ecx: &mut InterpCx<'mir, 'tcx, Self>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Self::Provenance>],
destination: &PlaceTy<'tcx, Self::Provenance>,
destination: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx>;
@@ -584,7 +584,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
fn_val: !,
_abi: CallAbi,
_args: &[FnArg<$tcx>],
_destination: &PlaceTy<$tcx, Self::Provenance>,
_destination: &MPlaceTy<$tcx, Self::Provenance>,
_target: Option<mir::BasicBlock>,
_unwind: mir::UnwindAction,
) -> InterpResult<$tcx> {
16 changes: 6 additions & 10 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
@@ -194,6 +194,12 @@ pub(super) enum Place<Prov: Provenance = CtfeProvenance> {
Local { frame: usize, local: mir::Local, offset: Option<Size> },
}

/// An evaluated place, together with its type.
///
/// This may reference a stack frame by its index, so `PlaceTy` should generally not be kept around
/// for longer than a single operation. Popping and then pushing a stack frame can make `PlaceTy`
/// point to the wrong destination. If the interpreter has multiple stacks, stack switching will
/// also invalidate a `PlaceTy`.
#[derive(Clone)]
pub struct PlaceTy<'tcx, Prov: Provenance = CtfeProvenance> {
place: Place<Prov>, // Keep this private; it helps enforce invariants.
@@ -494,16 +500,6 @@ where
Ok((mplace, len))
}

/// Converts a repr(simd) place into a place where `place_index` accesses the SIMD elements.
/// Also returns the number of elements.
pub fn place_to_simd(
&mut self,
place: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::Provenance>, u64)> {
let mplace = self.force_allocation(place)?;
self.mplace_to_simd(&mplace)
}

pub fn local_to_place(
&self,
frame: usize,
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
@@ -153,7 +153,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
),
};

let destination = self.eval_place(destination)?;
let destination = self.force_allocation(&self.eval_place(destination)?)?;
self.eval_fn_call(
fn_val,
(fn_sig.abi, fn_abi),
@@ -503,7 +503,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(caller_abi, caller_fn_abi): (Abi, &FnAbi<'tcx, Ty<'tcx>>),
args: &[FnArg<'tcx, M::Provenance>],
with_caller_location: bool,
destination: &PlaceTy<'tcx, M::Provenance>,
destination: &MPlaceTy<'tcx, M::Provenance>,
target: Option<mir::BasicBlock>,
mut unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
@@ -732,7 +732,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
});
}
// Protect return place for in-place return value passing.
M::protect_in_place_function_argument(self, destination)?;
M::protect_in_place_function_argument(self, &destination.clone().into())?;

// Don't forget to mark "initially live" locals as live.
self.storage_live_for_always_live_locals()?;
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/dataflow_const_prop.rs
Original file line number Diff line number Diff line change
@@ -929,7 +929,7 @@ impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for Dumm
_instance: ty::Instance<'tcx>,
_abi: rustc_target::spec::abi::Abi,
_args: &[rustc_const_eval::interpret::FnArg<'tcx, Self::Provenance>],
_destination: &rustc_const_eval::interpret::PlaceTy<'tcx, Self::Provenance>,
_destination: &rustc_const_eval::interpret::MPlaceTy<'tcx, Self::Provenance>,
_target: Option<BasicBlock>,
_unwind: UnwindAction,
) -> interpret::InterpResult<'tcx, Option<(&'mir Body<'tcx>, ty::Instance<'tcx>)>> {
@@ -947,7 +947,7 @@ impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for Dumm
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_instance: ty::Instance<'tcx>,
_args: &[rustc_const_eval::interpret::OpTy<'tcx, Self::Provenance>],
_destination: &rustc_const_eval::interpret::PlaceTy<'tcx, Self::Provenance>,
_destination: &rustc_const_eval::interpret::MPlaceTy<'tcx, Self::Provenance>,
_target: Option<BasicBlock>,
_unwind: UnwindAction,
) -> interpret::InterpResult<'tcx> {
2 changes: 1 addition & 1 deletion src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
@@ -381,7 +381,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
f: ty::Instance<'tcx>,
caller_abi: Abi,
args: &[Immediate<Provenance>],
dest: Option<&PlaceTy<'tcx, Provenance>>,
dest: Option<&MPlaceTy<'tcx, Provenance>>,
stack_pop: StackPopCleanup,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
6 changes: 3 additions & 3 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
@@ -950,7 +950,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
instance: ty::Instance<'tcx>,
abi: Abi,
args: &[FnArg<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
ret: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
@@ -977,7 +977,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
fn_val: DynSym,
abi: Abi,
args: &[FnArg<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
ret: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
@@ -990,7 +990,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
ecx: &mut MiriInterpCx<'mir, 'tcx>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
ret: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
21 changes: 10 additions & 11 deletions src/tools/miri/src/shims/backtrace.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
abi: Abi,
link_name: Symbol,
args: &[OpTy<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let [flags] = this.check_shim(abi, Abi::Rust, link_name, args)?;
@@ -32,7 +32,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
abi: Abi,
link_name: Symbol,
args: &[OpTy<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let tcx = this.tcx;
@@ -145,7 +145,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
abi: Abi,
link_name: Symbol,
args: &[OpTy<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let [ptr, flags] = this.check_shim(abi, Abi::Rust, link_name, args)?;
@@ -174,7 +174,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// `lo.col` is 0-based - add 1 to make it 1-based for the caller.
let colno: u32 = u32::try_from(lo.col.0.saturating_add(1)).unwrap_or(0);

let dest = this.force_allocation(dest)?;
if let ty::Adt(adt, _) = dest.layout.ty.kind() {
if !adt.repr().c() {
throw_ub_format!(
@@ -191,29 +190,29 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let filename_alloc =
this.allocate_str(&filename, MiriMemoryKind::Rust.into(), Mutability::Mut)?;

this.write_immediate(name_alloc.to_ref(this), &this.project_field(&dest, 0)?)?;
this.write_immediate(filename_alloc.to_ref(this), &this.project_field(&dest, 1)?)?;
this.write_immediate(name_alloc.to_ref(this), &this.project_field(dest, 0)?)?;
this.write_immediate(filename_alloc.to_ref(this), &this.project_field(dest, 1)?)?;
}
1 => {
this.write_scalar(
Scalar::from_target_usize(name.len().try_into().unwrap(), this),
&this.project_field(&dest, 0)?,
&this.project_field(dest, 0)?,
)?;
this.write_scalar(
Scalar::from_target_usize(filename.len().try_into().unwrap(), this),
&this.project_field(&dest, 1)?,
&this.project_field(dest, 1)?,
)?;
}
_ => throw_unsup_format!("unknown `miri_resolve_frame` flags {}", flags),
}

this.write_scalar(Scalar::from_u32(lineno), &this.project_field(&dest, 2)?)?;
this.write_scalar(Scalar::from_u32(colno), &this.project_field(&dest, 3)?)?;
this.write_scalar(Scalar::from_u32(lineno), &this.project_field(dest, 2)?)?;
this.write_scalar(Scalar::from_u32(colno), &this.project_field(dest, 3)?)?;

// Support a 4-field struct for now - this is deprecated
// and slated for removal.
if num_fields == 5 {
this.write_pointer(fn_ptr, &this.project_field(&dest, 4)?)?;
this.write_pointer(fn_ptr, &this.project_field(dest, 4)?)?;
}

Ok(())
4 changes: 2 additions & 2 deletions src/tools/miri/src/shims/ffi_support.rs
Original file line number Diff line number Diff line change
@@ -70,7 +70,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn call_external_c_and_store_return<'a>(
&mut self,
link_name: Symbol,
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
ptr: CodePtr,
libffi_args: Vec<libffi::high::Arg<'a>>,
) -> InterpResult<'tcx, ()> {
@@ -205,7 +205,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn call_external_c_fct(
&mut self,
link_name: Symbol,
dest: &PlaceTy<'tcx, Provenance>,
dest: &MPlaceTy<'tcx, Provenance>,
args: &[OpTy<'tcx, Provenance>],
) -> InterpResult<'tcx, bool> {
// Get the pointer to the function in the shared object file if it exists.
Loading