@@ -8,7 +8,8 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
88
99declare_clippy_lint ! {
1010 /// **What it does:** Checks for usage of invalid atomic
11- /// ordering in atomic loads/stores and memory fences.
11+ /// ordering in atomic loads/stores/exchanges/updates and
12+ /// memory fences.
1213///
1314/// **Why is this bad?** Using an invalid atomic ordering
1415/// will cause a panic at run-time.
@@ -17,22 +18,35 @@ declare_clippy_lint! {
1718///
1819/// **Example:**
1920/// ```rust,no_run
20- /// # use std::sync::atomic::{self, AtomicBool , Ordering};
21+ /// # use std::sync::atomic::{self, AtomicU8 , Ordering};
2122///
22- /// let x = AtomicBool ::new(true );
23+ /// let x = AtomicU8 ::new(0 );
2324///
25+ /// // Bad: `Release` and `AcqRel` cannot be used for `load`.
2426/// let _ = x.load(Ordering::Release);
2527/// let _ = x.load(Ordering::AcqRel);
2628///
27- /// x.store(false, Ordering::Acquire);
28- /// x.store(false, Ordering::AcqRel);
29+ /// // Bad: `Acquire` and `AcqRel` cannot be used for `store`.
30+ /// x.store(1, Ordering::Acquire);
31+ /// x.store(2, Ordering::AcqRel);
2932///
33+ /// // Bad: `Relaxed` cannot be used as a fence's ordering.
3034/// atomic::fence(Ordering::Relaxed);
3135/// atomic::compiler_fence(Ordering::Relaxed);
36+ ///
37+ /// // Bad: `Release` and `AcqRel` are both always invalid
38+ /// // for the failure ordering (the last arg).
39+ /// let _ = x.compare_exchange(1, 2, Ordering::SeqCst, Ordering::Release);
40+ /// let _ = x.compare_exchange_weak(2, 3, Ordering::AcqRel, Ordering::AcqRel);
41+ ///
42+ /// // Bad: The failure ordering is not allowed to be
43+ /// // stronger than the success order, and `SeqCst` is
44+ /// // stronger than `Relaxed`.
45+ /// let _ = x.fetch_update(Ordering::Relaxed, Ordering::SeqCst, |val| Some(val + val));
3246/// ```
3347pub INVALID_ATOMIC_ORDERING ,
3448 correctness,
35- "usage of invalid atomic ordering in atomic loads/stores and memory fences"
49+ "usage of invalid atomic ordering in atomic operations and memory fences"
3650}
3751
3852declare_lint_pass ! ( AtomicOrdering => [ INVALID_ATOMIC_ORDERING ] ) ;
@@ -127,9 +141,89 @@ fn check_memory_fence(cx: &LateContext<'_>, expr: &Expr<'_>) {
127141 }
128142}
129143
144+ fn opt_ordering_defid ( cx : & LateContext < ' _ > , ord_arg : & Expr < ' _ > ) -> Option < DefId > {
145+ if let ExprKind :: Path ( ref ord_qpath) = ord_arg. kind {
146+ cx. qpath_res ( ord_qpath, ord_arg. hir_id ) . opt_def_id ( )
147+ } else {
148+ None
149+ }
150+ }
151+
152+ fn check_atomic_compare_exchange ( cx : & LateContext < ' _ > , expr : & Expr < ' _ > ) {
153+ if_chain ! {
154+ if let ExprKind :: MethodCall ( ref method_path, _, args, _) = & expr. kind;
155+ let method = method_path. ident. name. as_str( ) ;
156+ if type_is_atomic( cx, & args[ 0 ] ) ;
157+ if method == "compare_exchange" || method == "compare_exchange_weak" || method == "fetch_update" ;
158+ let ( success_order_arg, failure_order_arg) = if method == "fetch_update" {
159+ ( & args[ 1 ] , & args[ 2 ] )
160+ } else {
161+ ( & args[ 3 ] , & args[ 4 ] )
162+ } ;
163+ if let Some ( fail_ordering_def_id) = opt_ordering_defid( cx, failure_order_arg) ;
164+ then {
165+ // Helper type holding on to some checking and error reporting data. Has
166+ // - (success ordering name,
167+ // - list of failure orderings forbidden by the success order,
168+ // - suggestion message)
169+ type OrdLintInfo = ( & ' static str , & ' static [ & ' static str ] , & ' static str ) ;
170+ let relaxed: OrdLintInfo = ( "Relaxed" , & [ "SeqCst" , "Acquire" ] , "ordering mode `Relaxed`" ) ;
171+ let acquire: OrdLintInfo = ( "Acquire" , & [ "SeqCst" ] , "ordering modes `Acquire` or `Relaxed`" ) ;
172+ let seq_cst: OrdLintInfo = ( "SeqCst" , & [ ] , "ordering modes `Acquire`, `SeqCst` or `Relaxed`" ) ;
173+ let release = ( "Release" , relaxed. 1 , relaxed. 2 ) ;
174+ let acqrel = ( "AcqRel" , acquire. 1 , acquire. 2 ) ;
175+ let search = [ relaxed, acquire, seq_cst, release, acqrel] ;
176+
177+ let success_lint_info = opt_ordering_defid( cx, success_order_arg)
178+ . and_then( |success_ord_def_id| -> Option <OrdLintInfo > {
179+ search
180+ . iter( )
181+ . find( |( ordering, ..) | {
182+ match_def_path( cx, success_ord_def_id,
183+ & [ "core" , "sync" , "atomic" , "Ordering" , ordering] )
184+ } )
185+ . copied( )
186+ } ) ;
187+
188+ if match_ordering_def_path( cx, fail_ordering_def_id, & [ "Release" , "AcqRel" ] ) {
189+ // If we don't know the success order is, use what we'd suggest
190+ // if it were maximally permissive.
191+ let suggested = success_lint_info. unwrap_or( seq_cst) . 2 ;
192+ span_lint_and_help(
193+ cx,
194+ INVALID_ATOMIC_ORDERING ,
195+ failure_order_arg. span,
196+ & format!(
197+ "{}'s failure ordering may not be `Release` or `AcqRel`" ,
198+ method,
199+ ) ,
200+ None ,
201+ & format!( "consider using {} instead" , suggested) ,
202+ ) ;
203+ } else if let Some ( ( success_ord_name, bad_ords_given_success, suggested) ) = success_lint_info {
204+ if match_ordering_def_path( cx, fail_ordering_def_id, bad_ords_given_success) {
205+ span_lint_and_help(
206+ cx,
207+ INVALID_ATOMIC_ORDERING ,
208+ failure_order_arg. span,
209+ & format!(
210+ "{}'s failure ordering may not be stronger than the success ordering of `{}`" ,
211+ method,
212+ success_ord_name,
213+ ) ,
214+ None ,
215+ & format!( "consider using {} instead" , suggested) ,
216+ ) ;
217+ }
218+ }
219+ }
220+ }
221+ }
222+
130223impl < ' tcx > LateLintPass < ' tcx > for AtomicOrdering {
131224 fn check_expr ( & mut self , cx : & LateContext < ' tcx > , expr : & ' tcx Expr < ' _ > ) {
132225 check_atomic_load_store ( cx, expr) ;
133226 check_memory_fence ( cx, expr) ;
227+ check_atomic_compare_exchange ( cx, expr) ;
134228 }
135229}
0 commit comments