Make uninit checks stricter but avoid issues for old hyper

5225225 · 5225225 · commit 85545dde6ca2 · 2022-08-01T20:33:21.000+01:00
diff --git a/compiler/rustc_const_eval/src/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/might_permit_raw_init.rs
@@ -1,7 +1,10 @@
 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_middle::ty::{
+    layout::{self, TyAndLayout},
+    ParamEnv, TyCtxt,
+};
 use rustc_session::Limit;
 use rustc_target::abi::InitKind;
 
@@ -12,7 +15,7 @@ pub fn might_permit_raw_init<'tcx>(
 ) -> bool {
     let strict = tcx.sess.opts.unstable_opts.strict_init_checks;
 
-    if strict {
+    if strict || kind == InitKind::Zero {
         let machine = CompileTimeInterpreter::new(Limit::new(0), false);
 
         let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);
@@ -35,6 +38,6 @@ pub fn might_permit_raw_init<'tcx>(
         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)
+        layout::might_permit_raw_init(ty, &layout_cx)
     }
 }
diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs
@@ -3492,3 +3492,86 @@ fn make_thin_self_ptr<'tcx>(
         ..tcx.layout_of(ty::ParamEnv::reveal_all().and(unit_ptr_ty)).unwrap()
     }
 }
+
+/// Determines if this type permits "raw" initialization by just transmuting some
+/// uninitialized memory into an instance of `T`.
+///
+/// This code is intentionally conservative, and will not detect
+/// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
+/// * uninit invalid &[T] where T has align 1 (only inside arrays)
+///
+/// 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 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<'tcx, C>(this: TyAndLayout<'tcx>, cx: &C) -> bool
+where
+    C: HasDataLayout + ty::layout::HasParamEnv<'tcx> + ty::layout::HasTyCtxt<'tcx>,
+{
+    return might_permit_raw_init_inner(this, cx, false);
+
+    fn might_permit_raw_init_inner<'tcx, C>(
+        this: TyAndLayout<'tcx>,
+        cx: &C,
+        inside_array: bool,
+    ) -> bool
+    where
+        C: HasDataLayout + ty::layout::HasParamEnv<'tcx> + ty::layout::HasTyCtxt<'tcx>,
+    {
+        if inside_array {
+            if let ty::Ref(_, inner, _) = this.ty.kind() {
+                if let ty::Slice(inner) = inner.kind() {
+                    let penv = ty::ParamEnv::reveal_all().and(*inner);
+                    if let Ok(l) = cx.tcx().layout_of(penv) {
+                        return l.layout.align().abi == Align::ONE;
+                    }
+                }
+
+                if let ty::Str = inner.kind() {
+                    return true;
+                }
+            }
+        }
+
+        // Check the ABI.
+        let valid = match this.abi {
+            Abi::Uninhabited => false, // definitely UB
+            Abi::Scalar(s) => s.is_always_valid(cx),
+            Abi::ScalarPair(s1, s2) => s1.is_always_valid(cx) && s2.is_always_valid(cx),
+            Abi::Vector { element: s, count } => count == 0 || s.is_always_valid(cx),
+            Abi::Aggregate { .. } => true, // Fields are checked below.
+        };
+        if !valid {
+            // This is definitely not okay.
+            return false;
+        }
+
+        // If we have not found an error yet, we need to recursively descend into fields.
+        match &this.fields {
+            FieldsShape::Primitive | FieldsShape::Union { .. } => {}
+            FieldsShape::Array { count, .. } => {
+                if *count > 0 && !might_permit_raw_init_inner(this.field(cx, 0), cx, true) {
+                    // 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 !might_permit_raw_init_inner(this.field(cx, idx), cx, inside_array) {
+                        // We found a field that is unhappy with this kind of initialization.
+                        return false;
+                    }
+                }
+            }
+        }
+
+        if matches!(this.variants, Variants::Multiple { .. }) {
+            // All uninit enums are automatically invalid, even if their discriminant includes all values.
+            return false;
+        }
+
+        true
+    }
+}
diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs
@@ -1485,72 +1485,4 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
             Abi::Aggregate { sized } => sized && self.size.bytes() == 0,
         }
     }
-
-    /// Determines if this type permits "raw" initialization by just transmuting some
-    /// memory into an instance of `T`.
-    ///
-    /// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
-    ///
-    /// 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)
-    ///
-    /// 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 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>,
-        C: HasDataLayout,
-    {
-        let scalar_allows_raw_init = move |s: Scalar| -> bool {
-            match init_kind {
-                InitKind::Zero => {
-                    // The range must contain 0.
-                    s.valid_range(cx).contains(0)
-                }
-                InitKind::Uninit => {
-                    // The range must include all values.
-                    s.is_always_valid(cx)
-                }
-            }
-        };
-
-        // Check the ABI.
-        let valid = match self.abi {
-            Abi::Uninhabited => false, // definitely UB
-            Abi::Scalar(s) => scalar_allows_raw_init(s),
-            Abi::ScalarPair(s1, s2) => scalar_allows_raw_init(s1) && scalar_allows_raw_init(s2),
-            Abi::Vector { element: s, count } => count == 0 || scalar_allows_raw_init(s),
-            Abi::Aggregate { .. } => true, // Fields are checked below.
-        };
-        if !valid {
-            // This is definitely not okay.
-            return false;
-        }
-
-        // 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 by default.
-            }
-            FieldsShape::Arbitrary { offsets, .. } => {
-                for idx in 0..offsets.len() {
-                    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;
-                    }
-                }
-            }
-        }
-
-        // FIXME(#66151): For now, we are conservative and do not check `self.variants`.
-        true
-    }
 }
diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs
@@ -64,6 +64,31 @@ enum ZeroIsValid {
     One(NonNull<()>) = 1,
 }
 
+#[rustfmt::skip]
+#[allow(dead_code)]
+enum SoManyVariants {
+    A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17,
+    A18, A19, A20, A21, A22, A23, A24, A25, A26, A27, A28, A29, A30, A31, A32,
+    A33, A34, A35, A36, A37, A38, A39, A40, A41, A42, A43, A44, A45, A46, A47,
+    A48, A49, A50, A51, A52, A53, A54, A55, A56, A57, A58, A59, A60, A61, A62,
+    A63, A64, A65, A66, A67, A68, A69, A70, A71, A72, A73, A74, A75, A76, A77,
+    A78, A79, A80, A81, A82, A83, A84, A85, A86, A87, A88, A89, A90, A91, A92,
+    A93, A94, A95, A96, A97, A98, A99, A100, A101, A102, A103, A104, A105, A106,
+    A107, A108, A109, A110, A111, A112, A113, A114, A115, A116, A117, A118, A119,
+    A120, A121, A122, A123, A124, A125, A126, A127, A128, A129, A130, A131, A132,
+    A133, A134, A135, A136, A137, A138, A139, A140, A141, A142, A143, A144, A145,
+    A146, A147, A148, A149, A150, A151, A152, A153, A154, A155, A156, A157, A158,
+    A159, A160, A161, A162, A163, A164, A165, A166, A167, A168, A169, A170, A171,
+    A172, A173, A174, A175, A176, A177, A178, A179, A180, A181, A182, A183, A184,
+    A185, A186, A187, A188, A189, A190, A191, A192, A193, A194, A195, A196, A197,
+    A198, A199, A200, A201, A202, A203, A204, A205, A206, A207, A208, A209, A210,
+    A211, A212, A213, A214, A215, A216, A217, A218, A219, A220, A221, A222, A223,
+    A224, A225, A226, A227, A228, A229, A230, A231, A232, A233, A234, A235, A236,
+    A237, A238, A239, A240, A241, A242, A243, A244, A245, A246, A247, A248, A249,
+    A250, A251, A252, A253, A254, A255, A256,
+}
+
+#[track_caller]
 fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
     let err = panic::catch_unwind(op).err();
     assert_eq!(
@@ -72,6 +97,19 @@ fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
     );
 }
 
+#[track_caller]
+// If strict mode is enabled, expect the msg. Otherwise, expect there to be no error.
+fn test_strict_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
+    let err = panic::catch_unwind(op).err();
+
+    let expectation = if cfg!(strict) { Some(&msg) } else { None };
+
+    assert_eq!(
+        err.as_ref().and_then(|a| a.downcast_ref::<&str>()),
+        expectation
+    );
+}
+
 fn main() {
     unsafe {
         // Uninhabited types
@@ -221,6 +259,67 @@ fn main() {
             "attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR>` uninitialized, which is invalid"
         );
 
+        test_panic_msg(
+            || mem::uninitialized::<&'static [u8]>(),
+            "attempted to leave type `&[u8]` uninitialized, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<&'static [u16]>(),
+            "attempted to leave type `&[u16]` uninitialized, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<SoManyVariants>(),
+            "attempted to leave type `SoManyVariants` uninitialized, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::zeroed::<[(&'static [u8], &'static str); 1]>(),
+            "attempted to zero-initialize type `[(&[u8], &str); 1]`, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<[&'static [u16]; 1]>(),
+            "attempted to leave type `[&[u16]; 1]` uninitialized, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::zeroed::<[NonNull<()>; 1]>(),
+            "attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<[NonNull<()>; 1]>(),
+            "attempted to leave type `[core::ptr::non_null::NonNull<()>; 1]` 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::zeroed::<[LR_NonZero; 1]>(),
+            "attempted to zero-initialize type `[LR_NonZero; 1]`, which is invalid"
+        );
+
+        test_panic_msg(
+            || mem::zeroed::<[LR_NonZero; 1]>(),
+            "attempted to zero-initialize type `[LR_NonZero; 1]`, 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"
+        );
+
+        test_panic_msg(
+            || mem::uninitialized::<(&'static [u8], &'static str)>(),
+            "attempted to leave type `(&[u8], &str)` uninitialized, which is invalid"
+        );
+
         // Some things that should work.
         let _val = mem::zeroed::<bool>();
         let _val = mem::zeroed::<LR>();
@@ -230,63 +329,34 @@ fn main() {
         let _val = mem::zeroed::<MaybeUninit<NonNull<u32>>>();
         let _val = mem::zeroed::<[!; 0]>();
         let _val = mem::zeroed::<ZeroIsValid>();
+        let _val = mem::zeroed::<SoManyVariants>();
+        let _val = mem::uninitialized::<[SoManyVariants; 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"
-            );
-
-            test_panic_msg(
-                || 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]>();
-            let _val = mem::uninitialized::<[NonNull<()>; 1]>();
-        }
+        // 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.
+        test_strict_panic_msg(
+            || mem::uninitialized::<i32>(),
+            "attempted to leave type `i32` uninitialized, which is invalid"
+        );
+
+        test_strict_panic_msg(
+            || mem::uninitialized::<*const ()>(),
+            "attempted to leave type `*const ()` uninitialized, which is invalid"
+        );
+
+        test_strict_panic_msg(
+            || mem::uninitialized::<[i32; 1]>(),
+            "attempted to leave type `[i32; 1]` uninitialized, which is invalid"
+        );
+
+        test_strict_panic_msg(
+            || mem::uninitialized::<[(&'static [u8], &'static str); 1]>(),
+            "attempted to leave type `[(&[u8], &str); 1]` uninitialized, which is invalid"
+        );
     }
 }
