Skip to content

Commit a86301f

Browse files
borsbors
committed
Auto merge of rust-lang#4999 - krishna-veerareddy:issue-4679-atomic-ordering, r=phansch
Add lint to detect usage of invalid atomic ordering Detect usage of invalid atomic ordering modes such as `Ordering::{Release, AcqRel}` in atomic loads and `Ordering::{Acquire, AcqRel}` in atomic stores. Fixes rust-lang#4679 changelog: Add lint [`invalid_atomic_ordering`]
2 parents 6271f56 + fe21ef4 commit a86301f

13 files changed

+737
-2
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,7 @@ Released 2018-09-13
11341134
[`integer_division`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_division
11351135
[`into_iter_on_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_array
11361136
[`into_iter_on_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_ref
1137+
[`invalid_atomic_ordering`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_atomic_ordering
11371138
[`invalid_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_ref
11381139
[`invalid_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_regex
11391140
[`invalid_upcast_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_upcast_comparisons

‎README.md

+1-1
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 344 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 345 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/atomic_ordering.rs

+102
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use crate::utils::{match_def_path, span_help_and_lint};
2+
use if_chain::if_chain;
3+
use rustc::declare_lint_pass;
4+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
5+
use rustc::ty;
6+
use rustc_hir::def_id::DefId;
7+
use rustc_hir::*;
8+
use rustc_session::declare_tool_lint;
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for usage of invalid atomic
12+
/// ordering in Atomic*::{load, store} calls.
13+
///
14+
/// **Why is this bad?** Using an invalid atomic ordering
15+
/// will cause a panic at run-time.
16+
///
17+
/// **Known problems:** None.
18+
///
19+
/// **Example:**
20+
/// ```rust,no_run
21+
/// # use std::sync::atomic::{AtomicBool, Ordering};
22+
///
23+
/// let x = AtomicBool::new(true);
24+
///
25+
/// let _ = x.load(Ordering::Release);
26+
/// let _ = x.load(Ordering::AcqRel);
27+
///
28+
/// x.store(false, Ordering::Acquire);
29+
/// x.store(false, Ordering::AcqRel);
30+
/// ```
31+
pub INVALID_ATOMIC_ORDERING,
32+
correctness,
33+
"usage of invalid atomic ordering in atomic load/store calls"
34+
}
35+
36+
declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]);
37+
38+
const ATOMIC_TYPES: [&str; 12] = [
39+
"AtomicBool",
40+
"AtomicI8",
41+
"AtomicI16",
42+
"AtomicI32",
43+
"AtomicI64",
44+
"AtomicIsize",
45+
"AtomicPtr",
46+
"AtomicU8",
47+
"AtomicU16",
48+
"AtomicU32",
49+
"AtomicU64",
50+
"AtomicUsize",
51+
];
52+
53+
fn type_is_atomic(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
54+
if let ty::Adt(&ty::AdtDef { did, .. }, _) = cx.tables.expr_ty(expr).kind {
55+
ATOMIC_TYPES
56+
.iter()
57+
.any(|ty| match_def_path(cx, did, &["core", "sync", "atomic", ty]))
58+
} else {
59+
false
60+
}
61+
}
62+
63+
fn match_ordering_def_path(cx: &LateContext<'_, '_>, did: DefId, orderings: &[&str]) -> bool {
64+
orderings
65+
.iter()
66+
.any(|ordering| match_def_path(cx, did, &["core", "sync", "atomic", "Ordering", ordering]))
67+
}
68+
69+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AtomicOrdering {
70+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
71+
if_chain! {
72+
if let ExprKind::MethodCall(ref method_path, _, args) = &expr.kind;
73+
let method = method_path.ident.name.as_str();
74+
if type_is_atomic(cx, &args[0]);
75+
if method == "load" || method == "store";
76+
let ordering_arg = if method == "load" { &args[1] } else { &args[2] };
77+
if let ExprKind::Path(ref ordering_qpath) = ordering_arg.kind;
78+
if let Some(ordering_def_id) = cx.tables.qpath_res(ordering_qpath, ordering_arg.hir_id).opt_def_id();
79+
then {
80+
if method == "load" &&
81+
match_ordering_def_path(cx, ordering_def_id, &["Release", "AcqRel"]) {
82+
span_help_and_lint(
83+
cx,
84+
INVALID_ATOMIC_ORDERING,
85+
ordering_arg.span,
86+
"atomic loads cannot have `Release` and `AcqRel` ordering",
87+
"consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`"
88+
);
89+
} else if method == "store" &&
90+
match_ordering_def_path(cx, ordering_def_id, &["Acquire", "AcqRel"]) {
91+
span_help_and_lint(
92+
cx,
93+
INVALID_ATOMIC_ORDERING,
94+
ordering_arg.span,
95+
"atomic stores cannot have `Acquire` and `AcqRel` ordering",
96+
"consider using ordering modes `Release`, `SeqCst` or `Relaxed`"
97+
);
98+
}
99+
}
100+
}
101+
}
102+
}

‎clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ pub mod arithmetic;
165165
pub mod as_conversions;
166166
pub mod assertions_on_constants;
167167
pub mod assign_ops;
168+
pub mod atomic_ordering;
168169
pub mod attrs;
169170
pub mod bit_mask;
170171
pub mod blacklisted_name;
@@ -466,6 +467,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
466467
&assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
467468
&assign_ops::ASSIGN_OP_PATTERN,
468469
&assign_ops::MISREFACTORED_ASSIGN_OP,
470+
&atomic_ordering::INVALID_ATOMIC_ORDERING,
469471
&attrs::DEPRECATED_CFG_ATTR,
470472
&attrs::DEPRECATED_SEMVER,
471473
&attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
@@ -984,6 +986,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
984986
store.register_early_pass(|| box as_conversions::AsConversions);
985987
store.register_early_pass(|| box utils::internal_lints::ProduceIce);
986988
store.register_late_pass(|| box let_underscore::LetUnderscore);
989+
store.register_late_pass(|| box atomic_ordering::AtomicOrdering);
987990

988991
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
989992
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1089,6 +1092,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
10891092
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
10901093
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
10911094
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
1095+
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
10921096
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
10931097
LintId::of(&attrs::DEPRECATED_SEMVER),
10941098
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
@@ -1509,6 +1513,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
15091513

15101514
store.register_group(true, "clippy::correctness", Some("clippy_correctness"), vec![
15111515
LintId::of(&approx_const::APPROX_CONSTANT),
1516+
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
15121517
LintId::of(&attrs::DEPRECATED_SEMVER),
15131518
LintId::of(&attrs::USELESS_ATTRIBUTE),
15141519
LintId::of(&bit_mask::BAD_BIT_MASK),

‎src/lintlist/mod.rs

+8-1
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; 344] = [
9+
pub const ALL_LINTS: [Lint; 345] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -833,6 +833,13 @@ pub const ALL_LINTS: [Lint; 344] = [
833833
deprecation: None,
834834
module: "methods",
835835
},
836+
Lint {
837+
name: "invalid_atomic_ordering",
838+
group: "correctness",
839+
desc: "usage of invalid atomic ordering in atomic load/store calls",
840+
deprecation: None,
841+
module: "atomic_ordering",
842+
},
836843
Lint {
837844
name: "invalid_regex",
838845
group: "correctness",

‎tests/ui/atomic_ordering_bool.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![warn(clippy::invalid_atomic_ordering)]
2+
3+
use std::sync::atomic::{AtomicBool, Ordering};
4+
5+
fn main() {
6+
let x = AtomicBool::new(true);
7+
8+
// Allowed load ordering modes
9+
let _ = x.load(Ordering::Acquire);
10+
let _ = x.load(Ordering::SeqCst);
11+
let _ = x.load(Ordering::Relaxed);
12+
13+
// Disallowed load ordering modes
14+
let _ = x.load(Ordering::Release);
15+
let _ = x.load(Ordering::AcqRel);
16+
17+
// Allowed store ordering modes
18+
x.store(false, Ordering::Release);
19+
x.store(false, Ordering::SeqCst);
20+
x.store(false, Ordering::Relaxed);
21+
22+
// Disallowed store ordering modes
23+
x.store(false, Ordering::Acquire);
24+
x.store(false, Ordering::AcqRel);
25+
}

‎tests/ui/atomic_ordering_bool.stderr

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error: atomic loads cannot have `Release` and `AcqRel` ordering
2+
--> $DIR/atomic_ordering_bool.rs:14:20
3+
|
4+
LL | let _ = x.load(Ordering::Release);
5+
| ^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::invalid-atomic-ordering` implied by `-D warnings`
8+
= help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`
9+
10+
error: atomic loads cannot have `Release` and `AcqRel` ordering
11+
--> $DIR/atomic_ordering_bool.rs:15:20
12+
|
13+
LL | let _ = x.load(Ordering::AcqRel);
14+
| ^^^^^^^^^^^^^^^^
15+
|
16+
= help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`
17+
18+
error: atomic stores cannot have `Acquire` and `AcqRel` ordering
19+
--> $DIR/atomic_ordering_bool.rs:23:20
20+
|
21+
LL | x.store(false, Ordering::Acquire);
22+
| ^^^^^^^^^^^^^^^^^
23+
|
24+
= help: consider using ordering modes `Release`, `SeqCst` or `Relaxed`
25+
26+
error: atomic stores cannot have `Acquire` and `AcqRel` ordering
27+
--> $DIR/atomic_ordering_bool.rs:24:20
28+
|
29+
LL | x.store(false, Ordering::AcqRel);
30+
| ^^^^^^^^^^^^^^^^
31+
|
32+
= help: consider using ordering modes `Release`, `SeqCst` or `Relaxed`
33+
34+
error: aborting due to 4 previous errors
35+

‎tests/ui/atomic_ordering_int.rs

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
#![warn(clippy::invalid_atomic_ordering)]
2+
3+
use std::sync::atomic::{AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, Ordering};
4+
5+
fn main() {
6+
// `AtomicI8` test cases
7+
let x = AtomicI8::new(0);
8+
9+
// Allowed load ordering modes
10+
let _ = x.load(Ordering::Acquire);
11+
let _ = x.load(Ordering::SeqCst);
12+
let _ = x.load(Ordering::Relaxed);
13+
14+
// Disallowed load ordering modes
15+
let _ = x.load(Ordering::Release);
16+
let _ = x.load(Ordering::AcqRel);
17+
18+
// Allowed store ordering modes
19+
x.store(1, Ordering::Release);
20+
x.store(1, Ordering::SeqCst);
21+
x.store(1, Ordering::Relaxed);
22+
23+
// Disallowed store ordering modes
24+
x.store(1, Ordering::Acquire);
25+
x.store(1, Ordering::AcqRel);
26+
27+
// `AtomicI16` test cases
28+
let x = AtomicI16::new(0);
29+
30+
let _ = x.load(Ordering::Acquire);
31+
let _ = x.load(Ordering::SeqCst);
32+
let _ = x.load(Ordering::Relaxed);
33+
let _ = x.load(Ordering::Release);
34+
let _ = x.load(Ordering::AcqRel);
35+
36+
x.store(1, Ordering::Release);
37+
x.store(1, Ordering::SeqCst);
38+
x.store(1, Ordering::Relaxed);
39+
x.store(1, Ordering::Acquire);
40+
x.store(1, Ordering::AcqRel);
41+
42+
// `AtomicI32` test cases
43+
let x = AtomicI32::new(0);
44+
45+
let _ = x.load(Ordering::Acquire);
46+
let _ = x.load(Ordering::SeqCst);
47+
let _ = x.load(Ordering::Relaxed);
48+
let _ = x.load(Ordering::Release);
49+
let _ = x.load(Ordering::AcqRel);
50+
51+
x.store(1, Ordering::Release);
52+
x.store(1, Ordering::SeqCst);
53+
x.store(1, Ordering::Relaxed);
54+
x.store(1, Ordering::Acquire);
55+
x.store(1, Ordering::AcqRel);
56+
57+
// `AtomicI64` test cases
58+
let x = AtomicI64::new(0);
59+
60+
let _ = x.load(Ordering::Acquire);
61+
let _ = x.load(Ordering::SeqCst);
62+
let _ = x.load(Ordering::Relaxed);
63+
let _ = x.load(Ordering::Release);
64+
let _ = x.load(Ordering::AcqRel);
65+
66+
x.store(1, Ordering::Release);
67+
x.store(1, Ordering::SeqCst);
68+
x.store(1, Ordering::Relaxed);
69+
x.store(1, Ordering::Acquire);
70+
x.store(1, Ordering::AcqRel);
71+
72+
// `AtomicIsize` test cases
73+
let x = AtomicIsize::new(0);
74+
75+
let _ = x.load(Ordering::Acquire);
76+
let _ = x.load(Ordering::SeqCst);
77+
let _ = x.load(Ordering::Relaxed);
78+
let _ = x.load(Ordering::Release);
79+
let _ = x.load(Ordering::AcqRel);
80+
81+
x.store(1, Ordering::Release);
82+
x.store(1, Ordering::SeqCst);
83+
x.store(1, Ordering::Relaxed);
84+
x.store(1, Ordering::Acquire);
85+
x.store(1, Ordering::AcqRel);
86+
}

0 commit comments

Comments
 (0)