Skip to content

Commit 938f8ba

Browse files
committed
Auto merge of rust-lang#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 rust-lang#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
2 parents 1e797e9 + ae5326b commit 938f8ba

File tree

3 files changed

+62
-5
lines changed

3 files changed

+62
-5
lines changed

clippy_utils/src/ty.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -606,10 +606,13 @@ fn is_uninit_value_valid_for_ty_fallback<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
606606
ty::Tuple(types) => types.iter().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
607607
// Unions are always fine right now.
608608
// This includes MaybeUninit, the main way people use uninitialized memory.
609-
// For ADTs, we could look at all fields just like for tuples, but that's potentially
610-
// exponential, so let's avoid doing that for now. Code doing that is sketchy enough to
611-
// just use an `#[allow()]`.
612-
ty::Adt(adt, _) => adt.is_union(),
609+
ty::Adt(adt, _) if adt.is_union() => true,
610+
// Types (e.g. `UnsafeCell<MaybeUninit<T>>`) that recursively contain only types that can be uninit
611+
// can themselves be uninit too.
612+
// This purposefully ignores enums as they may have a discriminant that can't be uninit.
613+
ty::Adt(adt, args) if adt.is_struct() => adt
614+
.all_fields()
615+
.all(|field| is_uninit_value_valid_for_ty(cx, field.ty(cx.tcx, args))),
613616
// For the rest, conservatively assume that they cannot be uninit.
614617
_ => false,
615618
}

tests/ui/uninit_vec.rs

+32
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,36 @@ fn main() {
150150
vec.set_len(10);
151151
}
152152
}
153+
154+
fn nested_union<T>() {
155+
let mut vec: Vec<UnsafeCell<MaybeUninit<T>>> = Vec::with_capacity(1);
156+
unsafe {
157+
vec.set_len(1);
158+
}
159+
}
160+
161+
struct Recursive<T>(*const Recursive<T>, MaybeUninit<T>);
162+
fn recursive_union<T>() {
163+
// Make sure we don't stack overflow on recursive types.
164+
// The pointer acts as the base case because it can't be uninit regardless of its pointee.
165+
166+
let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
167+
//~^ uninit_vec
168+
unsafe {
169+
vec.set_len(1);
170+
}
171+
}
172+
173+
#[repr(u8)]
174+
enum Enum<T> {
175+
Variant(T),
176+
}
177+
fn union_in_enum<T>() {
178+
// Enums can have a discriminant that can't be uninit, so this should still warn
179+
let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
180+
//~^ uninit_vec
181+
unsafe {
182+
vec.set_len(1);
183+
}
184+
}
153185
}

tests/ui/uninit_vec.stderr

+23-1
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,27 @@ LL | vec.set_len(10);
125125
|
126126
= help: initialize the buffer or wrap the content in `MaybeUninit`
127127

128-
error: aborting due to 12 previous errors
128+
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
129+
--> tests/ui/uninit_vec.rs:166:9
130+
|
131+
LL | let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
132+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
133+
...
134+
LL | vec.set_len(1);
135+
| ^^^^^^^^^^^^^^
136+
|
137+
= help: initialize the buffer or wrap the content in `MaybeUninit`
138+
139+
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
140+
--> tests/ui/uninit_vec.rs:179:9
141+
|
142+
LL | let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
143+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
144+
...
145+
LL | vec.set_len(1);
146+
| ^^^^^^^^^^^^^^
147+
|
148+
= help: initialize the buffer or wrap the content in `MaybeUninit`
149+
150+
error: aborting due to 14 previous errors
129151

0 commit comments

Comments
 (0)