Skip to content

Commit

Permalink
Auto merge of rust-lang#10553 - Nilstrieb:better-uninit, r=Alexendoo
Browse files Browse the repository at this point in the history
In uninit checking, add fallback for polymorphic types

After rust-lang#10520, we always assumed that polymorphic types do not allow to be left uninitialized. But we can do better, by peeking into polymorphic types and adding a few special cases for going through tuples, arrays (because the length may be polymorphic) and blanket allowing all unions (like MaybeUninit).

fixes rust-lang#10551

changelog: [uninit_vec]: fix false positive for polymorphic types
changelog: [uninit_assumed_init]: fix false positive for polymorphic types
  • Loading branch information
bors committed Mar 29, 2023
2 parents 58fb801 + 51b4d2a commit c5011e9
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 11 deletions.
22 changes: 19 additions & 3 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,25 @@ pub fn same_type_and_consts<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
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)
.unwrap_or_else(|_| is_uninit_value_valid_for_ty_fallback(cx, ty))
}

/// A fallback for polymorphic types, which are not supported by `check_validity_requirement`.
fn is_uninit_value_valid_for_ty_fallback<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
match *ty.kind() {
// The array length may be polymorphic, let's try the inner type.
ty::Array(component, _) => is_uninit_value_valid_for_ty(cx, component),
// Peek through tuples and try their fallbacks.
ty::Tuple(types) => types.iter().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
// Unions are always fine right now.
// This includes MaybeUninit, the main way people use uninitialized memory.
// For ADTs, we could look at all fields just like for tuples, but that's potentially
// exponential, so let's avoid doing that for now. Code doing that is sketchy enough to
// just use an `#[allow()]`.
ty::Adt(adt, _) => adt.is_union(),
// For the rest, conservatively assume that they cannot be uninit.
_ => false,
}
}

/// Gets an iterator over all predicates which apply to the given item.
Expand Down
18 changes: 15 additions & 3 deletions tests/ui/uninit.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![feature(stmt_expr_attributes)]
#![allow(clippy::let_unit_value, invalid_value)]

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

union MyOwnMaybeUninit {
value: u8,
Expand Down Expand Up @@ -30,12 +30,24 @@ fn main() {
let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };

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

polymorphic::<()>();
polymorphic_maybe_uninit_array::<10>();
polymorphic_maybe_uninit::<u8>();

fn polymorphic<T>() {
// We are conservative around polymorphic types.
let _: T = unsafe { mem::MaybeUninit::uninit().assume_init() };
let _: T = unsafe { MaybeUninit::uninit().assume_init() };
}

fn polymorphic_maybe_uninit_array<const N: usize>() {
// While the type is polymorphic, MaybeUninit<u8> is not.
let _: [MaybeUninit<u8>; N] = unsafe { MaybeUninit::uninit().assume_init() };
}

fn polymorphic_maybe_uninit<T>() {
// The entire type is polymorphic, but it's wrapped in a MaybeUninit.
let _: MaybeUninit<T> = unsafe { MaybeUninit::uninit().assume_init() };
}
}
10 changes: 5 additions & 5 deletions tests/ui/uninit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ LL | let _: usize = unsafe { MaybeUninit::uninit().assume_init() };
error: this call for this type may be undefined behavior
--> $DIR/uninit.rs:33:29
|
LL | let _: usize = unsafe { mem::MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | let _: usize = unsafe { MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to 3 previous errors

8 changes: 8 additions & 0 deletions tests/ui/uninit_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,12 @@ fn main() {
vec.set_len(10);
}
}

fn poly_maybe_uninit<T>() {
// We are conservative around polymorphic types.
let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
unsafe {
vec.set_len(10);
}
}
}

0 comments on commit c5011e9

Please sign in to comment.