Auto merge of rust-lang#147687 - cjgillot:noshallow-init-box, r=nnethercote

Forbid ShallowInitBox after box deref elaboration.

MIR currently contains a `ShallowInitBox` rvalue. Its principal usage is to allow for in-place initialization of boxes. Having it is necessary for drop elaboration to be correct with that in-place initialization.

As part of analysis->runtime MIR lowering, we canonicalize deref of boxes to use the stored raw pointer. But we did not perform the same change to the construction of the box.

This PR replaces `ShallowInitBox` by the pointer manipulation it represents.

Alternatives:
- fully remove `ShallowInitBox` and implement `Box` in-place initialization differently;
- remove the `ElaborateBoxDeref` pass and keep dereferencing `Box` in runtime MIR.

Author: bors

compiler/rustc_codegen_cranelift/src/base.rs

@@ -829,13 +829,6 @@ fn codegen_stmt<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, cur_block: Block, stmt:
                         fx.bcx.ins().nop();
                     }
                 }
-                Rvalue::ShallowInitBox(ref operand, content_ty) => {
-                    let content_ty = fx.monomorphize(content_ty);
-                    let box_layout = fx.layout_of(Ty::new_box(fx.tcx, content_ty));
-                    let operand = codegen_operand(fx, operand);
-                    let operand = operand.load_scalar(fx);
-                    lval.write_cvalue(fx, CValue::by_val(operand, box_layout));
-                }
                 Rvalue::NullaryOp(ref null_op, ty) => {
                     assert!(lval.layout().ty.is_sized(fx.tcx, fx.typing_env()));
                     let layout = fx.layout_of(fx.monomorphize(ty));
@@ -924,6 +917,7 @@ fn codegen_stmt<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, cur_block: Block, stmt:
                     lval.write_cvalue_transmute(fx, operand);
                 }
                 Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in codegen"),
+                Rvalue::ShallowInitBox(..) => bug!("`ShallowInitBox` in codegen"),
             }
         }
         StatementKind::StorageLive(_)

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -724,22 +724,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                     }
                 }
             }
-            mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
-                let operand = self.codegen_operand(bx, operand);
-                let val = operand.immediate();
-
-                let content_ty = self.monomorphize(content_ty);
-                let box_layout = bx.cx().layout_of(Ty::new_box(bx.tcx(), content_ty));
-
-                OperandRef { val: OperandValue::Immediate(val), layout: box_layout }
-            }
             mir::Rvalue::WrapUnsafeBinder(ref operand, binder_ty) => {
                 let operand = self.codegen_operand(bx, operand);
                 let binder_ty = self.monomorphize(binder_ty);
                 let layout = bx.cx().layout_of(binder_ty);
                 OperandRef { val: operand.val, layout }
             }
             mir::Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in codegen"),
+            mir::Rvalue::ShallowInitBox(..) => bug!("`ShallowInitBox` in codegen"),
         }
     }
 

compiler/rustc_index/src/lib.rs

@@ -50,3 +50,13 @@ macro_rules! static_assert_size {
         const _: (usize, usize) = ($size, ::std::mem::size_of::<$ty>());
     };
 }
+
+#[macro_export]
+macro_rules! indexvec {
+    ($expr:expr; $n:expr) => {
+        IndexVec::from_raw(vec![$expr; $n])
+    };
+    ($($expr:expr),* $(,)?) => {
+        IndexVec::from_raw(vec![$($expr),*])
+    };
+}

compiler/rustc_mir_transform/src/coroutine.rs

@@ -67,7 +67,7 @@ use rustc_hir as hir;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{CoroutineDesugaring, CoroutineKind};
 use rustc_index::bit_set::{BitMatrix, DenseBitSet, GrowableBitSet};
-use rustc_index::{Idx, IndexVec};
+use rustc_index::{Idx, IndexVec, indexvec};
 use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;
 use rustc_middle::ty::util::Discr;
@@ -289,7 +289,7 @@ impl<'tcx> TransformVisitor<'tcx> {
                 let poll_def_id = self.tcx.require_lang_item(LangItem::Poll, source_info.span);
                 let args = self.tcx.mk_args(&[self.old_ret_ty.into()]);
                 let (variant_idx, operands) = if is_return {
-                    (ZERO, IndexVec::from_raw(vec![val])) // Poll::Ready(val)
+                    (ZERO, indexvec![val]) // Poll::Ready(val)
                 } else {
                     (ONE, IndexVec::new()) // Poll::Pending
                 };
@@ -301,7 +301,7 @@ impl<'tcx> TransformVisitor<'tcx> {
                 let (variant_idx, operands) = if is_return {
                     (ZERO, IndexVec::new()) // None
                 } else {
-                    (ONE, IndexVec::from_raw(vec![val])) // Some(val)
+                    (ONE, indexvec![val]) // Some(val)
                 };
                 make_aggregate_adt(option_def_id, variant_idx, args, operands)
             }
@@ -337,12 +337,7 @@ impl<'tcx> TransformVisitor<'tcx> {
                 } else {
                     ZERO // CoroutineState::Yielded(val)
                 };
-                make_aggregate_adt(
-                    coroutine_state_def_id,
-                    variant_idx,
-                    args,
-                    IndexVec::from_raw(vec![val]),
-                )
+                make_aggregate_adt(coroutine_state_def_id, variant_idx, args, indexvec![val])
             }
         };
 
@@ -1122,7 +1117,7 @@ fn return_poll_ready_assign<'tcx>(tcx: TyCtxt<'tcx>, source_info: SourceInfo) ->
     }));
     let ready_val = Rvalue::Aggregate(
         Box::new(AggregateKind::Adt(poll_def_id, VariantIdx::from_usize(0), args, None, None)),
-        IndexVec::from_raw(vec![val]),
+        indexvec![val],
     );
     Statement::new(source_info, StatementKind::Assign(Box::new((Place::return_place(), ready_val))))
 }

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

@@ -480,6 +480,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
             Rvalue::Discriminant(place) => state.get_discr(place.as_ref(), &self.map),
             Rvalue::Use(operand) => return self.handle_operand(operand, state),
             Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in runtime MIR"),
+            Rvalue::ShallowInitBox(..) => bug!("`ShallowInitBox` in runtime MIR"),
             Rvalue::Ref(..) | Rvalue::RawPtr(..) => {
                 // We don't track such places.
                 return ValueOrPlace::TOP;
@@ -489,7 +490,6 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
             | Rvalue::Cast(..)
             | Rvalue::BinaryOp(..)
             | Rvalue::Aggregate(..)
-            | Rvalue::ShallowInitBox(..)
             | Rvalue::WrapUnsafeBinder(..) => {
                 // No modification is possible through these r-values.
                 return ValueOrPlace::TOP;

compiler/rustc_mir_transform/src/elaborate_box_derefs.rs

@@ -1,26 +1,29 @@
 //! This pass transforms derefs of Box into a deref of the pointer inside Box.
 //!
 //! Box is not actually a pointer so it is incorrect to dereference it directly.
+//!
+//! `ShallowInitBox` being a device for drop elaboration to understand deferred assignment to box
+//! contents, we do not need this any more on runtime MIR.
 
-use rustc_abi::FieldIdx;
-use rustc_hir::def_id::DefId;
+use rustc_abi::{FieldIdx, VariantIdx};
+use rustc_index::{IndexVec, indexvec};
 use rustc_middle::mir::visit::MutVisitor;
 use rustc_middle::mir::*;
 use rustc_middle::span_bug;
-use rustc_middle::ty::{Ty, TyCtxt};
+use rustc_middle::ty::{self, Ty, TyCtxt};
 
 use crate::patch::MirPatch;
 
 /// Constructs the types used when accessing a Box's pointer
 fn build_ptr_tys<'tcx>(
     tcx: TyCtxt<'tcx>,
     pointee: Ty<'tcx>,
-    unique_did: DefId,
-    nonnull_did: DefId,
+    unique_def: ty::AdtDef<'tcx>,
+    nonnull_def: ty::AdtDef<'tcx>,
 ) -> (Ty<'tcx>, Ty<'tcx>, Ty<'tcx>) {
     let args = tcx.mk_args(&[pointee.into()]);
-    let unique_ty = tcx.type_of(unique_did).instantiate(tcx, args);
-    let nonnull_ty = tcx.type_of(nonnull_did).instantiate(tcx, args);
+    let unique_ty = Ty::new_adt(tcx, unique_def, args);
+    let nonnull_ty = Ty::new_adt(tcx, nonnull_def, args);
     let ptr_ty = Ty::new_imm_ptr(tcx, pointee);
 
     (unique_ty, nonnull_ty, ptr_ty)
@@ -36,8 +39,8 @@ pub(super) fn build_projection<'tcx>(
 
 struct ElaborateBoxDerefVisitor<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
-    unique_did: DefId,
-    nonnull_did: DefId,
+    unique_def: ty::AdtDef<'tcx>,
+    nonnull_def: ty::AdtDef<'tcx>,
     local_decls: &'a mut LocalDecls<'tcx>,
     patch: MirPatch<'tcx>,
 }
@@ -64,7 +67,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for ElaborateBoxDerefVisitor<'a, 'tcx> {
             let source_info = self.local_decls[place.local].source_info;
 
             let (unique_ty, nonnull_ty, ptr_ty) =
-                build_ptr_tys(tcx, boxed_ty, self.unique_did, self.nonnull_did);
+                build_ptr_tys(tcx, boxed_ty, self.unique_def, self.nonnull_def);
 
             let ptr_local = self.patch.new_temp(ptr_ty, source_info.span);
 
@@ -86,6 +89,68 @@ impl<'a, 'tcx> MutVisitor<'tcx> for ElaborateBoxDerefVisitor<'a, 'tcx> {
 
         self.super_place(place, context, location);
     }
+
+    fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) {
+        self.super_statement(stmt, location);
+
+        let tcx = self.tcx;
+        let source_info = stmt.source_info;
+
+        if let StatementKind::Assign(box (_, ref mut rvalue)) = stmt.kind
+            && let Rvalue::ShallowInitBox(ref mut mutptr_to_u8, pointee) = *rvalue
+            && let ty::Adt(box_adt, box_args) = Ty::new_box(tcx, pointee).kind()
+        {
+            let args = tcx.mk_args(&[pointee.into()]);
+            let (unique_ty, nonnull_ty, ptr_ty) =
+                build_ptr_tys(tcx, pointee, self.unique_def, self.nonnull_def);
+            let adt_kind = |def: ty::AdtDef<'tcx>, args| {
+                Box::new(AggregateKind::Adt(def.did(), VariantIdx::ZERO, args, None, None))
+            };
+            let zst = |ty| {
+                Operand::Constant(Box::new(ConstOperand {
+                    span: source_info.span,
+                    user_ty: None,
+                    const_: Const::zero_sized(ty),
+                }))
+            };
+
+            let constptr = self.patch.new_temp(ptr_ty, source_info.span);
+            self.patch.add_assign(
+                location,
+                constptr.into(),
+                Rvalue::Cast(CastKind::Transmute, mutptr_to_u8.clone(), ptr_ty),
+            );
+
+            let nonnull = self.patch.new_temp(nonnull_ty, source_info.span);
+            self.patch.add_assign(
+                location,
+                nonnull.into(),
+                Rvalue::Aggregate(
+                    adt_kind(self.nonnull_def, args),
+                    indexvec![Operand::Move(constptr.into())],
+                ),
+            );
+
+            let unique = self.patch.new_temp(unique_ty, source_info.span);
+            let phantomdata_ty =
+                self.unique_def.non_enum_variant().fields[FieldIdx::ONE].ty(tcx, args);
+            self.patch.add_assign(
+                location,
+                unique.into(),
+                Rvalue::Aggregate(
+                    adt_kind(self.unique_def, args),
+                    indexvec![Operand::Move(nonnull.into()), zst(phantomdata_ty)],
+                ),
+            );
+
+            let global_alloc_ty =
+                box_adt.non_enum_variant().fields[FieldIdx::ONE].ty(tcx, box_args);
+            *rvalue = Rvalue::Aggregate(
+                adt_kind(*box_adt, box_args),
+                indexvec![Operand::Move(unique.into()), zst(global_alloc_ty)],
+            );
+        }
+    }
 }
 
 pub(super) struct ElaborateBoxDerefs;
@@ -97,18 +162,22 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateBoxDerefs {
 
         let unique_did = tcx.adt_def(def_id).non_enum_variant().fields[FieldIdx::ZERO].did;
 
-        let Some(nonnull_def) = tcx.type_of(unique_did).instantiate_identity().ty_adt_def() else {
+        let Some(unique_def) = tcx.type_of(unique_did).instantiate_identity().ty_adt_def() else {
             span_bug!(tcx.def_span(unique_did), "expected Box to contain Unique")
         };
 
-        let nonnull_did = nonnull_def.non_enum_variant().fields[FieldIdx::ZERO].did;
+        let nonnull_did = unique_def.non_enum_variant().fields[FieldIdx::ZERO].did;
+
+        let Some(nonnull_def) = tcx.type_of(nonnull_did).instantiate_identity().ty_adt_def() else {
+            span_bug!(tcx.def_span(nonnull_did), "expected Unique to contain Nonnull")
+        };
 
         let patch = MirPatch::new(body);
 
         let local_decls = &mut body.local_decls;
 
         let mut visitor =
-            ElaborateBoxDerefVisitor { tcx, unique_did, nonnull_did, local_decls, patch };
+            ElaborateBoxDerefVisitor { tcx, unique_def, nonnull_def, local_decls, patch };
 
         for (block, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
             visitor.visit_basic_block_data(block, data);
@@ -131,7 +200,7 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateBoxDerefs {
                             new_projections.get_or_insert_with(|| base.projection.to_vec());
 
                         let (unique_ty, nonnull_ty, ptr_ty) =
-                            build_ptr_tys(tcx, boxed_ty, unique_did, nonnull_did);
+                            build_ptr_tys(tcx, boxed_ty, unique_def, nonnull_def);
 
                         new_projections.extend_from_slice(&build_projection(unique_ty, nonnull_ty));
                         // While we can't project into `NonNull<_>` in a basic block

compiler/rustc_mir_transform/src/gvn.rs

@@ -1073,8 +1073,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
             }
 
             // Unsupported values.
-            Rvalue::ThreadLocalRef(..) | Rvalue::ShallowInitBox(..) => return None,
-            Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in runtime MIR"),
+            Rvalue::ThreadLocalRef(..) => return None,
+            Rvalue::CopyForDeref(_) | Rvalue::ShallowInitBox(..) => {
+                bug!("forbidden in runtime MIR: {rvalue:?}")
+            }
         };
         let ty = rvalue.ty(self.local_decls, self.tcx);
         Some(self.insert(ty, value))

compiler/rustc_mir_transform/src/validate.rs

@@ -656,7 +656,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
     ) {
         match elem {
             ProjectionElem::Deref
-                if self.body.phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) =>
+                if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) =>
             {
                 let base_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty;
 
@@ -1049,7 +1049,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     assert!(!adt_def.is_union());
                     let variant = &adt_def.variants()[idx];
                     if variant.fields.len() != fields.len() {
-                        self.fail(location, "adt has the wrong number of initialized fields");
+                        self.fail(location, format!(
+                            "adt {def_id:?} has the wrong number of initialized fields, expected {}, found {}",
+                            fields.len(),
+                            variant.fields.len(),
+                        ));
                     }
                     for (src, dest) in std::iter::zip(fields, &variant.fields) {
                         let dest_ty = self
@@ -1252,6 +1256,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 }
             }
             Rvalue::ShallowInitBox(operand, _) => {
+                if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
+                    self.fail(location, format!("ShallowInitBox after ElaborateBoxDerefs"))
+                }
+
                 let a = operand.ty(&self.body.local_decls, self.tcx);
                 check_kinds!(a, "Cannot shallow init type {:?}", ty::RawPtr(..));
             }

tests/mir-opt/box_expr.rs

@@ -6,7 +6,10 @@
 // EMIT_MIR box_expr.main.ElaborateDrops.diff
 fn main() {
     // CHECK-LABEL: fn main(
-    // CHECK:   [[box:_.*]] = ShallowInitBox(
+    // CHECK:   [[ptr:_.*]] = move {{_.*}} as *const S (Transmute);
+    // CHECK:   [[nonnull:_.*]] = NonNull::<S> { pointer: move [[ptr]] };
+    // CHECK:   [[unique:_.*]] = Unique::<S> { pointer: move [[nonnull]], _marker: const PhantomData::<S> };
+    // CHECK:   [[box:_.*]] = Box::<S>(move [[unique]], const std::alloc::Global);
     // CHECK:   [[ptr:_.*]] = copy (([[box]].0: std::ptr::Unique<S>).0: std::ptr::NonNull<S>) as *const S (Transmute);
     // CHECK:   (*[[ptr]]) = S::new() -> [return: [[ret:bb.*]], unwind: [[unwind:bb.*]]];
     // CHECK: [[ret]]: {

tests/mir-opt/const_prop/boxes.main.GVN.panic-abort.diff

@@ -11,7 +11,10 @@
       let mut _6: *mut u8;
       let mut _7: std::boxed::Box<i32>;
       let mut _8: *const i32;
-      let mut _9: *const i32;
+      let mut _9: std::ptr::NonNull<i32>;
+      let mut _10: std::ptr::Unique<i32>;
+      let mut _11: *const i32;
+      let mut _12: *const i32;
       scope 1 {
           debug x => _1;
       }
@@ -31,13 +34,21 @@
 
       bb1: {
           StorageLive(_7);
-          _7 = ShallowInitBox(move _6, i32);
-          _8 = copy ((_7.0: std::ptr::Unique<i32>).0: std::ptr::NonNull<i32>) as *const i32 (Transmute);
-          (*_8) = const 42_i32;
+-         _8 = move _6 as *const i32 (Transmute);
+-         _9 = NonNull::<i32> { pointer: move _8 };
+-         _10 = Unique::<i32> { pointer: move _9, _marker: const PhantomData::<i32> };
++         _8 = copy _6 as *const i32 (PtrToPtr);
++         _9 = NonNull::<i32> { pointer: copy _8 };
++         _10 = Unique::<i32> { pointer: copy _9, _marker: const PhantomData::<i32> };
+          _7 = Box::<i32>(move _10, const std::alloc::Global);
+-         _11 = copy ((_7.0: std::ptr::Unique<i32>).0: std::ptr::NonNull<i32>) as *const i32 (Transmute);
+-         (*_11) = const 42_i32;
++         _11 = copy _8;
++         (*_8) = const 42_i32;
           _3 = move _7;
           StorageDead(_7);
-          _9 = copy ((_3.0: std::ptr::Unique<i32>).0: std::ptr::NonNull<i32>) as *const i32 (Transmute);
-          _2 = copy (*_9);
+          _12 = copy ((_3.0: std::ptr::Unique<i32>).0: std::ptr::NonNull<i32>) as *const i32 (Transmute);
+          _2 = copy (*_12);
 -         _1 = Add(move _2, const 0_i32);
 -         StorageDead(_2);
 +         _1 = copy _2;