From 85545dde6ca2816c0403425f8b8fe178a1444ea0 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Sun, 17 Jul 2022 17:03:54 +0100 Subject: [PATCH 1/8] Make uninit checks stricter but avoid issues for old hyper --- .../src/might_permit_raw_init.rs | 9 +- compiler/rustc_middle/src/ty/layout.rs | 83 +++++++++ compiler/rustc_target/src/abi/mod.rs | 68 ------- .../intrinsics/panic-uninitialized-zeroed.rs | 176 ++++++++++++------ 4 files changed, 212 insertions(+), 124 deletions(-) diff --git a/compiler/rustc_const_eval/src/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/might_permit_raw_init.rs index f971c2238c7bb..49da32f314bb3 100644 --- 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 index dde55dd96554b..79b301ed89b54 100644 --- 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 . +/// +/// 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 index b35502d9ee42b..7fb0fb508b7a8 100644 --- 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 . - /// - /// 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(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 index 255151a96032c..58d32bb8662f6 100644 --- 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(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(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(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` 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::(), + "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::(), + "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::>(), + "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop`, \ + 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::(); let _val = mem::zeroed::(); @@ -230,63 +329,34 @@ fn main() { let _val = mem::zeroed::>>(); let _val = mem::zeroed::<[!; 0]>(); let _val = mem::zeroed::(); + let _val = mem::zeroed::(); + let _val = mem::uninitialized::<[SoManyVariants; 0]>(); let _val = mem::uninitialized::>(); let _val = mem::uninitialized::<[!; 0]>(); let _val = mem::uninitialized::<()>(); let _val = mem::uninitialized::(); - if cfg!(strict) { - test_panic_msg( - || mem::uninitialized::(), - "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::>(), - "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::(), - "attempted to zero-initialize type `LR_NonZero`, which is invalid" - ); - - test_panic_msg( - || mem::zeroed::>(), - "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop`, \ - which is invalid" - ); - } else { - // These are UB because they have not been officially blessed, but we await the resolution - // of before doing - // anything about that. - let _val = mem::uninitialized::(); - 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::(); - 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 before doing + // anything about that. + test_strict_panic_msg( + || mem::uninitialized::(), + "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" + ); } } From c84936ed46da398cc96e2cb7ca1a7159d97de2bd Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 18 Jul 2022 10:42:47 +0100 Subject: [PATCH 2/8] Fix docs --- compiler/rustc_middle/src/ty/layout.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 79b301ed89b54..2f84a67a06c63 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3498,7 +3498,7 @@ fn make_thin_self_ptr<'tcx>( /// /// 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) +/// * 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 From 6770c21bbad081f8ee29ebcaf7d55323dd4e9f3a Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 18 Jul 2022 15:26:47 +0100 Subject: [PATCH 3/8] Still do the type walking for zero init checks --- .../src/might_permit_raw_init.rs | 4 +- compiler/rustc_middle/src/ty/layout.rs | 80 +++++++++++++++---- .../intrinsics/panic-uninitialized-zeroed.rs | 35 ++++---- 3 files changed, 80 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_const_eval/src/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/might_permit_raw_init.rs index 49da32f314bb3..62d39d43c187c 100644 --- a/compiler/rustc_const_eval/src/might_permit_raw_init.rs +++ b/compiler/rustc_const_eval/src/might_permit_raw_init.rs @@ -15,7 +15,7 @@ pub fn might_permit_raw_init<'tcx>( ) -> bool { let strict = tcx.sess.opts.unstable_opts.strict_init_checks; - if strict || kind == InitKind::Zero { + if strict { let machine = CompileTimeInterpreter::new(Limit::new(0), false); let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine); @@ -38,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() }; - layout::might_permit_raw_init(ty, &layout_cx) + layout::might_permit_raw_init(ty, &layout_cx, kind) } } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 2f84a67a06c63..1a7334c0294fa 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3494,11 +3494,13 @@ fn make_thin_self_ptr<'tcx>( } /// Determines if this type permits "raw" initialization by just transmuting some -/// uninitialized memory into an instance of `T`. +/// 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) +/// * uninit `&[T]` where T has align 1 (only inside arrays). This includes `&str` +/// * zero init enums where a discriminant with tag 0 exists, but is invalid to be zeroed +/// * zero init type that does not allow zero init (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 @@ -3506,30 +3508,62 @@ fn make_thin_self_ptr<'tcx>( /// /// 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 +pub fn might_permit_raw_init<'tcx, C>(this: TyAndLayout<'tcx>, cx: &C, init_kind: InitKind) -> bool where C: HasDataLayout + ty::layout::HasParamEnv<'tcx> + ty::layout::HasTyCtxt<'tcx>, { - return might_permit_raw_init_inner(this, cx, false); + return might_permit_raw_init_inner(this, cx, init_kind, false); fn might_permit_raw_init_inner<'tcx, C>( this: TyAndLayout<'tcx>, cx: &C, + init_kind: InitKind, inside_array: bool, ) -> bool where C: HasDataLayout + ty::layout::HasParamEnv<'tcx> + ty::layout::HasTyCtxt<'tcx>, { + 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 => { + // FIXME(#66151) This should be "is the type a union", but that's pending on a + // resolution to https://github.com/rust-lang/unsafe-code-guidelines/issues/71 + // And likely a large number of crates fail with those rules, though no crater + // run has been done yet. + // + // The range must include all values. + s.is_always_valid(cx) + } + } + }; + + // These are bypasses in order to try not to break quite as many crates so quickly. + // They are being done only if we're inside an array, to ensure we don't panic on *less* + // things than we did before this change. + // See: https://github.com/rust-lang/rust/pull/99389 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; + match init_kind { + // FIXME(#66151) We need to ignore uninit slice references with an alignment of 1 + // (as in, &[u8] and &str) + // Since if we do not, old versions of `hyper` with no semver compatible fix + // (0.11, 0.12, 0.13) break. + InitKind::Uninit => { + if let ty::Ref(_, inner, _) = this.ty.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 + && l.layout.size() == Size::ZERO; + } } } - - if let ty::Str = inner.kind() { + // FIXME(#66151) We need to ignore all forms of zero data being made inside an + // array, because old versions of crossbeam make zeroed data, sometimes at an + // arbitrary type chosen by the user (such as for crossbeam-channel). + InitKind::Zero => { return true; } } @@ -3538,9 +3572,9 @@ where // 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::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 { @@ -3552,14 +3586,26 @@ where match &this.fields { FieldsShape::Primitive | FieldsShape::Union { .. } => {} FieldsShape::Array { count, .. } => { - if *count > 0 && !might_permit_raw_init_inner(this.field(cx, 0), cx, true) { + if *count > 0 + && !might_permit_raw_init_inner( + this.field(cx, 0), + cx, + init_kind, + /*inside_array*/ 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) { + if !might_permit_raw_init_inner( + this.field(cx, idx), + cx, + init_kind, + inside_array, + ) { // We found a field that is unhappy with this kind of initialization. return false; } @@ -3567,7 +3613,7 @@ where } } - if matches!(this.variants, Variants::Multiple { .. }) { + if matches!(this.variants, Variants::Multiple { .. }) && init_kind == InitKind::Uninit { // All uninit enums are automatically invalid, even if their discriminant includes all values. return false; } diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs index 58d32bb8662f6..24dba4c83dff7 100644 --- a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -274,42 +274,27 @@ fn main() { "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( + test_strict_panic_msg( || mem::zeroed::(), "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( + test_strict_panic_msg( || mem::zeroed::<[LR_NonZero; 1]>(), "attempted to zero-initialize type `[LR_NonZero; 1]`, which is invalid" ); - test_panic_msg( + test_strict_panic_msg( || mem::zeroed::>(), "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop`, \ which is invalid" @@ -355,8 +340,18 @@ fn main() { ); test_strict_panic_msg( - || mem::uninitialized::<[(&'static [u8], &'static str); 1]>(), - "attempted to leave type `[(&[u8], &str); 1]` uninitialized, which is invalid" + || mem::uninitialized::<[(&'static [u8], &'static str, &()); 1]>(), + "attempted to leave type `[(&[u8], &str, &()); 1]` uninitialized, which is invalid" + ); + + test_strict_panic_msg( + || mem::zeroed::<[(&'static [u8], &'static str); 1]>(), + "attempted to zero-initialize type `[(&[u8], &str); 1]`, which is invalid" + ); + + test_strict_panic_msg( + || mem::zeroed::<[NonNull<()>; 1]>(), + "attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid" ); } } From a2f44ac649af95a294520a5ec49080b94f495e91 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 18 Jul 2022 15:32:25 +0100 Subject: [PATCH 4/8] We ignore all references to size 0 align 1 data. --- compiler/rustc_middle/src/ty/layout.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 1a7334c0294fa..630418da0d9dd 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3498,7 +3498,7 @@ fn make_thin_self_ptr<'tcx>( /// /// This code is intentionally conservative, and will not detect /// * making uninitialized types who have a full valid range (ints, floats, raw pointers) -/// * uninit `&[T]` where T has align 1 (only inside arrays). This includes `&str` +/// * uninit `&T` where T has align 1 size 0 (only inside arrays). /// * zero init enums where a discriminant with tag 0 exists, but is invalid to be zeroed /// * zero init type that does not allow zero init (only inside arrays) /// @@ -3547,7 +3547,8 @@ where // See: https://github.com/rust-lang/rust/pull/99389 if inside_array { match init_kind { - // FIXME(#66151) We need to ignore uninit slice references with an alignment of 1 + // FIXME(#66151) We need to ignore uninit references with an alignment of 1 and + // size 0 // (as in, &[u8] and &str) // Since if we do not, old versions of `hyper` with no semver compatible fix // (0.11, 0.12, 0.13) break. From a007e7bfd775319c82b91b183fbc2a1d7bedb9d5 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 18 Jul 2022 15:45:31 +0100 Subject: [PATCH 5/8] Reword explanation for ignoring ZST 1-aligned references --- compiler/rustc_middle/src/ty/layout.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 630418da0d9dd..812141463ef97 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3496,6 +3496,9 @@ fn make_thin_self_ptr<'tcx>( /// Determines if this type permits "raw" initialization by just transmuting some /// memory into an instance of `T`. /// +/// If called with InitKind::Uninit, this function must always panic if a 0x01 filled buffer would +/// cause LLVM UB. +/// /// This code is intentionally conservative, and will not detect /// * making uninitialized types who have a full valid range (ints, floats, raw pointers) /// * uninit `&T` where T has align 1 size 0 (only inside arrays). @@ -3547,11 +3550,18 @@ where // See: https://github.com/rust-lang/rust/pull/99389 if inside_array { match init_kind { - // FIXME(#66151) We need to ignore uninit references with an alignment of 1 and - // size 0 - // (as in, &[u8] and &str) - // Since if we do not, old versions of `hyper` with no semver compatible fix - // (0.11, 0.12, 0.13) break. + // We panic if creating this type with all 0x01 bytes would + // cause LLVM UB. + // + // Therefore, in order for us to not panic, + // * the alignment of the pointer must be 1 + // (or we would have an unaligned pointer) + // + // * the statically known size of the pointee must be 0. + // (or we would emit dereferenceable) + // + // If this bypass didn't exist, old versions of `hyper` with no semver compatible + // fix (0.11, 0.12, 0.13) would panic, as they make uninit &[u8] and &str. InitKind::Uninit => { if let ty::Ref(_, inner, _) = this.ty.kind() { let penv = ty::ParamEnv::reveal_all().and(*inner); From 5ee555c2d06e4a5259af488de2b93a74b0349375 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Wed, 20 Jul 2022 01:13:56 +0100 Subject: [PATCH 6/8] Only panic for refs to slices and str, not dyn trait --- compiler/rustc_middle/src/ty/layout.rs | 14 ++++-- .../intrinsics/panic-uninitialized-zeroed.rs | 49 ++++++++++++------- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 812141463ef97..89c0cf4673001 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3564,10 +3564,16 @@ where // fix (0.11, 0.12, 0.13) would panic, as they make uninit &[u8] and &str. InitKind::Uninit => { if let ty::Ref(_, inner, _) = this.ty.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 - && l.layout.size() == Size::ZERO; + if let ty::Slice(slice_inner) = inner.kind() { + let penv = ty::ParamEnv::reveal_all().and(*slice_inner); + + if let Ok(l) = cx.tcx().layout_of(penv) { + return l.layout.align().abi == Align::ONE; + } + } + + if ty::Str == *inner.kind() { + return true; } } } diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs index 24dba4c83dff7..a5d450a9a14fd 100644 --- a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -284,25 +284,23 @@ fn main() { "attempted to leave type `[core::ptr::non_null::NonNull<()>; 1]` uninitialized, which is invalid" ); - test_strict_panic_msg( - || mem::zeroed::(), - "attempted to zero-initialize type `LR_NonZero`, which is invalid" + test_panic_msg( + || mem::uninitialized::<(&[u8], &str, &())>(), + "attempted to leave type `(&[u8], &str, &())` uninitialized, which is invalid" ); - test_strict_panic_msg( - || mem::zeroed::<[LR_NonZero; 1]>(), - "attempted to zero-initialize type `[LR_NonZero; 1]`, which is invalid" + test_panic_msg( + || mem::uninitialized::<[&(); 1]>(), + "attempted to leave type `[&(); 1]` uninitialized, which is invalid" ); - test_strict_panic_msg( - || mem::zeroed::>(), - "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop`, \ - which is invalid" + test_panic_msg( + || mem::uninitialized::<[&dyn Send; 1]>(), + "attempted to leave type `[&dyn core::marker::Send; 1]` uninitialized, which is invalid" ); - test_panic_msg( - || mem::uninitialized::<(&'static [u8], &'static str)>(), - "attempted to leave type `(&[u8], &str)` uninitialized, which is invalid" + || mem::uninitialized::<[*const dyn Send; 1]>(), + "attempted to leave type `[*const dyn core::marker::Send; 1]` uninitialized, which is invalid" ); // Some things that should work. @@ -339,19 +337,36 @@ fn main() { "attempted to leave type `[i32; 1]` uninitialized, which is invalid" ); + // These are UB, but making them panic breaks too many crates at the moment. test_strict_panic_msg( - || mem::uninitialized::<[(&'static [u8], &'static str, &()); 1]>(), - "attempted to leave type `[(&[u8], &str, &()); 1]` uninitialized, which is invalid" + || mem::zeroed::<[(&[u8], &str); 1]>(), + "attempted to zero-initialize type `[(&[u8], &str); 1]`, which is invalid" ); test_strict_panic_msg( - || mem::zeroed::<[(&'static [u8], &'static str); 1]>(), - "attempted to zero-initialize type `[(&[u8], &str); 1]`, which is invalid" + || mem::uninitialized::<[(&[u8], &str); 1]>(), + "attempted to leave type `[(&[u8], &str); 1]` uninitialized, which is invalid" ); test_strict_panic_msg( || mem::zeroed::<[NonNull<()>; 1]>(), "attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid" ); + + test_strict_panic_msg( + || mem::zeroed::(), + "attempted to zero-initialize type `LR_NonZero`, which is invalid" + ); + + test_strict_panic_msg( + || mem::zeroed::<[LR_NonZero; 1]>(), + "attempted to zero-initialize type `[LR_NonZero; 1]`, which is invalid" + ); + + test_strict_panic_msg( + || mem::zeroed::>(), + "attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop`, \ + which is invalid" + ); } } From eb1b384d744d63543123576e60c06538af27e177 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Wed, 20 Jul 2022 01:22:27 +0100 Subject: [PATCH 7/8] Adjust justification comment --- compiler/rustc_middle/src/ty/layout.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 89c0cf4673001..9fa60caa88f3c 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3553,12 +3553,17 @@ where // We panic if creating this type with all 0x01 bytes would // cause LLVM UB. // - // Therefore, in order for us to not panic, - // * the alignment of the pointer must be 1 - // (or we would have an unaligned pointer) + // Therefore, in order for us to not panic, it must either be a + // reference to [T] where T has align 1 (where we don't statically know + // the size, so we don't emit any dereferenceable), or a reference to str + // which acts much like a [u8]. // - // * the statically known size of the pointee must be 0. - // (or we would emit dereferenceable) + // We *do* need to panic for &dyn Trait, even though the layout of dyn Trait is + // size 0 align 1, because &dyn Trait holds a reference to a non-zero sized type, + // which also must be aligned. + // + // This even applies to *const dyn Trait, which holds a reference and therefore + // must be valid, so 1-initialization is not okay there. // // If this bypass didn't exist, old versions of `hyper` with no semver compatible // fix (0.11, 0.12, 0.13) would panic, as they make uninit &[u8] and &str. From a6ab54bcb5a53bc9479a33de8dddc7fc58f960b1 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Wed, 20 Jul 2022 01:53:11 +0100 Subject: [PATCH 8/8] Change comment to mention vtable --- compiler/rustc_middle/src/ty/layout.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 9fa60caa88f3c..f6e1376b9dcbb 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -3559,11 +3559,11 @@ where // which acts much like a [u8]. // // We *do* need to panic for &dyn Trait, even though the layout of dyn Trait is - // size 0 align 1, because &dyn Trait holds a reference to a non-zero sized type, - // which also must be aligned. + // size 0 align 1, because &dyn Trait holds a reference to the vtable, which + // must be aligned and dereferenceable. // - // This even applies to *const dyn Trait, which holds a reference and therefore - // must be valid, so 1-initialization is not okay there. + // This even applies to *const dyn Trait, which holds a reference to the vtable + // and therefore must be valid, so 1-initialization is not okay there. // // If this bypass didn't exist, old versions of `hyper` with no semver compatible // fix (0.11, 0.12, 0.13) would panic, as they make uninit &[u8] and &str.