Auto merge of #1851 - RalfJung:provenance-overhaul, r=RalfJung

adjust Miri to Pointer type overhaul

This is the Miri side of rust-lang/rust#87123.

This was a lot more work than I expected... lucky enough it is also (hopefully) the last large-scale refactoring I will do.^^

Fixes #224

Author: bors

rust-version

@@ -1 +1 @@
-3982eb35cabe3a99194d768d34a92347967c3fa2
+c78ebb7bdcfc924a20fd069891ffe1364d6814e7

src/bin/miri.rs

@@ -10,6 +10,7 @@ extern crate rustc_session;
 
 use std::convert::TryFrom;
 use std::env;
+use std::num::NonZeroU64;
 use std::path::PathBuf;
 use std::rc::Rc;
 use std::str::FromStr;
@@ -412,11 +413,16 @@ fn main() {
                     }
                 }
                 arg if arg.starts_with("-Zmiri-track-alloc-id=") => {
-                    let id: u64 = match arg.strip_prefix("-Zmiri-track-alloc-id=").unwrap().parse()
+                    let id = match arg
+                        .strip_prefix("-Zmiri-track-alloc-id=")
+                        .unwrap()
+                        .parse()
+                        .ok()
+                        .and_then(NonZeroU64::new)
                     {
-                        Ok(id) => id,
-                        Err(err) =>
-                            panic!("-Zmiri-track-alloc-id requires a valid `u64` argument: {}", err),
+                        Some(id) => id,
+                        None =>
+                            panic!("-Zmiri-track-alloc-id requires a valid non-zero `u64` argument"),
                     };
                     miri_config.tracked_alloc_id = Some(miri::AllocId(id));
                 }

src/data_race.rs

@@ -73,9 +73,9 @@ use rustc_middle::{mir, ty::layout::TyAndLayout};
 use rustc_target::abi::Size;
 
 use crate::{
-    ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MemoryKind, MiriEvalContext,
-    MiriEvalContextExt, MiriMemoryKind, OpTy, Pointer, RangeMap, Scalar, ScalarMaybeUninit, Tag,
-    ThreadId, VClock, VTimestamp, VectorIdx,
+    AllocId, AllocRange, ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MemoryKind,
+    MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, OpTy, Pointer, RangeMap, Scalar,
+    ScalarMaybeUninit, Tag, ThreadId, VClock, VTimestamp, VectorIdx,
 };
 
 pub type AllocExtra = VClockAlloc;
@@ -561,7 +561,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
             if lt { &rhs } else { &old }
         };
 
-        this.allow_data_races_mut(|this| this.write_immediate_to_mplace(**new_val, place))?;
+        this.allow_data_races_mut(|this| this.write_immediate(**new_val, &(*place).into()))?;
 
         this.validate_atomic_rmw(&place, atomic)?;
 
@@ -713,18 +713,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
             Ok(())
         }
     }
-
-    fn reset_vector_clocks(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
-        let this = self.eval_context_mut();
-        if let Some(data_race) = &mut this.memory.extra.data_race {
-            if data_race.multi_threaded.get() {
-                let alloc_meta =
-                    this.memory.get_alloc_extra_mut(ptr.alloc_id)?.0.data_race.as_mut().unwrap();
-                alloc_meta.reset_clocks(ptr.offset, size);
-            }
-        }
-        Ok(())
-    }
 }
 
 /// Vector clock metadata for a logical memory allocation.
@@ -769,14 +757,6 @@ impl VClockAlloc {
         }
     }
 
-    fn reset_clocks(&mut self, offset: Size, len: Size) {
-        let alloc_ranges = self.alloc_ranges.get_mut();
-        for (_, range) in alloc_ranges.iter_mut(offset, len) {
-            // Reset the portion of the range
-            *range = MemoryCellClocks::new(0, VectorIdx::MAX_INDEX);
-        }
-    }
-
     // Find an index, if one exists where the value
     // in `l` is greater than the value in `r`.
     fn find_gt_index(l: &VClock, r: &VClock) -> Option<VectorIdx> {
@@ -820,8 +800,7 @@ impl VClockAlloc {
         range: &MemoryCellClocks,
         action: &str,
         is_atomic: bool,
-        pointer: Pointer<Tag>,
-        len: Size,
+        ptr_dbg: Pointer<AllocId>,
     ) -> InterpResult<'tcx> {
         let (current_index, current_clocks) = global.current_thread_state();
         let write_clock;
@@ -863,15 +842,12 @@ impl VClockAlloc {
 
         // Throw the data-race detection.
         throw_ub_format!(
-            "Data race detected between {} on {} and {} on {}, memory({:?},offset={},size={})\
-            \n(current vector clock = {:?}, conflicting timestamp = {:?})",
+            "Data race detected between {} on {} and {} on {} at {:?} (current vector clock = {:?}, conflicting timestamp = {:?})",
             action,
             current_thread_info,
             other_action,
             other_thread_info,
-            pointer.alloc_id,
-            pointer.offset.bytes(),
-            len.bytes(),
+            ptr_dbg,
             current_clocks.clock,
             other_clock
         )
@@ -884,17 +860,23 @@ impl VClockAlloc {
     /// atomic read operations.
     pub fn read<'tcx>(
         &self,
-        pointer: Pointer<Tag>,
-        len: Size,
+        alloc_id: AllocId,
+        range: AllocRange,
         global: &GlobalState,
     ) -> InterpResult<'tcx> {
         if global.multi_threaded.get() {
             let (index, clocks) = global.current_thread_state();
             let mut alloc_ranges = self.alloc_ranges.borrow_mut();
-            for (_, range) in alloc_ranges.iter_mut(pointer.offset, len) {
+            for (offset, range) in alloc_ranges.iter_mut(range.start, range.size) {
                 if let Err(DataRace) = range.read_race_detect(&*clocks, index) {
                     // Report data-race.
-                    return Self::report_data_race(global, range, "Read", false, pointer, len);
+                    return Self::report_data_race(
+                        global,
+                        range,
+                        "Read",
+                        false,
+                        Pointer::new(alloc_id, offset),
+                    );
                 }
             }
             Ok(())
@@ -906,23 +888,22 @@ impl VClockAlloc {
     // Shared code for detecting data-races on unique access to a section of memory
     fn unique_access<'tcx>(
         &mut self,
-        pointer: Pointer<Tag>,
-        len: Size,
+        alloc_id: AllocId,
+        range: AllocRange,
         write_type: WriteType,
         global: &mut GlobalState,
     ) -> InterpResult<'tcx> {
         if global.multi_threaded.get() {
             let (index, clocks) = global.current_thread_state();
-            for (_, range) in self.alloc_ranges.get_mut().iter_mut(pointer.offset, len) {
+            for (offset, range) in self.alloc_ranges.get_mut().iter_mut(range.start, range.size) {
                 if let Err(DataRace) = range.write_race_detect(&*clocks, index, write_type) {
                     // Report data-race
                     return Self::report_data_race(
                         global,
                         range,
                         write_type.get_descriptor(),
                         false,
-                        pointer,
-                        len,
+                        Pointer::new(alloc_id, offset),
                     );
                 }
             }
@@ -938,11 +919,11 @@ impl VClockAlloc {
     /// operation
     pub fn write<'tcx>(
         &mut self,
-        pointer: Pointer<Tag>,
-        len: Size,
+        alloc_id: AllocId,
+        range: AllocRange,
         global: &mut GlobalState,
     ) -> InterpResult<'tcx> {
-        self.unique_access(pointer, len, WriteType::Write, global)
+        self.unique_access(alloc_id, range, WriteType::Write, global)
     }
 
     /// Detect data-races for an unsynchronized deallocate operation, will not perform
@@ -951,11 +932,11 @@ impl VClockAlloc {
     /// operation
     pub fn deallocate<'tcx>(
         &mut self,
-        pointer: Pointer<Tag>,
-        len: Size,
+        alloc_id: AllocId,
+        range: AllocRange,
         global: &mut GlobalState,
     ) -> InterpResult<'tcx> {
-        self.unique_access(pointer, len, WriteType::Deallocate, global)
+        self.unique_access(alloc_id, range, WriteType::Deallocate, global)
     }
 }
 
@@ -1002,12 +983,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         result
     }
 
-    /// Generic atomic operation implementation,
-    /// this accesses memory via get_raw instead of
-    /// get_raw_mut, due to issues calling get_raw_mut
-    /// for atomic loads from read-only memory.
-    /// FIXME: is this valid, or should get_raw_mut be used for
-    /// atomic-stores/atomic-rmw?
+    /// Generic atomic operation implementation
     fn validate_atomic_op<A: Debug + Copy>(
         &self,
         place: &MPlaceTy<'tcx, Tag>,
@@ -1023,25 +999,24 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         let this = self.eval_context_ref();
         if let Some(data_race) = &this.memory.extra.data_race {
             if data_race.multi_threaded.get() {
+                let size = place.layout.size;
+                let (alloc_id, base_offset, ptr) = this.memory.ptr_get_alloc(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 place_ptr = place.ptr.assert_ptr();
-                let size = place.layout.size;
                 let alloc_meta =
-                    &this.memory.get_alloc_extra(place_ptr.alloc_id)?.data_race.as_ref().unwrap();
+                    &this.memory.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();
                 log::trace!(
-                    "Atomic op({}) with ordering {:?} on memory({:?}, offset={}, size={})",
+                    "Atomic op({}) with ordering {:?} on {:?} (size={})",
                     description,
                     &atomic,
-                    place_ptr.alloc_id,
-                    place_ptr.offset.bytes(),
+                    ptr,
                     size.bytes()
                 );
 
                 // Perform the atomic operation.
                 data_race.maybe_perform_sync_operation(|index, mut clocks| {
-                    for (_, range) in
-                        alloc_meta.alloc_ranges.borrow_mut().iter_mut(place_ptr.offset, size)
+                    for (offset, range) in
+                        alloc_meta.alloc_ranges.borrow_mut().iter_mut(base_offset, size)
                     {
                         if let Err(DataRace) = op(range, &mut *clocks, index, atomic) {
                             mem::drop(clocks);
@@ -1050,8 +1025,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
                                 range,
                                 description,
                                 true,
-                                place_ptr,
-                                size,
+                                Pointer::new(alloc_id, offset),
                             )
                             .map(|_| true);
                         }
@@ -1063,12 +1037,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
 
                 // Log changes to atomic memory.
                 if log::log_enabled!(log::Level::Trace) {
-                    for (_, range) in alloc_meta.alloc_ranges.borrow().iter(place_ptr.offset, size)
+                    for (_offset, range) in alloc_meta.alloc_ranges.borrow().iter(base_offset, size)
                     {
                         log::trace!(
-                            "Updated atomic memory({:?}, offset={}, size={}) to {:#?}",
-                            place.ptr.assert_ptr().alloc_id,
-                            place_ptr.offset.bytes(),
+                            "Updated atomic memory({:?}, size={}) to {:#?}",
+                            ptr,
                             size.bytes(),
                             range.atomic_ops
                         );

src/diagnostics.rs

@@ -134,7 +134,7 @@ pub fn report_error<'tcx, 'mir>(
             let helps = match e.kind() {
                 Unsupported(UnsupportedOpInfo::NoMirFor(..)) =>
                     vec![(None, format!("make sure to use a Miri sysroot, which you can prepare with `cargo miri setup`"))],
-                Unsupported(UnsupportedOpInfo::ReadBytesAsPointer | UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
+                Unsupported(UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
                     panic!("Error should never be raised by Miri: {:?}", e.kind()),
                 Unsupported(_) =>
                     vec![(None, format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support"))],

src/eval.rs

@@ -164,15 +164,16 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     // Third argument (`argv`): created from `config.args`.
     let argv = {
         // Put each argument in memory, collect pointers.
-        let mut argvs = Vec::<Scalar<Tag>>::new();
+        let mut argvs = Vec::<Immediate<Tag>>::new();
         for arg in config.args.iter() {
             // Make space for `0` terminator.
             let size = u64::try_from(arg.len()).unwrap().checked_add(1).unwrap();
             let arg_type = tcx.mk_array(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)?;
-            argvs.push(arg_place.ptr);
+            ecx.mark_immutable(&*arg_place);
+            argvs.push(arg_place.to_ref(&ecx));
         }
         // Make an array with all these pointers, in the Miri memory.
         let argvs_layout = ecx.layout_of(
@@ -181,24 +182,26 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Machine.into())?;
         for (idx, arg) in argvs.into_iter().enumerate() {
             let place = ecx.mplace_field(&argvs_place, idx)?;
-            ecx.write_scalar(arg, &place.into())?;
+            ecx.write_immediate(arg, &place.into())?;
         }
-        ecx.memory.mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
+        ecx.mark_immutable(&*argvs_place);
         // A pointer to that place is the 3rd argument for main.
-        let argv = argvs_place.ptr;
+        let argv = argvs_place.to_ref(&ecx);
         // Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
         {
             let argc_place =
                 ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
             ecx.write_scalar(argc, &argc_place.into())?;
-            ecx.machine.argc = Some(argc_place.ptr);
+            ecx.mark_immutable(&*argc_place);
+            ecx.machine.argc = Some(*argc_place);
 
             let argv_place = ecx.allocate(
                 ecx.layout_of(tcx.mk_imm_ptr(tcx.types.unit))?,
                 MiriMemoryKind::Machine.into(),
             )?;
-            ecx.write_scalar(argv, &argv_place.into())?;
-            ecx.machine.argv = Some(argv_place.ptr);
+            ecx.write_immediate(argv, &argv_place.into())?;
+            ecx.mark_immutable(&*argv_place);
+            ecx.machine.argv = Some(*argv_place);
         }
         // Store command line as UTF-16 for Windows `GetCommandLineW`.
         {
@@ -217,12 +220,13 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
             let cmd_type = tcx.mk_array(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.ptr);
+            ecx.machine.cmd_line = Some(*cmd_place);
             // 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.mplace_field(&cmd_place, idx)?;
                 ecx.write_scalar(Scalar::from_u16(c), &place.into())?;
             }
+            ecx.mark_immutable(&*cmd_place);
         }
         argv
     };
@@ -233,7 +237,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     ecx.call_function(
         start_instance,
         Abi::Rust,
-        &[main_ptr.into(), argc.into(), argv.into()],
+        &[Scalar::from_pointer(main_ptr, &ecx).into(), argc.into(), argv],
         Some(&ret_place.into()),
         StackPopCleanup::None { cleanup: true },
     )?;
@@ -285,8 +289,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
             }
             ecx.process_diagnostics(info);
         }
-        let return_code =
-            ecx.read_scalar(&ret_place.into())?.check_init()?.to_machine_isize(&ecx)?;
+        let return_code = ecx.read_scalar(&ret_place.into())?.to_machine_isize(&ecx)?;
         Ok(return_code)
     })();