-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #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 #4679
changelog: Add lint [`invalid_atomic_ordering`]- Loading branch information
Showing
13 changed files
with
737 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| use crate::utils::{match_def_path, span_help_and_lint}; | ||
| use if_chain::if_chain; | ||
| use rustc::declare_lint_pass; | ||
| use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; | ||
| use rustc::ty; | ||
| use rustc_hir::def_id::DefId; | ||
| use rustc_hir::*; | ||
| use rustc_session::declare_tool_lint; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks for usage of invalid atomic | ||
| /// ordering in Atomic*::{load, store} calls. | ||
| /// | ||
| /// **Why is this bad?** Using an invalid atomic ordering | ||
| /// will cause a panic at run-time. | ||
| /// | ||
| /// **Known problems:** None. | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust,no_run | ||
| /// # use std::sync::atomic::{AtomicBool, Ordering}; | ||
| /// | ||
| /// let x = AtomicBool::new(true); | ||
| /// | ||
| /// let _ = x.load(Ordering::Release); | ||
| /// let _ = x.load(Ordering::AcqRel); | ||
| /// | ||
| /// x.store(false, Ordering::Acquire); | ||
| /// x.store(false, Ordering::AcqRel); | ||
| /// ``` | ||
| pub INVALID_ATOMIC_ORDERING, | ||
| correctness, | ||
| "usage of invalid atomic ordering in atomic load/store calls" | ||
| } | ||
|
|
||
| declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]); | ||
|
|
||
| const ATOMIC_TYPES: [&str; 12] = [ | ||
| "AtomicBool", | ||
| "AtomicI8", | ||
| "AtomicI16", | ||
| "AtomicI32", | ||
| "AtomicI64", | ||
| "AtomicIsize", | ||
| "AtomicPtr", | ||
| "AtomicU8", | ||
| "AtomicU16", | ||
| "AtomicU32", | ||
| "AtomicU64", | ||
| "AtomicUsize", | ||
| ]; | ||
|
|
||
| fn type_is_atomic(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { | ||
| if let ty::Adt(&ty::AdtDef { did, .. }, _) = cx.tables.expr_ty(expr).kind { | ||
| ATOMIC_TYPES | ||
| .iter() | ||
| .any(|ty| match_def_path(cx, did, &["core", "sync", "atomic", ty])) | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| fn match_ordering_def_path(cx: &LateContext<'_, '_>, did: DefId, orderings: &[&str]) -> bool { | ||
| orderings | ||
| .iter() | ||
| .any(|ordering| match_def_path(cx, did, &["core", "sync", "atomic", "Ordering", ordering])) | ||
| } | ||
|
|
||
| impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AtomicOrdering { | ||
| fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { | ||
| if_chain! { | ||
| if let ExprKind::MethodCall(ref method_path, _, args) = &expr.kind; | ||
| let method = method_path.ident.name.as_str(); | ||
| if type_is_atomic(cx, &args[0]); | ||
| if method == "load" || method == "store"; | ||
| let ordering_arg = if method == "load" { &args[1] } else { &args[2] }; | ||
| if let ExprKind::Path(ref ordering_qpath) = ordering_arg.kind; | ||
| if let Some(ordering_def_id) = cx.tables.qpath_res(ordering_qpath, ordering_arg.hir_id).opt_def_id(); | ||
| then { | ||
| if method == "load" && | ||
| match_ordering_def_path(cx, ordering_def_id, &["Release", "AcqRel"]) { | ||
| span_help_and_lint( | ||
| cx, | ||
| INVALID_ATOMIC_ORDERING, | ||
| ordering_arg.span, | ||
| "atomic loads cannot have `Release` and `AcqRel` ordering", | ||
| "consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`" | ||
| ); | ||
| } else if method == "store" && | ||
| match_ordering_def_path(cx, ordering_def_id, &["Acquire", "AcqRel"]) { | ||
| span_help_and_lint( | ||
| cx, | ||
| INVALID_ATOMIC_ORDERING, | ||
| ordering_arg.span, | ||
| "atomic stores cannot have `Acquire` and `AcqRel` ordering", | ||
| "consider using ordering modes `Release`, `SeqCst` or `Relaxed`" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #![warn(clippy::invalid_atomic_ordering)] | ||
|
|
||
| use std::sync::atomic::{AtomicBool, Ordering}; | ||
|
|
||
| fn main() { | ||
| let x = AtomicBool::new(true); | ||
|
|
||
| // Allowed load ordering modes | ||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
|
|
||
| // Disallowed load ordering modes | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| // Allowed store ordering modes | ||
| x.store(false, Ordering::Release); | ||
| x.store(false, Ordering::SeqCst); | ||
| x.store(false, Ordering::Relaxed); | ||
|
|
||
| // Disallowed store ordering modes | ||
| x.store(false, Ordering::Acquire); | ||
| x.store(false, Ordering::AcqRel); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| error: atomic loads cannot have `Release` and `AcqRel` ordering | ||
| --> $DIR/atomic_ordering_bool.rs:14:20 | ||
| | | ||
| LL | let _ = x.load(Ordering::Release); | ||
| | ^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = note: `-D clippy::invalid-atomic-ordering` implied by `-D warnings` | ||
| = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` | ||
|
|
||
| error: atomic loads cannot have `Release` and `AcqRel` ordering | ||
| --> $DIR/atomic_ordering_bool.rs:15:20 | ||
| | | ||
| LL | let _ = x.load(Ordering::AcqRel); | ||
| | ^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` | ||
|
|
||
| error: atomic stores cannot have `Acquire` and `AcqRel` ordering | ||
| --> $DIR/atomic_ordering_bool.rs:23:20 | ||
| | | ||
| LL | x.store(false, Ordering::Acquire); | ||
| | ^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: consider using ordering modes `Release`, `SeqCst` or `Relaxed` | ||
|
|
||
| error: atomic stores cannot have `Acquire` and `AcqRel` ordering | ||
| --> $DIR/atomic_ordering_bool.rs:24:20 | ||
| | | ||
| LL | x.store(false, Ordering::AcqRel); | ||
| | ^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: consider using ordering modes `Release`, `SeqCst` or `Relaxed` | ||
|
|
||
| error: aborting due to 4 previous errors | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| #![warn(clippy::invalid_atomic_ordering)] | ||
|
|
||
| use std::sync::atomic::{AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, Ordering}; | ||
|
|
||
| fn main() { | ||
| // `AtomicI8` test cases | ||
| let x = AtomicI8::new(0); | ||
|
|
||
| // Allowed load ordering modes | ||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
|
|
||
| // Disallowed load ordering modes | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| // Allowed store ordering modes | ||
| x.store(1, Ordering::Release); | ||
| x.store(1, Ordering::SeqCst); | ||
| x.store(1, Ordering::Relaxed); | ||
|
|
||
| // Disallowed store ordering modes | ||
| x.store(1, Ordering::Acquire); | ||
| x.store(1, Ordering::AcqRel); | ||
|
|
||
| // `AtomicI16` test cases | ||
| let x = AtomicI16::new(0); | ||
|
|
||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| x.store(1, Ordering::Release); | ||
| x.store(1, Ordering::SeqCst); | ||
| x.store(1, Ordering::Relaxed); | ||
| x.store(1, Ordering::Acquire); | ||
| x.store(1, Ordering::AcqRel); | ||
|
|
||
| // `AtomicI32` test cases | ||
| let x = AtomicI32::new(0); | ||
|
|
||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| x.store(1, Ordering::Release); | ||
| x.store(1, Ordering::SeqCst); | ||
| x.store(1, Ordering::Relaxed); | ||
| x.store(1, Ordering::Acquire); | ||
| x.store(1, Ordering::AcqRel); | ||
|
|
||
| // `AtomicI64` test cases | ||
| let x = AtomicI64::new(0); | ||
|
|
||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| x.store(1, Ordering::Release); | ||
| x.store(1, Ordering::SeqCst); | ||
| x.store(1, Ordering::Relaxed); | ||
| x.store(1, Ordering::Acquire); | ||
| x.store(1, Ordering::AcqRel); | ||
|
|
||
| // `AtomicIsize` test cases | ||
| let x = AtomicIsize::new(0); | ||
|
|
||
| let _ = x.load(Ordering::Acquire); | ||
| let _ = x.load(Ordering::SeqCst); | ||
| let _ = x.load(Ordering::Relaxed); | ||
| let _ = x.load(Ordering::Release); | ||
| let _ = x.load(Ordering::AcqRel); | ||
|
|
||
| x.store(1, Ordering::Release); | ||
| x.store(1, Ordering::SeqCst); | ||
| x.store(1, Ordering::Relaxed); | ||
| x.store(1, Ordering::Acquire); | ||
| x.store(1, Ordering::AcqRel); | ||
| } |
Oops, something went wrong.