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) #112855

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

Fznamznon
Copy link
Contributor

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

Fixes #109095
Fixes #112516

@Fznamznon Fznamznon added this to the LLVM 19.X Release milestone Oct 18, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 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/112855.diff

2 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+7-3)
  • (modified) clang/test/Sema/constexpr.c (+14)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 490c4a2fc525cd..bc7cce0bcd7fc2 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2503,7 +2503,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
@@ -2610,8 +2611,11 @@ 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)
+  // In C, all globals and constexpr variables should have constant
+  // initialization. For constexpr variables in C check that initializer is a
+  // constant initializer because they can be used in constant expressions.
+  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 8286cd2107d2f2..0bf3a41c18d76d 100644
--- a/clang/test/Sema/constexpr.c
+++ b/clang/test/Sema/constexpr.c
@@ -357,3 +357,17 @@ void infsNaNs() {
   constexpr double db5 = LD_SNAN; // expected-error {{constexpr initializer evaluates to nan which is not exactly representable in type 'const double'}}
   constexpr double db6 = INF;
 }
+
+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}}
+}

@tru
Copy link
Collaborator

tru commented Oct 28, 2024

@AaronBallman @shafik ?

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.

LGTM!

@tru
Copy link
Collaborator

tru commented Oct 29, 2024

This has conflicts now. Can someone fix it and I can merge it for 19.1.4

@AaronBallman
Copy link
Collaborator

This has conflicts now. Can someone fix it and I can merge it for 19.1.4

I took a shot at resolving the conflicts (I think Mariya is out on vacation currently).

@tru
Copy link
Collaborator

tru commented Nov 12, 2024

I had to resolve some conflicts again - can you have a look @AaronBallman ?

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.

Still LGTM!

@tru tru merged commit c9e8540 into llvm:release/19.x Nov 15, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
Copy link

@Fznamznon (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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:backport
Projects
Development

Successfully merging this pull request may close these issues.

5 participants