[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
Sun Nov 24 08:10:55 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 1/3] [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
>From de4a9d55e723d4662b7ad6264d33df524d56833c Mon Sep 17 00:00:00 2001
From: yronglin <yronglin777 at gmail.com>
Date: Sat, 23 Nov 2024 23:26:06 +0800
Subject: [PATCH 2/3] Revert "[clang] Fix a crash issue that caused by handling
of fields with initializers in nested anonymous unions"
This reverts commit 804b1032cb23cea8fa705a0d2130b1f95185c949.
---
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, 9 insertions(+), 52 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 95d28a8e35cab7..7ff35d73df5997 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3219,7 +3219,15 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
public:
/// Remove the C++11 in-class initializer from this member.
- void removeInClassInitializer();
+ 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;
+ }
+ }
/// 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 7a0644652f950b..2693cc0e95b4b2 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -269,7 +269,6 @@ 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 {
@@ -320,9 +319,6 @@ 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.
@@ -501,17 +497,6 @@ 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 01398d3800816a..f083ffff87a8ec 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4584,22 +4584,6 @@ 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 83a7a055742e7a..08615d4393f5d1 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1145,7 +1145,6 @@ 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
@@ -1442,13 +1441,6 @@ 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 c62c3381ada525..38f808a470aa87 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4088,7 +4088,6 @@ 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 8360b8fd7d8ee2..03a6800898d18f 100644
--- a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
@@ -115,14 +115,3 @@ 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
>From 882fd4e32395348514db2cde741bd9713f07f88b Mon Sep 17 00:00:00 2001
From: yronglin <yronglin777 at gmail.com>
Date: Mon, 25 Nov 2024 00:10:15 +0800
Subject: [PATCH 3/3] Recovery from the invalid in-class-initializer
Signed-off-by: yronglin <yronglin777 at gmail.com>
---
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Parse/ParseCXXInlineMethods.cpp | 3 +--
clang/lib/Sema/SemaDeclCXX.cpp | 24 +++++++++++--------
clang/lib/Sema/SemaInit.cpp | 12 ++++++++++
.../SemaCXX/cxx1y-initializer-aggregates.cpp | 11 +++++++++
5 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..b0dd4494b7c3b2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5287,7 +5287,7 @@ class Sema final : public SemaBase {
/// is complete.
void ActOnFinishCXXInClassMemberInitializer(Decl *VarDecl,
SourceLocation EqualLoc,
- Expr *Init);
+ ExprResult Init);
/// Handle a C++ member initializer using parentheses syntax.
MemInitResult
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index b461743833c82a..79e61af5f06850 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -722,8 +722,7 @@ void Parser::ParseLexedMemberInitializer(LateParsedMemberInitializer &MI) {
ExprResult Init = ParseCXXMemberInitializer(MI.Field, /*IsFunction=*/false,
EqualLoc);
- Actions.ActOnFinishCXXInClassMemberInitializer(MI.Field, EqualLoc,
- Init.get());
+ Actions.ActOnFinishCXXInClassMemberInitializer(MI.Field, EqualLoc, Init);
// The next token should be our artificial terminating EOF token.
if (Tok.isNot(tok::eof)) {
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 38f808a470aa87..19acac7bf6d09e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4080,24 +4080,28 @@ ExprResult Sema::ConvertMemberDefaultInitExpression(FieldDecl *FD,
void Sema::ActOnFinishCXXInClassMemberInitializer(Decl *D,
SourceLocation InitLoc,
- Expr *InitExpr) {
+ ExprResult InitExpr) {
// Pop the notional constructor scope we created earlier.
PopFunctionScopeInfo(nullptr, D);
- FieldDecl *FD = dyn_cast<FieldDecl>(D);
- assert((isa<MSPropertyDecl>(D) || FD->getInClassInitStyle() != ICIS_NoInit) &&
- "must set init style when field is created");
-
- if (!InitExpr) {
+ // Microsoft C++'s property declaration cannot have a default member
+ // initializer.
+ if (isa<MSPropertyDecl>(D)) {
D->setInvalidDecl();
- if (FD)
- FD->removeInClassInitializer();
return;
}
- if (DiagnoseUnexpandedParameterPack(InitExpr, UPPC_Initializer)) {
+ FieldDecl *FD = dyn_cast<FieldDecl>(D);
+ assert((FD && FD->getInClassInitStyle() != ICIS_NoInit) &&
+ "must set init style when field is created");
+
+ if (!InitExpr.isUsable() ||
+ DiagnoseUnexpandedParameterPack(InitExpr.get(), UPPC_Initializer)) {
FD->setInvalidDecl();
- FD->removeInClassInitializer();
+ ExprResult RecoveryInit =
+ CreateRecoveryExpr(InitLoc, InitLoc, {}, FD->getType());
+ if (RecoveryInit.isUsable())
+ FD->setInClassInitializer(RecoveryInit.get());
return;
}
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index f560865681fa5a..5f86a14a8026af 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -750,6 +750,18 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
if (Field->hasInClassInitializer()) {
if (VerifyOnly)
return;
+
+ // We do not want to aggressively set the hadError flag and cutoff
+ // parsing. Try to recover when in-class-initializer is a RecoveryExpr.
+ if (isa_and_nonnull<RecoveryExpr>(Field->getInClassInitializer())) {
+ if (Init < NumInits)
+ ILE->setInit(Init, Field->getInClassInitializer());
+ else
+ ILE->updateInit(SemaRef.Context, Init,
+ Field->getInClassInitializer());
+ return;
+ }
+
ExprResult DIE;
{
// Enter a default initializer rebuild context, then we can support
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