From 729f4cd9ae9a619677afaa4c389e000c03e5b22b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Feb 2020 09:21:02 +0100 Subject: [PATCH] we cannot short-circuit just becuase the Abi seems harmless also add tests for ScalarPair enums --- src/librustc_target/abi/mod.rs | 63 ++++--------------- .../intrinsics/panic-uninitialized-zeroed.rs | 48 +++++++++++++- 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index c043d87f69f..aa9474233a4 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -1045,7 +1045,7 @@ pub fn is_zst(&self) -> bool { /// 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 vonst value validator to see if + /// 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(self, cx: &C, zero: bool) -> Result where @@ -1067,59 +1067,22 @@ pub fn might_permit_raw_init(self, cx: &C, zero: bool) -> Result } }; - // Abi is the most informative here. - let res = match &self.abi { + // 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 { .. } => { - match self.variants { - Variants::Multiple { .. } => { - if zero { - // FIXME(#66151): - // could we identify the variant with discriminant 0, check that? - true - } else { - // FIXME(#66151): This needs to have some sort of discriminant, - // which cannot be undef. But for now we are conservative. - true - } - } - Variants::Single { .. } => { - // For aggregates, recurse. - match self.fields { - FieldPlacement::Union(..) => true, // An all-0 unit is fine. - FieldPlacement::Array { .. } => - // FIXME(#66151): The widely use smallvec 0.6 creates uninit arrays - // with any element type, so let us not (yet) complain about that. - /* count == 0 || - self.field(cx, 0).to_result()?.might_permit_raw_init(cx, zero)? */ - { - true - } - FieldPlacement::Arbitrary { .. } => { - // FIXME(#66151) cargo depends on sized-chunks 0.3.0 which - // has some illegal zero-initialization, so let us not (yet) - // complain about aggregates either. - /* let mut res = true; - // Check that all fields accept zero-init. - for idx in 0..offsets.len() { - let field = self.field(cx, idx).to_result()?; - if !field.might_permit_raw_init(cx, zero)? { - res = false; - break; - } - } - res */ - true - } - } - } - } - } + Abi::Aggregate { .. } => true, // Cannot be excluded *right now*. }; - trace!("might_permit_raw_init({:?}, zero={}) = {}", self.details, zero, res); - Ok(res) + if !valid { + // This is definitely not okay. + trace!("might_permit_raw_init({:?}, zero={}): not valid", self.details, zero); + return Ok(false); + } + + // If we have not found an error yet, we need to recursively descend. + // FIXME(#66151): For now, we are conservative and do not do this. + Ok(true) } } diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs index a1b2a1af2c4..be0df0ea254 100644 --- a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -7,9 +7,10 @@ #![allow(deprecated, invalid_value)] use std::{ - mem::{self, MaybeUninit}, + mem::{self, MaybeUninit, ManuallyDrop}, panic, ptr::NonNull, + num, }; #[allow(dead_code)] @@ -23,6 +24,18 @@ enum Bar {} #[allow(dead_code)] enum OneVariant { Variant(i32) } +// An enum with ScalarPair layout +#[allow(dead_code)] +enum LR { + Left(i64), + Right(i64), +} +#[allow(dead_code, non_camel_case_types)] +enum LR_NonZero { + Left(num::NonZeroI64), + Right(num::NonZeroI64), +} + fn test_panic_msg(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) { let err = panic::catch_unwind(op).err(); assert_eq!( @@ -33,7 +46,7 @@ fn test_panic_msg(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) { fn main() { unsafe { - // Uninitialized types + // Uninhabited types test_panic_msg( || mem::uninitialized::(), "attempted to instantiate uninhabited type `!`" @@ -93,6 +106,26 @@ fn main() { ); /* FIXME(#66151) we conservatively do not error here yet. + test_panic_msg( + || mem::uninitialized::(), + "attempted to leave type `LR_NonZero` uninitialized, which is invalid" + ); + test_panic_msg( + || mem::zeroed::(), + "attempted to zero-initialize type `LR_NonZero`, which is invalid" + ); + + test_panic_msg( + || mem::uninitialized::>(), + "attempted to leave type `std::mem::ManuallyDrop` uninitialized, \ + which is invalid" + ); + test_panic_msg( + || mem::zeroed::>(), + "attempted to zero-initialize type `std::mem::ManuallyDrop`, \ + which is invalid" + ); + test_panic_msg( || mem::uninitialized::<(NonNull, u32, u32)>(), "attempted to leave type `(std::ptr::NonNull, u32, u32)` uninitialized, \ @@ -105,13 +138,24 @@ fn main() { ); */ + // Types that can be zero, but not uninit. test_panic_msg( || mem::uninitialized::(), "attempted to leave type `bool` uninitialized, which is invalid" ); + test_panic_msg( + || mem::uninitialized::(), + "attempted to leave type `LR` uninitialized, which is invalid" + ); + test_panic_msg( + || mem::uninitialized::>(), + "attempted to leave type `std::mem::ManuallyDrop` uninitialized, which is invalid" + ); // Some things that should work. let _val = mem::zeroed::(); + let _val = mem::zeroed::(); + let _val = mem::zeroed::>(); let _val = mem::zeroed::(); let _val = mem::zeroed::>(); let _val = mem::zeroed::>>();