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

Use uninit checking from rustc #10520

Merged
merged 1 commit into from
Mar 21, 2023
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
19 changes: 9 additions & 10 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use rustc_infer::infer::{
use rustc_lint::LateContext;
use rustc_middle::mir::interpret::{ConstValue, Scalar};
use rustc_middle::ty::{
self, AdtDef, AliasTy, AssocKind, Binder, BoundRegion, FnSig, IntTy, List, ParamEnv, Predicate, PredicateKind,
Region, RegionKind, SubstsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
UintTy, VariantDef, VariantDiscr,
self, layout::ValidityRequirement, AdtDef, AliasTy, AssocKind, Binder, BoundRegion, FnSig, IntTy, List, ParamEnv,
Predicate, PredicateKind, Region, RegionKind, SubstsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
TypeVisitableExt, TypeVisitor, UintTy, VariantDef, VariantDiscr,
};
use rustc_middle::ty::{GenericArg, GenericArgKind};
use rustc_span::symbol::Ident;
Expand Down Expand Up @@ -538,13 +538,12 @@ pub fn same_type_and_consts<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
}

/// Checks if a given type looks safe to be uninitialized.
pub fn is_uninit_value_valid_for_ty(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
match *ty.kind() {
ty::Array(component, _) => is_uninit_value_valid_for_ty(cx, component),
ty::Tuple(types) => types.iter().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
ty::Adt(adt, _) => cx.tcx.lang_items().maybe_uninit() == Some(adt.did()),
_ => false,
}
pub fn is_uninit_value_valid_for_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
cx.tcx
.check_validity_requirement((ValidityRequirement::Uninit, cx.param_env.and(ty)))
// For types containing generic parameters we cannot get a layout to check.
// Therefore, we are conservative and assume that they don't allow uninit.
.unwrap_or(false)
}

/// Gets an iterator over all predicates which apply to the given item.
Expand Down
23 changes: 19 additions & 4 deletions tests/ui/uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

use std::mem::{self, MaybeUninit};

union MyOwnMaybeUninit {
value: u8,
uninit: (),
}

fn main() {
let _: usize = unsafe { MaybeUninit::uninit().assume_init() };

// edge case: For now we lint on empty arrays
Noratrieb marked this conversation as resolved.
Show resolved Hide resolved
let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };

// edge case: For now we accept unit tuples
Copy link
Member Author

Choose a reason for hiding this comment

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

I also changed this comment because this isn't really a "for now", ZSTs are simply valid here.

// This is OK, because ZSTs do not contain data.
let _: () = unsafe { MaybeUninit::uninit().assume_init() };

// This is OK, because `MaybeUninit` allows uninitialized data.
Expand All @@ -21,6 +23,19 @@ fn main() {
// This is OK, because all constitutent types are uninit-compatible.
let _: (MaybeUninit<usize>, [MaybeUninit<bool>; 2]) = unsafe { MaybeUninit::uninit().assume_init() };

// This is OK, because our own MaybeUninit is just as fine as the one from core.
let _: MyOwnMaybeUninit = unsafe { MaybeUninit::uninit().assume_init() };

// This is OK, because empty arrays don't contain data.
let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };

// Was a false negative.
let _: usize = unsafe { mem::MaybeUninit::uninit().assume_init() };

polymorphic::<()>();

fn polymorphic<T>() {
// We are conservative around polymorphic types.
let _: T = unsafe { mem::MaybeUninit::uninit().assume_init() };
}
}
12 changes: 6 additions & 6 deletions tests/ui/uninit.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
error: this call for this type may be undefined behavior
--> $DIR/uninit.rs:7:29
--> $DIR/uninit.rs:12:29
|
LL | let _: usize = unsafe { MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(clippy::uninit_assumed_init)]` on by default

error: this call for this type may be undefined behavior
--> $DIR/uninit.rs:10:31
--> $DIR/uninit.rs:33:29
|
LL | let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | let _: usize = unsafe { mem::MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this call for this type may be undefined behavior
--> $DIR/uninit.rs:25:29
--> $DIR/uninit.rs:39:29
|
LL | let _: usize = unsafe { mem::MaybeUninit::uninit().assume_init() };
LL | let _: T = unsafe { mem::MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/uninit_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ struct MyVec {
vec: Vec<u8>,
}

union MyOwnMaybeUninit {
value: u8,
uninit: (),
}

fn main() {
// with_capacity() -> set_len() should be detected
let mut vec: Vec<u8> = Vec::with_capacity(1000);
Expand Down Expand Up @@ -97,4 +102,26 @@ fn main() {
unsafe {
vec.set_len(0);
}

// ZSTs should not be detected
let mut vec: Vec<()> = Vec::with_capacity(1000);
unsafe {
vec.set_len(10);
}

// unions should not be detected
let mut vec: Vec<MyOwnMaybeUninit> = Vec::with_capacity(1000);
unsafe {
vec.set_len(10);
}

polymorphic::<()>();

fn polymorphic<T>() {
// We are conservative around polymorphic types.
let mut vec: Vec<T> = Vec::with_capacity(1000);
unsafe {
vec.set_len(10);
}
}
}
33 changes: 22 additions & 11 deletions tests/ui/uninit_vec.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:12:5
--> $DIR/uninit_vec.rs:17:5
|
LL | let mut vec: Vec<u8> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -11,7 +11,7 @@ LL | vec.set_len(200);
= note: `-D clippy::uninit-vec` implied by `-D warnings`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:18:5
--> $DIR/uninit_vec.rs:23:5
|
LL | vec.reserve(1000);
| ^^^^^^^^^^^^^^^^^^
Expand All @@ -22,7 +22,7 @@ LL | vec.set_len(200);
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` on empty `Vec` creates out-of-bound values
--> $DIR/uninit_vec.rs:24:5
--> $DIR/uninit_vec.rs:29:5
|
LL | let mut vec: Vec<u8> = Vec::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -31,7 +31,7 @@ LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^

error: calling `set_len()` on empty `Vec` creates out-of-bound values
--> $DIR/uninit_vec.rs:30:5
--> $DIR/uninit_vec.rs:35:5
|
LL | let mut vec: Vec<u8> = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -40,7 +40,7 @@ LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^

error: calling `set_len()` on empty `Vec` creates out-of-bound values
--> $DIR/uninit_vec.rs:35:5
--> $DIR/uninit_vec.rs:40:5
|
LL | let mut vec: Vec<u8> = Vec::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -49,7 +49,7 @@ LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:49:5
--> $DIR/uninit_vec.rs:54:5
|
LL | let mut vec: Vec<u8> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -60,7 +60,7 @@ LL | vec.set_len(200);
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:58:5
--> $DIR/uninit_vec.rs:63:5
|
LL | my_vec.vec.reserve(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -71,7 +71,7 @@ LL | my_vec.vec.set_len(200);
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:63:5
--> $DIR/uninit_vec.rs:68:5
|
LL | my_vec.vec = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -82,7 +82,7 @@ LL | my_vec.vec.set_len(200);
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:42:9
--> $DIR/uninit_vec.rs:47:9
|
LL | let mut vec: Vec<u8> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -92,7 +92,7 @@ LL | vec.set_len(200);
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:45:9
--> $DIR/uninit_vec.rs:50:9
|
LL | vec.reserve(1000);
| ^^^^^^^^^^^^^^^^^^
Expand All @@ -101,5 +101,16 @@ LL | vec.set_len(200);
|
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: aborting due to 10 previous errors
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:122:9
|
LL | let mut vec: Vec<T> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | unsafe {
LL | vec.set_len(10);
| ^^^^^^^^^^^^^^^
|
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: aborting due to 11 previous errors