From b01f2d11263ae67ddea62dc0ee989d44db8873bb Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sat, 31 Aug 2019 20:25:28 +0200 Subject: [PATCH] lint against `MaybeUninit::uninit().assume_init()` --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 + clippy_lints/src/methods/mod.rs | 65 +++++++++++++++++++++++++++++++++ clippy_lints/src/utils/paths.rs | 2 + src/lintlist/mod.rs | 9 ++++- tests/ui/uninit.rs | 23 ++++++++++++ tests/ui/uninit.stderr | 16 ++++++++ 8 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 tests/ui/uninit.rs create mode 100644 tests/ui/uninit.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 46c2be3010d..5a550e8d84f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1176,6 +1176,7 @@ Released 2018-09-13 [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented +[`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp [`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints diff --git a/README.md b/README.md index e5da763607b..bd97910f597 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 311 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 312 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0ae93c6147a..4383b8b733a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -804,6 +804,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con methods::STRING_EXTEND_CHARS, methods::SUSPICIOUS_MAP, methods::TEMPORARY_CSTRING_AS_PTR, + methods::UNINIT_ASSUMED_INIT, methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FOLD, methods::USELESS_ASREF, @@ -1116,6 +1117,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con methods::CLONE_DOUBLE_REF, methods::INTO_ITER_ON_ARRAY, methods::TEMPORARY_CSTRING_AS_PTR, + methods::UNINIT_ASSUMED_INIT, minmax::MIN_MAX, misc::CMP_NAN, misc::FLOAT_CMP, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 110d161471f..ea208953a40 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -951,6 +951,38 @@ "suspicious usage of map" } +declare_clippy_lint! { + /// **What it does:** Checks for `MaybeUninit::uninit().assume_init()`. + /// + /// **Why is this bad?** For most types, this is undefined behavior. + /// + /// **Known problems:** For now, we accept empty tuples and tuples / arrays + /// of `MaybeUninit`. There may be other types that allow uninitialized + /// data, but those are not yet rigorously defined. + /// + /// **Example:** + /// + /// ```rust + /// // Beware the UB + /// use std::mem::MaybeUninit; + /// + /// let _: usize = unsafe { MaybeUninit::uninit().assume_init() }; + /// ``` + /// + /// Note that the following is OK: + /// + /// ```rust + /// use std::mem::MaybeUninit; + /// + /// let _: [MaybeUninit; 5] = unsafe { + /// MaybeUninit::uninit().assume_init() + /// }; + /// ``` + pub UNINIT_ASSUMED_INIT, + correctness, + "`MaybeUninit::uninit().assume_init()`" +} + declare_lint_pass!(Methods => [ OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, @@ -991,6 +1023,7 @@ INTO_ITER_ON_ARRAY, INTO_ITER_ON_REF, SUSPICIOUS_MAP, + UNINIT_ASSUMED_INIT, ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { @@ -1038,6 +1071,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) { ["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0]), ["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]), ["count", "map"] => lint_suspicious_map(cx, expr), + ["assume_init"] => lint_maybe_uninit(cx, &arg_lists[0][0], expr), _ => {}, } @@ -2662,6 +2696,37 @@ fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: Ty<'_ } } +/// lint for `MaybeUninit::uninit().assume_init()` (we already have the latter) +fn lint_maybe_uninit(cx: &LateContext<'_, '_>, expr: &hir::Expr, outer: &hir::Expr) { + if_chain! { + if let hir::ExprKind::Call(ref callee, ref args) = expr.node; + if args.is_empty(); + if let hir::ExprKind::Path(ref path) = callee.node; + if match_qpath(path, &paths::MEM_MAYBEUNINIT_UNINIT); + if !is_maybe_uninit_ty_valid(cx, cx.tables.expr_ty_adjusted(outer)); + then { + span_lint( + cx, + UNINIT_ASSUMED_INIT, + outer.span, + "this call for this type may be undefined behavior" + ); + } + } +} + +fn is_maybe_uninit_ty_valid(cx: &LateContext<'_, '_>, ty: Ty<'_>) -> bool { + match ty.sty { + ty::Array(ref component, _) => is_maybe_uninit_ty_valid(cx, component), + ty::Tuple(ref types) => types.types().all(|ty| is_maybe_uninit_ty_valid(cx, ty)), + ty::Adt(ref adt, _) => { + // needs to be a MaybeUninit + match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT) + }, + _ => false, + } +} + fn lint_suspicious_map(cx: &LateContext<'_, '_>, expr: &hir::Expr) { span_help_and_lint( cx, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 55e1387fe99..9b88a0d3089 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -49,6 +49,8 @@ pub const LINT_PASS: [&str; 3] = ["rustc", "lint", "LintPass"]; pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"]; pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"]; +pub const MEM_MAYBEUNINIT: [&str; 4] = ["core", "mem", "maybe_uninit", "MaybeUninit"]; +pub const MEM_MAYBEUNINIT_UNINIT: [&str; 5] = ["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"]; pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"]; pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b7a6e486f0b..8d236323825 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 311] = [ +pub const ALL_LINTS: [Lint; 312] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1890,6 +1890,13 @@ deprecation: None, module: "panic_unimplemented", }, + Lint { + name: "uninit_assumed_init", + group: "correctness", + desc: "`MaybeUninit::uninit().assume_init()`", + deprecation: None, + module: "methods", + }, Lint { name: "unit_arg", group: "complexity", diff --git a/tests/ui/uninit.rs b/tests/ui/uninit.rs new file mode 100644 index 00000000000..a4424c490e7 --- /dev/null +++ b/tests/ui/uninit.rs @@ -0,0 +1,23 @@ +#![feature(stmt_expr_attributes)] + +use std::mem::MaybeUninit; + +#[allow(clippy::let_unit_value)] +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 + let _: () = unsafe { MaybeUninit::uninit().assume_init() }; + + // This is OK, because `MaybeUninit` allows uninitialized data. + let _: MaybeUninit = unsafe { MaybeUninit::uninit().assume_init() }; + + // This is OK, because all constitutent types are uninit-compatible. + let _: (MaybeUninit, MaybeUninit) = unsafe { MaybeUninit::uninit().assume_init() }; + + // This is OK, because all constitutent types are uninit-compatible. + let _: (MaybeUninit, [MaybeUninit; 2]) = unsafe { MaybeUninit::uninit().assume_init() }; +} diff --git a/tests/ui/uninit.stderr b/tests/ui/uninit.stderr new file mode 100644 index 00000000000..f4c45354aef --- /dev/null +++ b/tests/ui/uninit.stderr @@ -0,0 +1,16 @@ +error: this call for this type may be undefined behavior + --> $DIR/uninit.rs:7: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 + | +LL | let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +