Use constant eval to do strict validity checks

5225225 · 5225225 · commit b36163893625 · 2022-07-09T17:33:51.000+01:00
diff --git a/Cargo.lock b/Cargo.lock
@@ -3680,6 +3680,7 @@ dependencies = [
  "rustc_arena",
  "rustc_ast",
  "rustc_attr",
+ "rustc_const_eval",
  "rustc_data_structures",
  "rustc_errors",
  "rustc_fs_util",
diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
@@ -675,8 +675,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
             if intrinsic == sym::assert_zero_valid
                 && !layout.might_permit_raw_init(
                     fx,
-                    InitKind::Zero,
-                    fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
+                    InitKind::Zero) {
 
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
@@ -691,8 +690,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
             if intrinsic == sym::assert_uninit_valid
                 && !layout.might_permit_raw_init(
                     fx,
-                    InitKind::Uninit,
-                    fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
+                    InitKind::Uninit) {
 
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
diff --git 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
@@ -549,10 +549,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             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),
+            let do_panic = match (&intrinsic, strict_validity) {
+                (Inhabited, _) => layout.abi.is_uninhabited(),
+                (ZeroValid, true) => {
+                    !rustc_const_eval::is_zero_valid(bx.tcx(), source_info.span, layout)
+                }
+                (UninitValid, true) => {
+                    !rustc_const_eval::is_uninit_valid(bx.tcx(), source_info.span, layout)
+                }
+                (ZeroValid, false) => !layout.might_permit_raw_init(bx, InitKind::Zero),
+                (UninitValid, false) => !layout.might_permit_raw_init(bx, InitKind::Uninit),
             };
             if do_panic {
                 let msg_str = with_no_visible_paths!({
diff --git 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
@@ -413,35 +413,41 @@ 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.debugging_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 = if self.tcx.sess.opts.debugging_opts.strict_init_checks {
+                        !crate::is_zero_valid(*self.tcx, self.cur_span(), layout)
+                    } else {
+                        !layout.might_permit_raw_init(self, InitKind::Zero)
+                    };
+
+                    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.debugging_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 = if self.tcx.sess.opts.debugging_opts.strict_init_checks {
+                        !crate::is_uninit_valid(*self.tcx, self.cur_span(), layout)
+                    } else {
+                        !layout.might_permit_raw_init(self, InitKind::Uninit)
+                    };
+
+                    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
@@ -60,3 +60,41 @@ pub fn provide(providers: &mut Providers) {
         const_eval::deref_mir_constant(tcx, param_env, value)
     };
 }
+
+use crate::const_eval::CompileTimeInterpreter;
+use crate::interpret::{InterpCx, MemoryKind, OpTy};
+use rustc_middle::ty::{layout::TyAndLayout, ParamEnv, TyCtxt};
+use rustc_session::Limit;
+use rustc_span::Span;
+
+pub fn is_uninit_valid<'tcx>(tcx: TyCtxt<'tcx>, root_span: Span, ty: TyAndLayout<'tcx>) -> bool {
+    let machine = CompileTimeInterpreter::new(Limit::new(0), false);
+    let mut cx = InterpCx::new(tcx, root_span, ParamEnv::reveal_all(), machine);
+    let allocated = cx
+        .allocate(ty, MemoryKind::Machine(const_eval::MemoryKind::Heap))
+        .expect("failed to allocate for uninit check");
+    let ot: OpTy<'_, _> = allocated.into();
+    cx.validate_operand(&ot).is_ok()
+}
+
+pub fn is_zero_valid<'tcx>(tcx: TyCtxt<'tcx>, root_span: Span, ty: TyAndLayout<'tcx>) -> bool {
+    let machine = CompileTimeInterpreter::new(Limit::new(0), false);
+
+    let mut cx = InterpCx::new(tcx, root_span, ParamEnv::reveal_all(), machine);
+
+    // We could panic here... Or we could just return "yeah it's valid whatever". Or let
+    // codegen_panic_intrinsic return an error that halts compilation.
+    // I'm not exactly sure *when* this can fail. OOM?
+    let allocated = cx
+        .allocate(ty, MemoryKind::Machine(const_eval::MemoryKind::Heap))
+        .expect("failed to allocate for uninit check");
+
+    // Again, unclear what to do here if it fails.
+    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()
+}
diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs
@@ -1494,7 +1494,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
     /// 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
+    pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
     where
         Self: Copy,
         Ty: TyAbiInterface<'a, C>,
@@ -1507,13 +1507,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 +1529,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;
                     }
diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs
@@ -57,6 +57,13 @@ enum LR_NonZero {
 
 struct ZeroSized;
 
+#[allow(dead_code)]
+#[repr(i32)]
+enum ZeroIsValid {
+    Zero(u8) = 0,
+    One(NonNull<()>) = 1,
+}
+
 fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
     let err = panic::catch_unwind(op).err();
     assert_eq!(
@@ -152,33 +159,12 @@ fn main() {
             "attempted to zero-initialize type `*const dyn core::marker::Send`, which is invalid"
         );
 
-        /* FIXME(#66151) we conservatively do not error here yet.
-        test_panic_msg(
-            || mem::uninitialized::<LR_NonZero>(),
-            "attempted to leave type `LR_NonZero` uninitialized, which is invalid"
-        );
-        test_panic_msg(
-            || mem::zeroed::<LR_NonZero>(),
-            "attempted to zero-initialize type `LR_NonZero`, which is invalid"
-        );
-
-        test_panic_msg(
-            || mem::uninitialized::<ManuallyDrop<LR_NonZero>>(),
-            "attempted to leave type `std::mem::ManuallyDrop<LR_NonZero>` uninitialized, \
-             which is invalid"
-        );
-        test_panic_msg(
-            || mem::zeroed::<ManuallyDrop<LR_NonZero>>(),
-            "attempted to zero-initialize type `std::mem::ManuallyDrop<LR_NonZero>`, \
-             which is invalid"
-        );
-        */
-
         test_panic_msg(
             || mem::uninitialized::<(NonNull<u32>, u32, u32)>(),
             "attempted to leave type `(core::ptr::non_null::NonNull<u32>, u32, u32)` uninitialized, \
                 which is invalid"
         );
+
         test_panic_msg(
             || mem::zeroed::<(NonNull<u32>, u32, u32)>(),
             "attempted to zero-initialize type `(core::ptr::non_null::NonNull<u32>, u32, u32)`, \
@@ -196,11 +182,23 @@ fn main() {
                 which is invalid"
         );
 
+        test_panic_msg(
+            || mem::uninitialized::<LR_NonZero>(),
+            "attempted to leave type `LR_NonZero` uninitialized, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<ManuallyDrop<LR_NonZero>>(),
+            "attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR_NonZero>` uninitialized, \
+             which is invalid"
+        );
+
         test_panic_msg(
             || mem::uninitialized::<NoNullVariant>(),
             "attempted to leave type `NoNullVariant` uninitialized, \
                 which is invalid"
         );
+
         test_panic_msg(
             || mem::zeroed::<NoNullVariant>(),
             "attempted to zero-initialize type `NoNullVariant`, \
@@ -212,10 +210,12 @@ fn main() {
             || mem::uninitialized::<bool>(),
             "attempted to leave type `bool` uninitialized, which is invalid"
         );
+
         test_panic_msg(
             || mem::uninitialized::<LR>(),
             "attempted to leave type `LR` uninitialized, which is invalid"
         );
+
         test_panic_msg(
             || mem::uninitialized::<ManuallyDrop<LR>>(),
             "attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR>` uninitialized, which is invalid"
@@ -229,6 +229,7 @@ fn main() {
         let _val = mem::zeroed::<Option<&'static i32>>();
         let _val = mem::zeroed::<MaybeUninit<NonNull<u32>>>();
         let _val = mem::zeroed::<[!; 0]>();
+        let _val = mem::zeroed::<ZeroIsValid>();
         let _val = mem::uninitialized::<MaybeUninit<bool>>();
         let _val = mem::uninitialized::<[!; 0]>();
         let _val = mem::uninitialized::<()>();
@@ -259,12 +260,32 @@ fn main() {
                 || mem::zeroed::<[NonNull<()>; 1]>(),
                 "attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid"
             );
+
+            // FIXME(#66151) we conservatively do not error here yet (by default).
+            test_panic_msg(
+                || mem::zeroed::<LR_NonZero>(),
+                "attempted to zero-initialize type `LR_NonZero`, which is invalid"
+            );
+
+            test_panic_msg(
+                || mem::zeroed::<ManuallyDrop<LR_NonZero>>(),
+                "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop<LR_NonZero>`, \
+                 which is invalid"
+            );
         } else {
             // These are UB because they have not been officially blessed, but we await the resolution
             // of <https://github.com/rust-lang/unsafe-code-guidelines/issues/71> before doing
             // anything about that.
             let _val = mem::uninitialized::<i32>();
             let _val = mem::uninitialized::<*const ()>();
+
+            // These are UB, but best to test them to ensure we don't become unintentionally
+            // stricter.
+
+            // It's currently unchecked to create invalid enums and values inside arrays.
+            let _val = mem::zeroed::<LR_NonZero>();
+            let _val = mem::zeroed::<[LR_NonZero; 1]>();
+            let _val = mem::zeroed::<[NonNull<()>; 1]>();
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {`
`104`	`104`	`}`
`105`	`105`
`106`	`106`	`impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {`
`107`		`- pub(super) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {`
	`107`	`+ pub(crate) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {`
`108`	`108`	`CompileTimeInterpreter {`
`109`	`109`	`steps_remaining: const_eval_limit.0,`
`110`	`110`	`stack: Vec::new(),`