From 1a3bda648747e2d332f58988a12bbed1aefd4cda Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 13 Apr 2020 16:06:51 +0200 Subject: [PATCH 1/3] Miri: let push_frame hook also access and mutate the rest of the frame data --- src/librustc_mir/const_eval/machine.rs | 11 +++++--- src/librustc_mir/interpret/eval_context.rs | 31 +++++++++++++++++----- src/librustc_mir/interpret/machine.rs | 13 +++++---- src/librustc_mir/transform/const_prop.rs | 7 +++-- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index e926347147894..ac9ef803d3b6a 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -13,8 +13,8 @@ use rustc_middle::mir::AssertMessage; use rustc_span::symbol::Symbol; use crate::interpret::{ - self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy, - PlaceTy, Pointer, Scalar, + self, AllocId, Allocation, Frame, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, + OpTy, PlaceTy, Pointer, Scalar, }; use super::error::*; @@ -339,8 +339,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { } #[inline(always)] - fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { - Ok(()) + fn init_frame_extra( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + frame: Frame<'mir, 'tcx>, + ) -> InterpResult<'tcx, Frame<'mir, 'tcx>> { + Ok(frame) } fn before_access_global( diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index e0b5f634bf3df..a0c6240a8a0ae 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -159,6 +159,21 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> { } } +impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> { + pub fn with_extra(self, extra: Extra) -> Frame<'mir, 'tcx, Tag, Extra> { + Frame { + body: self.body, + instance: self.instance, + return_to_block: self.return_to_block, + return_place: self.return_place, + locals: self.locals, + block: self.block, + stmt: self.stmt, + extra, + } + } +} + impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> { /// Return the `SourceInfo` of the current instruction. pub fn current_source_info(&self) -> Option { @@ -586,8 +601,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ::log_settings::settings().indentation += 1; // first push a stack frame so we have access to the local substs - let extra = M::stack_push(self)?; - self.stack.push(Frame { + let pre_frame = Frame { body, block: Some(mir::START_BLOCK), return_to_block, @@ -597,8 +611,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { locals: IndexVec::new(), instance, stmt: 0, - extra, - }); + extra: (), + }; + let frame = M::init_frame_extra(self, pre_frame)?; + self.stack.push(frame); // don't allocate at all for trivial constants if body.local_decls.len() > 1 { @@ -725,11 +741,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Cleanup: deallocate all locals that are backed by an allocation. - for local in frame.locals { + for local in &frame.locals { self.deallocate_local(local.value)?; } - if M::stack_pop(self, frame.extra, unwinding)? == StackPopJump::NoJump { + let return_place = frame.return_place; + if M::after_stack_pop(self, frame, unwinding)? == StackPopJump::NoJump { // The hook already did everything. // We want to skip the `info!` below, hence early return. return Ok(()); @@ -743,7 +760,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Follow the normal return edge. // Validate the return value. Do this after deallocating so that we catch dangling // references. - if let Some(return_place) = frame.return_place { + if let Some(return_place) = return_place { if M::enforce_validity(self) { // Data got changed, better make sure it matches the type! // It is still possible that the return place held invalid data while diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index fd67b088c93cf..ffda0334a508f 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -279,13 +279,16 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } - /// Called immediately before a new stack frame got pushed. - fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, Self::FrameExtra>; + /// Called immediately before a new stack frame gets pushed. + fn init_frame_extra( + ecx: &mut InterpCx<'mir, 'tcx, Self>, + frame: Frame<'mir, 'tcx, Self::PointerTag>, + ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>; - /// Called immediately after a stack frame gets popped - fn stack_pop( + /// Called immediately after a stack frame got popped, but before jumping back to the caller. + fn after_stack_pop( _ecx: &mut InterpCx<'mir, 'tcx, Self>, - _extra: Self::FrameExtra, + _frame: Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>, _unwinding: bool, ) -> InterpResult<'tcx, StackPopJump> { // By default, we do not support unwinding from panics diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 5a00f206a7646..ed5c2543ec6bb 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -287,8 +287,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { } #[inline(always)] - fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { - Ok(()) + fn init_frame_extra( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + frame: Frame<'mir, 'tcx>, + ) -> InterpResult<'tcx, Frame<'mir, 'tcx>> { + Ok(frame) } } From 7b90ff9ae68cc23fe002c92f30a4aaf2039c5a8f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 13 Apr 2020 17:07:54 +0200 Subject: [PATCH 2/3] add after_stack_push hook; add public ImmTy::from_immediate method, and make ImmTy::imm field private --- src/librustc_mir/const_eval/eval_queries.rs | 40 +++++++++++---------- src/librustc_mir/interpret/eval_context.rs | 3 +- src/librustc_mir/interpret/machine.rs | 5 +++ src/librustc_mir/interpret/operand.rs | 11 ++++-- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 10 +++--- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs index 97cdb32e2cdf7..33f8397887336 100644 --- a/src/librustc_mir/const_eval/eval_queries.rs +++ b/src/librustc_mir/const_eval/eval_queries.rs @@ -1,7 +1,7 @@ use super::{error_to_const_error, CompileTimeEvalContext, CompileTimeInterpreter, MemoryExtra}; use crate::interpret::eval_nullary_intrinsic; use crate::interpret::{ - intern_const_alloc_recursive, Allocation, ConstValue, GlobalId, ImmTy, Immediate, InternKind, + intern_const_alloc_recursive, Allocation, ConstValue, GlobalId, Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RawConst, RefTracking, Scalar, ScalarMaybeUndef, StackPopCleanup, }; @@ -147,24 +147,26 @@ pub(super) fn op_to_const<'tcx>( match immediate { Ok(mplace) => to_const_value(mplace), // see comment on `let try_as_immediate` above - Err(ImmTy { imm: Immediate::Scalar(x), .. }) => match x { - ScalarMaybeUndef::Scalar(s) => ConstValue::Scalar(s), - ScalarMaybeUndef::Undef => to_const_value(op.assert_mem_place(ecx)), - }, - Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => { - let (data, start) = match a.not_undef().unwrap() { - Scalar::Ptr(ptr) => { - (ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), ptr.offset.bytes()) - } - Scalar::Raw { .. } => ( - ecx.tcx.intern_const_alloc(Allocation::from_byte_aligned_bytes(b"" as &[u8])), - 0, - ), - }; - let len = b.to_machine_usize(&ecx.tcx.tcx).unwrap(); - let start = start.try_into().unwrap(); - let len: usize = len.try_into().unwrap(); - ConstValue::Slice { data, start, end: start + len } + Err(imm) => match *imm { + Immediate::Scalar(x) => match x { + ScalarMaybeUndef::Scalar(s) => ConstValue::Scalar(s), + ScalarMaybeUndef::Undef => to_const_value(op.assert_mem_place(ecx)), + }, + Immediate::ScalarPair(a, b) => { + let (data, start) = match a.not_undef().unwrap() { + Scalar::Ptr(ptr) => { + (ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), ptr.offset.bytes()) + } + Scalar::Raw { .. } => ( + ecx.tcx.intern_const_alloc(Allocation::from_byte_aligned_bytes(b"" as &[u8])), + 0, + ), + }; + let len = b.to_machine_usize(&ecx.tcx.tcx).unwrap(); + let start = start.try_into().unwrap(); + let len: usize = len.try_into().unwrap(); + ConstValue::Slice { data, start, end: start + len } + } } } } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index a0c6240a8a0ae..f111eecb9450e 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -646,6 +646,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.frame_mut().locals = locals; } + M::after_stack_push(self)?; info!("ENTERING({}) {}", self.cur_frame(), self.frame().instance); if self.stack.len() > *self.tcx.sess.recursion_limit.get() { @@ -751,7 +752,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We want to skip the `info!` below, hence early return. return Ok(()); } - // Normal return. + // Normal return, figure out where to jump. if unwinding { // Follow the unwind edge. let unwind = next_block.expect("Encountered StackPopCleanup::None when unwinding!"); diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index ffda0334a508f..3c7cb73610ea6 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -285,6 +285,11 @@ pub trait Machine<'mir, 'tcx>: Sized { frame: Frame<'mir, 'tcx, Self::PointerTag>, ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>; + /// Called immediately after a stack frame got pushed and its locals got initialized. + fn after_stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + Ok(()) + } + /// Called immediately after a stack frame got popped, but before jumping back to the caller. fn after_stack_pop( _ecx: &mut InterpCx<'mir, 'tcx, Self>, diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3741f31927e94..893f4c1db7e0a 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -87,7 +87,7 @@ impl<'tcx, Tag> Immediate { // as input for binary and cast operations. #[derive(Copy, Clone, Debug)] pub struct ImmTy<'tcx, Tag = ()> { - pub(crate) imm: Immediate, + imm: Immediate, pub layout: TyAndLayout<'tcx>, } @@ -183,6 +183,11 @@ impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> { ImmTy { imm: val.into(), layout } } + #[inline] + pub fn from_immediate(imm: Immediate, layout: TyAndLayout<'tcx>) -> Self { + ImmTy { imm, layout } + } + #[inline] pub fn try_from_uint(i: impl Into, layout: TyAndLayout<'tcx>) -> Option { Some(Self::from_scalar(Scalar::try_from_uint(i, layout.size)?, layout)) @@ -424,7 +429,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(OpTy { op, layout }) } - /// Every place can be read from, so we can turn them into an operand + /// Every place can be read from, so we can turn them into an operand. + /// This will definitely return `Indirect` if the place is a `Ptr`, i.e., this + /// will never actually read from memory. #[inline(always)] pub fn place_to_op( &self, diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 828df9a0930f5..9ac4b3551fc43 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -247,7 +247,7 @@ impl<'tcx, Tag: ::std::fmt::Debug + Copy> OpTy<'tcx, Tag> { Operand::Immediate(_) if self.layout.is_zst() => { Ok(MPlaceTy::dangling(self.layout, cx)) } - Operand::Immediate(imm) => Err(ImmTy { imm, layout: self.layout }), + Operand::Immediate(imm) => Err(ImmTy::from_immediate(imm, self.layout)), } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 2d8551b2bbf1e..49fee1bddcb6d 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -407,7 +407,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let this_receiver_ptr = self.layout_of(receiver_ptr_ty)?.field(self, 0)?; // Adjust receiver argument. args[0] = - OpTy::from(ImmTy { layout: this_receiver_ptr, imm: receiver_place.ptr.into() }); + OpTy::from(ImmTy::from_immediate(receiver_place.ptr.into(), this_receiver_ptr)); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function self.eval_fn_call(drop_fn, caller_abi, &args, ret, unwind) @@ -436,10 +436,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => (instance, place), }; - let arg = ImmTy { - imm: place.to_ref(), - layout: self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, - }; + let arg = ImmTy::from_immediate( + place.to_ref(), + self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, + ); let ty = self.tcx.mk_unit(); // return type is () let dest = MPlaceTy::dangling(self.layout_of(ty)?, self); From f2d4c93c6c56d8cfe1d69646ba1f788823e6bc35 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 13 Apr 2020 18:05:05 +0200 Subject: [PATCH 3/3] fmt --- src/librustc_mir/const_eval/eval_queries.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs index 33f8397887336..3f0774767fd7a 100644 --- a/src/librustc_mir/const_eval/eval_queries.rs +++ b/src/librustc_mir/const_eval/eval_queries.rs @@ -158,7 +158,8 @@ pub(super) fn op_to_const<'tcx>( (ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), ptr.offset.bytes()) } Scalar::Raw { .. } => ( - ecx.tcx.intern_const_alloc(Allocation::from_byte_aligned_bytes(b"" as &[u8])), + ecx.tcx + .intern_const_alloc(Allocation::from_byte_aligned_bytes(b"" as &[u8])), 0, ), }; @@ -167,7 +168,7 @@ pub(super) fn op_to_const<'tcx>( let len: usize = len.try_into().unwrap(); ConstValue::Slice { data, start, end: start + len } } - } + }, } }