[clang] [clang] Fix a crash issue that caused by handling of fields with initializers in nested anonymous unions (PR #113049)

via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 20 07:46:44 PST 2024


https://github.com/yronglin updated https://github.com/llvm/llvm-project/pull/113049

>From 804b1032cb23cea8fa705a0d2130b1f95185c949 Mon Sep 17 00:00:00 2001
From: yronglin <yronglin777 at gmail.com>
Date: Wed, 20 Nov 2024 23:45:59 +0800
Subject: [PATCH] [clang] Fix a crash issue that caused by handling of fields
 with initializers in nested anonymous unions

Signed-off-by: yronglin <yronglin777 at gmail.com>
---
 clang/include/clang/AST/Decl.h                   | 10 +---------
 clang/include/clang/AST/DeclCXX.h                | 15 +++++++++++++++
 clang/lib/AST/Decl.cpp                           | 16 ++++++++++++++++
 clang/lib/AST/DeclCXX.cpp                        |  8 ++++++++
 clang/lib/Sema/SemaDeclCXX.cpp                   |  1 +
 .../SemaCXX/cxx1y-initializer-aggregates.cpp     | 11 +++++++++++
 6 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 7ff35d73df5997..95d28a8e35cab7 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3219,15 +3219,7 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
 
 public:
   /// Remove the C++11 in-class initializer from this member.
-  void removeInClassInitializer() {
-    assert(hasInClassInitializer() && "no initializer to remove");
-    StorageKind = ISK_NoInit;
-    if (BitField) {
-      // Read the bit width before we change the active union member.
-      Expr *ExistingBitWidth = InitAndBitWidth->BitWidth;
-      BitWidth = ExistingBitWidth;
-    }
-  }
+  void removeInClassInitializer();
 
   /// Determine whether this member captures the variable length array
   /// type.
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 2693cc0e95b4b2..7a0644652f950b 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -269,6 +269,7 @@ class CXXRecordDecl : public RecordDecl {
 
   friend void FunctionDecl::setIsPureVirtual(bool);
   friend void TagDecl::startDefinition();
+  friend void FieldDecl::removeInClassInitializer();
 
   /// Values used in DefinitionData fields to represent special members.
   enum SpecialMemberFlags {
@@ -319,6 +320,9 @@ class CXXRecordDecl : public RecordDecl {
     /// The number of virtual base class specifiers in VBases.
     unsigned NumVBases = 0;
 
+    /// The number of C++11 in-class-initializers in this class.
+    unsigned NumInClassInitializers = 0;
+
     /// Base classes of this class.
     ///
     /// FIXME: This is wasted space for a union.
@@ -497,6 +501,17 @@ class CXXRecordDecl : public RecordDecl {
   /// whenever a member is added to this record.
   void addedMember(Decl *D);
 
+  /// Decreasing the number of C++11 in-class-initializers, and update the
+  /// HasInClassInitializer if there is no in-class-initializer in this class.
+  ///
+  /// This routine helps maintain the number of C++11 in-class-initializers.
+  /// The RecordDecl::hasInClassInitializer() needs to be consistent with the
+  /// FieldDecl::hasInClassInitializer(), When calling
+  /// FieldDecl::hasInClassInitializer() to remove the in-class-initializer in
+  /// the field, we need to check whether there are any in-class-initializers in
+  /// this class, and update HasInClassInitializer to the correct value.
+  void removeInClassInitializer();
+
   void markedVirtualFunctionPure();
 
   /// Get the head of our list of friend declarations, possibly
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index f083ffff87a8ec..01398d3800816a 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4584,6 +4584,22 @@ void FieldDecl::setInClassInitializer(Expr *NewInit) {
   setLazyInClassInitializer(LazyDeclStmtPtr(NewInit));
 }
 
+void FieldDecl::removeInClassInitializer() {
+  assert(hasInClassInitializer() && "no initializer to remove");
+  StorageKind = ISK_NoInit;
+  if (BitField) {
+    // Read the bit width before we change the active union member.
+    Expr *ExistingBitWidth = InitAndBitWidth->BitWidth;
+    BitWidth = ExistingBitWidth;
+  }
+
+  // The RecordDecl::hasInClassInitializer() needs to be consistent with the
+  // FieldDecl::hasInClassInitializer(). Check the number of C++11
+  // in-class-initializers in the parent class.
+  if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(getParent()))
+    RD->removeInClassInitializer();
+}
+
 void FieldDecl::setLazyInClassInitializer(LazyDeclStmtPtr NewInit) {
   assert(hasInClassInitializer() && !getInClassInitializer());
   if (BitField)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 08615d4393f5d1..83a7a055742e7a 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1145,6 +1145,7 @@ void CXXRecordDecl::addedMember(Decl *D) {
         (Field->isAnonymousStructOrUnion() &&
          Field->getType()->getAsCXXRecordDecl()->hasInClassInitializer())) {
       data().HasInClassInitializer = true;
+      data().NumInClassInitializers++;
 
       // C++11 [class]p5:
       //   A default constructor is trivial if [...] no non-static data member
@@ -1441,6 +1442,13 @@ void CXXRecordDecl::addedMember(Decl *D) {
   }
 }
 
+void CXXRecordDecl::removeInClassInitializer() {
+  assert(data().NumInClassInitializers > 0 &&
+         "No member initializer in this class");
+  if (--data().NumInClassInitializers == 0)
+    data().HasInClassInitializer = false;
+}
+
 bool CXXRecordDecl::isLiteral() const {
   const LangOptions &LangOpts = getLangOpts();
   if (!(LangOpts.CPlusPlus20 ? hasConstexprDestructor()
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 38f808a470aa87..c62c3381ada525 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4088,6 +4088,7 @@ void Sema::ActOnFinishCXXInClassMemberInitializer(Decl *D,
   assert((isa<MSPropertyDecl>(D) || FD->getInClassInitStyle() != ICIS_NoInit) &&
          "must set init style when field is created");
 
+  /// FIXME: We might create an RecoveryExpr for the in-class-initializer.
   if (!InitExpr) {
     D->setInvalidDecl();
     if (FD)
diff --git a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
index 03a6800898d18f..8360b8fd7d8ee2 100644
--- a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
@@ -115,3 +115,14 @@ namespace nested_union {
   // of Test3, or we should exclude f(Test3) as a candidate.
   static_assert(f({1}) == 2, ""); // expected-error {{call to 'f' is ambiguous}}
 }
+
+// Fix crash issue https://github.com/llvm/llvm-project/issues/112560.
+// Make sure clang compiles the following code without crashing:
+namespace GH112560 {
+union U {
+  int f = ; // expected-error {{expected expression}}
+};
+void foo() {
+  U g{};
+}
+} // namespace GH112560



More information about the cfe-commits mailing list