Auto merge of rust-lang#10520 - Nilstrieb:uninit, r=Alexendoo

Use uninit checking from rustc rustc has proper heuristics for actually checking whether a type allows being left uninitialized (by asking CTFE). We can now use this for our helper instead of rolling our own bad version with false positives. I added this in rustc in rust-lang#108669 Fix rust-lang#10407 changelog: [`uninit_vec`]: fix false positives changelog: [`uninit_assumed_init`]: fix false positives
flip1995 · Mar 21, 2023 · 7b3c4aa · 7b3c4aa
2 parents 0a77f4c + 84b6049
commit 7b3c4aa
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 31 deletions.
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
@@ -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;
@@ -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.

diff --git a/tests/ui/uninit.rs b/tests/ui/uninit.rs
@@ -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
-    let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };
-
-    // edge case: For now we accept unit tuples
+    // This is OK, because ZSTs do not contain data.
     let _: () = unsafe { MaybeUninit::uninit().assume_init() };
 
     // This is OK, because `MaybeUninit` allows uninitialized data.
@@ -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() };
+    }
 }
diff --git a/tests/ui/uninit.stderr b/tests/ui/uninit.stderr
@@ -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

diff --git a/tests/ui/uninit_vec.rs b/tests/ui/uninit_vec.rs
@@ -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);
@@ -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);
+        }
+    }
 }
diff --git a/tests/ui/uninit_vec.stderr b/tests/ui/uninit_vec.stderr
@@ -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);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |     ^^^^^^^^^^^^^^^^^^
@@ -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();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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);
    |         ^^^^^^^^^^^^^^^^^^
@@ -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