about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-09-09 23:25:46 +0000
committerbors <bors@rust-lang.org>2024-09-09 23:25:46 +0000
commit938f8ba20aac34a2ef696627c8bf871844da67f9 (patch)
tree0998f0b14a96006468573326426e45bcc84f34f0
parent1e797e9a381906c9b249209dae053bbbd13b50ca (diff)
parentae5326b967d5a3d34d3c562882feeacd9839950c (diff)
downloadrust-938f8ba20aac34a2ef696627c8bf871844da67f9.tar.gz
rust-938f8ba20aac34a2ef696627c8bf871844da67f9.zip
Auto merge of #13367 - y21:issue13364, r=Manishearth
Visit struct fields recursively in uninit fallback check

This makes the fallback a bit more consistent with the other checks and rustc.

Fixes #13364. When using a generic type as the `Vec` element type like the issue title says, rustc's uninit check fails and our fallback is used, which didn't look at struct fields when it could.

changelog: none
-rw-r--r--clippy_utils/src/ty.rs11
-rw-r--r--tests/ui/uninit_vec.rs32
-rw-r--r--tests/ui/uninit_vec.stderr24
3 files changed, 62 insertions, 5 deletions
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index f80981c11af..5756e56c262 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -606,10 +606,13 @@ fn is_uninit_value_valid_for_ty_fallback<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
         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(),
+        ty::Adt(adt, _) if adt.is_union() => true,
+        // Types (e.g. `UnsafeCell<MaybeUninit<T>>`) that recursively contain only types that can be uninit
+        // can themselves be uninit too.
+        // This purposefully ignores enums as they may have a discriminant that can't be uninit.
+        ty::Adt(adt, args) if adt.is_struct() => adt
+            .all_fields()
+            .all(|field| is_uninit_value_valid_for_ty(cx, field.ty(cx.tcx, args))),
         // For the rest, conservatively assume that they cannot be uninit.
         _ => false,
     }
diff --git a/tests/ui/uninit_vec.rs b/tests/ui/uninit_vec.rs
index 0cc77a8775d..464f88140bd 100644
--- a/tests/ui/uninit_vec.rs
+++ b/tests/ui/uninit_vec.rs
@@ -150,4 +150,36 @@ fn main() {
             vec.set_len(10);
         }
     }
+
+    fn nested_union<T>() {
+        let mut vec: Vec<UnsafeCell<MaybeUninit<T>>> = Vec::with_capacity(1);
+        unsafe {
+            vec.set_len(1);
+        }
+    }
+
+    struct Recursive<T>(*const Recursive<T>, MaybeUninit<T>);
+    fn recursive_union<T>() {
+        // Make sure we don't stack overflow on recursive types.
+        // The pointer acts as the base case because it can't be uninit regardless of its pointee.
+
+        let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
+        //~^ uninit_vec
+        unsafe {
+            vec.set_len(1);
+        }
+    }
+
+    #[repr(u8)]
+    enum Enum<T> {
+        Variant(T),
+    }
+    fn union_in_enum<T>() {
+        // Enums can have a discriminant that can't be uninit, so this should still warn
+        let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
+        //~^ uninit_vec
+        unsafe {
+            vec.set_len(1);
+        }
+    }
 }
diff --git a/tests/ui/uninit_vec.stderr b/tests/ui/uninit_vec.stderr
index e8b77d653f0..e7c81cf792f 100644
--- a/tests/ui/uninit_vec.stderr
+++ b/tests/ui/uninit_vec.stderr
@@ -125,5 +125,27 @@ LL |             vec.set_len(10);
    |
    = help: initialize the buffer or wrap the content in `MaybeUninit`
 
-error: aborting due to 12 previous errors
+error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
+  --> tests/ui/uninit_vec.rs:166:9
+   |
+LL |         let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |             vec.set_len(1);
+   |             ^^^^^^^^^^^^^^
+   |
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
+
+error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
+  --> tests/ui/uninit_vec.rs:179:9
+   |
+LL |         let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |             vec.set_len(1);
+   |             ^^^^^^^^^^^^^^
+   |
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
+
+error: aborting due to 14 previous errors