Skip to content

Commit fed5644

Browse files
authored
[Sema] atomic_compare_exchange: check failure memory order (#74959)
For `__atomic_compare_exchange{,_n}/__c11_atomic_compare_exchange_{strong,weak}`, GCC checks both the success memory order and the failure memory order under the default -Winvalid-memory-model ("memory model" is confusing here and "memory order" is much more common in the atomic context). * The failure memory order, if a constant, must be one of relaxed/consume/acquire/seq_cst. Clang checks just the success memory order under the default -Watomic-memory-ordering. This patch checks the failure memory order.
1 parent 419c45a commit fed5644

File tree

4 files changed

+69
-22
lines changed

4 files changed

+69
-22
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8729,7 +8729,7 @@ def err_atomic_op_needs_atomic_int : Error<
87298729
"address argument to atomic operation must be a pointer to "
87308730
"%select{|atomic }0integer (%1 invalid)">;
87318731
def warn_atomic_op_has_invalid_memory_order : Warning<
8732-
"memory order argument to atomic operation is invalid">,
8732+
"%select{|success |failure }0memory order argument to atomic operation is invalid">,
87338733
InGroup<DiagGroup<"atomic-memory-ordering">>;
87348734
def err_atomic_op_has_invalid_synch_scope : Error<
87358735
"synchronization scope argument to atomic operation is invalid">;

clang/lib/Sema/SemaChecking.cpp

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5030,14 +5030,14 @@ bool Sema::CheckAMDGCNBuiltinFunctionCall(unsigned BuiltinID,
50305030
if (!llvm::isValidAtomicOrderingCABI(Ord))
50315031
return Diag(ArgExpr->getBeginLoc(),
50325032
diag::warn_atomic_op_has_invalid_memory_order)
5033-
<< ArgExpr->getSourceRange();
5033+
<< 0 << ArgExpr->getSourceRange();
50345034
switch (static_cast<llvm::AtomicOrderingCABI>(Ord)) {
50355035
case llvm::AtomicOrderingCABI::relaxed:
50365036
case llvm::AtomicOrderingCABI::consume:
50375037
if (BuiltinID == AMDGPU::BI__builtin_amdgcn_fence)
50385038
return Diag(ArgExpr->getBeginLoc(),
50395039
diag::warn_atomic_op_has_invalid_memory_order)
5040-
<< ArgExpr->getSourceRange();
5040+
<< 0 << ArgExpr->getSourceRange();
50415041
break;
50425042
case llvm::AtomicOrderingCABI::acquire:
50435043
case llvm::AtomicOrderingCABI::release:
@@ -8177,13 +8177,31 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
81778177
break;
81788178
}
81798179

8180+
// If the memory orders are constants, check they are valid.
81808181
if (SubExprs.size() >= 2 && Form != Init) {
8181-
if (std::optional<llvm::APSInt> Result =
8182-
SubExprs[1]->getIntegerConstantExpr(Context))
8183-
if (!isValidOrderingForOp(Result->getSExtValue(), Op))
8184-
Diag(SubExprs[1]->getBeginLoc(),
8185-
diag::warn_atomic_op_has_invalid_memory_order)
8186-
<< SubExprs[1]->getSourceRange();
8182+
std::optional<llvm::APSInt> Success =
8183+
SubExprs[1]->getIntegerConstantExpr(Context);
8184+
if (Success && !isValidOrderingForOp(Success->getSExtValue(), Op)) {
8185+
Diag(SubExprs[1]->getBeginLoc(),
8186+
diag::warn_atomic_op_has_invalid_memory_order)
8187+
<< /*success=*/(Form == C11CmpXchg || Form == GNUCmpXchg)
8188+
<< SubExprs[1]->getSourceRange();
8189+
}
8190+
if (SubExprs.size() >= 5) {
8191+
if (std::optional<llvm::APSInt> Failure =
8192+
SubExprs[3]->getIntegerConstantExpr(Context)) {
8193+
if (!llvm::is_contained(
8194+
{llvm::AtomicOrderingCABI::relaxed,
8195+
llvm::AtomicOrderingCABI::consume,
8196+
llvm::AtomicOrderingCABI::acquire,
8197+
llvm::AtomicOrderingCABI::seq_cst},
8198+
(llvm::AtomicOrderingCABI)Failure->getSExtValue())) {
8199+
Diag(SubExprs[3]->getBeginLoc(),
8200+
diag::warn_atomic_op_has_invalid_memory_order)
8201+
<< /*failure=*/2 << SubExprs[3]->getSourceRange();
8202+
}
8203+
}
8204+
}
81878205
}
81888206

81898207
if (auto ScopeModel = AtomicExpr::getScopeModel(Op)) {

clang/test/Sema/atomic-ops.c

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -339,18 +339,18 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
339339
(void)__c11_atomic_load(Ap, memory_order_relaxed);
340340
(void)__c11_atomic_load(Ap, memory_order_acquire);
341341
(void)__c11_atomic_load(Ap, memory_order_consume);
342-
(void)__c11_atomic_load(Ap, memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
343-
(void)__c11_atomic_load(Ap, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
342+
(void)__c11_atomic_load(Ap, memory_order_release); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
343+
(void)__c11_atomic_load(Ap, memory_order_acq_rel); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
344344
(void)__c11_atomic_load(Ap, memory_order_seq_cst);
345345
(void)__c11_atomic_load(Ap, val);
346-
(void)__c11_atomic_load(Ap, -1); // expected-warning {{memory order argument to atomic operation is invalid}}
347-
(void)__c11_atomic_load(Ap, 42); // expected-warning {{memory order argument to atomic operation is invalid}}
346+
(void)__c11_atomic_load(Ap, -1); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
347+
(void)__c11_atomic_load(Ap, 42); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
348348

349349
(void)__c11_atomic_store(Ap, val, memory_order_relaxed);
350-
(void)__c11_atomic_store(Ap, val, memory_order_acquire); // expected-warning {{memory order argument to atomic operation is invalid}}
351-
(void)__c11_atomic_store(Ap, val, memory_order_consume); // expected-warning {{memory order argument to atomic operation is invalid}}
350+
(void)__c11_atomic_store(Ap, val, memory_order_acquire); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
351+
(void)__c11_atomic_store(Ap, val, memory_order_consume); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
352352
(void)__c11_atomic_store(Ap, val, memory_order_release);
353-
(void)__c11_atomic_store(Ap, val, memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
353+
(void)__c11_atomic_store(Ap, val, memory_order_acq_rel); // expected-warning-re {{{{^}}memory order argument to atomic operation is invalid}}
354354
(void)__c11_atomic_store(Ap, val, memory_order_seq_cst);
355355

356356
(void)__c11_atomic_fetch_add(Ap, 1, memory_order_relaxed);
@@ -427,19 +427,35 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
427427
(void)__c11_atomic_exchange(Ap, val, memory_order_acq_rel);
428428
(void)__c11_atomic_exchange(Ap, val, memory_order_seq_cst);
429429

430+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, -1, memory_order_relaxed); // expected-warning {{success memory order argument to atomic operation is invalid}}
430431
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_relaxed, memory_order_relaxed);
431432
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_acquire, memory_order_relaxed);
432433
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_consume, memory_order_relaxed);
433434
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_release, memory_order_relaxed);
434435
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_acq_rel, memory_order_relaxed);
435-
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
436-
436+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_acquire);
437+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_consume);
438+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_release); // expected-warning {{failure memory order argument to atomic operation is invalid}}
439+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{failure memory order argument to atomic operation is invalid}}
440+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, memory_order_seq_cst);
441+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_seq_cst, -1); // expected-warning {{failure memory order argument to atomic operation is invalid}}
442+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_relaxed, memory_order_acquire);
443+
(void)__c11_atomic_compare_exchange_strong(Ap, p, val, memory_order_acquire, memory_order_seq_cst);
444+
445+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, -1, memory_order_relaxed); // expected-warning {{success memory order argument to atomic operation is invalid}}
437446
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_relaxed, memory_order_relaxed);
438447
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_acquire, memory_order_relaxed);
439448
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_consume, memory_order_relaxed);
440449
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_release, memory_order_relaxed);
441450
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_acq_rel, memory_order_relaxed);
442-
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_relaxed);
451+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_acquire);
452+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_consume);
453+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_release); // expected-warning {{failure memory order argument to atomic operation is invalid}}
454+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{failure memory order argument to atomic operation is invalid}}
455+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, memory_order_seq_cst);
456+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_seq_cst, -1); // expected-warning {{failure memory order argument to atomic operation is invalid}}
457+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_relaxed, memory_order_acquire);
458+
(void)__c11_atomic_compare_exchange_weak(Ap, p, val, memory_order_acquire, memory_order_seq_cst);
443459

444460
(void)__atomic_load_n(p, memory_order_relaxed);
445461
(void)__atomic_load_n(p, memory_order_acquire);
@@ -595,19 +611,32 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
595611
(void)__atomic_exchange(p, p, p, memory_order_acq_rel);
596612
(void)__atomic_exchange(p, p, p, memory_order_seq_cst);
597613

614+
(void)__atomic_compare_exchange(p, p, p, 0, -1, memory_order_relaxed); // expected-warning {{success memory order argument to atomic operation is invalid}}
598615
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_relaxed, memory_order_relaxed);
599616
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_acquire, memory_order_relaxed);
600617
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_consume, memory_order_relaxed);
601618
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_release, memory_order_relaxed);
602619
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_acq_rel, memory_order_relaxed);
603-
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_relaxed);
604-
620+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_acquire);
621+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_consume);
622+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_release); // expected-warning {{failure memory order argument to atomic operation is invalid}}
623+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{failure memory order argument to atomic operation is invalid}}
624+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, memory_order_seq_cst);
625+
(void)__atomic_compare_exchange(p, p, p, 0, memory_order_seq_cst, -1); // expected-warning {{memory order argument to atomic operation is invalid}}
626+
627+
(void)__atomic_compare_exchange_n(p, p, val, 0, -1, memory_order_relaxed); // expected-warning {{success memory order argument to atomic operation is invalid}}
605628
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_relaxed, memory_order_relaxed);
606629
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_acquire, memory_order_relaxed);
607630
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_consume, memory_order_relaxed);
608631
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_release, memory_order_relaxed);
609632
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_acq_rel, memory_order_relaxed);
610633
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_relaxed);
634+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_acquire);
635+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_consume);
636+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_release); // expected-warning {{failure memory order argument to atomic operation is invalid}}
637+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_acq_rel); // expected-warning {{failure memory order argument to atomic operation is invalid}}
638+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_seq_cst);
639+
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, -1); // expected-warning {{memory order argument to atomic operation is invalid}}
611640
}
612641

613642
void nullPointerWarning(void) {

clang/test/SemaCUDA/atomic-ops.cu

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ __device__ bool test_hip_atomic_cmpxchg_weak(int *ptr, int val, int desired) {
7676
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_SEQ_CST, __HIP_MEMORY_SCOPE_SINGLETHREAD);
7777
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_CONSUME, __HIP_MEMORY_SCOPE_SINGLETHREAD);
7878
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_ACQUIRE, __HIP_MEMORY_SCOPE_SINGLETHREAD);
79-
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_ACQ_REL, __HIP_MEMORY_SCOPE_SINGLETHREAD);
79+
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_ACQ_REL, __HIP_MEMORY_SCOPE_SINGLETHREAD); // expected-warning {{failure memory order argument to atomic operation is invalid}}
8080
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
8181
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
8282
flag = __hip_atomic_compare_exchange_weak(ptr, &val, desired, __ATOMIC_CONSUME, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);

0 commit comments

Comments
 (0)