interpret: make MemPlace, Place, Operand types private to the interpreter

Author: RalfJung

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -79,7 +79,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
     intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
     // we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway
 
-    debug!("eval_body_using_ecx done: {:?}", *ret);
+    debug!("eval_body_using_ecx done: {:?}", ret);
     Ok(ret)
 }
 
@@ -147,7 +147,7 @@ pub(super) fn op_to_const<'tcx>(
     // We know `offset` is relative to the allocation, so we can use `into_parts`.
     let to_const_value = |mplace: &MPlaceTy<'_>| {
         debug!("to_const_value(mplace: {:?})", mplace);
-        match mplace.ptr.into_parts() {
+        match mplace.ptr().into_parts() {
             (Some(alloc_id), offset) => {
                 let alloc = ecx.tcx.global_alloc(alloc_id).unwrap_memory();
                 ConstValue::ByRef { alloc, offset }
@@ -370,7 +370,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
                     inner = true;
                 }
             };
-            let alloc_id = mplace.ptr.provenance.unwrap();
+            let alloc_id = mplace.ptr().provenance.unwrap();
 
             // Validation failed, report an error. This is always a hard error.
             if let Err(error) = validation {

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -30,7 +30,7 @@ pub(crate) fn const_caller_location(
     if intern_const_alloc_recursive(&mut ecx, InternKind::Constant, &loc_place).is_err() {
         bug!("intern_const_alloc_recursive should not error in this case")
     }
-    ConstValue::Scalar(Scalar::from_maybe_pointer(loc_place.ptr, &tcx))
+    ConstValue::Scalar(Scalar::from_maybe_pointer(loc_place.ptr(), &tcx))
 }
 
 // We forbid type-level constants that contain more than `VALTREE_MAX_NODES` nodes.

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -5,7 +5,7 @@ use crate::const_eval::CanAccessStatics;
 use crate::interpret::MPlaceTy;
 use crate::interpret::{
     intern_const_alloc_recursive, ConstValue, ImmTy, Immediate, InternKind, MemPlaceMeta,
-    MemoryKind, Place, Projectable, Scalar,
+    MemoryKind, PlaceTy, Projectable, Scalar,
 };
 use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
 use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
@@ -318,7 +318,7 @@ fn valtree_into_mplace<'tcx>(
                     let len_scalar = Scalar::from_target_usize(len as u64, &tcx);
 
                     Immediate::ScalarPair(
-                        Scalar::from_maybe_pointer((*pointee_place).ptr, &tcx),
+                        Scalar::from_maybe_pointer(pointee_place.ptr(), &tcx),
                         len_scalar,
                     )
                 }
@@ -383,5 +383,5 @@ fn valtree_into_mplace<'tcx>(
 }
 
 fn dump_place<'tcx>(ecx: &CompileTimeEvalContext<'tcx, 'tcx>, place: &MPlaceTy<'tcx>) {
-    trace!("{:?}", ecx.dump_place(Place::Ptr(**place)));
+    trace!("{:?}", ecx.dump_place(&PlaceTy::from(place.clone())));
 }

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -21,8 +21,8 @@ use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayou
 
 use super::{
     AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
-    MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance,
-    Scalar, StackPopJump,
+    MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer, PointerArithmetic,
+    Projectable, Provenance, Scalar, StackPopJump,
 };
 use crate::errors::{self, ErroneousConstUsed};
 use crate::util;
@@ -155,17 +155,26 @@ pub enum StackPopCleanup {
 }
 
 /// State of a local variable including a memoized layout
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct LocalState<'tcx, Prov: Provenance = AllocId> {
-    pub value: LocalValue<Prov>,
+    value: LocalValue<Prov>,
     /// Don't modify if `Some`, this is only used to prevent computing the layout twice.
     /// Avoids computing the layout of locals that are never actually initialized.
-    pub layout: Cell<Option<TyAndLayout<'tcx>>>,
+    layout: Cell<Option<TyAndLayout<'tcx>>>,
+}
+
+impl<Prov: Provenance> std::fmt::Debug for LocalState<'_, Prov> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("LocalState")
+            .field("value", &self.value)
+            .field("ty", &self.layout.get().map(|l| l.ty))
+            .finish()
+    }
 }
 
 /// Current value of a local variable
 #[derive(Copy, Clone, Debug)] // Miri debug-prints these
-pub enum LocalValue<Prov: Provenance = AllocId> {
+pub(super) enum LocalValue<Prov: Provenance = AllocId> {
     /// This local is not currently alive, and cannot be used at all.
     Dead,
     /// A normal, live local.
@@ -176,10 +185,27 @@ pub enum LocalValue<Prov: Provenance = AllocId> {
     Live(Operand<Prov>),
 }
 
-impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> {
+impl<'tcx, Prov: Provenance> LocalState<'tcx, Prov> {
+    pub fn make_live_uninit(&mut self) {
+        self.value = LocalValue::Live(Operand::Immediate(Immediate::Uninit));
+    }
+
+    /// This is a hack because Miri needs a way to visit all the provenance in a `LocalState`
+    /// without having a layout or `TyCtxt` available, and we want to keep the `Operand` type
+    /// private.
+    pub fn as_mplace_or_imm(
+        &self,
+    ) -> Option<Either<(Pointer<Option<Prov>>, MemPlaceMeta<Prov>), Immediate<Prov>>> {
+        match self.value {
+            LocalValue::Dead => None,
+            LocalValue::Live(Operand::Indirect(mplace)) => Some(Left((mplace.ptr, mplace.meta))),
+            LocalValue::Live(Operand::Immediate(imm)) => Some(Right(imm)),
+        }
+    }
+
     /// Read the local's value or error if the local is not yet live or not live anymore.
     #[inline(always)]
-    pub fn access(&self) -> InterpResult<'tcx, &Operand<Prov>> {
+    pub(super) fn access(&self) -> InterpResult<'tcx, &Operand<Prov>> {
         match &self.value {
             LocalValue::Dead => throw_ub!(DeadLocal), // could even be "invalid program"?
             LocalValue::Live(val) => Ok(val),
@@ -189,10 +215,10 @@ impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> {
     /// Overwrite the local. If the local can be overwritten in place, return a reference
     /// to do so; otherwise return the `MemPlace` to consult instead.
     ///
-    /// Note: This may only be invoked from the `Machine::access_local_mut` hook and not from
-    /// anywhere else. You may be invalidating machine invariants if you do!
+    /// Note: Before calling this, call the `before_access_local_mut` machine hook! You may be
+    /// invalidating machine invariants otherwise!
     #[inline(always)]
-    pub fn access_mut(&mut self) -> InterpResult<'tcx, &mut Operand<Prov>> {
+    pub(super) fn access_mut(&mut self) -> InterpResult<'tcx, &mut Operand<Prov>> {
         match &mut self.value {
             LocalValue::Dead => throw_ub!(DeadLocal), // could even be "invalid program"?
             LocalValue::Live(val) => Ok(val),
@@ -694,7 +720,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         &self,
         mplace: &MPlaceTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, Option<(Size, Align)>> {
-        self.size_and_align_of(&mplace.meta, &mplace.layout)
+        self.size_and_align_of(&mplace.meta(), &mplace.layout)
     }
 
     #[instrument(skip(self, body, return_place, return_to_block), level = "debug")]
@@ -826,7 +852,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 .expect("return place should always be live");
             let dest = self.frame().return_place.clone();
             let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
-            trace!("return value: {:?}", self.dump_place(*dest));
+            trace!("return value: {:?}", self.dump_place(&dest));
             // 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.
@@ -974,7 +1000,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             // Need to allocate some memory, since `Immediate::Uninit` cannot be unsized.
             let dest_place = self.allocate_dyn(layout, MemoryKind::Stack, meta)?;
-            Operand::Indirect(*dest_place)
+            Operand::Indirect(*dest_place.mplace())
         } else {
             assert!(!meta.has_meta()); // we're dropping the metadata
             // Just make this an efficient immediate.
@@ -1068,8 +1094,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     #[must_use]
-    pub fn dump_place(&self, place: Place<M::Provenance>) -> PlacePrinter<'_, 'mir, 'tcx, M> {
-        PlacePrinter { ecx: self, place }
+    pub fn dump_place(
+        &self,
+        place: &PlaceTy<'tcx, M::Provenance>,
+    ) -> PlacePrinter<'_, 'mir, 'tcx, M> {
+        PlacePrinter { ecx: self, place: *place.place() }
     }
 
     #[must_use]

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -25,7 +25,7 @@ use rustc_ast::Mutability;
 
 use super::{
     AllocId, Allocation, ConstAllocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy,
-    ValueVisitor,
+    Projectable, ValueVisitor,
 };
 use crate::const_eval;
 use crate::errors::{DanglingPtrInFinal, UnsupportedUntypedPointer};
@@ -177,7 +177,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
             if let ty::Dynamic(_, _, ty::Dyn) =
                 tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind()
             {
-                let ptr = mplace.meta.unwrap_meta().to_pointer(&tcx)?;
+                let ptr = mplace.meta().unwrap_meta().to_pointer(&tcx)?;
                 if let Some(alloc_id) = ptr.provenance {
                     // Explicitly choose const mode here, since vtables are immutable, even
                     // if the reference of the fat pointer is mutable.
@@ -191,7 +191,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
             }
             // Check if we have encountered this pointer+layout combination before.
             // Only recurse for allocation-backed pointers.
-            if let Some(alloc_id) = mplace.ptr.provenance {
+            if let Some(alloc_id) = mplace.ptr().provenance {
                 // Compute the mode with which we intern this. Our goal here is to make as many
                 // statics as we can immutable so they can be placed in read-only memory by LLVM.
                 let ref_mode = match self.mode {
@@ -267,7 +267,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
 
                     // If there is no provenance in this allocation, it does not contain references
                     // that point to another allocation, and we can avoid the interning walk.
-                    if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr, size, align)? {
+                    if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr(), size, align)? {
                         if !alloc.has_provenance() {
                             return Ok(false);
                         }
@@ -353,7 +353,7 @@ pub fn intern_const_alloc_recursive<
         leftover_allocations,
         // The outermost allocation must exist, because we allocated it with
         // `Memory::allocate`.
-        ret.ptr.provenance.unwrap(),
+        ret.ptr().provenance.unwrap(),
         base_intern_mode,
         Some(ret.layout.ty),
     );
@@ -466,7 +466,7 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>>
     ) -> InterpResult<'tcx, ConstAllocation<'tcx>> {
         let dest = self.allocate(layout, MemoryKind::Stack)?;
         f(self, &dest.clone().into())?;
-        let mut alloc = self.memory.alloc_map.remove(&dest.ptr.provenance.unwrap()).unwrap().1;
+        let mut alloc = self.memory.alloc_map.remove(&dest.ptr().provenance.unwrap()).unwrap().1;
         alloc.mutability = Mutability::Not;
         Ok(self.tcx.mk_const_alloc(alloc))
     }

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -466,7 +466,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));
         self.go_to_block(ret);
         Ok(true)
     }

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -18,7 +18,7 @@ use crate::const_eval::CheckAlignment;
 
 use super::{
     AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
-    InterpResult, MPlaceTy, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
+    InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance, Scalar,
 };
 
 /// Data returned by Machine::stack_pop,
@@ -237,22 +237,22 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
         right: &ImmTy<'tcx, Self::Provenance>,
     ) -> InterpResult<'tcx, (Scalar<Self::Provenance>, bool, Ty<'tcx>)>;
 
-    /// Called to write the specified `local` from the `frame`.
+    /// Called before writing the specified `local` of the `frame`.
     /// Since writing a ZST is not actually accessing memory or locals, this is never invoked
     /// for ZST reads.
     ///
     /// Due to borrow checker trouble, we indicate the `frame` as an index rather than an `&mut
     /// Frame`.
-    #[inline]
-    fn access_local_mut<'a>(
-        ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-        frame: usize,
-        local: mir::Local,
-    ) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
+    #[inline(always)]
+    fn before_access_local_mut<'a>(
+        _ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
+        _frame: usize,
+        _local: mir::Local,
+    ) -> InterpResult<'tcx>
     where
         'tcx: 'mir,
     {
-        ecx.stack_mut()[frame].locals[local].access_mut()
+        Ok(())
     }
 
     /// Called before a basic block terminator is executed.

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -20,16 +20,21 @@ mod visitor;
 
 pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
-pub use self::eval_context::{Frame, FrameInfo, InterpCx, LocalState, LocalValue, StackPopCleanup};
+pub use self::eval_context::{Frame, FrameInfo, InterpCx, StackPopCleanup};
 pub use self::intern::{intern_const_alloc_recursive, InternKind};
 pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump};
 pub use self::memory::{AllocKind, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind};
-pub use self::operand::{ImmTy, Immediate, OpTy, Operand, Readable};
-pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy, Writeable};
+pub use self::operand::{ImmTy, Immediate, OpTy, Readable};
+pub use self::place::{MPlaceTy, MemPlaceMeta, PlaceTy, Writeable};
 pub use self::projection::Projectable;
 pub use self::terminator::FnArg;
 pub use self::validity::{CtfeValidationMode, RefTracking};
 pub use self::visitor::ValueVisitor;
 
+use self::{
+    operand::Operand,
+    place::{MemPlace, Place},
+};
+
 pub(crate) use self::intrinsics::eval_nullary_intrinsic;
 use eval_context::{from_known_layout, mir_assign_valid_types};