From 2bed079103fc125294fef254575e5dc9c709dd60 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 09:20:27 +0000 Subject: [PATCH] Compute layout instead of manually procesisng the layout restriction attributes --- compiler/rustc_lint/src/builtin.rs | 67 ++++++++++++++------------- src/test/ui/lint/invalid_value.stderr | 63 +++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 700bf4a0aca9..5bc130e14c5b 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -52,7 +52,7 @@ use rustc_span::edition::Edition; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, InnerSpan, Span}; -use rustc_target::abi::VariantIdx; +use rustc_target::abi::{Abi, VariantIdx}; use rustc_trait_selection::traits::{self, misc::can_type_implement_copy}; use crate::nonstandard_style::{method_context, MethodLateContext}; @@ -2418,9 +2418,9 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { Self { span: Some(span), ..self } } - fn nested(self, nested: InitError) -> InitError { + fn nested(self, nested: impl Into>) -> InitError { assert!(self.nested.is_none()); - Self { nested: Some(Box::new(nested)), ..self } + Self { nested: nested.into().map(Box::new), ..self } } } @@ -2489,18 +2489,47 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { fn variant_find_init_error<'tcx>( cx: &LateContext<'tcx>, + ty: Ty<'tcx>, variant: &VariantDef, substs: ty::SubstsRef<'tcx>, descr: &str, init: InitKind, ) -> Option { - variant.fields.iter().find_map(|field| { + let field_err = variant.fields.iter().find_map(|field| { ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|err| { InitError::from(format!("in this {descr}")) .spanned(cx.tcx.def_span(field.did)) .nested(err) }) - }) + }); + + // Check if this ADT has a constrained layout (like `NonNull` and friends). + let layout = cx.tcx.layout_of(cx.param_env.and(ty)).unwrap(); + + match &layout.abi { + Abi::Scalar(scalar) | Abi::ScalarPair(scalar, _) => { + let range = scalar.valid_range(cx); + if !range.contains(0) { + Some( + InitError::from(format!("`{}` must be non-null", ty)).nested(field_err), + ) + } else if init == InitKind::Uninit && !scalar.is_always_valid(cx) { + // Prefer reporting on the fields over the entire struct for uninit, + // as the information bubbles out and it may be unclear why the type can't + // be null from just its outside signature. + Some( + InitError::from(format!( + "`{}` must be initialized inside its custom valid range", + ty, + )) + .nested(field_err), + ) + } else { + field_err + } + } + _ => field_err, + } } /// Return `Some` only if we are sure this type does *not* @@ -2540,36 +2569,11 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { } // Recurse and checks for some compound types. (but not unions) Adt(adt_def, substs) if !adt_def.is_union() => { - // First check if this ADT has a layout attribute (like `NonNull` and friends). - use std::ops::Bound; - match cx.tcx.layout_scalar_valid_range(adt_def.did()) { - // We exploit here that `layout_scalar_valid_range` will never - // return `Bound::Excluded`. (And we have tests checking that we - // handle the attribute correctly.) - // We don't add a span since users cannot declare such types anyway. - (Bound::Included(lo), Bound::Included(hi)) if 0 < lo && lo < hi => { - return Some(format!("`{}` must be non-null", ty).into()); - } - (Bound::Included(lo), Bound::Unbounded) if 0 < lo => { - return Some(format!("`{}` must be non-null", ty).into()); - } - (Bound::Included(_), _) | (_, Bound::Included(_)) - if init == InitKind::Uninit => - { - return Some( - format!( - "`{}` must be initialized inside its custom valid range", - ty, - ) - .into(), - ); - } - _ => {} - } // Handle structs. if adt_def.is_struct() { return variant_find_init_error( cx, + ty, adt_def.non_enum_variant(), substs, "struct field", @@ -2600,6 +2604,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { // There is only one potentially inhabited variant. So we can recursively check that variant! return variant_find_init_error( cx, + ty, &first_variant.0, substs, "field of the only potentially inhabited enum variant", diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index 7b452325ccb4..c5e99c9d25c9 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -34,6 +34,7 @@ LL | let _val: Wrap<&'static T> = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Wrap<&T>` must be non-null note: in this struct field --> $DIR/invalid_value.rs:17:18 | @@ -50,6 +51,7 @@ LL | let _val: Wrap<&'static T> = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Wrap<&T>` must be non-null note: in this struct field --> $DIR/invalid_value.rs:17:18 | @@ -162,6 +164,7 @@ LL | let _val: Ref = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Ref` must be non-null note: in this struct field --> $DIR/invalid_value.rs:14:12 | @@ -178,6 +181,7 @@ LL | let _val: Ref = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Ref` must be non-null note: in this struct field --> $DIR/invalid_value.rs:14:12 | @@ -216,6 +220,7 @@ LL | let _val: Wrap = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Wrap` must be non-null note: in this struct field --> $DIR/invalid_value.rs:17:18 | @@ -232,6 +237,7 @@ LL | let _val: Wrap = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Wrap` must be non-null note: in this struct field --> $DIR/invalid_value.rs:17:18 | @@ -248,6 +254,7 @@ LL | let _val: WrapEnum = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `WrapEnum` must be non-null note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:18:28 | @@ -264,6 +271,7 @@ LL | let _val: WrapEnum = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `WrapEnum` must be non-null note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:18:28 | @@ -285,6 +293,7 @@ note: in this struct field | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: `RefPair` must be non-null note: in this struct field --> $DIR/invalid_value.rs:15:16 | @@ -306,6 +315,7 @@ note: in this struct field | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: `RefPair` must be non-null note: in this struct field --> $DIR/invalid_value.rs:15:16 | @@ -334,6 +344,12 @@ LL | let _val: NonNull = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `std::ptr::NonNull` must be non-null +note: in this struct field + --> $SRC_DIR/core/src/ptr/non_null.rs:LL:COL + | +LL | pointer: *const T, + | ^^^^^^^^^^^^^^^^^ + = note: raw pointers must not be uninitialized error: the type `(NonZeroU32, i32)` does not permit zero-initialization --> $DIR/invalid_value.rs:95:39 @@ -356,6 +372,19 @@ LL | let _val: (NonZeroU32, i32) = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `std::num::NonZeroU32` must be non-null +note: in this struct field + --> $SRC_DIR/core/src/num/nonzero.rs:LL:COL + | +LL | / nonzero_integers! { +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU8(u8); +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU16(u16); +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU32(u32); +... | +LL | | #[stable(feature = "signed_nonzero", since = "1.34.0")] #[rustc_const_stable(feature = "signed_nonzero", since = "1.34.0")] NonZeroIs... +LL | | } + | |_^ + = note: integers must not be uninitialized + = note: this error originates in the macro `nonzero_integers` (in Nightly builds, run with -Z macro-backtrace for more info) error: the type `*const dyn Send` does not permit zero-initialization --> $DIR/invalid_value.rs:98:37 @@ -440,6 +469,7 @@ LL | let _val: OneFruitNonZero = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `OneFruitNonZero` must be non-null note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:39:12 | @@ -456,12 +486,26 @@ LL | let _val: OneFruitNonZero = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `OneFruitNonZero` must be non-null note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:39:12 | LL | Banana(NonZeroU32), | ^^^^^^^^^^ = note: `std::num::NonZeroU32` must be non-null +note: in this struct field + --> $SRC_DIR/core/src/num/nonzero.rs:LL:COL + | +LL | / nonzero_integers! { +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU8(u8); +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU16(u16); +LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable(feature = "nonzero", since = "1.28.0")] NonZeroU32(u32); +... | +LL | | #[stable(feature = "signed_nonzero", since = "1.34.0")] #[rustc_const_stable(feature = "signed_nonzero", since = "1.34.0")] NonZeroIs... +LL | | } + | |_^ + = note: integers must not be uninitialized + = note: this error originates in the macro `nonzero_integers` (in Nightly builds, run with -Z macro-backtrace for more info) error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:112:26 @@ -483,6 +527,7 @@ LL | let _val: Wrap = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | + = note: `Wrap` must be initialized inside its custom valid range note: in this struct field --> $DIR/invalid_value.rs:17:18 | @@ -500,6 +545,12 @@ LL | let _val: NonBig = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `NonBig` must be initialized inside its custom valid range +note: in this struct field + --> $DIR/invalid_value.rs:23:26 + | +LL | pub(crate) struct NonBig(u64); + | ^^^ + = note: integers must not be uninitialized error: the type `Fruit` does not permit being left uninitialized --> $DIR/invalid_value.rs:121:27 @@ -581,6 +632,12 @@ LL | let _val: WrapAroundRange = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `WrapAroundRange` must be initialized inside its custom valid range +note: in this struct field + --> $DIR/invalid_value.rs:49:35 + | +LL | pub(crate) struct WrapAroundRange(u8); + | ^^ + = note: integers must not be uninitialized error: the type `Result` does not permit being left uninitialized --> $DIR/invalid_value.rs:144:38 @@ -651,6 +708,12 @@ LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `std::ptr::NonNull` must be non-null +note: in this struct field + --> $SRC_DIR/core/src/ptr/non_null.rs:LL:COL + | +LL | pointer: *const T, + | ^^^^^^^^^^^^^^^^^ + = note: raw pointers must not be uninitialized error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:159:26