[clang][Sema] Handle zero initialization of atomic pointers

[rahulana-quic]

Currently, atomic pointer initializers can reach CodeGen without the expected null-to-pointer cast, triggering an assert. This change ensures that atomic pointer initializers are correctly handled for scalar initializations and init-list elements.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (rahulana-quic)

Changes

Currently, atomic pointer initializers can reach CodeGen without the expected null-to-pointer cast, triggering an assert. This change ensures that atomic pointer initializers are correctly handled for scalar initializations and init-list elements.

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaInit.cpp (+47-14)
• (added) clang/test/CodeGen/atomic_zero_initialize.c (+20)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ff278bc7471bd..3c75e40d1ce59 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -1611,8 +1611,8 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity,
     // Fall through for subaggregate initialization
   } else if (ElemType->isScalarType() || ElemType->isAtomicType()) {
     // FIXME: Need to handle atomic aggregate types with implicit init lists.
-    return CheckScalarType(Entity, IList, ElemType, Index,
-                           StructuredList, StructuredIndex);
+    return CheckScalarType(Entity, IList, ElemType, Index, StructuredList,
+                           StructuredIndex);
   } else if (const ArrayType *arrayType =
                  SemaRef.Context.getAsArrayType(ElemType)) {
     // arrayType can be incomplete if we're initializing a flexible
@@ -6899,8 +6899,26 @@ void InitializationSequence::InitializeFrom(Sema &S,
       return;
 
     // Handle initialization in C
+    // Check for atomic destinations and unwrap to the underlying value type if
+    // the source is non-atomic. Perform the atomic conversion later
+    bool NeedAtomicConversion = false;
+    if (const AtomicType *AtomicDest = DestType->getAs<AtomicType>()) {
+      bool SourceIsAtomicOrPtrToAtomic =
+          (SourceType->isAtomicType() ||
+           (SourceType->isPointerType() &&
+            SourceType->getPointeeType()->isAtomicType()));
+      if (SourceType.isNull() || !SourceIsAtomicOrPtrToAtomic) {
+        DestType = AtomicDest->getValueType();
+        NeedAtomicConversion = true;
+      }
+    }
+
     AddCAssignmentStep(DestType);
     MaybeProduceObjCObject(S, *this, Entity);
+
+    // Add atomic conversion if needed
+    if (!Failed() && NeedAtomicConversion)
+      AddAtomicConversionStep(Entity.getType());
     return;
   }
 
@@ -7004,13 +7022,25 @@ void InitializationSequence::InitializeFrom(Sema &S,
   //      initializer expression to the cv-unqualified version of the
   //      destination type; no user-defined conversions are considered.
 
-  ImplicitConversionSequence ICS
-    = S.TryImplicitConversion(Initializer, DestType,
-                              /*SuppressUserConversions*/true,
-                              Sema::AllowedExplicit::None,
-                              /*InOverloadResolution*/ false,
-                              /*CStyle=*/Kind.isCStyleOrFunctionalCast(),
-                              allowObjCWritebackConversion);
+  // Check for atomic destinations and unwrap to the underlying value type if
+  // the source is non-atomic. Perform the atomic conversion later
+  bool NeedAtomicConversion = false;
+  if (const AtomicType *AtomicDest = DestType->getAs<AtomicType>()) {
+    bool SourceIsAtomicOrPtrToAtomic =
+        (SourceType->isAtomicType() ||
+         (SourceType->isPointerType() &&
+          SourceType->getPointeeType()->isAtomicType()));
+    if (SourceType.isNull() || !SourceIsAtomicOrPtrToAtomic) {
+      DestType = AtomicDest->getValueType();
+      NeedAtomicConversion = true;
+    }
+  }
+
+  ImplicitConversionSequence ICS = S.TryImplicitConversion(
+      Initializer, DestType,
+      /*SuppressUserConversions*/ true, Sema::AllowedExplicit::None,
+      /*InOverloadResolution*/ false,
+      /*CStyle=*/Kind.isCStyleOrFunctionalCast(), allowObjCWritebackConversion);
 
   if (ICS.isStandard() &&
       ICS.Standard.Second == ICK_Writeback_Conversion) {
@@ -7049,6 +7079,10 @@ void InitializationSequence::InitializeFrom(Sema &S,
     AddConversionSequenceStep(ICS, DestType, TopLevelOfInitList);
 
     MaybeProduceObjCObject(S, *this, Entity);
+
+    // Add atomic conversion if needed
+    if (!Failed() && NeedAtomicConversion)
+      AddAtomicConversionStep(Entity.getType());
   }
 }
 
@@ -8516,11 +8550,10 @@ ExprResult InitializationSequence::Perform(Sema &S,
                                         Step->Type, InitialCurInit.get());
 
       bool Complained;
-      if (S.DiagnoseAssignmentResult(ConvTy, Kind.getLocation(),
-                                     Step->Type, SourceType,
-                                     InitialCurInit.get(),
-                                     getAssignmentAction(Entity, true),
-                                     &Complained)) {
+      if (S.DiagnoseAssignmentResult(
+              ConvTy, Kind.getLocation(), Entity.getType(), SourceType,
+              InitialCurInit.get(), getAssignmentAction(Entity, true),
+              &Complained)) {
         PrintInitLocationNote(S, Entity);
         return ExprError();
       } else if (Complained)
diff --git a/clang/test/CodeGen/atomic_zero_initialize.c b/clang/test/CodeGen/atomic_zero_initialize.c
new file mode 100644
index 0000000000000..56b6868919853
--- /dev/null
+++ b/clang/test/CodeGen/atomic_zero_initialize.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK-C
+// RUN: %clang_cc1 -emit-llvm -o - %s -x c++ | FileCheck %s --check-prefixes=CHECK-CXX
+
+// CHECK-LABEL: @initlist_atomic_pointer_zero
+struct range_tree_node_s {
+  long base;
+  long left;
+  struct range_tree_node_s *_Atomic right;
+};
+
+void initlist_atomic_pointer_zero() {
+  (struct range_tree_node_s){.right = 0};
+}
+
+// CHECK-LABEL: @scalar_atomic_init
+void scalar_atomic_init() {
+  // CHECK-C: store ptr null
+  // CHECK-CXX: store ptr null
+  _Atomic(int*) p = 0;
+}

[rahulana-quic]

cc: @efriedma-quic

[efriedma-quic]

Unrelated change?

[efriedma-quic]

Note for other looking at this: this changes SourceType to Entity.getType(). I think from internal discussions, this mostly affects the diagnostics.

[efriedma-quic]

This triggers "warning: expression result unused". Maybe make this a little more clear?

[efriedma-quic]

This CHECK line will be ignored because you didn't specify CHECK in --check-prefixes.

[efriedma-quic]

AddCAssignmentStep eventually leads to code in InitializationSequence::Perform handling SK_CAssignment, which calls CheckSingleAssignmentConstraints... which seems to have an underlying issue. We end up with a similar problematic AST for:

void f() {
  _Atomic(int*)p;
  p = 0;
}

(This apparently doesn't trigger the codegen crash, though.)