-
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.
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.- Loading branch information
1 parent
390eacf
commit c3ad468
Showing
7 changed files
with
700 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::hir::def_id::DefId; | ||
| use rustc::hir::*; | ||
| use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; | ||
| use rustc::ty; | ||
| 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,ignore | ||
| /// # 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, | ||
| "lint 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,196 @@ | ||
| #![warn(clippy::invalid_atomic_ordering)] | ||
|
|
||
| use std::sync::atomic::{ | ||
| AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicPtr, AtomicU16, AtomicU32, AtomicU64, | ||
| AtomicU8, AtomicUsize, Ordering, | ||
| }; | ||
|
|
||
| fn main() { | ||
| // `AtomicBool` test cases | ||
| 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); | ||
|
|
||
| // `AtomicI8` test cases | ||
| let x = AtomicI8::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); | ||
|
|
||
| // `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); | ||
|
|
||
| // `AtomicPtr` test cases | ||
| let ptr = &mut 5; | ||
| let other_ptr = &mut 10; | ||
| let x = AtomicPtr::new(ptr); | ||
|
|
||
| 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(other_ptr, Ordering::Release); | ||
| x.store(other_ptr, Ordering::SeqCst); | ||
| x.store(other_ptr, Ordering::Relaxed); | ||
| x.store(other_ptr, Ordering::Acquire); | ||
| x.store(other_ptr, Ordering::AcqRel); | ||
|
|
||
| // `AtomicU8` test cases | ||
| let x = AtomicU8::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); | ||
|
|
||
| // `AtomicU16` test cases | ||
| let x = AtomicU16::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); | ||
|
|
||
| // `AtomicU32` test cases | ||
| let x = AtomicU32::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); | ||
|
|
||
| // `AtomicU64` test cases | ||
| let x = AtomicU64::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); | ||
|
|
||
| // `AtomicUsize` test cases | ||
| let x = AtomicUsize::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.