Skip to content

Commit

Permalink
Rollup merge of rust-lang#66044 - RalfJung:uninit-lint, r=oli-obk
Browse files Browse the repository at this point in the history
Improve uninit/zeroed lint

* Also warn when creating a raw pointer with a NULL vtable.
* Also identify `MaybeUninit::uninit().assume_init()` and `MaybeUninit::zeroed().assume_init()` as dangerous.
  • Loading branch information
JohnTitor authored Nov 7, 2019
2 parents 1988f28 + bb37d00 commit 12ffe5f
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/libcore/mem/maybe_uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ impl<T> MaybeUninit<T> {
/// ```
#[stable(feature = "maybe_uninit", since = "1.36.0")]
#[inline(always)]
#[cfg_attr(all(not(bootstrap)), rustc_diagnostic_item = "assume_init")]
pub unsafe fn assume_init(self) -> T {
intrinsics::panic_if_uninhabited::<T>();
ManuallyDrop::into_inner(self.value)
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,9 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> {

/// Check if a `DefId`'s path matches the given absolute type path usage.
///
/// Anonymous scopes such as `extern` imports are matched with `kw::Invalid`;
/// inherent `impl` blocks are matched with the name of the type.
///
/// # Examples
///
/// ```rust,ignore (no context or def id available)
Expand Down
26 changes: 24 additions & 2 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1909,8 +1909,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
// `Invalid` represents the empty string and matches that.
const TRANSMUTE_PATH: &[Symbol] =
&[sym::core, sym::intrinsics, kw::Invalid, sym::transmute];
const MU_ZEROED_PATH: &[Symbol] =
&[sym::core, sym::mem, sym::maybe_uninit, sym::MaybeUninit, sym::zeroed];
const MU_UNINIT_PATH: &[Symbol] =
&[sym::core, sym::mem, sym::maybe_uninit, sym::MaybeUninit, sym::uninit];

if let hir::ExprKind::Call(ref path_expr, ref args) = expr.kind {
// Find calls to `mem::{uninitialized,zeroed}` methods.
if let hir::ExprKind::Path(ref qpath) = path_expr.kind {
let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?;

Expand All @@ -1925,8 +1930,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
return Some(InitKind::Zeroed);
}
}
// FIXME: Also detect `MaybeUninit::zeroed().assume_init()` and
// `MaybeUninit::uninit().assume_init()`.
}
} else if let hir::ExprKind::MethodCall(_, _, ref args) = expr.kind {
// Find problematic calls to `MaybeUninit::assume_init`.
let def_id = cx.tables.type_dependent_def_id(expr.hir_id)?;
if cx.tcx.is_diagnostic_item(sym::assume_init, def_id) {
// This is a call to *some* method named `assume_init`.
// See if the `self` parameter is one of the dangerous constructors.
if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind {
if let hir::ExprKind::Path(ref qpath) = path_expr.kind {
let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?;
if cx.match_def_path(def_id, MU_ZEROED_PATH) {
return Some(InitKind::Zeroed);
} else if cx.match_def_path(def_id, MU_UNINIT_PATH) {
return Some(InitKind::Uninit);
}
}
}
}
}

Expand All @@ -1947,6 +1967,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
Adt(..) if ty.is_box() => Some((format!("`Box` must be non-null"), None)),
FnPtr(..) => Some((format!("Function pointers must be non-null"), None)),
Never => Some((format!("The never type (`!`) has no valid value"), None)),
RawPtr(tm) if matches!(tm.ty.kind, Dynamic(..)) => // raw ptr to dyn Trait
Some((format!("The vtable of a wide raw pointer must be non-null"), None)),
// Primitive types with other constraints.
Bool if init == InitKind::Uninit =>
Some((format!("Booleans must be `true` or `false`"), None)),
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#![feature(box_patterns)]
#![feature(box_syntax)]
#![feature(nll)]
#![feature(matches_macro)]

#![recursion_limit="256"]

Expand Down
4 changes: 4 additions & 0 deletions src/libsyntax_pos/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ symbols! {
associated_type_bounds,
associated_type_defaults,
associated_types,
assume_init,
async_await,
async_closure,
attr,
Expand Down Expand Up @@ -417,6 +418,8 @@ symbols! {
match_beginning_vert,
match_default_bindings,
may_dangle,
maybe_uninit,
MaybeUninit,
mem,
member_constraints,
message,
Expand Down Expand Up @@ -709,6 +712,7 @@ symbols! {
underscore_imports,
underscore_lifetimes,
uniform_paths,
uninit,
uninitialized,
universal_impl_trait,
unmarked_api,
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/lint/uninitialized-zeroed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ fn main() {
let _val: NonNull<i32> = mem::zeroed(); //~ ERROR: does not permit zero-initialization
let _val: NonNull<i32> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized

let _val: *const dyn Send = mem::zeroed(); //~ ERROR: does not permit zero-initialization
let _val: *const dyn Send = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized

// Things that can be zero, but not uninit.
let _val: bool = mem::zeroed();
let _val: bool = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
Expand All @@ -82,10 +85,16 @@ fn main() {
let _val: &'static [i32] = mem::transmute((0usize, 0usize)); //~ ERROR: does not permit zero-initialization
let _val: NonZeroU32 = mem::transmute(0); //~ ERROR: does not permit zero-initialization

// `MaybeUninit` cases
let _val: NonNull<i32> = MaybeUninit::zeroed().assume_init(); //~ ERROR: does not permit zero-initialization
let _val: NonNull<i32> = MaybeUninit::uninit().assume_init(); //~ ERROR: does not permit being left uninitialized
let _val: bool = MaybeUninit::uninit().assume_init(); //~ ERROR: does not permit being left uninitialized

// Some more types that should work just fine.
let _val: Option<&'static i32> = mem::zeroed();
let _val: Option<fn()> = mem::zeroed();
let _val: MaybeUninit<&'static i32> = mem::zeroed();
let _val: i32 = mem::zeroed();
let _val: bool = MaybeUninit::zeroed().assume_init();
}
}
69 changes: 62 additions & 7 deletions src/test/ui/lint/uninitialized-zeroed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,30 @@ LL | let _val: NonNull<i32> = mem::uninitialized();
|
= note: std::ptr::NonNull<i32> must be non-null

error: the type `*const dyn std::marker::Send` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:70:37
|
LL | let _val: *const dyn Send = mem::zeroed();
| ^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead
|
= note: The vtable of a wide raw pointer must be non-null

error: the type `*const dyn std::marker::Send` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:71:37
|
LL | let _val: *const dyn Send = mem::uninitialized();
| ^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead
|
= note: The vtable of a wide raw pointer must be non-null

error: the type `bool` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:72:26
--> $DIR/uninitialized-zeroed.rs:75:26
|
LL | let _val: bool = mem::uninitialized();
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -319,7 +341,7 @@ LL | let _val: bool = mem::uninitialized();
= note: Booleans must be `true` or `false`

error: the type `Wrap<char>` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:75:32
--> $DIR/uninitialized-zeroed.rs:78:32
|
LL | let _val: Wrap<char> = mem::uninitialized();
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -334,7 +356,7 @@ LL | struct Wrap<T> { wrapped: T }
| ^^^^^^^^^^

error: the type `NonBig` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:78:28
--> $DIR/uninitialized-zeroed.rs:81:28
|
LL | let _val: NonBig = mem::uninitialized();
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -345,7 +367,7 @@ LL | let _val: NonBig = mem::uninitialized();
= note: NonBig must be initialized inside its custom valid range

error: the type `&'static i32` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:81:34
--> $DIR/uninitialized-zeroed.rs:84:34
|
LL | let _val: &'static i32 = mem::transmute(0usize);
| ^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -356,7 +378,7 @@ LL | let _val: &'static i32 = mem::transmute(0usize);
= note: References must be non-null

error: the type `&'static [i32]` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:82:36
--> $DIR/uninitialized-zeroed.rs:85:36
|
LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -367,7 +389,7 @@ LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize));
= note: References must be non-null

error: the type `std::num::NonZeroU32` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:83:32
--> $DIR/uninitialized-zeroed.rs:86:32
|
LL | let _val: NonZeroU32 = mem::transmute(0);
| ^^^^^^^^^^^^^^^^^
Expand All @@ -377,5 +399,38 @@ LL | let _val: NonZeroU32 = mem::transmute(0);
|
= note: std::num::NonZeroU32 must be non-null

error: aborting due to 30 previous errors
error: the type `std::ptr::NonNull<i32>` does not permit zero-initialization
--> $DIR/uninitialized-zeroed.rs:89:34
|
LL | let _val: NonNull<i32> = MaybeUninit::zeroed().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead
|
= note: std::ptr::NonNull<i32> must be non-null

error: the type `std::ptr::NonNull<i32>` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:90:34
|
LL | let _val: NonNull<i32> = MaybeUninit::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead
|
= note: std::ptr::NonNull<i32> must be non-null

error: the type `bool` does not permit being left uninitialized
--> $DIR/uninitialized-zeroed.rs:91:26
|
LL | let _val: bool = MaybeUninit::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead
|
= note: Booleans must be `true` or `false`

error: aborting due to 35 previous errors

0 comments on commit 12ffe5f

Please sign in to comment.