Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that's UB #98919

Merged
merged 5 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
3 changes: 0 additions & 3 deletions library/core/src/mem/maybe_uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ use crate::slice;
/// // The equivalent code with `MaybeUninit<i32>`:
/// 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<T>`]
/// is considered initialized (under the current implementation; this does not constitute
Expand Down
10 changes: 5 additions & 5 deletions library/core/src/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,14 +665,14 @@ pub unsafe fn zeroed<T>() -> 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::<bool>()` 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
///
/// 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nearly all types

For which types mem::uninitialized is sound to call?

It it maybe the time to actually hard-remove the function, so that legacy pre-MaybeUninit code would get a compile error instead of run-time error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For which types

Inhabited ZSTs and values where every byte of it is inside a union or is padding. So (), (MaybeUninit<u32>, ()), [u32; 0], (MaybeUninit<u16>, MaybeUninit<u8>), etc. Not many.

Hard-removing it is tracked in #98862 to some extent. This PR only talks about what is UB, not what surface area the stdlib exposes.

/// including integer types and arrays of integer types, and even if the result is unused.
///
/// [uninit]: MaybeUninit::uninit
/// [assume_init]: MaybeUninit::assume_init
Expand Down
15 changes: 13 additions & 2 deletions src/test/ui/lint/uninitialized-zeroed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@ 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

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
Expand All @@ -114,13 +126,12 @@ fn main() {
let _val: Option<&'static i32> = mem::zeroed();
let _val: Option<fn()> = 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();
}
}
60 changes: 52 additions & 8 deletions src/test/ui/lint/uninitialized-zeroed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ LL | let _val: (i32, !) = mem::uninitialized();
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` 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
Expand Down Expand Up @@ -414,8 +414,52 @@ 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<T>` instead, and only call `assume_init` after initialization is done
|
= 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<T>` 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<T>` 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<T>` 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:104:34
--> $DIR/uninitialized-zeroed.rs:116:34
|
LL | let _val: &'static i32 = mem::transmute(0usize);
| ^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -426,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:105:36
--> $DIR/uninitialized-zeroed.rs:117:36
|
LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -437,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:106:32
--> $DIR/uninitialized-zeroed.rs:118:32
|
LL | let _val: NonZeroU32 = mem::transmute(0);
| ^^^^^^^^^^^^^^^^^
Expand All @@ -448,7 +492,7 @@ LL | let _val: NonZeroU32 = mem::transmute(0);
= note: `std::num::NonZeroU32` must be non-null

error: the type `NonNull<i32>` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:109:34
--> $DIR/uninitialized-zeroed.rs:121:34
|
LL | let _val: NonNull<i32> = MaybeUninit::zeroed().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -459,7 +503,7 @@ LL | let _val: NonNull<i32> = MaybeUninit::zeroed().assume_init();
= note: `std::ptr::NonNull<i32>` must be non-null

error: the type `NonNull<i32>` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:110:34
--> $DIR/uninitialized-zeroed.rs:122:34
|
LL | let _val: NonNull<i32> = MaybeUninit::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -470,7 +514,7 @@ LL | let _val: NonNull<i32> = MaybeUninit::uninit().assume_init();
= note: `std::ptr::NonNull<i32>` must be non-null

error: the type `bool` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:111:26
--> $DIR/uninitialized-zeroed.rs:123:26
|
LL | let _val: bool = MaybeUninit::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -480,5 +524,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 43 previous errors

1 change: 1 addition & 0 deletions src/test/ui/sanitize/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/uninit.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down