EricWF created this revision.
EricWF added reviewers: majnemer, jfb, rsmith.
EricWF added a subscriber: cfe-commits.
Clang currently issues diagnostics if the success memory order argument is
invalid, but it doesn't check the failure ordering argument for cmpxchg atomic
operations.
This patch adds diagnostics if the failure ordering is either
`memory_order_release` or `memory_order_acq_rel`.
https://reviews.llvm.org/D22711
Files:
lib/Sema/SemaChecking.cpp
test/Sema/atomic-ops.c
Index: test/Sema/atomic-ops.c
===================================================================
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -355,13 +355,27 @@
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_acq_rel, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_acquire);
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_consume);
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_seq_cst);
+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_relaxed, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_acquire, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_consume, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_release, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_acq_rel, memory_order_relaxed);
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_acquire);
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_consume);
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_seq_cst);
+
(void)__atomic_load_n(p, memory_order_relaxed);
(void)__atomic_load_n(p, memory_order_acquire);
(void)__atomic_load_n(p, memory_order_consume);
@@ -495,12 +509,26 @@
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_acq_rel, memory_order_relaxed);
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_relaxed);
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_relaxed);
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_acquire);
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_consume);
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_seq_cst);
+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_relaxed, memory_order_relaxed);
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_acquire, memory_order_relaxed);
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_consume, memory_order_relaxed);
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_release, memory_order_relaxed);
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_acq_rel, memory_order_relaxed);
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_relaxed);
+
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_relaxed);
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_acquire);
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_consume);
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+ (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_seq_cst);
}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2246,7 +2246,8 @@
return false;
}
-static bool isValidOrderingForOp(int64_t Ordering, AtomicExpr::AtomicOp Op) {
+static bool isValidOrderingForOp(int64_t Ordering, AtomicExpr::AtomicOp Op,
+ bool IsFailureOrdering = false) {
if (!llvm::isValidAtomicOrderingCABI(Ordering))
return false;
@@ -2268,6 +2269,14 @@
OrderingCABI != llvm::AtomicOrderingCABI::acquire &&
OrderingCABI != llvm::AtomicOrderingCABI::acq_rel;
+ case AtomicExpr::AO__c11_atomic_compare_exchange_strong:
+ case AtomicExpr::AO__c11_atomic_compare_exchange_weak:
+ case AtomicExpr::AO__atomic_compare_exchange:
+ case AtomicExpr::AO__atomic_compare_exchange_n:
+ return !IsFailureOrdering ||
+ (OrderingCABI != llvm::AtomicOrderingCABI::release &&
+ OrderingCABI != llvm::AtomicOrderingCABI::acq_rel);
+
default:
return true;
}
@@ -2599,7 +2608,15 @@
diag::warn_atomic_op_has_invalid_memory_order)
<< SubExprs[1]->getSourceRange();
}
-
+ if (Form == GNUCmpXchg || Form == C11CmpXchg) {
+ llvm::APSInt Result(32);
+ if (SubExprs[3]->isIntegerConstantExpr(Result, Context) &&
+ !isValidOrderingForOp(Result.getSExtValue(), Op,
+ /*IsFailureOrdering*/true))
+ Diag(SubExprs[3]->getLocStart(),
+ diag::warn_atomic_op_has_invalid_memory_order)
+ << SubExprs[3]->getSourceRange();
+ }
AtomicExpr *AE = new (Context) AtomicExpr(TheCall->getCallee()->getLocStart(),
SubExprs, ResultType, Op,
TheCall->getRParenLoc());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
