Use a ConstValue instead.

Author: cjgillot

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

@@ -3,21 +3,23 @@
 //! Currently, this pass only propagates scalar values.
 
 use rustc_const_eval::const_eval::CheckAlignment;
-use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
+use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, PlaceTy, Projectable};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::DefKind;
-use rustc_middle::mir::interpret::{AllocId, ConstAllocation, ConstValue, InterpResult, Scalar};
+use rustc_middle::mir::interpret::{
+    AllocId, ConstAllocation, ConstValue, GlobalAlloc, InterpResult, Scalar,
+};
 use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
 use rustc_middle::mir::*;
-use rustc_middle::ty::layout::TyAndLayout;
+use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_mir_dataflow::value_analysis::{
     Map, PlaceIndex, State, TrackElem, ValueAnalysis, ValueAnalysisWrapper, ValueOrPlace,
 };
 use rustc_mir_dataflow::{lattice::FlatSet, Analysis, Results, ResultsVisitor};
 use rustc_span::def_id::DefId;
 use rustc_span::DUMMY_SP;
-use rustc_target::abi::{Align, FieldIdx, VariantIdx};
+use rustc_target::abi::{Align, FieldIdx, Size, VariantIdx, FIRST_VARIANT};
 
 use crate::MirPass;
 
@@ -546,110 +548,130 @@ impl<'tcx, 'locals> Collector<'tcx, 'locals> {
 
     fn try_make_constant(
         &self,
+        ecx: &mut InterpCx<'tcx, 'tcx, DummyMachine>,
         place: Place<'tcx>,
         state: &State<FlatSet<Scalar>>,
         map: &Map,
     ) -> Option<ConstantKind<'tcx>> {
         let ty = place.ty(self.local_decls, self.patch.tcx).ty;
         let place = map.find(place.as_ref())?;
-        if let FlatSet::Elem(Scalar::Int(value)) = state.get_idx(place, map) {
-            Some(ConstantKind::Val(ConstValue::Scalar(value.into()), ty))
+        let layout = ecx.layout_of(ty).ok()?;
+        if layout.abi.is_scalar() {
+            let value = propagatable_scalar(*ecx.tcx, place, state, map)?;
+            Some(ConstantKind::Val(ConstValue::Scalar(value), ty))
         } else {
-            let valtree = self.try_make_valtree(place, ty, state, map)?;
-            let constant = ty::Const::new_value(self.patch.tcx, valtree, ty);
-            Some(ConstantKind::Ty(constant))
+            let alloc_id = ecx
+                .intern_with_temp_alloc(layout, |ecx, dest| {
+                    try_write_constant(ecx, dest, place, ty, state, map)
+                })
+                .ok()?;
+            Some(ConstantKind::Val(ConstValue::Indirect { alloc_id, offset: Size::ZERO }, ty))
         }
     }
+}
 
-    fn try_make_valtree(
-        &self,
-        place: PlaceIndex,
-        ty: Ty<'tcx>,
-        state: &State<FlatSet<Scalar>>,
-        map: &Map,
-    ) -> Option<ty::ValTree<'tcx>> {
-        let tcx = self.patch.tcx;
-        match ty.kind() {
-            // ZSTs.
-            ty::FnDef(..) => Some(ty::ValTree::zst()),
-
-            // Scalars.
-            ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => {
-                if let FlatSet::Elem(Scalar::Int(value)) = state.get_idx(place, map) {
-                    Some(ty::ValTree::Leaf(value))
-                } else {
-                    None
-                }
+fn propagatable_scalar<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    place: PlaceIndex,
+    state: &State<FlatSet<Scalar>>,
+    map: &Map,
+) -> Option<Scalar> {
+    if let FlatSet::Elem(value) = state.get_idx(place, map) {
+        if let Scalar::Ptr(pointer, _) = value {
+            let (alloc_id, _) = pointer.into_parts();
+            match tcx.global_alloc(alloc_id) {
+                // Do not propagate pointers to functions and vtables as they may
+                // lose identify during codegen, which is a miscompilation.
+                GlobalAlloc::Function(_) | GlobalAlloc::VTable(..) => return None,
+                GlobalAlloc::Memory(_) | GlobalAlloc::Static(_) => {}
             }
+        }
+        Some(value)
+    } else {
+        None
+    }
+}
 
-            // Unsupported for now.
-            ty::Array(_, _) => None,
-
-            ty::Tuple(elem_tys) => {
-                let branches = elem_tys
-                    .iter()
-                    .enumerate()
-                    .map(|(i, ty)| {
-                        let field = map.apply(place, TrackElem::Field(FieldIdx::from_usize(i)))?;
-                        self.try_make_valtree(field, ty, state, map)
-                    })
-                    .collect::<Option<Vec<_>>>()?;
-                Some(ty::ValTree::Branch(tcx.arena.alloc_from_iter(branches.into_iter())))
+#[instrument(level = "trace", skip(ecx, state, map))]
+fn try_write_constant<'tcx>(
+    ecx: &mut InterpCx<'_, 'tcx, DummyMachine>,
+    dest: &PlaceTy<'tcx>,
+    place: PlaceIndex,
+    ty: Ty<'tcx>,
+    state: &State<FlatSet<Scalar>>,
+    map: &Map,
+) -> InterpResult<'tcx> {
+    let layout = ecx.layout_of(ty)?;
+    match ty.kind() {
+        // ZSTs. Nothing to do.
+        ty::FnDef(..) => {}
+
+        // Scalars.
+        _ if layout.abi.is_scalar() => {
+            let value = propagatable_scalar(*ecx.tcx, place, state, map).ok_or(err_inval!(ConstPropNonsense))?;
+            ecx.write_immediate(Immediate::Scalar(value), dest)?;
+        }
+        // Those are scalars, must be handled above.
+        ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => bug!(),
+
+        ty::Tuple(elem_tys) => {
+            for (i, elem) in elem_tys.iter().enumerate() {
+                let field = map.apply(place, TrackElem::Field(FieldIdx::from_usize(i))).ok_or(err_inval!(ConstPropNonsense))?;
+                let field_dest = ecx.project_field(dest, i)?;
+                try_write_constant(ecx, &field_dest, field, elem, state, map)?;
             }
+        }
 
-            ty::Adt(def, args) => {
-                if def.is_union() {
-                    return None;
-                }
+        ty::Adt(def, args) => {
+            if def.is_union() {
+                throw_inval!(ConstPropNonsense)
+            }
 
-                let (variant_idx, variant_def, variant_place) = if def.is_enum() {
-                    let discr = map.apply(place, TrackElem::Discriminant)?;
-                    let FlatSet::Elem(Scalar::Int(discr)) = state.get_idx(discr, map) else {
-                        return None;
-                    };
-                    let discr_bits = discr.assert_bits(discr.size());
-                    let (variant, _) =
-                        def.discriminants(tcx).find(|(_, var)| discr_bits == var.val)?;
-                    let variant_place = map.apply(place, TrackElem::Variant(variant))?;
-                    let variant_int = ty::ValTree::Leaf(variant.as_u32().into());
-                    (Some(variant_int), def.variant(variant), variant_place)
-                } else {
-                    (None, def.non_enum_variant(), place)
+            let (variant_idx, variant_def, variant_place, variant_dest) = if def.is_enum() {
+                let discr = map.apply(place, TrackElem::Discriminant).ok_or(err_inval!(ConstPropNonsense))?;
+                let FlatSet::Elem(Scalar::Int(discr)) = state.get_idx(discr, map) else {
+                    throw_inval!(ConstPropNonsense)
                 };
-
-                let branches = variant_def
-                    .fields
-                    .iter_enumerated()
-                    .map(|(i, field)| {
-                        let ty = field.ty(tcx, args);
-                        let field = map.apply(variant_place, TrackElem::Field(i))?;
-                        self.try_make_valtree(field, ty, state, map)
-                    })
-                    .collect::<Option<Vec<_>>>()?;
-                Some(ty::ValTree::Branch(
-                    tcx.arena.alloc_from_iter(variant_idx.into_iter().chain(branches)),
-                ))
+                let discr_bits = discr.assert_bits(discr.size());
+                let (variant, _) = def.discriminants(*ecx.tcx).find(|(_, var)| discr_bits == var.val).ok_or(err_inval!(ConstPropNonsense))?;
+                let variant_place = map.apply(place, TrackElem::Variant(variant)).ok_or(err_inval!(ConstPropNonsense))?;
+                let variant_dest = ecx.project_downcast(dest, variant)?;
+                (variant, def.variant(variant), variant_place, variant_dest)
+            } else {
+                (FIRST_VARIANT, def.non_enum_variant(), place, dest.clone())
+            };
+
+            for (i, field) in variant_def.fields.iter_enumerated() {
+                let ty = field.ty(*ecx.tcx, args);
+                let field = map.apply(variant_place, TrackElem::Field(i)).ok_or(err_inval!(ConstPropNonsense))?;
+                let field_dest = ecx.project_field(&variant_dest, i.as_usize())?;
+                try_write_constant(ecx, &field_dest, field, ty, state, map)?;
             }
+            ecx.write_discriminant(variant_idx, dest)?;
+        }
+
+        // Unsupported for now.
+        ty::Array(_, _)
 
-            // Do not attempt to support indirection in constants.
-            ty::Ref(..) | ty::RawPtr(..) | ty::FnPtr(..) | ty::Str | ty::Slice(_) => None,
-
-            ty::Never
-            | ty::Foreign(..)
-            | ty::Alias(..)
-            | ty::Param(_)
-            | ty::Bound(..)
-            | ty::Placeholder(..)
-            | ty::Closure(..)
-            | ty::Generator(..)
-            | ty::Dynamic(..) => None,
-
-            ty::Error(_)
-            | ty::Infer(..)
-            | ty::GeneratorWitness(..)
-            | ty::GeneratorWitnessMIR(..) => bug!(),
+        // Do not attempt to support indirection in constants.
+        | ty::Ref(..) | ty::RawPtr(..) | ty::FnPtr(..) | ty::Str | ty::Slice(_)
+
+        | ty::Never
+        | ty::Foreign(..)
+        | ty::Alias(..)
+        | ty::Param(_)
+        | ty::Bound(..)
+        | ty::Placeholder(..)
+        | ty::Closure(..)
+        | ty::Generator(..)
+        | ty::Dynamic(..) => throw_inval!(ConstPropNonsense),
+
+        ty::Error(_) | ty::Infer(..) | ty::GeneratorWitness(..) | ty::GeneratorWitnessMIR(..) => {
+            bug!()
         }
     }
+
+    Ok(())
 }
 
 impl<'mir, 'tcx>
@@ -667,8 +689,13 @@ impl<'mir, 'tcx>
     ) {
         match &statement.kind {
             StatementKind::Assign(box (_, rvalue)) => {
-                OperandCollector { state, visitor: self, map: &results.analysis.0.map }
-                    .visit_rvalue(rvalue, location);
+                OperandCollector {
+                    state,
+                    visitor: self,
+                    ecx: &mut results.analysis.0.ecx,
+                    map: &results.analysis.0.map,
+                }
+                .visit_rvalue(rvalue, location);
             }
             _ => (),
         }
@@ -686,7 +713,12 @@ impl<'mir, 'tcx>
                 // Don't overwrite the assignment if it already uses a constant (to keep the span).
             }
             StatementKind::Assign(box (place, _)) => {
-                if let Some(value) = self.try_make_constant(place, state, &results.analysis.0.map) {
+                if let Some(value) = self.try_make_constant(
+                    &mut results.analysis.0.ecx,
+                    place,
+                    state,
+                    &results.analysis.0.map,
+                ) {
                     self.patch.assignments.insert(location, value);
                 }
             }
@@ -701,8 +733,13 @@ impl<'mir, 'tcx>
         terminator: &'mir Terminator<'tcx>,
         location: Location,
     ) {
-        OperandCollector { state, visitor: self, map: &results.analysis.0.map }
-            .visit_terminator(terminator, location);
+        OperandCollector {
+            state,
+            visitor: self,
+            ecx: &mut results.analysis.0.ecx,
+            map: &results.analysis.0.map,
+        }
+        .visit_terminator(terminator, location);
     }
 }
 
@@ -757,6 +794,7 @@ impl<'tcx> MutVisitor<'tcx> for Patch<'tcx> {
 struct OperandCollector<'tcx, 'map, 'locals, 'a> {
     state: &'a State<FlatSet<Scalar>>,
     visitor: &'a mut Collector<'tcx, 'locals>,
+    ecx: &'map mut InterpCx<'tcx, 'tcx, DummyMachine>,
     map: &'map Map,
 }
 
@@ -769,15 +807,17 @@ impl<'tcx> Visitor<'tcx> for OperandCollector<'tcx, '_, '_, '_> {
         location: Location,
     ) {
         if let PlaceElem::Index(local) = elem
-            && let Some(value) = self.visitor.try_make_constant(local.into(), self.state, self.map)
+            && let Some(value) = self.visitor.try_make_constant(self.ecx, local.into(), self.state, self.map)
         {
             self.visitor.patch.before_effect.insert((location, local.into()), value);
         }
     }
 
     fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
         if let Some(place) = operand.place() {
-            if let Some(value) = self.visitor.try_make_constant(place, self.state, self.map) {
+            if let Some(value) =
+                self.visitor.try_make_constant(self.ecx, place, self.state, self.map)
+            {
                 self.visitor.patch.before_effect.insert((location, place), value);
             } else if !place.projection.is_empty() {
                 // Try to propagate into `Index` projections.
@@ -802,8 +842,9 @@ impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for Dumm
     }
 
     fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool {
-        unimplemented!()
+        false
     }
+
     fn alignment_check_failed(
         _ecx: &InterpCx<'mir, 'tcx, Self>,
         _has: Align,

tests/mir-opt/const_debuginfo.main.ConstDebugInfo.diff

@@ -42,7 +42,7 @@
                               let _10: std::option::Option<u16>;
                               scope 7 {
 -                                 debug o => _10;
-+                                 debug o => const Option::<u16>::Some(99);
++                                 debug o => const Option::<u16>::Some(99_u16);
                                   let _17: u32;
                                   let _18: u32;
                                   scope 8 {
@@ -82,7 +82,7 @@
           _15 = const false;
           _16 = const 123_u32;
           StorageLive(_10);
-          _10 = const Option::<u16>::Some(99);
+          _10 = const Option::<u16>::Some(99_u16);
           _17 = const 32_u32;
           _18 = const 32_u32;
           StorageLive(_11);
@@ -98,3 +98,7 @@
       }
   }
 
+  alloc10 (size: 4, align: 2) {
+      01 00 63 00                                     │ ..c.
+  }
+  

tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.panic-abort.diff

@@ -43,7 +43,7 @@
 -         _6 = CheckedAdd(_4, _5);
 -         assert(!move (_6.1: bool), "attempt to compute `{} + {}`, which would overflow", move _4, move _5) -> [success: bb1, unwind unreachable];
 +         _5 = const 2_i32;
-+         _6 = const (3, false);
++         _6 = const (3_i32, false);
 +         assert(!const false, "attempt to compute `{} + {}`, which would overflow", const 1_i32, const 2_i32) -> [success: bb1, unwind unreachable];
       }
 
@@ -76,5 +76,13 @@
           StorageDead(_1);
           return;
       }
++ }
++ 
++ alloc5 (size: 8, align: 4) {
++     00 00 00 80 01 __ __ __                         │ .....░░░
++ }
++ 
++ alloc4 (size: 8, align: 4) {
++     03 00 00 00 00 __ __ __                         │ .....░░░
   }
 

tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.panic-unwind.diff

@@ -43,7 +43,7 @@
 -         _6 = CheckedAdd(_4, _5);
 -         assert(!move (_6.1: bool), "attempt to compute `{} + {}`, which would overflow", move _4, move _5) -> [success: bb1, unwind continue];
 +         _5 = const 2_i32;
-+         _6 = const (3, false);
++         _6 = const (3_i32, false);
 +         assert(!const false, "attempt to compute `{} + {}`, which would overflow", const 1_i32, const 2_i32) -> [success: bb1, unwind continue];
       }
 
@@ -76,5 +76,13 @@
           StorageDead(_1);
           return;
       }
++ }
++ 
++ alloc5 (size: 8, align: 4) {
++     00 00 00 80 01 __ __ __                         │ .....░░░
++ }
++ 
++ alloc4 (size: 8, align: 4) {
++     03 00 00 00 00 __ __ __                         │ .....░░░
   }