interpret: fix overlapping aggregate initialization

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -1,5 +1,5 @@
 use itertools::Itertools as _;
-use rustc_abi::{self as abi, FIRST_VARIANT};
+use rustc_abi::{self as abi, BackendRepr, FIRST_VARIANT};
 use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::layout::{HasTyCtxt, HasTypingEnv, LayoutOf, TyAndLayout};
 use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
@@ -25,6 +25,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         match *rvalue {
             mir::Rvalue::Use(ref operand) => {
                 let cg_operand = self.codegen_operand(bx, operand);
+                // Crucially, we do *not* use `OperandValue::Ref` for types with
+                // `BackendRepr::Scalar | BackendRepr::ScalarPair`. This ensures we match the MIR
+                // semantics regarding when assignment operators allow overlap of LHS and RHS.
+                if matches!(
+                    cg_operand.layout.backend_repr,
+                    BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..),
+                ) {
+                    debug_assert!(!matches!(cg_operand.val, OperandValue::Ref(..)));
+                }
                 // FIXME: consider not copying constants through stack. (Fixable by codegen'ing
                 // constants into `OperandValue::Ref`; why don’t we do that yet if we don’t?)
                 cg_operand.val.store(bx, dest);

compiler/rustc_const_eval/src/interpret/place.rs

@@ -858,7 +858,7 @@ where
     /// Also, if you use this you are responsible for validating that things get copied at the
     /// right type.
     #[instrument(skip(self), level = "trace")]
-    fn copy_op_no_validate(
+    pub(super) fn copy_op_no_validate(
         &mut self,
         src: &impl Projectable<'tcx, M::Provenance>,
         dest: &impl Writeable<'tcx, M::Provenance>,

compiler/rustc_const_eval/src/interpret/step.rs

@@ -310,7 +310,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         operands: &IndexSlice<FieldIdx, mir::Operand<'tcx>>,
         dest: &PlaceTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx> {
-        self.write_uninit(dest)?; // make sure all the padding ends up as uninit
         let (variant_index, variant_dest, active_field_index) = match *kind {
             mir::AggregateKind::Adt(_, variant_index, _, _, active_field_index) => {
                 let variant_dest = self.project_downcast(dest, variant_index)?;
@@ -346,9 +345,20 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             let field_index = active_field_index.unwrap_or(field_index);
             let field_dest = self.project_field(&variant_dest, field_index)?;
             let op = self.eval_operand(operand, Some(field_dest.layout))?;
-            self.copy_op(&op, &field_dest)?;
+            // We validate manually below so we don't have to do it here.
+            self.copy_op_no_validate(&op, &field_dest, /*allow_transmute*/ false)?;
         }
-        self.write_discriminant(variant_index, dest)
+        self.write_discriminant(variant_index, dest)?;
+        // Validate that the entire thing is valid, and reset padding that might be in between the
+        // fields.
+        if M::enforce_validity(self, dest.layout()) {
+            self.validate_operand(
+                dest,
+                M::enforce_validity_recursively(self, dest.layout()),
+                /*reset_provenance_and_padding*/ true,
+            )?;
+        }
+        interp_ok(())
     }
 
     /// Repeats `operand` into the destination. `dest` must have array type, and that type

compiler/rustc_middle/src/mir/syntax.rs

@@ -327,9 +327,11 @@ pub enum StatementKind<'tcx> {
     /// interesting for optimizations? Do we want to allow such optimizations?
     ///
     /// **Needs clarification**: We currently require that the LHS place not overlap with any place
-    /// read as part of computation of the RHS for some rvalues (generally those not producing
-    /// primitives). This requirement is under discussion in [#68364]. As a part of this discussion,
-    /// it is also unclear in what order the components are evaluated.
+    /// read as part of computation of the RHS for some rvalues. This requirement is under
+    /// discussion in [#68364]. Specifically, overlap is permitted only for assignments of a type
+    /// with `BackendRepr::Scalar | BackendRepr::ScalarPair` where all the scalar fields are
+    /// [`Scalar::Initialized`][rustc_abi::Scalar::Initialized]. As a part of this discussion, it is
+    /// also unclear in what order the components are evaluated.
     ///
     /// [#68364]: https://github.com/rust-lang/rust/issues/68364
     ///

src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.rs

@@ -1,4 +1,6 @@
 //@revisions: stack tree
+// Ensure this even hits the aliasing model
+//@compile-flags: -Zmiri-disable-validation
 //@[tree]compile-flags: -Zmiri-tree-borrows
 //@error-in-other-file: pointer not dereferenceable
 

src/tools/miri/tests/fail/both_borrows/issue-miri-1050-2.rs

@@ -1,4 +1,6 @@
 //@revisions: stack tree
+// Ensure this even hits the aliasing model
+//@compile-flags: -Zmiri-disable-validation
 //@[tree]compile-flags: -Zmiri-tree-borrows
 //@error-in-other-file: is a dangling pointer
 use std::ptr::NonNull;

src/tools/miri/tests/fail/both_borrows/return_invalid_shr.stack.stderr

@@ -11,7 +11,7 @@ help: <TAG> was created by a SharedReadOnly retag at offsets [0x4..0x8]
    |
 LL |     let ret = unsafe { &(*xraw).1 };
    |                        ^^^^^^^^^^
-help: <TAG> was later invalidated at offsets [0x0..0x8] by a write access
+help: <TAG> was later invalidated at offsets [0x4..0x8] by a write access
   --> tests/fail/both_borrows/return_invalid_shr.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/both_borrows/return_invalid_shr.tree.stderr

@@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Frozen
    |
 LL |     let ret = unsafe { &(*xraw).1 };
    |                        ^^^^^^^^^^
-help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8]
+help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8]
   --> tests/fail/both_borrows/return_invalid_shr.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr

@@ -14,7 +14,7 @@ help: <TAG> was created by a SharedReadOnly retag at offsets [0x4..0x8]
    |
 LL |     let ret = Some(unsafe { &(*xraw).1 });
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: <TAG> was later invalidated at offsets [0x0..0x8] by a write access
+help: <TAG> was later invalidated at offsets [0x4..0x8] by a write access
   --> tests/fail/both_borrows/return_invalid_shr_option.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.tree.stderr

@@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Frozen
    |
 LL |     let ret = Some(unsafe { &(*xraw).1 });
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8]
+help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8]
   --> tests/fail/both_borrows/return_invalid_shr_option.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr

@@ -14,7 +14,7 @@ help: <TAG> was created by a SharedReadOnly retag at offsets [0x4..0x8]
    |
 LL |     let ret = (unsafe { &(*xraw).1 },);
    |               ^^^^^^^^^^^^^^^^^^^^^^^^
-help: <TAG> was later invalidated at offsets [0x0..0x8] by a write access
+help: <TAG> was later invalidated at offsets [0x4..0x8] by a write access
   --> tests/fail/both_borrows/return_invalid_shr_tuple.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.tree.stderr

@@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Frozen
    |
 LL |     let ret = (unsafe { &(*xraw).1 },);
    |               ^^^^^^^^^^^^^^^^^^^^^^^^
-help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8]
+help: the accessed tag <TAG> later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8]
   --> tests/fail/both_borrows/return_invalid_shr_tuple.rs:LL:CC
    |
 LL |     unsafe { *xraw = (42, 23) }; // unfreeze

src/tools/miri/tests/fail/overlapping_assignment_aggregate.rs

@@ -0,0 +1,18 @@
+//! This is like `pass/overlapping_assignment_aggregate_scalar.rs` but with a non-scalar
+//! type, and that makes it definite UB.
+#![feature(custom_mir, core_intrinsics)]
+#![allow(internal_features)]
+
+use std::intrinsics::mir::*;
+
+#[custom_mir(dialect = "runtime")]
+fn main() {
+    mir! {
+        let _1: ([u8; 1],);
+        {
+            _1.0 = [0_u8; 1];
+            _1 = (_1.0, ); //~ERROR: overlapping ranges
+            Return()
+        }
+    }
+}

src/tools/miri/tests/fail/overlapping_assignment_aggregate.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: `copy_nonoverlapping` called on overlapping ranges
+  --> tests/fail/overlapping_assignment_aggregate.rs:LL:CC
+   |
+LL |             _1 = (_1.0, );
+   |             ^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail/overlapping_assignment_aggregate.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/validity/nonzero.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: constructing invalid value: encountered 0, but expect
   --> tests/fail/validity/nonzero.rs:LL:CC
    |
 LL |     let _x = Some(unsafe { NonZero(0) });
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |                            ^^^^^^^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/pass/overlapping_assignment_aggregate_scalar.rs

@@ -0,0 +1,19 @@
+#![feature(custom_mir, core_intrinsics)]
+#![allow(internal_features)]
+
+use std::intrinsics::mir::*;
+
+#[custom_mir(dialect = "runtime")]
+fn main() {
+    mir! {
+        let _1: (u8,);
+        {
+            _1.0 = 0_u8;
+            // This is a scalar type, so overlap is (for now) not UB.
+            // However, we used to treat such overlapping assignments incorrectly
+            // (see <https://github.com/rust-lang/rust/issues/146383#issuecomment-3273224645>).
+            _1 = (_1.0, );
+            Return()
+        }
+    }
+}