From b5554722ffb720e66a4e22ee2917b2c038498161 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:11:52 +0000 Subject: [PATCH 01/17] Add helper method to `ScalarInt` --- compiler/rustc_middle/src/ty/consts/int.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/compiler/rustc_middle/src/ty/consts/int.rs b/compiler/rustc_middle/src/ty/consts/int.rs index 7436f0f6f4d33..f3186e1c30c3d 100644 --- a/compiler/rustc_middle/src/ty/consts/int.rs +++ b/compiler/rustc_middle/src/ty/consts/int.rs @@ -245,6 +245,18 @@ impl ScalarInt { self.to_bits(size) } + // Tries to convert the `ScalarInt` to `bool`. Fails if the `size` of the `ScalarInt` + // in not equal to `Size { raw: 1 }` or if the value is not 0 or 1 and returns the `size` + // value of the `ScalarInt` in that case. + #[inline] + pub fn try_to_bool(self) -> Result { + match self.try_to_u8()? { + 0 => Ok(false), + 1 => Ok(true), + _ => Err(self.size()), + } + } + // Tries to convert the `ScalarInt` to `u8`. Fails if the `size` of the `ScalarInt` // in not equal to `Size { raw: 1 }` and returns the `size` value of the `ScalarInt` in // that case. From 2a94a2d385657dfa89bf2d46d2a6114be70afef0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:12:19 +0000 Subject: [PATCH 02/17] Prefer not accessing the private field of newtype_index types --- compiler/rustc_span/src/def_id.rs | 2 +- compiler/rustc_span/src/hygiene.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_span/src/def_id.rs b/compiler/rustc_span/src/def_id.rs index bbeabdb55a72a..f5555846d20a7 100644 --- a/compiler/rustc_span/src/def_id.rs +++ b/compiler/rustc_span/src/def_id.rs @@ -34,7 +34,7 @@ impl CrateNum { impl fmt::Display for CrateNum { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.private, f) + fmt::Display::fmt(&self.as_u32(), f) } } diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 191186af6fa08..99a8b03fa39ad 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -76,7 +76,7 @@ pub struct ExpnId { impl fmt::Debug for ExpnId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Generate crate_::{{expn_}}. - write!(f, "{:?}::{{{{expn{}}}}}", self.krate, self.local_id.private) + write!(f, "{:?}::{{{{expn{}}}}}", self.krate, self.local_id.as_u32()) } } From 2b8963a94c9faa980ab534a2ee1a4118865c6645 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:14:43 +0000 Subject: [PATCH 03/17] Some manual formatting of let..else statements --- compiler/rustc_ty_utils/src/layout.rs | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 7a1cc1e9e6ded..fbc055b5d238f 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -325,8 +325,8 @@ fn layout_of_uncached<'tcx>( // Extract the number of elements from the layout of the array field: let FieldsShape::Array { count, .. } = cx.layout_of(f0_ty)?.layout.fields() else { - return Err(LayoutError::Unknown(ty)); - }; + return Err(LayoutError::Unknown(ty)); + }; (*e_ty, *count, true) } else { @@ -351,14 +351,14 @@ fn layout_of_uncached<'tcx>( // Compute the ABI of the element type: let e_ly = cx.layout_of(e_ty)?; let Abi::Scalar(e_abi) = e_ly.abi else { - // This error isn't caught in typeck, e.g., if - // the element type of the vector is generic. - tcx.sess.fatal(&format!( - "monomorphising SIMD type `{}` with a non-primitive-scalar \ - (integer/float/pointer) element type `{}`", - ty, e_ty - )) - }; + // This error isn't caught in typeck, e.g., if + // the element type of the vector is generic. + tcx.sess.fatal(&format!( + "monomorphising SIMD type `{}` with a non-primitive-scalar \ + (integer/float/pointer) element type `{}`", + ty, e_ty + )) + }; // Compute the size and alignment of the vector: let size = e_ly.size.checked_mul(e_len, dl).ok_or(LayoutError::SizeOverflow(ty))?; @@ -597,8 +597,8 @@ fn generator_layout<'tcx>( let subst_field = |ty: Ty<'tcx>| EarlyBinder(ty).subst(tcx, substs); let Some(info) = tcx.generator_layout(def_id) else { - return Err(LayoutError::Unknown(ty)); - }; + return Err(LayoutError::Unknown(ty)); + }; let (ineligible_locals, assignments) = generator_saved_local_eligibility(&info); // Build a prefix layout, including "promoting" all ineligible @@ -701,8 +701,8 @@ fn generator_layout<'tcx>( variant.variants = Variants::Single { index }; let FieldsShape::Arbitrary { offsets, memory_index } = variant.fields else { - bug!(); - }; + bug!(); + }; // Now, stitch the promoted and variant-only fields back together in // the order they are mentioned by our GeneratorLayout. From c3eb8f27786482cac6b28b13eda8af0e919a556c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:17:08 +0000 Subject: [PATCH 04/17] `rustc_layout_scalar_valid_range` can be applied to scalar pairs and affects teh first scalar --- compiler/rustc_const_eval/src/interpret/validity.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index cd7a472c0f0df..d69192ef6ed65 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -785,15 +785,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } } Abi::ScalarPair(a_layout, b_layout) => { - // There is no `rustc_layout_scalar_valid_range_start` for pairs, so - // we would validate these things as we descend into the fields, - // but that can miss bugs in layout computation. Layout computation - // is subtle due to enums having ScalarPair layout, where one field - // is the discriminant. - if cfg!(debug_assertions) - && !a_layout.is_uninit_valid() - && !b_layout.is_uninit_valid() - { + if !a_layout.is_uninit_valid() && !b_layout.is_uninit_valid() { // We can only proceed if *both* scalars need to be initialized. // FIXME: find a way to also check ScalarPair when one side can be uninit but // the other must be init. From 6773e7ee562eb6a3c50bc9f284f767c09b87dfbb Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:17:32 +0000 Subject: [PATCH 05/17] More manual formatting --- compiler/rustc_const_eval/src/transform/validate.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 860dee5898057..a4bd81fb73f8f 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -226,12 +226,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { let check_equal = |this: &Self, location, f_ty| { if !this.mir_assign_valid_types(ty, f_ty) { this.fail( - location, - format!( - "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is `{:?}`", - parent, f, ty, f_ty + location, + format!( + "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is `{:?}`", + parent, f, ty, f_ty + ) ) - ) } }; From 9909cb902f76bbd99e74d9ff91718d9f5a2eba80 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 08:17:55 +0000 Subject: [PATCH 06/17] Make the ICEs in the mir typechecker have more spans helpful --- compiler/rustc_const_eval/src/transform/validate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index a4bd81fb73f8f..bf700d3122465 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -81,6 +81,7 @@ struct TypeChecker<'a, 'tcx> { } impl<'a, 'tcx> TypeChecker<'a, 'tcx> { + #[track_caller] fn fail(&self, location: Location, msg: impl AsRef) { let span = self.body.source_info(location).span; // We use `delay_span_bug` as we might see broken MIR when other errors have already From 5cbf17290923c04487e031f882846d1320832eff Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 09:10:31 +0000 Subject: [PATCH 07/17] Print a trace through types to show how to get to the problematic type --- compiler/rustc_lint/src/builtin.rs | 106 +++++++++++------- .../validate_uninhabited_zsts.32bit.stderr | 5 + .../validate_uninhabited_zsts.64bit.stderr | 5 + src/test/ui/lint/invalid_value.stderr | 49 +++++--- 4 files changed, 110 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index ada3c3b67fb05..700bf4a0aca9b 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -57,8 +57,6 @@ use rustc_trait_selection::traits::{self, misc::can_type_implement_copy}; use crate::nonstandard_style::{method_context, MethodLateContext}; -use std::fmt::Write; - // hardwired lints from librustc_middle pub use rustc_session::lint::builtin::*; @@ -2408,8 +2406,34 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { } /// Information about why a type cannot be initialized this way. - /// Contains an error message and optionally a span to point at. - type InitError = (String, Option); + struct InitError { + message: String, + /// Spans from struct fields and similar can be obtained from just the type. + span: Option, + /// Used to report a trace through adts. + nested: Option>, + } + impl InitError { + fn spanned(self, span: Span) -> InitError { + Self { span: Some(span), ..self } + } + + fn nested(self, nested: InitError) -> InitError { + assert!(self.nested.is_none()); + Self { nested: Some(Box::new(nested)), ..self } + } + } + + impl<'a> From<&'a str> for InitError { + fn from(s: &'a str) -> Self { + s.to_owned().into() + } + } + impl From for InitError { + fn from(message: String) -> Self { + Self { message, span: None, nested: None } + } + } /// Test if this constant is all-0. fn is_zero(expr: &hir::Expr<'_>) -> bool { @@ -2471,17 +2495,10 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { init: InitKind, ) -> Option { variant.fields.iter().find_map(|field| { - ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|(mut msg, span)| { - if span.is_none() { - // Point to this field, should be helpful for figuring - // out where the source of the error is. - let span = cx.tcx.def_span(field.did); - write!(&mut msg, " (in this {descr})").unwrap(); - (msg, Some(span)) - } else { - // Just forward. - (msg, span) - } + 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) }) }) } @@ -2496,30 +2513,30 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { use rustc_type_ir::sty::TyKind::*; match ty.kind() { // Primitive types that don't like 0 as a value. - Ref(..) => Some(("references must be non-null".to_string(), None)), - Adt(..) if ty.is_box() => Some(("`Box` must be non-null".to_string(), None)), - FnPtr(..) => Some(("function pointers must be non-null".to_string(), None)), - Never => Some(("the `!` type has no valid value".to_string(), None)), + Ref(..) => Some("references must be non-null".into()), + Adt(..) if ty.is_box() => Some("`Box` must be non-null".into()), + FnPtr(..) => Some("function pointers must be non-null".into()), + Never => Some("the `!` type has no valid value".into()), RawPtr(tm) if matches!(tm.ty.kind(), Dynamic(..)) => // raw ptr to dyn Trait { - Some(("the vtable of a wide raw pointer must be non-null".to_string(), None)) + Some("the vtable of a wide raw pointer must be non-null".into()) } // Primitive types with other constraints. Bool if init == InitKind::Uninit => { - Some(("booleans must be either `true` or `false`".to_string(), None)) + Some("booleans must be either `true` or `false`".into()) } Char if init == InitKind::Uninit => { - Some(("characters must be a valid Unicode codepoint".to_string(), None)) + Some("characters must be a valid Unicode codepoint".into()) } Int(_) | Uint(_) if init == InitKind::Uninit => { - Some(("integers must not be uninitialized".to_string(), None)) + Some("integers must not be uninitialized".into()) } Float(_) if init == InitKind::Uninit => { - Some(("floats must not be uninitialized".to_string(), None)) + Some("floats must not be uninitialized".into()) } RawPtr(_) if init == InitKind::Uninit => { - Some(("raw pointers must not be uninitialized".to_string(), None)) + Some("raw pointers must not be uninitialized".into()) } // Recurse and checks for some compound types. (but not unions) Adt(adt_def, substs) if !adt_def.is_union() => { @@ -2531,21 +2548,21 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { // 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), None)); + 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), None)); + return Some(format!("`{}` must be non-null", ty).into()); } (Bound::Included(_), _) | (_, Bound::Included(_)) if init == InitKind::Uninit => { - return Some(( + return Some( format!( "`{}` must be initialized inside its custom valid range", ty, - ), - None, - )); + ) + .into(), + ); } _ => {} } @@ -2576,7 +2593,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { Some((variant, definitely_inhabited)) }); let Some(first_variant) = potential_variants.next() else { - return Some(("enums with no inhabited variants have no valid value".to_string(), Some(span))); + return Some(InitError::from("enums with no inhabited variants have no valid value").spanned(span)); }; // So we have at least one potentially inhabited variant. Might we have two? let Some(second_variant) = potential_variants.next() else { @@ -2600,10 +2617,9 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { .filter(|(_variant, definitely_inhabited)| *definitely_inhabited) .count(); if definitely_inhabited > 1 { - return Some(( - "enums with multiple inhabited variants have to be initialized to a variant".to_string(), - Some(span), - )); + return Some(InitError::from( + "enums with multiple inhabited variants have to be initialized to a variant", + ).spanned(span)); } } // We couldn't find anything wrong here. @@ -2632,8 +2648,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { // using zeroed or uninitialized memory. // We are extremely conservative with what we warn about. let conjured_ty = cx.typeck_results().expr_ty(expr); - if let Some((msg, span)) = - with_no_trimmed_paths!(ty_find_init_error(cx, conjured_ty, init)) + if let Some(mut err) = with_no_trimmed_paths!(ty_find_init_error(cx, conjured_ty, init)) { // FIXME(davidtwco): make translatable cx.struct_span_lint( @@ -2659,10 +2674,17 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { "help: use `MaybeUninit` instead, \ and only call `assume_init` after initialization is done", ); - if let Some(span) = span { - lint.span_note(span, &msg); - } else { - lint.note(&msg); + loop { + if let Some(span) = err.span { + lint.span_note(span, &err.message); + } else { + lint.note(&err.message); + } + if let Some(e) = err.nested { + err = *e; + } else { + break; + } } lint }, diff --git a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr index 63639729a2a91..8b4d845b30eaa 100644 --- a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr +++ b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr @@ -40,6 +40,11 @@ LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | +note: in this struct field + --> $DIR/validate_uninhabited_zsts.rs:16:22 + | +LL | pub struct Empty(Void); + | ^^^^ note: enums with no inhabited variants have no valid value --> $DIR/validate_uninhabited_zsts.rs:13:5 | diff --git a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr index 63639729a2a91..8b4d845b30eaa 100644 --- a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr +++ b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr @@ -40,6 +40,11 @@ LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | +note: in this struct field + --> $DIR/validate_uninhabited_zsts.rs:16:22 + | +LL | pub struct Empty(Void); + | ^^^^ note: enums with no inhabited variants have no valid value --> $DIR/validate_uninhabited_zsts.rs:13:5 | diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index 76afb765f0f05..7b452325ccb4f 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -34,11 +34,12 @@ 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: references must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: references must be non-null error: the type `Wrap<&T>` does not permit being left uninitialized --> $DIR/invalid_value.rs:58:38 @@ -49,11 +50,12 @@ 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: references must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: references must be non-null error: the type `!` does not permit zero-initialization --> $DIR/invalid_value.rs:65:23 @@ -160,11 +162,12 @@ 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: references must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); | ^^^^^^^^^^^^ + = note: references must be non-null error: the type `Ref` does not permit being left uninitialized --> $DIR/invalid_value.rs:78:25 @@ -175,11 +178,12 @@ 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: references must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); | ^^^^^^^^^^^^ + = note: references must be non-null error: the type `fn()` does not permit zero-initialization --> $DIR/invalid_value.rs:80:26 @@ -212,11 +216,12 @@ 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: function pointers must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: function pointers must be non-null error: the type `Wrap` does not permit being left uninitialized --> $DIR/invalid_value.rs:84:32 @@ -227,11 +232,12 @@ 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: function pointers must be non-null (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: function pointers must be non-null error: the type `WrapEnum` does not permit zero-initialization --> $DIR/invalid_value.rs:86:36 @@ -242,11 +248,12 @@ 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: function pointers must be non-null (in this field of the only potentially inhabited enum variant) +note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } | ^ + = note: function pointers must be non-null error: the type `WrapEnum` does not permit being left uninitialized --> $DIR/invalid_value.rs:87:36 @@ -257,11 +264,12 @@ 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: function pointers must be non-null (in this field of the only potentially inhabited enum variant) +note: in this field of the only potentially inhabited enum variant --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } | ^ + = note: function pointers must be non-null error: the type `Wrap<(RefPair, i32)>` does not permit zero-initialization --> $DIR/invalid_value.rs:89:42 @@ -272,11 +280,17 @@ LL | let _val: Wrap<(RefPair, i32)> = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | -note: references must be non-null (in this struct field) +note: in this struct field + --> $DIR/invalid_value.rs:17:18 + | +LL | struct Wrap { wrapped: T } + | ^^^^^^^^^^ +note: in this struct field --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); | ^^^^^^^^^^^^^^^^^^^ + = note: references must be non-null error: the type `Wrap<(RefPair, i32)>` does not permit being left uninitialized --> $DIR/invalid_value.rs:90:42 @@ -287,11 +301,17 @@ LL | let _val: Wrap<(RefPair, i32)> = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | -note: references must be non-null (in this struct field) +note: in this struct field + --> $DIR/invalid_value.rs:17:18 + | +LL | struct Wrap { wrapped: T } + | ^^^^^^^^^^ +note: in this struct field --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); | ^^^^^^^^^^^^^^^^^^^ + = note: references must be non-null error: the type `NonNull` does not permit zero-initialization --> $DIR/invalid_value.rs:92:34 @@ -420,11 +440,12 @@ 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: `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) +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 error: the type `OneFruitNonZero` does not permit being left uninitialized --> $DIR/invalid_value.rs:108:37 @@ -435,11 +456,12 @@ 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: `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) +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 error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:112:26 @@ -461,11 +483,12 @@ 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: characters must be a valid Unicode codepoint (in this struct field) +note: in this struct field --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ + = note: characters must be a valid Unicode codepoint error: the type `NonBig` does not permit being left uninitialized --> $DIR/invalid_value.rs:118:28 From 2bed079103fc125294fef254575e5dc9c709dd60 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 09:20:27 +0000 Subject: [PATCH 08/17] 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 700bf4a0aca9b..5bc130e14c5b5 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 7b452325ccb4f..c5e99c9d25c90 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 From ccaa28bf694a4a8081e2a2e0e4c84842f4205ed0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 09:43:52 +0000 Subject: [PATCH 09/17] Don't try to compute the layout of generic types. --- compiler/rustc_lint/src/builtin.rs | 46 +++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 5bc130e14c5b5..b45ba0dcd5fda 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2504,32 +2504,32 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { }); // 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 + if let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty)) { + match &layout.abi { + Abi::Scalar(scalar) | Abi::ScalarPair(scalar, _) => { + let range = scalar.valid_range(cx); + if !range.contains(0) { + return 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. + return Some( + InitError::from(format!( + "`{}` must be initialized inside its custom valid range", + ty, + )) + .nested(field_err), + ); + } } + _ => {} } - _ => field_err, } + field_err } /// Return `Some` only if we are sure this type does *not* From 545fccaab4215e3abb5d1023119202520e2da795 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 10:12:49 +0000 Subject: [PATCH 10/17] Add a test for scalar pair layout validation --- .../ui/consts/const-eval/ub-nonnull.32bit.stderr | 13 ++++++++++++- .../ui/consts/const-eval/ub-nonnull.64bit.stderr | 13 ++++++++++++- src/test/ui/consts/const-eval/ub-nonnull.rs | 9 ++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr index dbd05b8f4249a..b24e0cc37aa65 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr @@ -65,6 +65,17 @@ LL | const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; 14 00 00 00 │ .... } -error: aborting due to 7 previous errors +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-nonnull.rs:50:1 + | +LL | const NULL_FAT_PTR: NonNull = unsafe { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1 + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. + = note: the raw bytes of the constant (size: 8, align: 4) { + 00 00 00 00 ╾─alloc26─╼ │ ....╾──╼ + } + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr index 5a1ac09bd35ac..92b8d017c0b77 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr @@ -65,6 +65,17 @@ LL | const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; 14 00 00 00 │ .... } -error: aborting due to 7 previous errors +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-nonnull.rs:50:1 + | +LL | const NULL_FAT_PTR: NonNull = unsafe { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1 + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. + = note: the raw bytes of the constant (size: 16, align: 8) { + 00 00 00 00 00 00 00 00 ╾───────alloc26───────╼ │ ........╾──────╼ + } + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs index d22a99cd01e68..49092582267c8 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.rs +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -1,5 +1,5 @@ // stderr-per-bitwidth -#![feature(rustc_attrs)] +#![feature(rustc_attrs, ptr_metadata)] #![allow(invalid_value)] // make sure we cannot allow away the errors tested here use std::mem; @@ -47,4 +47,11 @@ struct RestrictedRange2(u32); const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; //~^ ERROR it is undefined behavior to use this value +const NULL_FAT_PTR: NonNull = unsafe { +//~^ ERROR it is undefined behavior to use this value + let x: &dyn Send = &42; + let meta = std::ptr::metadata(x); + mem::transmute((0_usize, meta)) +}; + fn main() {} From 98c550ecc8ea51db79bc4a83f9fb518220add79c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 10:17:50 +0000 Subject: [PATCH 11/17] Reinstate the previous compact form of "in this field" errors --- compiler/rustc_lint/src/builtin.rs | 16 +++++-- src/test/ui/lint/invalid_value.stderr | 63 +++++++++------------------ 2 files changed, 33 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index b45ba0dcd5fda..2f3a2dcb81962 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -57,6 +57,8 @@ use rustc_trait_selection::traits::{self, misc::can_type_implement_copy}; use crate::nonstandard_style::{method_context, MethodLateContext}; +use std::fmt::Write; + // hardwired lints from librustc_middle pub use rustc_session::lint::builtin::*; @@ -2496,10 +2498,16 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { init: InitKind, ) -> Option { 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) + ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|mut err| { + if err.span.is_none() { + err.span = Some(cx.tcx.def_span(field.did)); + write!(&mut err.message, " (in this {descr})").unwrap(); + err + } else { + InitError::from(format!("in this {descr}")) + .spanned(cx.tcx.def_span(field.did)) + .nested(err) + } }) }); diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index c5e99c9d25c90..e9449605e35c3 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -35,12 +35,11 @@ LL | let _val: Wrap<&'static T> = mem::zeroed(); | 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 +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: references must be non-null error: the type `Wrap<&T>` does not permit being left uninitialized --> $DIR/invalid_value.rs:58:38 @@ -52,12 +51,11 @@ LL | let _val: Wrap<&'static T> = mem::uninitialized(); | 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 +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: references must be non-null error: the type `!` does not permit zero-initialization --> $DIR/invalid_value.rs:65:23 @@ -165,12 +163,11 @@ LL | let _val: Ref = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Ref` must be non-null -note: in this struct field +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); | ^^^^^^^^^^^^ - = note: references must be non-null error: the type `Ref` does not permit being left uninitialized --> $DIR/invalid_value.rs:78:25 @@ -182,12 +179,11 @@ LL | let _val: Ref = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Ref` must be non-null -note: in this struct field +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); | ^^^^^^^^^^^^ - = note: references must be non-null error: the type `fn()` does not permit zero-initialization --> $DIR/invalid_value.rs:80:26 @@ -221,12 +217,11 @@ LL | let _val: Wrap = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap` must be non-null -note: in this struct field +note: function pointers must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: function pointers must be non-null error: the type `Wrap` does not permit being left uninitialized --> $DIR/invalid_value.rs:84:32 @@ -238,12 +233,11 @@ LL | let _val: Wrap = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap` must be non-null -note: in this struct field +note: function pointers must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: function pointers must be non-null error: the type `WrapEnum` does not permit zero-initialization --> $DIR/invalid_value.rs:86:36 @@ -255,12 +249,11 @@ LL | let _val: WrapEnum = mem::zeroed(); | 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 +note: function pointers must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } | ^ - = note: function pointers must be non-null error: the type `WrapEnum` does not permit being left uninitialized --> $DIR/invalid_value.rs:87:36 @@ -272,12 +265,11 @@ LL | let _val: WrapEnum = mem::uninitialized(); | 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 +note: function pointers must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } | ^ - = note: function pointers must be non-null error: the type `Wrap<(RefPair, i32)>` does not permit zero-initialization --> $DIR/invalid_value.rs:89:42 @@ -288,18 +280,16 @@ LL | let _val: Wrap<(RefPair, i32)> = mem::zeroed(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | -note: in this struct field +note: `RefPair` must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: `RefPair` must be non-null -note: in this struct field +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); | ^^^^^^^^^^^^^^^^^^^ - = note: references must be non-null error: the type `Wrap<(RefPair, i32)>` does not permit being left uninitialized --> $DIR/invalid_value.rs:90:42 @@ -310,18 +300,16 @@ LL | let _val: Wrap<(RefPair, i32)> = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | -note: in this struct field +note: `RefPair` must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: `RefPair` must be non-null -note: in this struct field +note: references must be non-null (in this struct field) --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); | ^^^^^^^^^^^^^^^^^^^ - = note: references must be non-null error: the type `NonNull` does not permit zero-initialization --> $DIR/invalid_value.rs:92:34 @@ -344,12 +332,11 @@ 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 +note: raw pointers must not be uninitialized (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 @@ -372,7 +359,7 @@ 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 +note: integers must not be uninitialized (in this struct field) --> $SRC_DIR/core/src/num/nonzero.rs:LL:COL | LL | / nonzero_integers! { @@ -383,7 +370,6 @@ LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable 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 @@ -470,12 +456,11 @@ LL | let _val: OneFruitNonZero = mem::zeroed(); | 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 +note: `std::num::NonZeroU32` must be non-null (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 error: the type `OneFruitNonZero` does not permit being left uninitialized --> $DIR/invalid_value.rs:108:37 @@ -487,13 +472,12 @@ LL | let _val: OneFruitNonZero = mem::uninitialized(); | 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 +note: `std::num::NonZeroU32` must be non-null (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 +note: integers must not be uninitialized (in this struct field) --> $SRC_DIR/core/src/num/nonzero.rs:LL:COL | LL | / nonzero_integers! { @@ -504,7 +488,6 @@ LL | | #[stable(feature = "nonzero", since = "1.28.0")] #[rustc_const_stable 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 @@ -528,12 +511,11 @@ LL | let _val: Wrap = mem::uninitialized(); | 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 +note: characters must be a valid Unicode codepoint (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ - = note: characters must be a valid Unicode codepoint error: the type `NonBig` does not permit being left uninitialized --> $DIR/invalid_value.rs:118:28 @@ -545,12 +527,11 @@ 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 +note: integers must not be uninitialized (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 @@ -632,12 +613,11 @@ 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 +note: integers must not be uninitialized (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 @@ -708,12 +688,11 @@ 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 +note: raw pointers must not be uninitialized (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 From 2e79f5f9f8b16444e86cb611d1f819023317d974 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 12:04:26 +0000 Subject: [PATCH 12/17] Move a comment to the right place --- compiler/rustc_const_eval/src/interpret/validity.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index d69192ef6ed65..0e85c7d11bce5 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -785,10 +785,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } } Abi::ScalarPair(a_layout, b_layout) => { + // We can only proceed if *both* scalars need to be initialized. + // FIXME: find a way to also check ScalarPair when one side can be uninit but + // the other must be init. if !a_layout.is_uninit_valid() && !b_layout.is_uninit_valid() { - // We can only proceed if *both* scalars need to be initialized. - // FIXME: find a way to also check ScalarPair when one side can be uninit but - // the other must be init. let (a, b) = self.read_immediate(op, "initiailized scalar value")?.to_scalar_pair(); self.visit_scalar(a, a_layout)?; From fcb1f1874f8c1b40b2ba5228a905c7875b21ce88 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 15:30:52 +0000 Subject: [PATCH 13/17] Don't show fields from other crates --- compiler/rustc_lint/src/builtin.rs | 4 ++- src/test/ui/lint/invalid_value.stderr | 38 +++------------------------ 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 2f3a2dcb81962..4bde57881f083 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2499,7 +2499,9 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { ) -> Option { let field_err = variant.fields.iter().find_map(|field| { ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|mut err| { - if err.span.is_none() { + if !field.did.is_local() { + err + } else if err.span.is_none() { err.span = Some(cx.tcx.def_span(field.did)); write!(&mut err.message, " (in this {descr})").unwrap(); err diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index e9449605e35c3..9f0f51f4d9726 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -332,11 +332,7 @@ 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: raw pointers must not be uninitialized (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 @@ -359,18 +355,7 @@ 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: integers must not be uninitialized (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: this error originates in the macro `nonzero_integers` (in Nightly builds, run with -Z macro-backtrace for more info) + = note: integers must not be uninitialized error: the type `*const dyn Send` does not permit zero-initialization --> $DIR/invalid_value.rs:98:37 @@ -477,18 +462,7 @@ note: `std::num::NonZeroU32` must be non-null (in this field of the only potenti | LL | Banana(NonZeroU32), | ^^^^^^^^^^ -note: integers must not be uninitialized (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: this error originates in the macro `nonzero_integers` (in Nightly builds, run with -Z macro-backtrace for more info) + = note: integers must not be uninitialized error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:112:26 @@ -688,11 +662,7 @@ 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: raw pointers must not be uninitialized (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 From d7f5d784d7dd759944ddb8c224544b6f89c9fdc8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 7 Nov 2022 10:26:14 +0000 Subject: [PATCH 14/17] Simplify and document range layout computation --- compiler/rustc_abi/src/layout.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 39ea7a85be652..11e7b80f85efd 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -382,28 +382,26 @@ pub trait LayoutCalculator { let (start, end) = scalar_valid_range; match st.abi { Abi::Scalar(ref mut scalar) | Abi::ScalarPair(ref mut scalar, _) => { - // the asserts ensure that we are not using the - // `#[rustc_layout_scalar_valid_range(n)]` - // attribute to widen the range of anything as that would probably - // result in UB somewhere - // FIXME(eddyb) the asserts are probably not needed, - // as larger validity ranges would result in missed + // Enlarging validity ranges would result in missed // optimizations, *not* wrongly assuming the inner - // value is valid. e.g. unions enlarge validity ranges, + // value is valid. e.g. unions already enlarge validity ranges, // because the values may be uninitialized. + // + // Because of that we only check that the start and end + // of the range is representable with this scalar type. + + let max_value = scalar.size(dl).unsigned_int_max(); if let Bound::Included(start) = start { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. - let valid_range = scalar.valid_range_mut(); - assert!(valid_range.start <= start); - valid_range.start = start; + assert!(start <= max_value, "{start} > {max_value}"); + scalar.valid_range_mut().start = start; } if let Bound::Included(end) = end { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. - let valid_range = scalar.valid_range_mut(); - assert!(valid_range.end >= end); - valid_range.end = end; + assert!(end <= max_value, "{end} > {max_value}"); + scalar.valid_range_mut().end = end; } // Update `largest_niche` if we have introduced a larger niche. From 9b6f8e500c2d2dedfa838b18b93952486c12b6f5 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 7 Nov 2022 10:26:23 +0000 Subject: [PATCH 15/17] Add a test for OOB ranges --- src/test/ui/layout/valid_range_oob.rs | 15 +++++++++++++++ src/test/ui/layout/valid_range_oob.stderr | 6 ++++++ 2 files changed, 21 insertions(+) create mode 100644 src/test/ui/layout/valid_range_oob.rs create mode 100644 src/test/ui/layout/valid_range_oob.stderr diff --git a/src/test/ui/layout/valid_range_oob.rs b/src/test/ui/layout/valid_range_oob.rs new file mode 100644 index 0000000000000..74aa47fe40549 --- /dev/null +++ b/src/test/ui/layout/valid_range_oob.rs @@ -0,0 +1,15 @@ +// failure-status: 101 +// normalize-stderr-test "note: .*\n\n" -> "" +// normalize-stderr-test "thread 'rustc' panicked.*\n" -> "" +// rustc-env:RUST_BACKTRACE=0 + +#![feature(rustc_attrs)] + +#[rustc_layout_scalar_valid_range_end(257)] +struct Foo(i8); + +// Need to do in a constant, as runtime codegen +// does not compute the layout of `Foo` in check builds. +const FOO: Foo = unsafe { Foo(1) }; + +fn main() {} diff --git a/src/test/ui/layout/valid_range_oob.stderr b/src/test/ui/layout/valid_range_oob.stderr new file mode 100644 index 0000000000000..7398f01643f6e --- /dev/null +++ b/src/test/ui/layout/valid_range_oob.stderr @@ -0,0 +1,6 @@ +error: internal compiler error: unexpected panic + +query stack during panic: +#0 [layout_of] computing layout of `Foo` +#1 [eval_to_allocation_raw] const-evaluating + checking `FOO` +end of query stack From 5446a52b4bcdca0938f6393732de6e0c13305d9b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 7 Nov 2022 10:48:40 +0000 Subject: [PATCH 16/17] Add a `because` to errors derived from fields --- compiler/rustc_lint/src/builtin.rs | 43 +++++++++++++-------------- src/test/ui/lint/invalid_value.stderr | 24 +++++++-------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 4bde57881f083..3aee97e790863 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2410,7 +2410,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { /// Information about why a type cannot be initialized this way. struct InitError { message: String, - /// Spans from struct fields and similar can be obtained from just the type. + /// Spans from struct fields and similar that can be obtained from just the type. span: Option, /// Used to report a trace through adts. nested: Option>, @@ -2497,7 +2497,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { descr: &str, init: InitKind, ) -> Option { - let field_err = variant.fields.iter().find_map(|field| { + let mut field_err = variant.fields.iter().find_map(|field| { ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|mut err| { if !field.did.is_local() { err @@ -2515,28 +2515,27 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { // Check if this ADT has a constrained layout (like `NonNull` and friends). if let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty)) { - match &layout.abi { - Abi::Scalar(scalar) | Abi::ScalarPair(scalar, _) => { - let range = scalar.valid_range(cx); - if !range.contains(0) { - return 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. - return Some( - InitError::from(format!( - "`{}` must be initialized inside its custom valid range", - ty, - )) - .nested(field_err), - ); + if let Abi::Scalar(scalar) | Abi::ScalarPair(scalar, _) = &layout.abi { + let range = scalar.valid_range(cx); + let msg = if !range.contains(0) { + "must be non-null" + } 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. + + "must be initialized inside its custom valid range" + } else { + return field_err; + }; + if let Some(field_err) = &mut field_err { + // Most of the time, if the field error is the same as the struct error, + // the struct error only happens because of the field error. + if field_err.message.contains(msg) { + field_err.message = format!("because {}", field_err.message); } } - _ => {} + return Some(InitError::from(format!("`{ty}` {msg}")).nested(field_err)); } } field_err diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index 9f0f51f4d9726..afa09a074ecf1 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -35,7 +35,7 @@ LL | let _val: Wrap<&'static T> = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap<&T>` must be non-null -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } @@ -51,7 +51,7 @@ LL | let _val: Wrap<&'static T> = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap<&T>` must be non-null -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } @@ -163,7 +163,7 @@ LL | let _val: Ref = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Ref` must be non-null -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); @@ -179,7 +179,7 @@ LL | let _val: Ref = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Ref` must be non-null -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:14:12 | LL | struct Ref(&'static i32); @@ -217,7 +217,7 @@ LL | let _val: Wrap = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap` must be non-null -note: function pointers must be non-null (in this struct field) +note: because function pointers must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } @@ -233,7 +233,7 @@ LL | let _val: Wrap = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `Wrap` must be non-null -note: function pointers must be non-null (in this struct field) +note: because function pointers must be non-null (in this struct field) --> $DIR/invalid_value.rs:17:18 | LL | struct Wrap { wrapped: T } @@ -249,7 +249,7 @@ LL | let _val: WrapEnum = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `WrapEnum` must be non-null -note: function pointers must be non-null (in this field of the only potentially inhabited enum variant) +note: because function pointers must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } @@ -265,7 +265,7 @@ LL | let _val: WrapEnum = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `WrapEnum` must be non-null -note: function pointers must be non-null (in this field of the only potentially inhabited enum variant) +note: because function pointers must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:18:28 | LL | enum WrapEnum { Wrapped(T) } @@ -285,7 +285,7 @@ note: `RefPair` must be non-null (in this struct field) | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); @@ -305,7 +305,7 @@ note: `RefPair` must be non-null (in this struct field) | LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ -note: references must be non-null (in this struct field) +note: because references must be non-null (in this struct field) --> $DIR/invalid_value.rs:15:16 | LL | struct RefPair((&'static i32, i32)); @@ -441,7 +441,7 @@ LL | let _val: OneFruitNonZero = mem::zeroed(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `OneFruitNonZero` must be non-null -note: `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) +note: because `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:39:12 | LL | Banana(NonZeroU32), @@ -457,7 +457,7 @@ LL | let _val: OneFruitNonZero = mem::uninitialized(); | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | = note: `OneFruitNonZero` must be non-null -note: `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) +note: because `std::num::NonZeroU32` must be non-null (in this field of the only potentially inhabited enum variant) --> $DIR/invalid_value.rs:39:12 | LL | Banana(NonZeroU32), From 208bb933e7a6d8820646e3e367e2bf188d85e485 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 7 Nov 2022 10:52:48 +0000 Subject: [PATCH 17/17] Use "must be init" instead of "must not be uninit" everywhere --- compiler/rustc_lint/src/builtin.rs | 8 +++----- src/test/ui/lint/invalid_value.stderr | 22 +++++++++++----------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 3aee97e790863..5c9ea1234f1c0 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2568,13 +2568,11 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { Some("characters must be a valid Unicode codepoint".into()) } Int(_) | Uint(_) if init == InitKind::Uninit => { - Some("integers must not be uninitialized".into()) - } - Float(_) if init == InitKind::Uninit => { - Some("floats must not be uninitialized".into()) + Some("integers must be initialized".into()) } + Float(_) if init == InitKind::Uninit => Some("floats must be initialized".into()), RawPtr(_) if init == InitKind::Uninit => { - Some("raw pointers must not be uninitialized".into()) + Some("raw pointers must be initialized".into()) } // Recurse and checks for some compound types. (but not unions) Adt(adt_def, substs) if !adt_def.is_union() => { diff --git a/src/test/ui/lint/invalid_value.stderr b/src/test/ui/lint/invalid_value.stderr index afa09a074ecf1..5370660d6c185 100644 --- a/src/test/ui/lint/invalid_value.stderr +++ b/src/test/ui/lint/invalid_value.stderr @@ -99,7 +99,7 @@ LL | let _val: (i32, !) = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | - = note: integers must not be uninitialized + = note: integers must be initialized error: the type `Void` does not permit zero-initialization --> $DIR/invalid_value.rs:71:26 @@ -332,7 +332,7 @@ 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: raw pointers must not be uninitialized + = note: raw pointers must be initialized error: the type `(NonZeroU32, i32)` does not permit zero-initialization --> $DIR/invalid_value.rs:95:39 @@ -355,7 +355,7 @@ 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: integers must not be uninitialized + = note: integers must be initialized error: the type `*const dyn Send` does not permit zero-initialization --> $DIR/invalid_value.rs:98:37 @@ -462,7 +462,7 @@ note: because `std::num::NonZeroU32` must be non-null (in this field of the only | LL | Banana(NonZeroU32), | ^^^^^^^^^^ - = note: integers must not be uninitialized + = note: integers must be initialized error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:112:26 @@ -501,7 +501,7 @@ 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: integers must not be uninitialized (in this struct field) +note: integers must be initialized (in this struct field) --> $DIR/invalid_value.rs:23:26 | LL | pub(crate) struct NonBig(u64); @@ -542,7 +542,7 @@ LL | let _val: i32 = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | - = note: integers must not be uninitialized + = note: integers must be initialized error: the type `f32` does not permit being left uninitialized --> $DIR/invalid_value.rs:130:25 @@ -553,7 +553,7 @@ LL | let _val: f32 = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | - = note: floats must not be uninitialized + = note: floats must be initialized error: the type `*const ()` does not permit being left uninitialized --> $DIR/invalid_value.rs:133:31 @@ -564,7 +564,7 @@ LL | let _val: *const () = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | - = note: raw pointers must not be uninitialized + = note: raw pointers must be initialized error: the type `*const [()]` does not permit being left uninitialized --> $DIR/invalid_value.rs:136:33 @@ -575,7 +575,7 @@ LL | let _val: *const [()] = mem::uninitialized(); | this code causes undefined behavior when executed | help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done | - = note: raw pointers must not be uninitialized + = note: raw pointers must be initialized error: the type `WrapAroundRange` does not permit being left uninitialized --> $DIR/invalid_value.rs:139:37 @@ -587,7 +587,7 @@ 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: integers must not be uninitialized (in this struct field) +note: integers must be initialized (in this struct field) --> $DIR/invalid_value.rs:49:35 | LL | pub(crate) struct WrapAroundRange(u8); @@ -662,7 +662,7 @@ 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: raw pointers must not be uninitialized + = note: raw pointers must be initialized error: the type `bool` does not permit being left uninitialized --> $DIR/invalid_value.rs:159:26