Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Fix C23 constexpr crashes #112708

Merged
merged 2 commits into from
Oct 18, 2024
Merged

[clang] Fix C23 constexpr crashes #112708

merged 2 commits into from
Oct 18, 2024

Conversation

Fznamznon
Copy link
Contributor

@Fznamznon Fznamznon commented Oct 17, 2024

Before using a constexpr variable that is not properly initialized check that it is valid.

Fixes #109095
Fixes #112516

Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

Before using a constexpr variable that is not properly initialized check that it is valid.

Fixes #109095 Fixes #112516


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

2 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+4-2)
  • (modified) clang/test/Sema/constexpr.c (+17)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index f083ffff87a8ec..c642fcf8785151 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2512,7 +2512,8 @@ bool VarDecl::isUsableInConstantExpressions(const ASTContext &Context) const {
   if (!DefVD->mightBeUsableInConstantExpressions(Context))
     return false;
   //   ... and its initializer is a constant initializer.
-  if (Context.getLangOpts().CPlusPlus && !DefVD->hasConstantInitialization())
+  if ((Context.getLangOpts().CPlusPlus || getLangOpts().C23) &&
+      !DefVD->hasConstantInitialization())
     return false;
   // C++98 [expr.const]p1:
   //   An integral constant-expression can involve only [...] const variables
@@ -2620,7 +2621,8 @@ bool VarDecl::hasICEInitializer(const ASTContext &Context) const {
 
 bool VarDecl::hasConstantInitialization() const {
   // In C, all globals (and only globals) have constant initialization.
-  if (hasGlobalStorage() && !getASTContext().getLangOpts().CPlusPlus)
+  if (hasGlobalStorage() && !getASTContext().getLangOpts().CPlusPlus &&
+      !isConstexpr())
     return true;
 
   // In C++, it depends on whether the evaluation at the point of definition
diff --git a/clang/test/Sema/constexpr.c b/clang/test/Sema/constexpr.c
index eaa000b3b97758..3dcb0b3a7d95fd 100644
--- a/clang/test/Sema/constexpr.c
+++ b/clang/test/Sema/constexpr.c
@@ -374,3 +374,20 @@ void constexprif() {
 void constevalif() {
   if consteval (300) {} //expected-error {{expected '(' after 'if'}}
 }
+
+struct S11 {
+  int len;
+};
+void ghissue112516() {
+  struct S11 *s11 = 0;
+  constexpr int num = s11->len; // expected-error {{constexpr variable 'num' must be initialized by a constant expression}}
+  void *Arr[num];
+}
+
+void ghissue109095() {
+  constexpr char c[] = { 'a' };
+  constexpr int i = c[1]; // expected-error {{constexpr variable 'i' must be initialized by a constant expression}}\
+                          // expected-note {{declared here}}
+  _Static_assert(i == c[0]); // expected-error {{static assertion expression is not an integral constant expression}}\
+                             // expected-note {{initializer of 'i' is not a constant expression}}
+}

@AaronBallman AaronBallman requested a review from tbaederr October 17, 2024 13:57
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem reasonable to me, and likely worth backporting (CC @tbaederr for a second set of eyes just to be sure)

@tbaederr
Copy link
Contributor

Agreed, LGTM. The first comment in VarDecl::hasConstantInitialization() could use some love, since it's no longer true for global constexpr variables.

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Fznamznon Fznamznon merged commit abfba7d into llvm:main Oct 18, 2024
8 checks passed
@Fznamznon Fznamznon added this to the LLVM 19.X Release milestone Oct 18, 2024
@Fznamznon
Copy link
Contributor Author

/cherry-pick abfba7d

@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

Failed to cherry-pick: abfba7d

https://github.com/llvm/llvm-project/actions/runs/11400407910

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

Fznamznon added a commit to Fznamznon/llvm-project that referenced this pull request Oct 18, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
@Fznamznon
Copy link
Contributor Author

Release branch cherry-pick #112855

Fznamznon added a commit to Fznamznon/llvm-project that referenced this pull request Oct 18, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 21, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
tru pushed a commit to Fznamznon/llvm-project that referenced this pull request Nov 12, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
tru pushed a commit to Fznamznon/llvm-project that referenced this pull request Nov 15, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
nikic pushed a commit to rust-lang/llvm-project that referenced this pull request Nov 20, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category release:cherry-pick-failed
Projects
5 participants