[clang][sema] make sure arguments of __atomic_exchange complete type

[knightXun]

make sure the args of __atomic_exchange is complete type, make it consistent with GCC issue: #74464

[llvmbot]

@llvm/pr-subscribers-clang

Author: flyingcat (knightXun)

Changes

make sure the args of __atomic_exchange is complete type, make it consistent with GCC issue: #74464

---

Full diff: https://github.com/llvm/llvm-project/pull/75135.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaChecking.cpp (+12)
• (modified) clang/test/Sema/atomic-ops.c (+3-1)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index cdb6e9584e955..a88d3a6928edd 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -7885,6 +7885,18 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange, if ((IsOpenCL || IsHIP || IsScoped) && Op != AtomicExpr::AO__opencl_atomic_init) ++AdjustedNumArgs; + + // Check if the arguments are CompleteType + if (Op == AtomicExpr::AO__atomic_exchange) { + for (auto Arg : Args) { + auto ValType = Arg->getType(); + if (ValType->isPointerType() && !ValType->isNullPtrType() && + RequireCompleteType(Arg->getBeginLoc(), ValType->getPointeeType(), + diag::err_incomplete_type)) { + return ExprError(); + } + } + } // Check we have the right number of arguments. if (Args.size() < AdjustedNumArgs) { Diag(CallRange.getEnd(), diag::err_typecheck_call_too_few_args) diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c index 4fa1223b3038f..6f1d3c4b86ef9 100644 --- a/clang/test/Sema/atomic-ops.c +++ b/clang/test/Sema/atomic-ops.c @@ -19,6 +19,7 @@ // Basic parsing/Sema tests for __c11_atomic_* +#include "clang-c/Index.h" #include <stdatomic.h> struct S { char c[3]; }; @@ -130,7 +131,7 @@ _Static_assert(__atomic_always_lock_free(8, &i64), ""); void f(_Atomic(int) *i, const _Atomic(int) *ci, _Atomic(int*) *p, _Atomic(float) *f, _Atomic(double) *d, _Atomic(long double) *ld, - int *I, const int *CI, + int *I, const int *CI,char c[], int **P, float *F, double *D, struct S *s1, struct S *s2) { __c11_atomic_init(I, 5); // expected-error {{pointer to _Atomic}} __c11_atomic_init(ci, 5); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}} @@ -192,6 +193,7 @@ void f(_Atomic(int) *i, const _Atomic(int) *ci, (int)__atomic_exchange(s1, s2, s2, memory_order_seq_cst); // expected-error {{operand of type 'void'}} __atomic_exchange(I, I, I, memory_order_seq_cst); __atomic_exchange(CI, I, I, memory_order_seq_cst); // expected-error {{address argument to atomic operation must be a pointer to non-const type ('const int *' invalid)}} + __atomic_exchange(&c, I, I, memory_order_seq_cst); // expected-error {{incomplete type 'char[]' where a complete type is required}} __atomic_exchange(I, I, CI, memory_order_seq_cst); // expected-warning {{passing 'const int *' to parameter of type 'int *' discards qualifiers}} __c11_atomic_fetch_add(i, 1, memory_order_seq_cst); 

[knightXun]

cc @shafik @cor3ntin

[jyknight]

Why only __atomic_exchange? Presumably we need to be doing the same with every other of the atomic builtins as well (unless they already have this check and we only missed it on exchange?)

[knightXun]

Why only __atomic_exchange? Presumably we need to be doing the same with every other of the atomic builtins as well (unless they already have this check and we only missed it on exchange?)

I agree! I will do this check on every other of the atomic builtins.

[shafik]

We should add a release note for this change.

[erichkeane]

When doing so, please see if you can 'extract' this to a function, something like;

RequireAtomicArgs(Args) kind of thing.