Rollup merge of #115536 - RalfJung:interpreter-privacy, r=oli-obk

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

Outside the interpreter, only the typed versions should be used.

Author: matthiaskrgr

src/borrow_tracker/stacked_borrows/mod.rs

@@ -615,7 +615,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
     ) -> InterpResult<'tcx, Option<Provenance>> {
         let this = self.eval_context_mut();
         // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
-        this.check_ptr_access_align(place.ptr, size, Align::ONE, CheckInAllocMsg::InboundsTest)?;
+        this.check_ptr_access_align(place.ptr(), size, Align::ONE, CheckInAllocMsg::InboundsTest)?;
 
         // It is crucial that this gets called on all code paths, to ensure we track tag creation.
         let log_creation = |this: &MiriInterpCx<'mir, 'tcx>,
@@ -682,7 +682,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
             trace!(
                 "reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
                 new_tag,
-                place.ptr,
+                place.ptr(),
                 place.layout.ty,
             );
             // Don't update any stacks for a zero-sized access; borrow stacks are per-byte and this
@@ -692,19 +692,19 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
             // attempt to use it for a non-zero-sized access.
             // Dangling slices are a common case here; it's valid to get their length but with raw
             // pointer tagging for example all calls to get_unchecked on them are invalid.
-            if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr) {
+            if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr()) {
                 log_creation(this, Some((alloc_id, base_offset, orig_tag)))?;
                 // Still give it the new provenance, it got retagged after all.
                 return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag }));
             } else {
                 // This pointer doesn't come with an AllocId. :shrug:
                 log_creation(this, None)?;
                 // Provenance unchanged.
-                return Ok(place.ptr.provenance);
+                return Ok(place.ptr().provenance);
             }
         }
 
-        let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?;
+        let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr())?;
         log_creation(this, Some((alloc_id, base_offset, orig_tag)))?;
 
         trace!(

src/borrow_tracker/tree_borrows/mod.rs

@@ -172,7 +172,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
         let this = self.eval_context_mut();
         // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
         this.check_ptr_access_align(
-            place.ptr,
+            place.ptr(),
             ptr_size,
             Align::ONE,
             CheckInAllocMsg::InboundsTest,
@@ -197,7 +197,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
         };
 
         trace!("Reborrow of size {:?}", ptr_size);
-        let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr) {
+        let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr()) {
             Ok(data) => {
                 // Unlike SB, we *do* a proper retag for size 0 if can identify the allocation.
                 // After all, the pointer may be lazily initialized outside this initial range.
@@ -210,18 +210,18 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
                 trace!(
                     "reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
                     new_tag,
-                    place.ptr,
+                    place.ptr(),
                     place.layout.ty,
                 );
                 log_creation(this, None)?;
                 // Keep original provenance.
-                return Ok(place.ptr.provenance);
+                return Ok(place.ptr().provenance);
             }
         };
         log_creation(this, Some((alloc_id, base_offset, parent_prov)))?;
 
         let orig_tag = match parent_prov {
-            ProvenanceExtra::Wildcard => return Ok(place.ptr.provenance), // TODO: handle wildcard pointers
+            ProvenanceExtra::Wildcard => return Ok(place.ptr().provenance), // TODO: handle wildcard pointers
             ProvenanceExtra::Concrete(tag) => tag,
         };
 

src/concurrency/data_race.rs

@@ -1018,7 +1018,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         // be 8-aligned).
         let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
         this.check_ptr_access_align(
-            place.ptr,
+            place.ptr(),
             place.layout.size,
             align,
             CheckInAllocMsg::MemoryAccessTest,
@@ -1031,7 +1031,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         // We avoid `get_ptr_alloc` since we do *not* want to run the access hooks -- the actual
         // access will happen later.
         let (alloc_id, _offset, _prov) =
-            this.ptr_try_get_alloc_id(place.ptr).expect("there are no zero-sized atomic accesses");
+            this.ptr_try_get_alloc_id(place.ptr()).expect("there are no zero-sized atomic accesses");
         if this.get_alloc_mutability(alloc_id)? == Mutability::Not {
             // FIXME: make this prettier, once these messages have separate title/span/help messages.
             throw_ub_format!(
@@ -1135,15 +1135,15 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         if let Some(data_race) = &this.machine.data_race {
             if data_race.race_detecting() {
                 let size = place.layout.size;
-                let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr)?;
+                let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr())?;
                 // Load and log the atomic operation.
                 // Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option.
                 let alloc_meta = this.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();
                 log::trace!(
                     "Atomic op({}) with ordering {:?} on {:?} (size={})",
                     description,
                     &atomic,
-                    place.ptr,
+                    place.ptr(),
                     size.bytes()
                 );
 
@@ -1186,7 +1186,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
                     {
                         log::trace!(
                             "Updated atomic memory({:?}, size={}) to {:#?}",
-                            place.ptr,
+                            place.ptr(),
                             size.bytes(),
                             mem_clocks.atomic_ops
                         );

src/concurrency/thread.rs

@@ -8,6 +8,7 @@ use std::task::Poll;
 use std::time::{Duration, SystemTime};
 
 use log::trace;
+use either::Either;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def_id::DefId;
@@ -259,8 +260,15 @@ impl VisitTags for Frame<'_, '_, Provenance, FrameExtra<'_>> {
         return_place.visit_tags(visit);
         // Locals.
         for local in locals.iter() {
-            if let LocalValue::Live(value) = &local.value {
-                value.visit_tags(visit);
+            match local.as_mplace_or_imm() {
+                None => {}
+                Some(Either::Left((ptr, meta))) => {
+                    ptr.visit_tags(visit);
+                    meta.visit_tags(visit);
+                }
+                Some(Either::Right(imm)) => {
+                    imm.visit_tags(visit);
+                }
             }
         }
 

src/concurrency/weak_memory.rs

@@ -481,7 +481,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
         place: &MPlaceTy<'tcx, Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_ref();
-        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
+        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
         if let crate::AllocExtra {
             weak_memory: Some(alloc_buffers),
             data_race: Some(alloc_clocks),
@@ -512,7 +512,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
         init: Scalar<Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
+        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
         if let (
             crate::AllocExtra { weak_memory: Some(alloc_buffers), .. },
             crate::MiriMachine { data_race: Some(global), threads, .. },
@@ -539,7 +539,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
     ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_ref();
         if let Some(global) = &this.machine.data_race {
-            let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
+            let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
             if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() {
                 if atomic == AtomicReadOrd::SeqCst {
                     global.sc_read(&this.machine.threads);
@@ -577,7 +577,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
         init: Scalar<Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr)?;
+        let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr())?;
         if let (
             crate::AllocExtra { weak_memory: Some(alloc_buffers), .. },
             crate::MiriMachine { data_race: Some(global), threads, .. },
@@ -627,7 +627,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
                 global.sc_read(&this.machine.threads);
             }
             let size = place.layout.size;
-            let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
+            let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
             if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() {
                 let buffer = alloc_buffers
                     .get_or_create_store_buffer(alloc_range(base_offset, size), init)?;

src/diagnostics.rs

@@ -376,9 +376,9 @@ pub fn report_error<'tcx, 'mir>(
     for (i, frame) in ecx.active_thread_stack().iter().enumerate() {
         trace!("-------------------");
         trace!("Frame {}", i);
-        trace!("    return: {:?}", *frame.return_place);
+        trace!("    return: {:?}", frame.return_place);
         for (i, local) in frame.locals.iter().enumerate() {
-            trace!("    local {}: {:?}", i, local.value);
+            trace!("    local {}: {:?}", i, local);
         }
     }
 

src/eval.rs

@@ -242,10 +242,7 @@ impl MainThreadState {
                 },
             Done => {
                 // Figure out exit code.
-                let ret_place = MPlaceTy::from_aligned_ptr(
-                    this.machine.main_fn_ret_place.unwrap().ptr,
-                    this.machine.layouts.isize,
-                );
+                let ret_place = this.machine.main_fn_ret_place.clone().unwrap();
                 let exit_code = this.read_target_isize(&ret_place)?;
                 // Need to call this ourselves since we are not going to return to the scheduler
                 // loop, and we want the main thread TLS to not show up as memory leaks.
@@ -308,7 +305,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
             let arg_type = Ty::new_array(tcx, tcx.types.u8, size);
             let arg_place =
                 ecx.allocate(ecx.layout_of(arg_type)?, MiriMemoryKind::Machine.into())?;
-            ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr, size)?;
+            ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr(), size)?;
             ecx.mark_immutable(&arg_place);
             argvs.push(arg_place.to_ref(&ecx));
         }
@@ -332,15 +329,15 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
                 ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
             ecx.write_scalar(argc, &argc_place)?;
             ecx.mark_immutable(&argc_place);
-            ecx.machine.argc = Some(*argc_place);
+            ecx.machine.argc = Some(argc_place.ptr());
 
             let argv_place = ecx.allocate(
                 ecx.layout_of(Ty::new_imm_ptr(tcx, tcx.types.unit))?,
                 MiriMemoryKind::Machine.into(),
             )?;
             ecx.write_immediate(argv, &argv_place)?;
             ecx.mark_immutable(&argv_place);
-            ecx.machine.argv = Some(*argv_place);
+            ecx.machine.argv = Some(argv_place.ptr());
         }
         // Store command line as UTF-16 for Windows `GetCommandLineW`.
         {
@@ -351,7 +348,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
                 Ty::new_array(tcx, tcx.types.u16, u64::try_from(cmd_utf16.len()).unwrap());
             let cmd_place =
                 ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Machine.into())?;
-            ecx.machine.cmd_line = Some(*cmd_place);
+            ecx.machine.cmd_line = Some(cmd_place.ptr());
             // Store the UTF-16 string. We just allocated so we know the bounds are fine.
             for (idx, &c) in cmd_utf16.iter().enumerate() {
                 let place = ecx.project_field(&cmd_place, idx)?;
@@ -364,7 +361,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
 
     // Return place (in static memory so that it does not count as leak).
     let ret_place = ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
-    ecx.machine.main_fn_ret_place = Some(*ret_place);
+    ecx.machine.main_fn_ret_place = Some(ret_place.clone());
     // Call start function.
 
     match entry_type {

src/helpers.rs

@@ -381,7 +381,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // Store how far we proceeded into the place so far. Everything to the left of
         // this offset has already been handled, in the sense that the frozen parts
         // have had `action` called on them.
-        let start_addr = place.ptr.addr();
+        let start_addr = place.ptr().addr();
         let mut cur_addr = start_addr;
         // Called when we detected an `UnsafeCell` at the given offset and size.
         // Calls `action` and advances `cur_ptr`.
@@ -413,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             let mut visitor = UnsafeCellVisitor {
                 ecx: this,
                 unsafe_cell_action: |place| {
-                    trace!("unsafe_cell_action on {:?}", place.ptr);
+                    trace!("unsafe_cell_action on {:?}", place.ptr());
                     // We need a size to go on.
                     let unsafe_cell_size = this
                         .size_and_align_of_mplace(place)?
@@ -422,7 +422,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         .unwrap_or_else(|| place.layout.size);
                     // Now handle this `UnsafeCell`, unless it is empty.
                     if unsafe_cell_size != Size::ZERO {
-                        unsafe_cell_action(&place.ptr, unsafe_cell_size)
+                        unsafe_cell_action(&place.ptr(), unsafe_cell_size)
                     } else {
                         Ok(())
                     }
@@ -432,7 +432,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
         // The part between the end_ptr and the end of the place is also frozen.
         // So pretend there is a 0-sized `UnsafeCell` at the end.
-        unsafe_cell_action(&place.ptr.offset(size, this)?, Size::ZERO)?;
+        unsafe_cell_action(&place.ptr().offset(size, this)?, Size::ZERO)?;
         // Done!
         return Ok(());
 
@@ -994,10 +994,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     }
 
     /// Mark a machine allocation that was just created as immutable.
-    fn mark_immutable(&mut self, mplace: &MemPlace<Provenance>) {
+    fn mark_immutable(&mut self, mplace: &MPlaceTy<'tcx, Provenance>) {
         let this = self.eval_context_mut();
         // This got just allocated, so there definitely is a pointer here.
-        let provenance = mplace.ptr.into_pointer_or_addr().unwrap().provenance;
+        let provenance = mplace.ptr().into_pointer_or_addr().unwrap().provenance;
         this.alloc_mark_immutable(provenance.get_alloc_id().unwrap()).unwrap();
     }
 

src/machine.rs

@@ -396,14 +396,14 @@ pub struct MiriMachine<'mir, 'tcx> {
     pub(crate) env_vars: EnvVars<'tcx>,
 
     /// Return place of the main function.
-    pub(crate) main_fn_ret_place: Option<MemPlace<Provenance>>,
+    pub(crate) main_fn_ret_place: Option<MPlaceTy<'tcx, Provenance>>,
 
     /// Program arguments (`Option` because we can only initialize them after creating the ecx).
     /// These are *pointers* to argc/argv because macOS.
     /// We also need the full command line as one string because of Windows.
-    pub(crate) argc: Option<MemPlace<Provenance>>,
-    pub(crate) argv: Option<MemPlace<Provenance>>,
-    pub(crate) cmd_line: Option<MemPlace<Provenance>>,
+    pub(crate) argc: Option<Pointer<Option<Provenance>>>,
+    pub(crate) argv: Option<Pointer<Option<Provenance>>>,
+    pub(crate) cmd_line: Option<Pointer<Option<Provenance>>>,
 
     /// TLS state.
     pub(crate) tls: TlsData<'tcx>,
@@ -670,7 +670,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
     ) -> InterpResult<'tcx> {
         let place = this.allocate(val.layout, MiriMemoryKind::ExternStatic.into())?;
         this.write_immediate(*val, &place)?;
-        Self::add_extern_static(this, name, place.ptr);
+        Self::add_extern_static(this, name, place.ptr());
         Ok(())
     }
 
@@ -686,7 +686,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
                 Self::add_extern_static(
                     this,
                     "environ",
-                    this.machine.env_vars.environ.as_ref().unwrap().ptr,
+                    this.machine.env_vars.environ.as_ref().unwrap().ptr(),
                 );
                 // A couple zero-initialized pointer-sized extern statics.
                 // Most of them are for weak symbols, which we all set to null (indicating that the
@@ -703,7 +703,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
                 Self::add_extern_static(
                     this,
                     "environ",
-                    this.machine.env_vars.environ.as_ref().unwrap().ptr,
+                    this.machine.env_vars.environ.as_ref().unwrap().ptr(),
                 );
             }
             "android" => {
@@ -1415,7 +1415,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         local: mir::Local,
         mplace: &MPlaceTy<'tcx, Provenance>,
     ) -> InterpResult<'tcx> {
-        let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr.provenance else {
+        let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr().provenance else {
             panic!("after_local_allocated should only be called on fresh allocations");
         };
         let local_decl = &ecx.active_thread_stack()[frame].body.local_decls[local];

src/shims/backtrace.rs

@@ -89,7 +89,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 }
 
                 this.write_immediate(
-                    Immediate::new_slice(Scalar::from_maybe_pointer(alloc.ptr, this), len, this),
+                    Immediate::new_slice(Scalar::from_maybe_pointer(alloc.ptr(), this), len, this),
                     dest,
                 )?;
             }