[clang] [AST][RecoveryExpr] Fix a crash on c89/c90 invalid InitListExpr (#88008) (PR #88014)
Ding Fei via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 15 17:47:07 PDT 2024
https://github.com/danix800 updated https://github.com/llvm/llvm-project/pull/88014
>From 9b293d37ec5c0193e3ad4e1bab2d382ebe54d7b6 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Tue, 9 Apr 2024 00:26:03 +0800
Subject: [PATCH 1/5] [AST][RecoveryExpr] Fix a crash on c89/c90 invalid
InitListExpr (#88008)
Use refactored CheckForConstantInitializer() to skip checking expr with error.
---
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Sema/SemaDecl.cpp | 28 ++++++++-----------
clang/lib/Sema/SemaExpr.cpp | 3 +-
.../test/Sema/recover-expr-gh88008-nocrash.c | 11 ++++++++
4 files changed, 25 insertions(+), 19 deletions(-)
create mode 100644 clang/test/Sema/recover-expr-gh88008-nocrash.c
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d93ac7863b721d..79bf44116d0acc 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3431,7 +3431,7 @@ class Sema final : public SemaBase {
bool ConstexprSupported, bool CLinkageMayDiffer);
/// type checking declaration initializers (C99 6.7.8)
- bool CheckForConstantInitializer(Expr *e, QualType t);
+ bool CheckForConstantInitializer(Expr *Init, unsigned DiagID);
QualType deduceVarTypeFromInitializer(VarDecl *VDecl, DeclarationName Name,
QualType Type, TypeSourceInfo *TSI,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8b3b9d020db572..6529e12e107fb7 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12668,7 +12668,7 @@ void Sema::CheckMSVCRTEntryPoint(FunctionDecl *FD) {
}
}
-bool Sema::CheckForConstantInitializer(Expr *Init, QualType DclT) {
+bool Sema::CheckForConstantInitializer(Expr *Init, unsigned DiagID) {
// FIXME: Need strict checking. In C89, we need to check for
// any assignment, increment, decrement, function-calls, or
// commas outside of a sizeof. In C99, it's the same list,
@@ -12686,8 +12686,7 @@ bool Sema::CheckForConstantInitializer(Expr *Init, QualType DclT) {
const Expr *Culprit;
if (Init->isConstantInitializer(Context, false, &Culprit))
return false;
- Diag(Culprit->getExprLoc(), diag::err_init_element_not_constant)
- << Culprit->getSourceRange();
+ Diag(Culprit->getExprLoc(), DiagID) << Culprit->getSourceRange();
return true;
}
@@ -13805,29 +13804,24 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
// OpenCL v1.2 s6.5.3: __constant locals must be constant-initialized.
// This is true even in C++ for OpenCL.
} else if (VDecl->getType().getAddressSpace() == LangAS::opencl_constant) {
- CheckForConstantInitializer(Init, DclT);
+ CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
- // Otherwise, C++ does not restrict the initializer.
+ // Otherwise, C++ does not restrict the initializer.
} else if (getLangOpts().CPlusPlus) {
// do nothing
// C99 6.7.8p4: All the expressions in an initializer for an object that has
// static storage duration shall be constant expressions or string literals.
} else if (VDecl->getStorageClass() == SC_Static) {
- CheckForConstantInitializer(Init, DclT);
+ CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
- // C89 is stricter than C99 for aggregate initializers.
- // C89 6.5.7p3: All the expressions [...] in an initializer list
- // for an object that has aggregate or union type shall be
- // constant expressions.
+ // C89 is stricter than C99 for aggregate initializers.
+ // C89 6.5.7p3: All the expressions [...] in an initializer list
+ // for an object that has aggregate or union type shall be
+ // constant expressions.
} else if (!getLangOpts().C99 && VDecl->getType()->isAggregateType() &&
isa<InitListExpr>(Init)) {
- const Expr *Culprit;
- if (!Init->isConstantInitializer(Context, false, &Culprit)) {
- Diag(Culprit->getExprLoc(),
- diag::ext_aggregate_init_not_constant)
- << Culprit->getSourceRange();
- }
+ CheckForConstantInitializer(Init, diag::ext_aggregate_init_not_constant);
}
if (auto *E = dyn_cast<ExprWithCleanups>(Init))
@@ -13960,7 +13954,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
// Avoid duplicate diagnostics for constexpr variables.
if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl() &&
!VDecl->isConstexpr())
- CheckForConstantInitializer(Init, DclT);
+ CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
}
QualType InitType = Init->getType();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 24f354f1c72498..4b846bb2a1bb43 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7880,7 +7880,8 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
if (!LiteralExpr->isTypeDependent() &&
!LiteralExpr->isValueDependent() &&
!literalType->isDependentType()) // C99 6.5.2.5p3
- if (CheckForConstantInitializer(LiteralExpr, literalType))
+ if (CheckForConstantInitializer(LiteralExpr,
+ diag::err_init_element_not_constant))
return ExprError();
} else if (literalType.getAddressSpace() != LangAS::opencl_private &&
literalType.getAddressSpace() != LangAS::Default) {
diff --git a/clang/test/Sema/recover-expr-gh88008-nocrash.c b/clang/test/Sema/recover-expr-gh88008-nocrash.c
new file mode 100644
index 00000000000000..5500b33dd0e85d
--- /dev/null
+++ b/clang/test/Sema/recover-expr-gh88008-nocrash.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c90
+
+struct S {
+ int v;
+};
+
+struct T; // expected-note {{forward declaration of 'struct T'}}
+
+void gh88008_nocrash(struct T *t) {
+ struct S s = { .v = t->y }; // expected-error {{incomplete definition of type 'struct T'}}
+}
>From 7ba3d7242e4c02d8b44f4797670a386765e5dbc1 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Tue, 9 Apr 2024 12:52:17 +0800
Subject: [PATCH 2/5] add release note for this fix
---
clang/docs/ReleaseNotes.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index de5fa99bff5dbe..f87e1dd8dbce8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -423,6 +423,8 @@ Bug Fixes in This Version
- Fixed a regression in CTAD that a friend declaration that befriends itself may cause
incorrect constraint substitution. (#GH86769).
+- Fixed an assertion failure on invalid InitListExpr in C90 mode (#GH88008).
+
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>From c7123aa4b5a6a9a2e88bb054517bce91839797be Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Tue, 16 Apr 2024 00:39:20 +0800
Subject: [PATCH 3/5] add default for DiagID
---
clang/include/clang/Sema/Sema.h | 3 ++-
clang/lib/Sema/SemaDecl.cpp | 6 +++---
clang/lib/Sema/SemaExpr.cpp | 3 +--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 79bf44116d0acc..7a0f996ac469c1 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3431,7 +3431,8 @@ class Sema final : public SemaBase {
bool ConstexprSupported, bool CLinkageMayDiffer);
/// type checking declaration initializers (C99 6.7.8)
- bool CheckForConstantInitializer(Expr *Init, unsigned DiagID);
+ bool CheckForConstantInitializer(
+ Expr *Init, unsigned DiagID = diag::err_init_element_not_constant);
QualType deduceVarTypeFromInitializer(VarDecl *VDecl, DeclarationName Name,
QualType Type, TypeSourceInfo *TSI,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6529e12e107fb7..d4a0cb9fe50c27 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13804,7 +13804,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
// OpenCL v1.2 s6.5.3: __constant locals must be constant-initialized.
// This is true even in C++ for OpenCL.
} else if (VDecl->getType().getAddressSpace() == LangAS::opencl_constant) {
- CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
+ CheckForConstantInitializer(Init);
// Otherwise, C++ does not restrict the initializer.
} else if (getLangOpts().CPlusPlus) {
@@ -13813,7 +13813,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
// C99 6.7.8p4: All the expressions in an initializer for an object that has
// static storage duration shall be constant expressions or string literals.
} else if (VDecl->getStorageClass() == SC_Static) {
- CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
+ CheckForConstantInitializer(Init);
// C89 is stricter than C99 for aggregate initializers.
// C89 6.5.7p3: All the expressions [...] in an initializer list
@@ -13954,7 +13954,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
// Avoid duplicate diagnostics for constexpr variables.
if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl() &&
!VDecl->isConstexpr())
- CheckForConstantInitializer(Init, diag::err_init_element_not_constant);
+ CheckForConstantInitializer(Init);
}
QualType InitType = Init->getType();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4b846bb2a1bb43..945263e2606664 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7880,8 +7880,7 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
if (!LiteralExpr->isTypeDependent() &&
!LiteralExpr->isValueDependent() &&
!literalType->isDependentType()) // C99 6.5.2.5p3
- if (CheckForConstantInitializer(LiteralExpr,
- diag::err_init_element_not_constant))
+ if (CheckForConstantInitializer(LiteralExpr))
return ExprError();
} else if (literalType.getAddressSpace() != LangAS::opencl_private &&
literalType.getAddressSpace() != LangAS::Default) {
>From 12290e49c5222a0c96d97e18be7015c9a85ded33 Mon Sep 17 00:00:00 2001
From: Ding Fei <danix800 at gmail.com>
Date: Tue, 16 Apr 2024 08:40:14 +0800
Subject: [PATCH 4/5] Update clang/docs/ReleaseNotes.rst
Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f87e1dd8dbce8c..96e0d8bde55c04 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -423,7 +423,7 @@ Bug Fixes in This Version
- Fixed a regression in CTAD that a friend declaration that befriends itself may cause
incorrect constraint substitution. (#GH86769).
-- Fixed an assertion failure on invalid InitListExpr in C90 mode (#GH88008).
+- Fixed an assertion failure on invalid InitListExpr in C89 mode (#GH88008).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>From aecd048b8e08da49370368eedbb1dbde187e0c02 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Tue, 16 Apr 2024 08:46:26 +0800
Subject: [PATCH 5/5] add missing header (diag for sema)
---
clang/include/clang/Sema/Sema.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7a0f996ac469c1..6576a73c31fb6b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -58,6 +58,7 @@
#include "clang/Sema/Scope.h"
#include "clang/Sema/SemaBase.h"
#include "clang/Sema/SemaConcept.h"
+#include "clang/Sema/SemaDiagnostic.h"
#include "clang/Sema/TypoCorrection.h"
#include "clang/Sema/Weak.h"
#include "llvm/ADT/ArrayRef.h"
More information about the cfe-commits
mailing list