Rollup merge of rust-lang#97323 - 5225225:strict_init_checks, r=oli-obk

Introduce stricter checks for might_permit_raw_init under a debug flag

This is intended to be a version of the strict checks tried out in rust-lang#79296, but also checking number validity (under the assumption that `let _ = std::mem::uninitialized::<u32>()` is UB, which seems to be what rust-lang/unsafe-code-guidelines#71 is leaning towards.)

Author: Dylan-DPC

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

@@ -58,6 +58,7 @@ 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;
@@ -671,7 +672,12 @@ fn codegen_regular_intrinsic_call<'tcx>(
                 return;
             }
 
-            if intrinsic == sym::assert_zero_valid && !layout.might_permit_raw_init(fx, /*zero:*/ true) {
+            if intrinsic == sym::assert_zero_valid
+                && !layout.might_permit_raw_init(
+                    fx,
+                    InitKind::Zero,
+                    fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
+
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
                         fx,
@@ -682,7 +688,12 @@ fn codegen_regular_intrinsic_call<'tcx>(
                 return;
             }
 
-            if intrinsic == sym::assert_uninit_valid && !layout.might_permit_raw_init(fx, /*zero:*/ false) {
+            if intrinsic == sym::assert_uninit_valid
+                && !layout.might_permit_raw_init(
+                    fx,
+                    InitKind::Uninit,
+                    fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
+
                 with_no_trimmed_paths!({
                     crate::base::codegen_panic(
                         fx,

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, WrappingRange};
+use rustc_target::abi::{self, HasDataLayout, InitKind, WrappingRange};
 use rustc_target::spec::abi::Abi;
 
 /// Used by `FunctionCx::codegen_terminator` for emitting common patterns
@@ -521,6 +521,7 @@ 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
@@ -543,8 +544,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             let layout = bx.layout_of(ty);
             let do_panic = match intrinsic {
                 Inhabited => layout.abi.is_uninhabited(),
-                ZeroValid => !layout.might_permit_raw_init(bx, /*zero:*/ true),
-                UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false),
+                ZeroValid => !layout.might_permit_raw_init(bx, InitKind::Zero, strict_validity),
+                UninitValid => !layout.might_permit_raw_init(bx, InitKind::Uninit, strict_validity),
             };
             if do_panic {
                 let msg_str = with_no_visible_paths!({
@@ -678,6 +679,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             source_info,
             target,
             cleanup,
+            self.cx.tcx().sess.opts.debugging_opts.strict_init_checks,
         ) {
             return;
         }

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, Primitive, Size};
+use rustc_target::abi::{Abi, Align, InitKind, Primitive, Size};
 
 use super::{
     util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
@@ -408,7 +408,11 @@ 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, /*zero:*/ true)
+                    && !layout.might_permit_raw_init(
+                        self,
+                        InitKind::Zero,
+                        self.tcx.sess.opts.debugging_opts.strict_init_checks,
+                    )
                 {
                     M::abort(
                         self,
@@ -419,7 +423,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     )?;
                 }
                 if intrinsic_name == sym::assert_uninit_valid
-                    && !layout.might_permit_raw_init(self, /*zero:*/ false)
+                    && !layout.might_permit_raw_init(
+                        self,
+                        InitKind::Uninit,
+                        self.tcx.sess.opts.debugging_opts.strict_init_checks,
+                    )
                 {
                     M::abort(
                         self,

compiler/rustc_session/src/options.rs

@@ -1495,6 +1495,8 @@ options! {
         "hash algorithm of source files in debug info (`md5`, `sha1`, or `sha256`)"),
     stack_protector: StackProtector = (StackProtector::None, parse_stack_protector, [TRACKED],
         "control stack smash protection strategy (`rustc --print stack-protector-strategies` for details)"),
+    strict_init_checks: bool = (false, parse_bool, [TRACKED],
+        "control if mem::uninitialized and mem::zeroed panic on more UB"),
     strip: Strip = (Strip::None, parse_strip, [UNTRACKED],
         "tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
     split_dwarf_kind: SplitDwarfKind = (SplitDwarfKind::Split, parse_split_dwarf_kind, [UNTRACKED],

compiler/rustc_target/src/abi/mod.rs

@@ -894,6 +894,15 @@ impl Scalar {
             Scalar::Union { .. } => true,
         }
     }
+
+    /// Returns `true` if this type can be left uninit.
+    #[inline]
+    pub fn is_uninit_valid(&self) -> bool {
+        match *self {
+            Scalar::Initialized { .. } => false,
+            Scalar::Union { .. } => true,
+        }
+    }
 }
 
 /// Describes how the fields of a type are located in memory.
@@ -1355,6 +1364,14 @@ pub struct PointeeInfo {
     pub address_space: AddressSpace,
 }
 
+/// Used in `might_permit_raw_init` to indicate the kind of initialisation
+/// that is checked to be valid
+#[derive(Copy, Clone, Debug)]
+pub enum InitKind {
+    Zero,
+    Uninit,
+}
+
 /// Trait that needs to be implemented by the higher-level type representation
 /// (e.g. `rustc_middle::ty::Ty`), to provide `rustc_target::abi` functionality.
 pub trait TyAbiInterface<'a, C>: Sized {
@@ -1461,26 +1478,37 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
 
     /// Determines if this type permits "raw" initialization by just transmuting some
     /// memory into an instance of `T`.
-    /// `zero` indicates if the memory is zero-initialized, or alternatively
-    /// left entirely uninitialized.
+    ///
+    /// `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 is conservative: in doubt, it will answer `true`.
     ///
     /// 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, zero: bool) -> bool
+    pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
     where
         Self: Copy,
         Ty: TyAbiInterface<'a, C>,
         C: HasDataLayout,
     {
         let scalar_allows_raw_init = move |s: Scalar| -> bool {
-            if zero {
-                // The range must contain 0.
-                s.valid_range(cx).contains(0)
-            } else {
-                // The range must include all values.
-                s.is_always_valid(cx)
+            match init_kind {
+                InitKind::Zero => {
+                    // The range must contain 0.
+                    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)
+                    }
+                }
             }
         };
 
@@ -1500,12 +1528,19 @@ 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 { .. } => {
-                // FIXME(#66151): For now, we are conservative and do not check arrays.
+            FieldsShape::Array { count, .. } => {
+                // 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, zero) {
+                    if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
                         // We found a field that is unhappy with this kind of initialization.
                         return false;
                     }

src/test/ui/intrinsics/panic-uninitialized-zeroed.rs

@@ -1,8 +1,9 @@
 // run-pass
 // needs-unwind
 // ignore-wasm32-bare compiled with panic=abort by default
-// revisions: mir thir
+// revisions: mir thir strict
 // [thir]compile-flags: -Zthir-unsafeck
+// [strict]compile-flags: -Zstrict-init-checks
 // ignore-tidy-linelength
 
 // This test checks panic emitted from `mem::{uninitialized,zeroed}`.
@@ -54,6 +55,8 @@ enum LR_NonZero {
     Right(num::NonZeroI64),
 }
 
+struct ZeroSized;
+
 fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
     let err = panic::catch_unwind(op).err();
     assert_eq!(
@@ -228,11 +231,40 @@ fn main() {
         let _val = mem::zeroed::<[!; 0]>();
         let _val = mem::uninitialized::<MaybeUninit<bool>>();
         let _val = mem::uninitialized::<[!; 0]>();
+        let _val = mem::uninitialized::<()>();
+        let _val = mem::uninitialized::<ZeroSized>();
+
+        if cfg!(strict) {
+            test_panic_msg(
+                || mem::uninitialized::<i32>(),
+                "attempted to leave type `i32` uninitialized, which is invalid"
+            );
+
+            test_panic_msg(
+                || mem::uninitialized::<*const ()>(),
+                "attempted to leave type `*const ()` uninitialized, which is invalid"
+            );
+
+            test_panic_msg(
+                || mem::uninitialized::<[i32; 1]>(),
+                "attempted to leave type `[i32; 1]` uninitialized, which is invalid"
+            );
+
+            test_panic_msg(
+                || mem::zeroed::<NonNull<()>>(),
+                "attempted to zero-initialize type `core::ptr::non_null::NonNull<()>`, which is invalid"
+            );
 
-        // 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 ()>();
+            test_panic_msg(
+                || mem::zeroed::<[NonNull<()>; 1]>(),
+                "attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, 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 ()>();
+        }
     }
 }