[C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

[aaron.ballman]

Summary

The relevant language rule from C11 is 6.5.16.1p1: "the left operand is an atomic, qualified, or unqualified pointer, and the right is a null pointer constant; or". We correctly handled qualified or unqualified pointer types, but failed to handle atomic-qualified pointer types. Now we look through the atomic qualification before testing the constraint requirements.

Fixes https://github.com/llvm/llvm-project/issues/49563

[aaron.ballman]

Added an additional test case (assignment of a null pointer constant to an atomic function pointer) that was mentioned in a comment on the original bug report).

[aaron.ballman]

Added a -pedantic run line and used (void *)0 as a null pointer constant for the last test.

[erichkeane]

clang/lib/Sema/SemaExpr.cpp

10307

Does the LHSType here need switching too? Or are we just going to use the 'normal' pointer conversion rules here?

[aaron.ballman]

clang/lib/Sema/SemaExpr.cpp

10307

The intent is to follow the normal conversion rules here -- this just allows us to check those rules properly when the LHS type is atomic as well as when it's non-atomic.

[sbc100]

We bisected a build failure on the emscripten waterfall to this change:

https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8782932845295577777/overview
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8782932845295577777/+/u/Build_Emscripten__upstream_/stdout

cache:INFO: generating system library: sysroot/lib/wasm32-emscripten/libc-mt.a... (this will be cached in "/b/s/w/ir/x/w/install/emscripten/cache/sysroot/lib/wasm32-emscripten/libc-mt.a" for subsequent builds)
/b/s/w/ir/x/w/install/emscripten/system/lib/pthread/emscripten_yield.c:10:26: error: cannot compile this static initializer yet
static _Atomic pthread_t crashed_thread_id = NULL;
                         ^
1 error generated.
emcc: error: '/b/s/w/ir/x/w/install/bin/clang -target wasm32-unknown-emscripten -fignore-exceptions -fvisibility=default -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -D__EMSCRIPTEN_SHARED_MEMORY__=1 -D__EMSCRIPTEN_WASM_WORKERS__=1 -Werror=implicit-function-declaration --sysroot=/b/s/w/ir/x/w/install/emscripten/cache/sysroot -Xclang -iwithsysroot/include/fakesdl -Xclang -iwithsysroot/include/compat -O2 -Wall -Werror -fno-unroll-loops -std=c99 -D_XOPEN_SOURCE=700 -Wno-unused-result -Os -fno-inline-functions -fno-builtin -Wno-ignored-attributes -Wno-macro-redefined -Wno-shift-op-parentheses -Wno-string-plus-int -Wno-missing-braces -Wno-logical-op-parentheses -Wno-bitwise-op-parentheses -Wno-unused-but-set-variable -Wno-unused-variable -Wno-unused-label -Wno-pointer-sign -g3 -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc/musl/src/internal -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc/musl/src/include -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc -I/b/s/w/ir/x/w/install/emscripten/system/lib/pthread -pthread -DNDEBUG -ffile-prefix-map=/b/s/w/ir/x/w/install/emscripten=/emsdk/emscripten -fdebug-compilation-dir=/emsdk/emscripten -c -matomics -mbulk-memory /b/s/w/ir/x/w/install/emscripten/system/lib/pthread/emscripten_yield.c -o /b/s/w/ir/x/w/install/emscripten/cache/build/libc-mt-tmp/emscripten_yield.o' failed (returned 1)
embuilder: error: Subprocess 11/1107 failed (returned 1)! (cmdline: /b/s/w/ir/x/w/install/emscripten/emcc -O2 -Wall -Werror -fno-unroll-loops -std=c99 -D_XOPEN_SOURCE=700 -Wno-unused-result -Os -fno-inline-functions -fno-builtin -Wno-ignored-attributes -Wno-macro-redefined -Wno-shift-op-parentheses -Wno-string-plus-int -Wno-missing-braces -Wno-logical-op-parentheses -Wno-bitwise-op-parentheses -Wno-unused-but-set-variable -Wno-unused-variable -Wno-unused-label -Wno-pointer-sign -g -sSTRICT -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc/musl/src/internal -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc/musl/src/include -I/b/s/w/ir/x/w/install/emscripten/system/lib/libc -I/b/s/w/ir/x/w/install/emscripten/system/lib/pthread -pthread -sWASM_WORKERS -DNDEBUG -ffile-prefix-map=/b/s/w/ir/x/w/install/emscripten=/emsdk/emscripten -fdebug-compilation-dir=/emsdk/emscripten -c /b/s/w/ir/x/w/install/emscripten/system/lib/pthread/emscripten_yield.c -o /b/s/w/ir/x/w/install/emscripten/cache/build/libc-mt-tmp/emscripten_yield.o)
Exception thrown in build step.

Prior to this change this code compiled fine.

[bgraur]

We see the same compilation error as the one reported by @sbc100 in several other C libraries.

Here's a short reproducer: https://godbolt.org/z/Y8eds47na

The tests imply that this should work. Please revert.

[aaron.ballman]

In D148730#4294781, @bgraur wrote:

We see the same compilation error as the one reported by @sbc100 in several other C libraries.

Here's a short reproducer: https://godbolt.org/z/Y8eds47na

The tests imply that this should work. Please revert.

Thank you (both) for reporting this issue so quickly! I believe I've addressed it with 1395cde24b3641e284bb1daae7d56c189a2635e3, but if you're still having problems after that commit, let me know and I'll revert both changes to investigate further.

[bgraur]

@aaron.ballman your patch (1395cde24b3641e284bb1daae7d56c189a2635e3) fixed all issues we encountered. Thanks!!

[aaron.ballman]

In D148730#4298401, @bgraur wrote:

@aaron.ballman your patch (1395cde24b3641e284bb1daae7d56c189a2635e3) fixed all issues we encountered. Thanks!!

Excellent, thank you for the confirmation!

clang/docs/ReleaseNotes.rst

 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Implemented `DR2397 <https://wg21.link/CWG2397>`_ which allows ``auto`` specifier for pointers
   and reference to arrays.
 
 C Language Changes
 ------------------
 - Support for outputs from asm goto statements along indirect edges has been
   added. (`#53562 <https://github.com/llvm/llvm-project/issues/53562>`_)
+- Fixed a bug that prevented initialization of an ``_Atomic``-qualified pointer
+  from a null pointer constant.
 
 C2x Feature Support
 ^^^^^^^^^^^^^^^^^^^
 - Implemented the ``unreachable`` macro in freestanding ``<stddef.h>`` for
   `WG14 N2826 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2826.pdf>`_
 
 - Removed the ``ATOMIC_VAR_INIT`` macro in C2x and later standards modes, which
   implements `WG14 N2886 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2886.htm>`_

clang/lib/AST/Expr.cpp

case CXXConstCastExprClass: {
 
     // Handle misc casts we want to ignore.
     if (CE->getCastKind() == CK_NoOp ||
         CE->getCastKind() == CK_LValueToRValue ||
         CE->getCastKind() == CK_ToUnion ||
         CE->getCastKind() == CK_ConstructorConversion ||
         CE->getCastKind() == CK_NonAtomicToAtomic ||
         CE->getCastKind() == CK_AtomicToNonAtomic ||
+        CE->getCastKind() == CK_NullToPointer ||
         CE->getCastKind() == CK_IntToOCLSampler)
       return CE->getSubExpr()->isConstantInitializer(Ctx, false, Culprit);
 
     break;
   }
   case MaterializeTemporaryExprClass:
     return cast<MaterializeTemporaryExpr>(this)
         ->getSubExpr()

clang/lib/Sema/SemaExpr.cpp

Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
   // Suppress this for references: C++ 8.5.3p5.
   if (!LHSType->isReferenceType()) {
     // FIXME: We potentially allocate here even if ConvertRHS is false.
     RHS = DefaultFunctionArrayLvalueConversion(RHS.get(), Diagnose);
     if (RHS.isInvalid())
       return Incompatible;
   }
 
+  // The constraints are expressed in terms of the atomic, qualified, or
+  // unqualified type of the LHS.
+  QualType LHSTypeAfterConversion = LHSType.getAtomicUnqualifiedType();
+
   // C99 6.5.16.1p1: the left operand is a pointer and the right is
   // a null pointer constant.
-  if ((LHSType->isPointerType() || LHSType->isObjCObjectPointerType() ||
-       LHSType->isBlockPointerType()) &&
+  if ((LHSTypeAfterConversion->isPointerType() ||
+       LHSTypeAfterConversion->isObjCObjectPointerType() ||
+       LHSTypeAfterConversion->isBlockPointerType()) &&
       RHS.get()->isNullPointerConstant(Context,
                                        Expr::NPC_ValueDependentIsNull)) {
     if (Diagnose || ConvertRHS) {
       CastKind Kind;
       CXXCastPath Path;
       CheckPointerConversion(RHS.get(), LHSType, Kind, Path,
                              /*IgnoreBaseAccess=*/false, Diagnose);
       if (ConvertRHS)
         RHS = ImpCastExprToType(RHS.get(), LHSType, Kind, VK_PRValue, &Path);
     }
     return Compatible;
   }
 
   // OpenCL queue_t type assignment.
   if (LHSType->isQueueT() && RHS.get()->isNullPointerConstant(
                                  Context, Expr::NPC_ValueDependentIsNull)) {

clang/test/Sema/atomic-expr.c

-// RUN: %clang_cc1 %s -verify -fsyntax-only
-// RUN: %clang_cc1 %s -verify=off -fsyntax-only -Wno-atomic-access
-// off-no-diagnostics
+// RUN: %clang_cc1 %s -verify=expected,access -fsyntax-only
+// RUN: %clang_cc1 %s -std=c2x -verify=expected,access -fsyntax-only
+// RUN: %clang_cc1 %s -std=c2x -pedantic -verify=expected,access -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Wno-atomic-access
 
 _Atomic(unsigned int) data1;
 int _Atomic data2;
 
 // Shift operations
 
 int func_01 (int x) {
   return data1 << x;
void func_15(void) {
   _Static_assert(__builtin_types_compatible_p(__typeof__(x += 2), int), "incorrect");
 }
 
 // Ensure that member access of an atomic structure or union type is properly
 // diagnosed as being undefined behavior; Issue 54563.
 void func_16(void) {
   // LHS member access.
   _Atomic struct { int val; } x, *xp;
-  x.val = 12;   // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  xp->val = 12; // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  x.val = 12;   // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  xp->val = 12; // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
 
   _Atomic union {
     int ival;
     float fval;
   } y, *yp;
-  y.ival = 12;     // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  yp->fval = 1.2f; // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  y.ival = 12;     // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  yp->fval = 1.2f; // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
 
   // RHS member access.
-  int xval = x.val; // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  xval = xp->val;   // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  int yval = y.ival; // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  yval = yp->ival;   // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  int xval = x.val; // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  xval = xp->val;   // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  int yval = y.ival; // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  yval = yp->ival;   // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
 
   // Using the type specifier instead of the type qualifier.
   _Atomic(struct { int val; }) z;
-  z.val = 12;       // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
-  int zval = z.val; // expected-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  z.val = 12;       // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
+  int zval = z.val; // access-error {{accessing a member of an atomic structure or union is undefined behavior}}
 
   // Don't diagnose in an unevaluated context, however.
   (void)sizeof(x.val);
   (void)sizeof(xp->val);
   (void)sizeof(y.ival);
   (void)sizeof(yp->ival);
 }
+
+// Ensure that we correctly implement assignment constraints from C2x 6.5.16.1.
+void func_17(void) {
+  // The left operand has atomic ... arithmetic type, and the right operand has
+  // arithmetic type;
+  _Atomic int i = 0;
+  _Atomic float f = 0.0f;
+
+  // the left operand has an atomic ... version of a structure or union type
+  // compatible with the type of the right operand;
+  struct S { int i; } non_atomic_s;
+  _Atomic struct S s = non_atomic_s;
+
+  union U { int i; float f; } non_atomic_u;
+  _Atomic union U u = non_atomic_u;
+
+  // the left operand has atomic ... pointer type, and (considering the type
+  // the left operand would have after lvalue conversion) both operands are
+  // pointers to qualified or unqualified versions of compatible types, and the
+  // type pointed to by the left operand has all the qualifiers of the type
+  // pointed to by the right operand;
+  const int *cip = 0;
+  volatile const int *vcip = 0;
+  const int * const cicp = 0;
+  _Atomic(const int *) acip = cip;
+  _Atomic(const int *) bad_acip = vcip; // expected-warning {{initializing '_Atomic(const int *)' with an expression of type 'const volatile int *' discards qualifiers}}
+  _Atomic(const int *) acip2 = cicp;
+  _Atomic(int *) aip = &i; // expected-warning {{incompatible pointer types initializing '_Atomic(int *)' with an expression of type '_Atomic(int) *'}} \
+
+  // the left operand has atomic ... pointer type, and (considering the type
+  // the left operand would have after lvalue conversion) one operand is a
+  // pointer to an object type, and the other is a pointer to a qualified or
+  // unqualified version of void, and the type pointed to by the left operand
+  // has all the qualifiers of the type pointed to by the right operand;
+  const void *cvp = 0;
+  _Atomic(const int *) acip3 = cvp;
+  _Atomic(const void *) acvip = cip;
+  _Atomic(const int *) acip4 = vcip;   // expected-warning {{initializing '_Atomic(const int *)' with an expression of type 'const volatile int *' discards qualifiers}}
+  _Atomic(const void *) acvip2 = vcip; // expected-warning {{initializing '_Atomic(const void *)' with an expression of type 'const volatile int *' discards qualifiers}}
+  _Atomic(const int *) acip5 = cicp;
+  _Atomic(const void *) acvip3 = cicp;
+
+#if __STDC_VERSION__ >= 202000L
+  // the left operand has an atomic ... version of the nullptr_t type and the
+  // right operand is a null pointer constant or its type is nullptr_t
+  typedef typeof(nullptr) nullptr_t;
+  nullptr_t n;
+  _Atomic nullptr_t cn2 = n;
+  _Atomic nullptr_t cn3 = nullptr;
+#endif // __STDC_VERSION__ >= 202000L
+
+  // the left operand is an atomic ... pointer, and the right operand is a null
+  // pointer constant or its type is nullptr_t;
+  _Atomic(int *) aip2 = 0;
+#if __STDC_VERSION__ >= 202000L
+  _Atomic(int *) ip2 = n;
+  _Atomic(int *) ip3 = nullptr;
+  _Atomic(const int *) ip4 = nullptr;
+#endif // __STDC_VERSION__ >= 202000L
+}
+
+// Ensure that the assignment constraints also work at file scope.
+_Atomic int ai = 0;
+_Atomic float af = 0.0f;
+_Atomic(int *) aip1 = 0;
+
+struct S { int a; } non_atomic_s;
+_Atomic struct S as = non_atomic_s; // expected-error {{initializer element is not a compile-time constant}}
+
+const int *cip = 0;
+_Atomic(const int *) acip1 = cip; // expected-error {{initializer element is not a compile-time constant}}
+
+const void *cvp = 0;
+_Atomic(const int *) acip2 = cvp; // expected-error {{initializer element is not a compile-time constant}}
+
+#if __STDC_VERSION__ >= 202000L
+  // the left operand has an atomic ... version of the nullptr_t type and the
+  // right operand is a null pointer constant or its type is nullptr_t
+  typedef typeof(nullptr) nullptr_t;
+  nullptr_t n;
+  _Atomic nullptr_t cn2 = n; // expected-error {{initializer element is not a compile-time constant}}
+  _Atomic(int *) aip2 = nullptr;
+#endif // __STDC_VERSION__ >= 202000L
+
+// FIXME: &ai is an address constant, so this should be accepted as an
+// initializer, but the bit-cast inserted due to the pointer conversion is
+// tripping up the test for whether the initializer is a constant expression.
+// The warning is correct but the error is not.
+_Atomic(int *) aip3 = &ai; /* expected-warning {{incompatible pointer types initializing '_Atomic(int *)' with an expression of type '_Atomic(int) *'}}
+                              expected-error {{initializer element is not a compile-time constant}}
+                            */
+
+// Test the behavior when converting the null pointer constant to an atomic
+// function pointer.
+_Atomic(int (*)(char)) afp = (void *)0;