OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

new lint: mem-replace-with-uninit

This commit is contained in:
Andre Bogus 2019-09-13 18:39:14 +02:00
parent cdaa93d695
commit 8d884c8a1a
8 changed files with 171 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1050,6 +1050,7 @@ Released 2018-09-13
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
[`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
[`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
[`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op

2
README.md
View File

@ -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 314 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 315 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:

2
clippy_lints/src/lib.rs
View File

@ -783,6 +783,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
matches::SINGLE_MATCH,
mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
mem_replace::MEM_REPLACE_WITH_UNINIT,
methods::CHARS_LAST_CMP,
methods::CHARS_NEXT_CMP,
methods::CLONE_DOUBLE_REF,
@ -1117,6 +1118,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
loops::REVERSE_RANGE_LOOP,
loops::WHILE_IMMUTABLE_CONDITION,
mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
mem_replace::MEM_REPLACE_WITH_UNINIT,
methods::CLONE_DOUBLE_REF,
methods::INTO_ITER_ON_ARRAY,
methods::TEMPORARY_CSTRING_AS_PTR,

126
clippy_lints/src/mem_replace.rs
View File

@ -1,4 +1,6 @@
use crate::utils::{match_def_path, match_qpath, paths, snippet_with_applicability, span_lint_and_sugg};
use crate::utils::{
match_def_path, match_qpath, paths, snippet_with_applicability, span_help_and_lint, span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc::hir::{Expr, ExprKind, MutMutable, QPath};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@ -32,7 +34,40 @@ declare_clippy_lint! {
"replacing an `Option` with `None` instead of `take()`"
}
declare_lint_pass!(MemReplace => [MEM_REPLACE_OPTION_WITH_NONE]);
declare_clippy_lint! {
/// **What it does:** Checks for `mem::replace(&mut _, mem::uninitialized())`
/// and `mem::replace(&mut _, mem::zeroed())`.
///
/// **Why is this bad?** This will lead to undefined behavior even if the
/// value is overwritten later, because the uninitialized value may be
/// observed in the case of a panic.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```
/// use std::mem;
///# fn may_panic(v: Vec<i32>) -> Vec<i32> { v }
///
/// #[allow(deprecated, invalid_value)]
/// fn myfunc (v: &mut Vec<i32>) {
/// let taken_v = unsafe { mem::replace(v, mem::uninitialized()) };
/// let new_v = may_panic(taken_v); // undefined behavior on panic
/// mem::forget(mem::replace(v, new_v));
/// }
/// ```
///
/// The [take_mut](https://docs.rs/take_mut) crate offers a sound solution,
/// at the cost of either lazily creating a replacement value or aborting
/// on panic, to ensure that the uninitialized value cannot be observed.
pub MEM_REPLACE_WITH_UNINIT,
correctness,
"`mem::replace(&mut _, mem::uninitialized())` or `mem::replace(&mut _, mem::zeroed())`"
}
declare_lint_pass!(MemReplace =>
[MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@ -45,37 +80,66 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
if match_def_path(cx, def_id, &paths::MEM_REPLACE);
// Check that second argument is `Option::None`
if let ExprKind::Path(ref replacement_qpath) = func_args[1].node;
if match_qpath(replacement_qpath, &paths::OPTION_NONE);
then {
// Since this is a late pass (already type-checked),
// and we already know that the second argument is an
// `Option`, we do not need to check the first
// argument's type. All that's left is to get
// replacee's path.
let replaced_path = match func_args[0].node {
ExprKind::AddrOf(MutMutable, ref replaced) => {
if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node {
replaced_path
} else {
return
}
},
ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path,
_ => return,
};
if let ExprKind::Path(ref replacement_qpath) = func_args[1].node {
if match_qpath(replacement_qpath, &paths::OPTION_NONE) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
MEM_REPLACE_OPTION_WITH_NONE,
expr.span,
"replacing an `Option` with `None`",
"consider `Option::take()` instead",
format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)),
applicability,
);
// Since this is a late pass (already type-checked),
// and we already know that the second argument is an
// `Option`, we do not need to check the first
// argument's type. All that's left is to get
// replacee's path.
let replaced_path = match func_args[0].node {
ExprKind::AddrOf(MutMutable, ref replaced) => {
if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node {
replaced_path
} else {
return
}
},
ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path,
_ => return,
};
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
MEM_REPLACE_OPTION_WITH_NONE,
expr.span,
"replacing an `Option` with `None`",
"consider `Option::take()` instead",
format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)),
applicability,
);
}
}
if let ExprKind::Call(ref repl_func, ref repl_args) = func_args[1].node {
if_chain! {
if repl_args.is_empty();
if let ExprKind::Path(ref repl_func_qpath) = repl_func.node;
if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
then {
if match_def_path(cx, repl_def_id, &paths::MEM_UNINITIALIZED) {
span_help_and_lint(
cx,
MEM_REPLACE_WITH_UNINIT,
expr.span,
"replacing with `mem::uninitialized()`",
"consider using the `take_mut` crate instead",
);
} else if match_def_path(cx, repl_def_id, &paths::MEM_ZEROED) &&
!cx.tables.expr_ty(&func_args[1]).is_primitive() {
span_help_and_lint(
cx,
MEM_REPLACE_WITH_UNINIT,
expr.span,
"replacing with `mem::zeroed()`",
"consider using a default value or the `take_mut` crate instead",
);
}
}
}
}
}
}
}

2
clippy_lints/src/utils/paths.rs
View File

@ -52,6 +52,8 @@ 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 MEM_UNINITIALIZED: [&str; 3] = ["core", "mem", "uninitialized"];
pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"];
pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"];
pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
pub const OPS_MODULE: [&str; 2] = ["core", "ops"];

9
src/lintlist/mod.rs
View File

@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 314] = [
pub const ALL_LINTS: [Lint; 315] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
@ -1043,6 +1043,13 @@ pub const ALL_LINTS: [Lint; 314] = [
deprecation: None,
module: "mem_replace",
},
Lint {
name: "mem_replace_with_uninit",
group: "correctness",
desc: "`mem::replace(&mut _, mem::uninitialized())` or `mem::replace(&mut _, mem::zeroed())`",
deprecation: None,
module: "mem_replace",
},
Lint {
name: "min_max",
group: "correctness",

35
tests/ui/repl_uninit.rs Normal file
View File

@ -0,0 +1,35 @@
#![allow(deprecated, invalid_value)]
#![warn(clippy::all)]
use std::mem;
fn might_panic<X>(x: X) -> X {
// in practice this would be a possibly-panicky operation
x
}
fn main() {
let mut v = vec![0i32; 4];
// the following is UB if `might_panic` panics
unsafe {
let taken_v = mem::replace(&mut v, mem::uninitialized());
let new_v = might_panic(taken_v);
std::mem::forget(mem::replace(&mut v, new_v));
}
unsafe {
let taken_v = mem::replace(&mut v, mem::zeroed());
let new_v = might_panic(taken_v);
std::mem::forget(mem::replace(&mut v, new_v));
}
// this is silly but OK, because usize is a primitive type
let mut u: usize = 42;
let uref = &mut u;
let taken_u = unsafe { mem::replace(uref, mem::zeroed()) };
*uref = taken_u + 1;
// this is still not OK, because uninit
let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) };
*uref = taken_u + 1;
}

27
tests/ui/repl_uninit.stderr Normal file
View File

@ -0,0 +1,27 @@
error: replacing with `mem::uninitialized()`
--> $DIR/repl_uninit.rs:15:23
|
LL | let taken_v = mem::replace(&mut v, mem::uninitialized());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::mem-replace-with-uninit` implied by `-D warnings`
= help: consider using the `take_mut` crate instead
error: replacing with `mem::zeroed()`
--> $DIR/repl_uninit.rs:21:23
|
LL | let taken_v = mem::replace(&mut v, mem::zeroed());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using a default value or the `take_mut` crate instead
error: replacing with `mem::uninitialized()`
--> $DIR/repl_uninit.rs:33:28
|
LL | let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using the `take_mut` crate instead
error: aborting due to 3 previous errors