Auto merge of #2455 - RalfJung:scalar-always-init, r=RalfJung

adjust for earlier init checking in the core engine

Miri side of #100043

Author: bors

rust-version

@@ -1 +1 @@
-e1b28cd2f16bd5b832183d7968cae3bb9213e78d
+4065b89b1e7287047d7d6c65e7abd7b8ee70bcf0

src/concurrency/data_race.rs

@@ -447,7 +447,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         offset: u64,
         layout: TyAndLayout<'tcx>,
         atomic: AtomicReadOrd,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_ref();
         let value_place = this.deref_operand_and_offset(op, offset, layout)?;
         this.read_scalar_atomic(&value_place, atomic)
@@ -458,7 +458,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         &mut self,
         op: &OpTy<'tcx, Provenance>,
         offset: u64,
-        value: impl Into<ScalarMaybeUninit<Provenance>>,
+        value: impl Into<Scalar<Provenance>>,
         layout: TyAndLayout<'tcx>,
         atomic: AtomicWriteOrd,
     ) -> InterpResult<'tcx> {
@@ -472,7 +472,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         &self,
         place: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicReadOrd,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_ref();
         this.atomic_access_check(place)?;
         // This will read from the last store in the modification order of this location. In case
@@ -490,7 +490,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
     /// Perform an atomic write operation at the memory location.
     fn write_scalar_atomic(
         &mut self,
-        val: ScalarMaybeUninit<Provenance>,
+        val: Scalar<Provenance>,
         dest: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicWriteOrd,
     ) -> InterpResult<'tcx> {
@@ -530,12 +530,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
 
         this.validate_atomic_rmw(place, atomic)?;
 
-        this.buffered_atomic_rmw(
-            val.to_scalar_or_uninit(),
-            place,
-            atomic,
-            old.to_scalar_or_uninit(),
-        )?;
+        this.buffered_atomic_rmw(val.to_scalar(), place, atomic, old.to_scalar())?;
         Ok(old)
     }
 
@@ -544,9 +539,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
     fn atomic_exchange_scalar(
         &mut self,
         place: &MPlaceTy<'tcx, Provenance>,
-        new: ScalarMaybeUninit<Provenance>,
+        new: Scalar<Provenance>,
         atomic: AtomicRwOrd,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_mut();
         this.atomic_access_check(place)?;
 
@@ -574,7 +569,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
 
         this.validate_overlapping_atomic(place)?;
         let old = this.allow_data_races_mut(|this| this.read_immediate(&place.into()))?;
-        let lt = this.binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar()?.to_bool()?;
+        let lt = this.binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar().to_bool()?;
 
         let new_val = if min {
             if lt { &old } else { &rhs }
@@ -586,12 +581,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
 
         this.validate_atomic_rmw(place, atomic)?;
 
-        this.buffered_atomic_rmw(
-            new_val.to_scalar_or_uninit(),
-            place,
-            atomic,
-            old.to_scalar_or_uninit(),
-        )?;
+        this.buffered_atomic_rmw(new_val.to_scalar(), place, atomic, old.to_scalar())?;
 
         // Return the old value.
         Ok(old)
@@ -607,7 +597,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         &mut self,
         place: &MPlaceTy<'tcx, Provenance>,
         expect_old: &ImmTy<'tcx, Provenance>,
-        new: ScalarMaybeUninit<Provenance>,
+        new: Scalar<Provenance>,
         success: AtomicRwOrd,
         fail: AtomicReadOrd,
         can_fail_spuriously: bool,
@@ -627,31 +617,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         // If the operation would succeed, but is "weak", fail some portion
         // of the time, based on `success_rate`.
         let success_rate = 1.0 - this.machine.cmpxchg_weak_failure_rate;
-        let cmpxchg_success = eq.to_scalar()?.to_bool()?
+        let cmpxchg_success = eq.to_scalar().to_bool()?
             && if can_fail_spuriously {
                 this.machine.rng.get_mut().gen_bool(success_rate)
             } else {
                 true
             };
-        let res = Immediate::ScalarPair(
-            old.to_scalar_or_uninit(),
-            Scalar::from_bool(cmpxchg_success).into(),
-        );
+        let res = Immediate::ScalarPair(old.to_scalar(), Scalar::from_bool(cmpxchg_success));
 
         // Update ptr depending on comparison.
         // if successful, perform a full rw-atomic validation
         // otherwise treat this as an atomic load with the fail ordering.
         if cmpxchg_success {
             this.allow_data_races_mut(|this| this.write_scalar(new, &place.into()))?;
             this.validate_atomic_rmw(place, success)?;
-            this.buffered_atomic_rmw(new, place, success, old.to_scalar_or_uninit())?;
+            this.buffered_atomic_rmw(new, place, success, old.to_scalar())?;
         } else {
             this.validate_atomic_load(place, fail)?;
             // A failed compare exchange is equivalent to a load, reading from the latest store
             // in the modification order.
             // Since `old` is only a value and not the store element, we need to separately
             // find it in our store buffer and perform load_impl on it.
-            this.perform_read_on_buffered_latest(place, fail, old.to_scalar_or_uninit())?;
+            this.perform_read_on_buffered_latest(place, fail, old.to_scalar())?;
         }
 
         // Return the old value.

src/concurrency/weak_memory.rs

@@ -77,9 +77,7 @@ use std::{
     collections::VecDeque,
 };
 
-use rustc_const_eval::interpret::{
-    alloc_range, AllocRange, InterpResult, MPlaceTy, ScalarMaybeUninit,
-};
+use rustc_const_eval::interpret::{alloc_range, AllocRange, InterpResult, MPlaceTy, Scalar};
 use rustc_data_structures::fx::FxHashMap;
 
 use crate::*;
@@ -130,10 +128,10 @@ struct StoreElement {
     /// The timestamp of the storing thread when it performed the store
     timestamp: VTimestamp,
     /// The value of this store
-    // FIXME: this means the store is either fully initialized or fully uninitialized;
+    // FIXME: this means the store must be fully initialized;
     // we will have to change this if we want to support atomics on
-    // partially initialized data.
-    val: ScalarMaybeUninit<Provenance>,
+    // (partially) uninitialized data.
+    val: Scalar<Provenance>,
 
     /// Timestamp of first loads from this store element by each thread
     /// Behind a RefCell to keep load op take &self
@@ -180,7 +178,7 @@ impl StoreBufferAlloc {
     fn get_or_create_store_buffer<'tcx>(
         &self,
         range: AllocRange,
-        init: ScalarMaybeUninit<Provenance>,
+        init: Scalar<Provenance>,
     ) -> InterpResult<'tcx, Ref<'_, StoreBuffer>> {
         let access_type = self.store_buffers.borrow().access_type(range);
         let pos = match access_type {
@@ -205,7 +203,7 @@ impl StoreBufferAlloc {
     fn get_or_create_store_buffer_mut<'tcx>(
         &mut self,
         range: AllocRange,
-        init: ScalarMaybeUninit<Provenance>,
+        init: Scalar<Provenance>,
     ) -> InterpResult<'tcx, &mut StoreBuffer> {
         let buffers = self.store_buffers.get_mut();
         let access_type = buffers.access_type(range);
@@ -226,7 +224,7 @@ impl StoreBufferAlloc {
 }
 
 impl<'mir, 'tcx: 'mir> StoreBuffer {
-    fn new(init: ScalarMaybeUninit<Provenance>) -> Self {
+    fn new(init: Scalar<Provenance>) -> Self {
         let mut buffer = VecDeque::new();
         buffer.reserve(STORE_BUFFER_LIMIT);
         let mut ret = Self { buffer };
@@ -259,7 +257,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
         is_seqcst: bool,
         rng: &mut (impl rand::Rng + ?Sized),
         validate: impl FnOnce() -> InterpResult<'tcx>,
-    ) -> InterpResult<'tcx, (ScalarMaybeUninit<Provenance>, LoadRecency)> {
+    ) -> InterpResult<'tcx, (Scalar<Provenance>, LoadRecency)> {
         // Having a live borrow to store_buffer while calling validate_atomic_load is fine
         // because the race detector doesn't touch store_buffer
 
@@ -284,7 +282,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
 
     fn buffered_write(
         &mut self,
-        val: ScalarMaybeUninit<Provenance>,
+        val: Scalar<Provenance>,
         global: &DataRaceState,
         thread_mgr: &ThreadManager<'_, '_>,
         is_seqcst: bool,
@@ -375,7 +373,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
     /// ATOMIC STORE IMPL in the paper (except we don't need the location's vector clock)
     fn store_impl(
         &mut self,
-        val: ScalarMaybeUninit<Provenance>,
+        val: Scalar<Provenance>,
         index: VectorIdx,
         thread_clock: &VClock,
         is_seqcst: bool,
@@ -417,11 +415,7 @@ impl StoreElement {
     /// buffer regardless of subsequent loads by the same thread; if the earliest load of another
     /// thread doesn't happen before the current one, then no subsequent load by the other thread
     /// can happen before the current one.
-    fn load_impl(
-        &self,
-        index: VectorIdx,
-        clocks: &ThreadClockSet,
-    ) -> ScalarMaybeUninit<Provenance> {
+    fn load_impl(&self, index: VectorIdx, clocks: &ThreadClockSet) -> Scalar<Provenance> {
         let _ = self.loads.borrow_mut().try_insert(index, clocks.clock[index]);
         self.val
     }
@@ -464,10 +458,10 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
 
     fn buffered_atomic_rmw(
         &mut self,
-        new_val: ScalarMaybeUninit<Provenance>,
+        new_val: Scalar<Provenance>,
         place: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicRwOrd,
-        init: ScalarMaybeUninit<Provenance>,
+        init: Scalar<Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
@@ -492,9 +486,9 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
         &self,
         place: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicReadOrd,
-        latest_in_mo: ScalarMaybeUninit<Provenance>,
+        latest_in_mo: Scalar<Provenance>,
         validate: impl FnOnce() -> InterpResult<'tcx>,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> 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)?;
@@ -529,10 +523,10 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
 
     fn buffered_atomic_write(
         &mut self,
-        val: ScalarMaybeUninit<Provenance>,
+        val: Scalar<Provenance>,
         dest: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicWriteOrd,
-        init: ScalarMaybeUninit<Provenance>,
+        init: Scalar<Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr)?;
@@ -576,7 +570,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
         &self,
         place: &MPlaceTy<'tcx, Provenance>,
         atomic: AtomicReadOrd,
-        init: ScalarMaybeUninit<Provenance>,
+        init: Scalar<Provenance>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_ref();
 

src/helpers.rs

@@ -116,8 +116,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let instance = this.resolve_path(path);
         let cid = GlobalId { instance, promoted: None };
         let const_val = this.eval_to_allocation(cid)?;
-        let const_val = this.read_scalar(&const_val.into())?;
-        const_val.check_init()
+        this.read_scalar(&const_val.into())
     }
 
     /// Helper function to get a `libc` constant as a `Scalar`.
@@ -567,7 +566,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_mut();
         let errno_place = this.last_error_place()?;
-        this.read_scalar(&errno_place.into())?.check_init()
+        this.read_scalar(&errno_place.into())
     }
 
     /// This function tries to produce the most similar OS error from the `std::io::ErrorKind`
@@ -680,22 +679,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         op: &OpTy<'tcx, Provenance>,
         offset: u64,
         layout: TyAndLayout<'tcx>,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_ref();
         let value_place = this.deref_operand_and_offset(op, offset, layout)?;
         this.read_scalar(&value_place.into())
     }
 
+    fn write_immediate_at_offset(
+        &mut self,
+        op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+        value: &ImmTy<'tcx, Provenance>,
+    ) -> InterpResult<'tcx, ()> {
+        let this = self.eval_context_mut();
+        let value_place = this.deref_operand_and_offset(op, offset, value.layout)?;
+        this.write_immediate(**value, &value_place.into())
+    }
+
     fn write_scalar_at_offset(
         &mut self,
         op: &OpTy<'tcx, Provenance>,
         offset: u64,
-        value: impl Into<ScalarMaybeUninit<Provenance>>,
+        value: impl Into<Scalar<Provenance>>,
         layout: TyAndLayout<'tcx>,
     ) -> InterpResult<'tcx, ()> {
-        let this = self.eval_context_mut();
-        let value_place = this.deref_operand_and_offset(op, offset, layout)?;
-        this.write_scalar(value, &value_place.into())
+        self.write_immediate_at_offset(op, offset, &ImmTy::from_scalar(value.into(), layout))
     }
 
     /// Parse a `timespec` struct and return it as a `std::time::Duration`. It returns `None`

src/machine.rs

@@ -149,7 +149,7 @@ static_assert_size!(Pointer<Provenance>, 24);
 // #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 //static_assert_size!(Pointer<Option<Provenance>>, 24);
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-static_assert_size!(ScalarMaybeUninit<Provenance>, 32);
+static_assert_size!(Scalar<Provenance>, 32);
 
 impl interpret::Provenance for Provenance {
     /// We use absolute addresses in the `offset` of a `Pointer<Provenance>`.
@@ -581,7 +581,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
     }
 
     #[inline(always)]
-    fn force_int_for_alignment_check(ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
+    fn use_addr_for_alignment_check(ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
         ecx.machine.check_alignment == AlignmentCheck::Int
     }
 
@@ -590,11 +590,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         ecx.machine.validate
     }
 
-    #[inline(always)]
-    fn enforce_number_init(_ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
-        true
-    }
-
     #[inline(always)]
     fn enforce_abi(ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
         ecx.machine.enforce_abi