Auto merge of #99033 - 5225225:interpreter-validity-checks, r=oli-obk

Use constant eval to do strict mem::uninit/zeroed validity checks

I'm not sure about the code organisation here, I just dumped the check in rustc_const_eval at the root. Not hard to move it elsewhere, in any case.

Also, this means cranelift codegen intrinsics lose the strict checks, since they don't seem to depend on rustc_const_eval, and I didn't see a point in keeping around two copies.

I also left comments in the is_zero_valid methods about "uhhh help how do i do this", those apply to both methods equally.

Also rustc_codegen_ssa now depends on rustc_const_eval... is this okay?

Pinging `@RalfJung` since you were the one who mentioned this to me, so I'm assuming you're interested.

Haven't had a chance to run full tests on this since it's really warm, and it's 1AM, I'll check out any failures/comments in the morning :)

11 files changed, 116 insertions, 72 deletions

diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
index eafae1cdc8a..4b2207f3758 100644
--- a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
+++ b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
@@ -58,7 +58,6 @@ pub(crate) use llvm::codegen_llvm_intrinsic_call;
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::subst::SubstsRef;
 use rustc_span::symbol::{kw, sym, Symbol};
-use rustc_target::abi::InitKind;
 
 use crate::prelude::*;
 use cranelift_codegen::ir::AtomicRmwOp;
@@ -672,12 +671,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
                 return;
             }
 
-            if intrinsic == sym::assert_zero_valid
-                && !layout.might_permit_raw_init(
-                    fx,
-                    InitKind::Zero,
-                    fx.tcx.sess.opts.unstable_opts.strict_init_checks) {
-
+            if intrinsic == sym::assert_zero_valid && !fx.tcx.permits_zero_init(layout) {
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
                         fx,
@@ -688,12 +682,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
                 return;
             }
 
-            if intrinsic == sym::assert_uninit_valid
-                && !layout.might_permit_raw_init(
-                    fx,
-                    InitKind::Uninit,
-                    fx.tcx.sess.opts.unstable_opts.strict_init_checks) {
-
+            if intrinsic == sym::assert_uninit_valid && !fx.tcx.permits_uninit_init(layout) {
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
                         fx,
diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml
index faabea92f5a..81c8b9ceb13 100644
--- a/compiler/rustc_codegen_ssa/Cargo.toml
+++ b/compiler/rustc_codegen_ssa/Cargo.toml
@@ -40,6 +40,7 @@ rustc_metadata = { path = "../rustc_metadata" }
 rustc_query_system = { path = "../rustc_query_system" }
 rustc_target = { path = "../rustc_target" }
 rustc_session = { path = "../rustc_session" }
+rustc_const_eval = { path = "../rustc_const_eval" }
 
 [dependencies.object]
 version = "0.29.0"
diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
index 745da821c9d..773c55cf551 100644
--- a/compiler/rustc_codegen_ssa/src/mir/block.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -22,7 +22,7 @@ use rustc_span::source_map::Span;
 use rustc_span::{sym, Symbol};
 use rustc_symbol_mangling::typeid_for_fnabi;
 use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
-use rustc_target::abi::{self, HasDataLayout, InitKind, WrappingRange};
+use rustc_target::abi::{self, HasDataLayout, WrappingRange};
 use rustc_target::spec::abi::Abi;
 
 /// Used by `FunctionCx::codegen_terminator` for emitting common patterns
@@ -528,7 +528,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         source_info: mir::SourceInfo,
         target: Option<mir::BasicBlock>,
         cleanup: Option<mir::BasicBlock>,
-        strict_validity: bool,
     ) -> bool {
         // Emit a panic or a no-op for `assert_*` intrinsics.
         // These are intrinsics that compile to panics so that we can get a message
@@ -547,12 +546,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         });
         if let Some(intrinsic) = panic_intrinsic {
             use AssertIntrinsic::*;
+
             let ty = instance.unwrap().substs.type_at(0);
             let layout = bx.layout_of(ty);
             let do_panic = match intrinsic {
                 Inhabited => layout.abi.is_uninhabited(),
-                ZeroValid => !layout.might_permit_raw_init(bx, InitKind::Zero, strict_validity),
-                UninitValid => !layout.might_permit_raw_init(bx, InitKind::Uninit, strict_validity),
+                ZeroValid => !bx.tcx().permits_zero_init(layout),
+                UninitValid => !bx.tcx().permits_uninit_init(layout),
             };
             if do_panic {
                 let msg_str = with_no_visible_paths!({
@@ -687,7 +687,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             source_info,
             target,
             cleanup,
-            self.cx.tcx().sess.opts.unstable_opts.strict_init_checks,
         ) {
             return;
         }
diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs
index 29ab1d18771..e00e667fb71 100644
--- a/compiler/rustc_const_eval/src/const_eval/machine.rs
+++ b/compiler/rustc_const_eval/src/const_eval/machine.rs
@@ -104,7 +104,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
 }
 
 impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
-    pub(super) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
+    pub(crate) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
         CompileTimeInterpreter {
             steps_remaining: const_eval_limit.0,
             stack: Vec::new(),
diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs
index 3039b10373d..242e3879e3a 100644
--- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs
+++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs
@@ -15,7 +15,7 @@ use rustc_middle::ty::layout::LayoutOf as _;
 use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_span::symbol::{sym, Symbol};
-use rustc_target::abi::{Abi, Align, InitKind, Primitive, Size};
+use rustc_target::abi::{Abi, Align, Primitive, Size};
 
 use super::{
     util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
@@ -411,35 +411,33 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         ),
                     )?;
                 }
-                if intrinsic_name == sym::assert_zero_valid
-                    && !layout.might_permit_raw_init(
-                        self,
-                        InitKind::Zero,
-                        self.tcx.sess.opts.unstable_opts.strict_init_checks,
-                    )
-                {
-                    M::abort(
-                        self,
-                        format!(
-                            "aborted execution: attempted to zero-initialize type `{}`, which is invalid",
-                            ty
-                        ),
-                    )?;
+
+                if intrinsic_name == sym::assert_zero_valid {
+                    let should_panic = !self.tcx.permits_zero_init(layout);
+
+                    if should_panic {
+                        M::abort(
+                            self,
+                            format!(
+                                "aborted execution: attempted to zero-initialize type `{}`, which is invalid",
+                                ty
+                            ),
+                        )?;
+                    }
                 }
-                if intrinsic_name == sym::assert_uninit_valid
-                    && !layout.might_permit_raw_init(
-                        self,
-                        InitKind::Uninit,
-                        self.tcx.sess.opts.unstable_opts.strict_init_checks,
-                    )
-                {
-                    M::abort(
-                        self,
-                        format!(
-                            "aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
-                            ty
-                        ),
-                    )?;
+
+                if intrinsic_name == sym::assert_uninit_valid {
+                    let should_panic = !self.tcx.permits_uninit_init(layout);
+
+                    if should_panic {
+                        M::abort(
+                            self,
+                            format!(
+                                "aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
+                                ty
+                            ),
+                        )?;
+                    }
                 }
             }
             sym::simd_insert => {
diff --git a/compiler/rustc_const_eval/src/lib.rs b/compiler/rustc_const_eval/src/lib.rs
index bba3e99f7b1..1063f03d0e2 100644
--- a/compiler/rustc_const_eval/src/lib.rs
+++ b/compiler/rustc_const_eval/src/lib.rs
@@ -33,11 +33,13 @@ extern crate rustc_middle;
 pub mod const_eval;
 mod errors;
 pub mod interpret;
+mod might_permit_raw_init;
 pub mod transform;
 pub mod util;
 
 use rustc_middle::ty;
 use rustc_middle::ty::query::Providers;
+use rustc_target::abi::InitKind;
 
 pub fn provide(providers: &mut Providers) {
     const_eval::provide(providers);
@@ -59,4 +61,8 @@ pub fn provide(providers: &mut Providers) {
         let (param_env, value) = param_env_and_value.into_parts();
         const_eval::deref_mir_constant(tcx, param_env, value)
     };
+    providers.permits_uninit_init =
+        |tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Uninit);
+    providers.permits_zero_init =
+        |tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Zero);
 }
diff --git a/compiler/rustc_const_eval/src/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/might_permit_raw_init.rs
new file mode 100644
index 00000000000..f971c2238c7
--- /dev/null
+++ b/compiler/rustc_const_eval/src/might_permit_raw_init.rs
@@ -0,0 +1,40 @@
+use crate::const_eval::CompileTimeInterpreter;
+use crate::interpret::{InterpCx, MemoryKind, OpTy};
+use rustc_middle::ty::layout::LayoutCx;
+use rustc_middle::ty::{layout::TyAndLayout, ParamEnv, TyCtxt};
+use rustc_session::Limit;
+use rustc_target::abi::InitKind;
+
+pub fn might_permit_raw_init<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    ty: TyAndLayout<'tcx>,
+    kind: InitKind,
+) -> bool {
+    let strict = tcx.sess.opts.unstable_opts.strict_init_checks;
+
+    if strict {
+        let machine = CompileTimeInterpreter::new(Limit::new(0), false);
+
+        let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);
+
+        let allocated = cx
+            .allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
+            .expect("OOM: failed to allocate for uninit check");
+
+        if kind == InitKind::Zero {
+            cx.write_bytes_ptr(
+                allocated.ptr,
+                std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()),
+            )
+            .expect("failed to write bytes for zero valid check");
+        }
+
+        let ot: OpTy<'_, _> = allocated.into();
+
+        // Assume that if it failed, it's a validation failure.
+        cx.validate_operand(&ot).is_ok()
+    } else {
+        let layout_cx = LayoutCx { tcx, param_env: ParamEnv::reveal_all() };
+        ty.might_permit_raw_init(&layout_cx, kind)
+    }
+}
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
index bdae7e5fcd6..0581ef41f66 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -2053,4 +2053,12 @@ rustc_queries! {
         desc { |tcx| "looking up generator diagnostic data of `{}`", tcx.def_path_str(key) }
         separate_provide_extern
     }
+
+    query permits_uninit_init(key: TyAndLayout<'tcx>) -> bool {
+        desc { "checking to see if {:?} permits being left uninit", key.ty }
+    }
+
+    query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
+        desc { "checking to see if {:?} permits being left zeroed", key.ty }
+    }
 }
diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs
index 3d662ed5de4..2452bcf6a61 100644
--- a/compiler/rustc_middle/src/ty/query.rs
+++ b/compiler/rustc_middle/src/ty/query.rs
@@ -28,6 +28,7 @@ use crate::traits::query::{
 use crate::traits::specialization_graph;
 use crate::traits::{self, ImplSource};
 use crate::ty::fast_reject::SimplifiedType;
+use crate::ty::layout::TyAndLayout;
 use crate::ty::subst::{GenericArg, SubstsRef};
 use crate::ty::util::AlwaysRequiresDrop;
 use crate::ty::GeneratorDiagnosticData;
diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs
index 6fbafeb1d32..54774314313 100644
--- a/compiler/rustc_query_impl/src/keys.rs
+++ b/compiler/rustc_query_impl/src/keys.rs
@@ -6,7 +6,7 @@ use rustc_middle::mir;
 use rustc_middle::traits;
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::subst::{GenericArg, SubstsRef};
-use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt};
 use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::{Span, DUMMY_SP};
 
@@ -385,6 +385,16 @@ impl<'tcx> Key for Ty<'tcx> {
     }
 }
 
+impl<'tcx> Key for TyAndLayout<'tcx> {
+    #[inline(always)]
+    fn query_crate_is_local(&self) -> bool {
+        true
+    }
+    fn default_span(&self, _: TyCtxt<'_>) -> Span {
+        DUMMY_SP
+    }
+}
+
 impl<'tcx> Key for (Ty<'tcx>, Ty<'tcx>) {
     #[inline(always)]
     fn query_crate_is_local(&self) -> bool {
diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs
index d1eafd6ac5f..6f4d073d704 100644
--- a/compiler/rustc_target/src/abi/mod.rs
+++ b/compiler/rustc_target/src/abi/mod.rs
@@ -1372,7 +1372,7 @@ pub struct PointeeInfo {
 
 /// Used in `might_permit_raw_init` to indicate the kind of initialisation
 /// that is checked to be valid
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum InitKind {
     Zero,
     Uninit,
@@ -1487,14 +1487,18 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
     ///
     /// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
     ///
-    /// `strict` is an opt-in debugging flag added in #97323 that enables more checks.
+    /// This code is intentionally conservative, and will not detect
+    /// * zero init of an enum whose 0 variant does not allow zero initialization
+    /// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
+    /// * Any form of invalid value being made inside an array (unless the value is uninhabited)
     ///
-    /// This is conservative: in doubt, it will answer `true`.
+    /// A strict form of these checks that uses const evaluation exists in
+    /// `rustc_const_eval::might_permit_raw_init`, and a tracking issue for making these checks
+    /// stricter is <https://github.com/rust-lang/rust/issues/66151>.
     ///
-    /// FIXME: Once we removed all the conservatism, we could alternatively
-    /// create an all-0/all-undef constant and run the const value validator to see if
-    /// this is a valid value for the given type.
-    pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
+    /// FIXME: Once all the conservatism is removed from here, and the checks are ran by default,
+    /// we can use the const evaluation checks always instead.
+    pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
     where
         Self: Copy,
         Ty: TyAbiInterface<'a, C>,
@@ -1507,13 +1511,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
                     s.valid_range(cx).contains(0)
                 }
                 InitKind::Uninit => {
-                    if strict {
-                        // The type must be allowed to be uninit (which means "is a union").
-                        s.is_uninit_valid()
-                    } else {
-                        // The range must include all values.
-                        s.is_always_valid(cx)
-                    }
+                    // The range must include all values.
+                    s.is_always_valid(cx)
                 }
             }
         };
@@ -1534,19 +1533,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
         // If we have not found an error yet, we need to recursively descend into fields.
         match &self.fields {
             FieldsShape::Primitive | FieldsShape::Union { .. } => {}
-            FieldsShape::Array { count, .. } => {
+            FieldsShape::Array { .. } => {
                 // FIXME(#66151): For now, we are conservative and do not check arrays by default.
-                if strict
-                    && *count > 0
-                    && !self.field(cx, 0).might_permit_raw_init(cx, init_kind, strict)
-                {
-                    // Found non empty array with a type that is unhappy about this kind of initialization
-                    return false;
-                }
             }
             FieldsShape::Arbitrary { offsets, .. } => {
                 for idx in 0..offsets.len() {
-                    if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
+                    if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
                         // We found a field that is unhappy with this kind of initialization.
                         return false;
                     }