From 3e567bcd4f32d413770d464192a2ed6ee636883b Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 4 Jul 2022 23:57:41 +0100 Subject: [PATCH 1/5] Make invalid-value trigger on uninit primitives --- compiler/rustc_lint/src/builtin.rs | 9 +++++++ src/test/ui/lint/uninitialized-zeroed.rs | 6 +++-- src/test/ui/lint/uninitialized-zeroed.stderr | 27 ++++++++++++++------ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index bd58021f78fc0..5b617fb2e2733 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2475,6 +2475,15 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { Char if init == InitKind::Uninit => { Some(("characters must be a valid Unicode codepoint".to_string(), None)) } + Int(_) | Uint(_) if init == InitKind::Uninit => { + Some(("integers must not be uninitialized".to_string(), None)) + } + Float(_) if init == InitKind::Uninit => { + Some(("floats must not be uninitialized".to_string(), None)) + } + RawPtr(_) if init == InitKind::Uninit => { + Some(("raw pointers must not be uninitialized".to_string(), None)) + } // Recurse and checks for some compound types. Adt(adt_def, substs) if !adt_def.is_union() => { // First check if this ADT has a layout attribute (like `NonNull` and friends). diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index 5cd323c01db8c..2d1ee6492af55 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -100,6 +100,9 @@ fn main() { let _val: [bool; 2] = mem::zeroed(); let _val: [bool; 2] = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: i32 = mem::zeroed(); + let _val: i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + // Transmute-from-0 let _val: &'static i32 = mem::transmute(0usize); //~ ERROR: does not permit zero-initialization let _val: &'static [i32] = mem::transmute((0usize, 0usize)); //~ ERROR: does not permit zero-initialization @@ -114,13 +117,12 @@ fn main() { let _val: Option<&'static i32> = mem::zeroed(); let _val: Option = mem::zeroed(); let _val: MaybeUninit<&'static i32> = mem::zeroed(); - let _val: i32 = mem::zeroed(); let _val: bool = MaybeUninit::zeroed().assume_init(); let _val: [bool; 0] = MaybeUninit::uninit().assume_init(); let _val: [!; 0] = MaybeUninit::zeroed().assume_init(); + // Some things that happen to work due to rustc implementation details, // but are not guaranteed to keep working. - let _val: i32 = mem::uninitialized(); let _val: OneFruit = mem::uninitialized(); } } diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index 88121a1836da4..69fce32153ce5 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -97,7 +97,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: the `!` type has no valid value + = note: integers must not be uninitialized error: the type `Void` does not permit zero-initialization --> $DIR/uninitialized-zeroed.rs:57:26 @@ -414,8 +414,19 @@ LL | let _val: [bool; 2] = mem::uninitialized(); | = note: booleans must be either `true` or `false` +error: the type `i32` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:104:25 + | +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 + error: the type `&i32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:104:34 + --> $DIR/uninitialized-zeroed.rs:107:34 | LL | let _val: &'static i32 = mem::transmute(0usize); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -426,7 +437,7 @@ LL | let _val: &'static i32 = mem::transmute(0usize); = note: references must be non-null error: the type `&[i32]` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:105:36 + --> $DIR/uninitialized-zeroed.rs:108:36 | LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -437,7 +448,7 @@ LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); = note: references must be non-null error: the type `NonZeroU32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:106:32 + --> $DIR/uninitialized-zeroed.rs:109:32 | LL | let _val: NonZeroU32 = mem::transmute(0); | ^^^^^^^^^^^^^^^^^ @@ -448,7 +459,7 @@ LL | let _val: NonZeroU32 = mem::transmute(0); = note: `std::num::NonZeroU32` must be non-null error: the type `NonNull` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:109:34 + --> $DIR/uninitialized-zeroed.rs:112:34 | LL | let _val: NonNull = MaybeUninit::zeroed().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -459,7 +470,7 @@ LL | let _val: NonNull = MaybeUninit::zeroed().assume_init(); = note: `std::ptr::NonNull` must be non-null error: the type `NonNull` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:110:34 + --> $DIR/uninitialized-zeroed.rs:113:34 | LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -470,7 +481,7 @@ LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); = note: `std::ptr::NonNull` must be non-null error: the type `bool` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:111:26 + --> $DIR/uninitialized-zeroed.rs:114:26 | LL | let _val: bool = MaybeUninit::uninit().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -480,5 +491,5 @@ LL | let _val: bool = MaybeUninit::uninit().assume_init(); | = note: booleans must be either `true` or `false` -error: aborting due to 39 previous errors +error: aborting due to 40 previous errors From 57ddb2d02ed61666125602c399e4492a1e8d1f51 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Tue, 5 Jul 2022 08:03:06 +0100 Subject: [PATCH 2/5] Creating uninitialized integers is UB --- library/core/src/mem/maybe_uninit.rs | 3 --- library/core/src/mem/mod.rs | 11 +++-------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 997494c769ec7..2490c07679dd0 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -54,9 +54,6 @@ use crate::slice; /// // The equivalent code with `MaybeUninit`: /// let x: i32 = unsafe { MaybeUninit::uninit().assume_init() }; // undefined behavior! ⚠️ /// ``` -/// (Notice that the rules around uninitialized integers are not finalized yet, but -/// until they are, it is advisable to avoid them.) -/// /// On top of that, remember that most types have additional invariants beyond merely /// being considered initialized at the type level. For example, a `1`-initialized [`Vec`] /// is considered initialized (under the current implementation; this does not constitute diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index 20b2d5e2681c2..20be406c0e5ac 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -665,14 +665,9 @@ pub unsafe fn zeroed() -> T { /// correctly: it has the same effect as [`MaybeUninit::uninit().assume_init()`][uninit]. /// As the [`assume_init` documentation][assume_init] explains, /// [the Rust compiler assumes][inv] that values are properly initialized. -/// As a consequence, calling e.g. `mem::uninitialized::()` causes immediate -/// undefined behavior for returning a `bool` that is not definitely either `true` -/// or `false`. Worse, truly uninitialized memory like what gets returned here -/// is special in that the compiler knows that it does not have a fixed value. -/// This makes it undefined behavior to have uninitialized data in a variable even -/// if that variable has an integer type. -/// (Notice that the rules around uninitialized integers are not finalized yet, but -/// until they are, it is advisable to avoid them.) +/// +/// Therefore, it is immediate undefined behavior to call this function on nearly all types, +/// including integer types and arrays of integer types, and even if the result is unused. /// /// [uninit]: MaybeUninit::uninit /// [assume_init]: MaybeUninit::assume_init From 73a30f8d8bb7a2da0e72d9c04c2cfd8db11b35f3 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Tue, 5 Jul 2022 08:50:42 +0100 Subject: [PATCH 3/5] More tests for invalid_value lint --- src/test/ui/lint/uninitialized-zeroed.rs | 9 ++++ src/test/ui/lint/uninitialized-zeroed.stderr | 47 +++++++++++++++++--- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index 2d1ee6492af55..dae258407ebb0 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -103,6 +103,15 @@ fn main() { let _val: i32 = mem::zeroed(); let _val: i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: f32 = mem::zeroed(); + let _val: f32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: *const () = mem::zeroed(); + let _val: *const () = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: *const [()] = mem::zeroed(); + let _val: *const [()] = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + // Transmute-from-0 let _val: &'static i32 = mem::transmute(0usize); //~ ERROR: does not permit zero-initialization let _val: &'static [i32] = mem::transmute((0usize, 0usize)); //~ ERROR: does not permit zero-initialization diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index 69fce32153ce5..b46042e7be43f 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -425,8 +425,41 @@ LL | let _val: i32 = mem::uninitialized(); | = note: integers must not be uninitialized +error: the type `f32` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:107:25 + | +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 + +error: the type `*const ()` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:110:31 + | +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 + +error: the type `*const [()]` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:113:33 + | +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 + error: the type `&i32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:107:34 + --> $DIR/uninitialized-zeroed.rs:116:34 | LL | let _val: &'static i32 = mem::transmute(0usize); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -437,7 +470,7 @@ LL | let _val: &'static i32 = mem::transmute(0usize); = note: references must be non-null error: the type `&[i32]` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:108:36 + --> $DIR/uninitialized-zeroed.rs:117:36 | LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -448,7 +481,7 @@ LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); = note: references must be non-null error: the type `NonZeroU32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:109:32 + --> $DIR/uninitialized-zeroed.rs:118:32 | LL | let _val: NonZeroU32 = mem::transmute(0); | ^^^^^^^^^^^^^^^^^ @@ -459,7 +492,7 @@ LL | let _val: NonZeroU32 = mem::transmute(0); = note: `std::num::NonZeroU32` must be non-null error: the type `NonNull` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:112:34 + --> $DIR/uninitialized-zeroed.rs:121:34 | LL | let _val: NonNull = MaybeUninit::zeroed().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -470,7 +503,7 @@ LL | let _val: NonNull = MaybeUninit::zeroed().assume_init(); = note: `std::ptr::NonNull` must be non-null error: the type `NonNull` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:113:34 + --> $DIR/uninitialized-zeroed.rs:122:34 | LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -481,7 +514,7 @@ LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); = note: `std::ptr::NonNull` must be non-null error: the type `bool` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:114:26 + --> $DIR/uninitialized-zeroed.rs:123:26 | LL | let _val: bool = MaybeUninit::uninit().assume_init(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -491,5 +524,5 @@ LL | let _val: bool = MaybeUninit::uninit().assume_init(); | = note: booleans must be either `true` or `false` -error: aborting due to 40 previous errors +error: aborting due to 43 previous errors From 5e8f95ba7d53f022c658aa9094c470f57e7a6076 Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Tue, 5 Jul 2022 17:50:33 +0100 Subject: [PATCH 4/5] Re-add some justification --- library/core/src/mem/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index 20be406c0e5ac..d2dd2941d590f 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -666,6 +666,11 @@ pub unsafe fn zeroed() -> T { /// As the [`assume_init` documentation][assume_init] explains, /// [the Rust compiler assumes][inv] that values are properly initialized. /// +/// Truly uninitialized memory like what gets returned here +/// is special in that the compiler knows that it does not have a fixed value. +/// This makes it undefined behavior to have uninitialized data in a variable even +/// if that variable has an integer type. +/// /// Therefore, it is immediate undefined behavior to call this function on nearly all types, /// including integer types and arrays of integer types, and even if the result is unused. /// From be324607ea90340001b00b4554502c8ec937313d Mon Sep 17 00:00:00 2001 From: 5225225 <5225225@mailbox.org> Date: Mon, 29 Aug 2022 21:28:35 +0100 Subject: [PATCH 5/5] Fix tests due to stricter invalid_value --- src/test/ui/sanitize/memory.rs | 1 + src/tools/clippy/tests/ui/uninit.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/sanitize/memory.rs b/src/test/ui/sanitize/memory.rs index 83f79679c6e67..adda51f6be0ec 100644 --- a/src/test/ui/sanitize/memory.rs +++ b/src/test/ui/sanitize/memory.rs @@ -14,6 +14,7 @@ #![feature(core_intrinsics)] #![feature(start)] #![feature(bench_black_box)] +#![allow(invalid_value)] use std::hint::black_box; use std::mem::MaybeUninit; diff --git a/src/tools/clippy/tests/ui/uninit.rs b/src/tools/clippy/tests/ui/uninit.rs index dac5ce272c026..2113173170868 100644 --- a/src/tools/clippy/tests/ui/uninit.rs +++ b/src/tools/clippy/tests/ui/uninit.rs @@ -1,5 +1,5 @@ #![feature(stmt_expr_attributes)] -#![allow(clippy::let_unit_value)] +#![allow(clippy::let_unit_value, invalid_value)] use std::mem::{self, MaybeUninit};