Skip to content

Commit 1e6adf7

Browse files
llogiqllogiq
committed
New lint: mem_replace_with_uninit
1 parent f30bf69 commit 1e6adf7

File tree

9 files changed

+125
-36
lines changed

9 files changed

+125
-36
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,7 @@ Released 2018-09-13
10501050
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
10511051
[`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
10521052
[`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
1053+
[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
10531054
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
10541055
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
10551056
[`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are 313 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 314 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1212

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
781781
matches::SINGLE_MATCH,
782782
mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
783783
mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
784+
mem_replace::MEM_REPLACE_WITH_UNINIT,
784785
methods::CHARS_LAST_CMP,
785786
methods::CHARS_NEXT_CMP,
786787
methods::CLONE_DOUBLE_REF,
@@ -1116,6 +1117,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
11161117
loops::REVERSE_RANGE_LOOP,
11171118
loops::WHILE_IMMUTABLE_CONDITION,
11181119
mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
1120+
mem_replace::MEM_REPLACE_WITH_UNINIT,
11191121
methods::CLONE_DOUBLE_REF,
11201122
methods::INTO_ITER_ON_ARRAY,
11211123
methods::TEMPORARY_CSTRING_AS_PTR,

‎clippy_lints/src/mem_replace.rs

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
use crate::utils::{match_def_path, match_qpath, paths, snippet_with_applicability, span_lint_and_sugg};
1+
use crate::utils::{match_def_path, match_qpath, paths, snippet_with_applicability,
2+
span_help_and_lint, span_lint_and_sugg};
23
use if_chain::if_chain;
34
use rustc::hir::{Expr, ExprKind, MutMutable, QPath};
45
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -32,7 +33,40 @@ declare_clippy_lint! {
3233
"replacing an `Option` with `None` instead of `take()`"
3334
}
3435

35-
declare_lint_pass!(MemReplace => [MEM_REPLACE_OPTION_WITH_NONE]);
36+
declare_clippy_lint! {
37+
/// **What it does:** Checks for `mem::replace(&mut _, mem::uninitialized())`
38+
/// and `mem::replace(&mut _, mem::zeroed())`.
39+
///
40+
/// **Why is this bad?** This will lead to undefined behavior even if the
41+
/// value is overwritten later, because the uninitialized value may be
42+
/// observed in the case of a panic.
43+
///
44+
/// **Known problems:** None.
45+
///
46+
/// **Example:**
47+
///
48+
/// ```
49+
/// use std::mem;
50+
///# fn may_panic(v: Vec<i32>) -> Vec<i32> { v }
51+
///
52+
/// #[allow(deprecated, invalid_value)]
53+
/// fn myfunc (v: &mut Vec<i32>) {
54+
/// let taken_v = unsafe { mem::replace(v, mem::uninitialized()) };
55+
/// let new_v = may_panic(taken_v); // undefined behavior on panic
56+
/// mem::forget(mem::replace(&mut v, new_v));
57+
/// }
58+
/// ```
59+
///
60+
/// The [take_mut](https://docs.rs/take_mut) crate offers a sound solution,
61+
/// at the cost of aborting on panic, to ensure that the uninitialized
62+
/// value cannot be observed.
63+
pub MEM_REPLACE_WITH_UNINIT,
64+
correctness,
65+
"`mem::replace(&mut _, mem::uninitialized())` or `mem::zeroed()`"
66+
}
67+
68+
declare_lint_pass!(MemReplace =>
69+
[MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT]);
3670

3771
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
3872
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@@ -46,36 +80,47 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
4680

4781
// Check that second argument is `Option::None`
4882
if let ExprKind::Path(ref replacement_qpath) = func_args[1].node;
49-
if match_qpath(replacement_qpath, &paths::OPTION_NONE);
50-
5183
then {
52-
// Since this is a late pass (already type-checked),
53-
// and we already know that the second argument is an
54-
// `Option`, we do not need to check the first
55-
// argument's type. All that's left is to get
56-
// replacee's path.
57-
let replaced_path = match func_args[0].node {
58-
ExprKind::AddrOf(MutMutable, ref replaced) => {
59-
if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node {
60-
replaced_path
61-
} else {
62-
return
63-
}
64-
},
65-
ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path,
66-
_ => return,
67-
};
84+
if match_qpath(replacement_qpath, &paths::OPTION_NONE) {
85+
86+
// Since this is a late pass (already type-checked),
87+
// and we already know that the second argument is an
88+
// `Option`, we do not need to check the first
89+
// argument's type. All that's left is to get
90+
// replacee's path.
91+
let replaced_path = match func_args[0].node {
92+
ExprKind::AddrOf(MutMutable, ref replaced) => {
93+
if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node {
94+
replaced_path
95+
} else {
96+
return
97+
}
98+
},
99+
ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path,
100+
_ => return,
101+
};
68102

69-
let mut applicability = Applicability::MachineApplicable;
70-
span_lint_and_sugg(
71-
cx,
72-
MEM_REPLACE_OPTION_WITH_NONE,
73-
expr.span,
74-
"replacing an `Option` with `None`",
75-
"consider `Option::take()` instead",
76-
format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)),
77-
applicability,
78-
);
103+
let mut applicability = Applicability::MachineApplicable;
104+
span_lint_and_sugg(
105+
cx,
106+
MEM_REPLACE_OPTION_WITH_NONE,
107+
expr.span,
108+
"replacing an `Option` with `None`",
109+
"consider `Option::take()` instead",
110+
format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)),
111+
applicability,
112+
);
113+
}
114+
if match_qpath(replacement_qpath, &paths::MEM_UNINITIALIZED) ||
115+
match_qpath(replacement_qpath, &paths::MEM_ZEROED) {
116+
span_help_and_lint(
117+
cx,
118+
MEM_REPLACE_WITH_UNINIT,
119+
expr.span,
120+
"replacing with `mem::uninitialized()`",
121+
"consider using the `take_mut` crate instead",
122+
);
123+
}
79124
}
80125
}
81126
}

‎clippy_lints/src/utils/paths.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
5252
pub const MEM_MAYBEUNINIT: [&str; 4] = ["core", "mem", "maybe_uninit", "MaybeUninit"];
5353
pub const MEM_MAYBEUNINIT_UNINIT: [&str; 5] = ["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"];
5454
pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"];
55+
pub const MEM_UNINITIALIZED: [&str; 3] = ["core", "mem", "uninitialized"];
56+
pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"];
5557
pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"];
5658
pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
5759
pub const OPS_MODULE: [&str; 2] = ["core", "ops"];

‎src/lintlist/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 313] = [
9+
pub const ALL_LINTS: [Lint; 314] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1043,6 +1043,13 @@ pub const ALL_LINTS: [Lint; 313] = [
10431043
deprecation: None,
10441044
module: "mem_replace",
10451045
},
1046+
Lint {
1047+
name: "mem_replace_with_uninit",
1048+
group: "correctness",
1049+
desc: "`mem::replace(&mut _, mem::uninitialized())` or `mem::zeroed()`",
1050+
deprecation: None,
1051+
module: "mem_replace",
1052+
},
10461053
Lint {
10471054
name: "min_max",
10481055
group: "correctness",

‎tests/ui/mem_replace.fixed

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,30 @@
88
// except according to those terms.
99

1010
// run-rustfix
11-
#![allow(unused_imports)]
11+
#![allow(unused_imports, deprecated, invalid_value)]
1212
#![warn(clippy::all, clippy::style, clippy::mem_replace_option_with_none)]
1313

1414
use std::mem;
1515

16+
fn might_panic(v: Vec<i32>) -> Vec<i32> { v }
17+
1618
fn main() {
1719
let mut an_option = Some(1);
1820
let _ = an_option.take();
1921
let an_option = &mut Some(1);
2022
let _ = an_option.take();
23+
24+
let mut v = vec![0i32; 4];
25+
// the following is UB if `might_panic` panics
26+
unsafe {
27+
let taken_v = mem::replace(&mut v, mem::uninitialized());
28+
let new_v = might_panic(taken_v);
29+
std::mem::forget(mem::replace(&mut v, new_v));
30+
}
31+
32+
unsafe {
33+
let taken_v = mem::replace(&mut v, mem::zeroed());
34+
let new_v = might_panic(taken_v);
35+
std::mem::forget(mem::replace(&mut v, new_v));
36+
}
2137
}

‎tests/ui/mem_replace.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,30 @@
88
// except according to those terms.
99

1010
// run-rustfix
11-
#![allow(unused_imports)]
11+
#![allow(unused_imports, deprecated, invalid_value)]
1212
#![warn(clippy::all, clippy::style, clippy::mem_replace_option_with_none)]
1313

1414
use std::mem;
1515

16+
fn might_panic(v: Vec<i32>) -> Vec<i32> { v }
17+
1618
fn main() {
1719
let mut an_option = Some(1);
1820
let _ = mem::replace(&mut an_option, None);
1921
let an_option = &mut Some(1);
2022
let _ = mem::replace(an_option, None);
23+
24+
let mut v = vec![0i32; 4];
25+
// the following is UB if `might_panic` panics
26+
unsafe {
27+
let taken_v = mem::replace(&mut v, mem::uninitialized());
28+
let new_v = might_panic(taken_v);
29+
std::mem::forget(mem::replace(&mut v, new_v));
30+
}
31+
32+
unsafe {
33+
let taken_v = mem::replace(&mut v, mem::zeroed());
34+
let new_v = might_panic(taken_v);
35+
std::mem::forget(mem::replace(&mut v, new_v));
36+
}
2137
}

‎tests/ui/mem_replace.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: replacing an `Option` with `None`
2-
--> $DIR/mem_replace.rs:18:13
2+
--> $DIR/mem_replace.rs:20:13
33
|
44
LL | let _ = mem::replace(&mut an_option, None);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
66
|
77
= note: `-D clippy::mem-replace-option-with-none` implied by `-D warnings`
88

99
error: replacing an `Option` with `None`
10-
--> $DIR/mem_replace.rs:20:13
10+
--> $DIR/mem_replace.rs:22:13
1111
|
1212
LL | let _ = mem::replace(an_option, None);
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`

0 commit comments

Comments
 (0)