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

Move const qualification of array to its elements #131366

Closed
wants to merge 1 commit into from

Conversation

spavloff
Copy link
Collaborator

Const-qualification of an array caused by constexpr specifier can produce QualType, where the const qualifier is set both as fast qualifier and as a qualifier of the array element type. It can result in a compiler crash, because such QualType does not compare equal to the same type but without extra qualification.

As a fix, the const qualifier is moved to the array element type when setting the implicit const.

It fixes #97005 (Clang crashed in ASTContext::getCommonSugaredType).

Const-qualification of an array caused by constexpr specifier can
produce QualType, where the const qualifier is set both as fast
qualifier and as a qualifier of the array element type. It can result in
a compiler crash, because such QualType does not compare equal to the
same type but without extra qualification.

As a fix, the const qualifier is moved to the array element type when
setting the implicit const.

It fixes llvm#97005 (Clang
crashed in ASTContext::getCommonSugaredType).
@spavloff spavloff self-assigned this Mar 14, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2025

@llvm/pr-subscribers-clang

Author: Serge Pavlov (spavloff)

Changes

Const-qualification of an array caused by constexpr specifier can produce QualType, where the const qualifier is set both as fast qualifier and as a qualifier of the array element type. It can result in a compiler crash, because such QualType does not compare equal to the same type but without extra qualification.

As a fix, the const qualifier is moved to the array element type when setting the implicit const.

It fixes #97005 (Clang crashed in ASTContext::getCommonSugaredType).


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

4 Files Affected:

  • (modified) clang/lib/Sema/SemaType.cpp (+6-1)
  • (modified) clang/test/AST/ByteCode/constexpr.c (+1-1)
  • (modified) clang/test/Sema/constexpr.c (+1-1)
  • (added) clang/test/SemaCXX/constexpr-implicit-const-97005.cpp (+8)
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 11943c0b53591..dee678aa69e83 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5613,8 +5613,13 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
   //  A constexpr specifier used in an object declaration declares the object
   //  as const.
   if (D.getDeclSpec().getConstexprSpecifier() == ConstexprSpecKind::Constexpr &&
-      T->isObjectType())
+      T->isObjectType()) {
     T.addConst();
+    // C++ 9.3.3.4p3: Any type of the form "cv-qualifier-seq array of N U" is
+    // adjusted to "array of N cv-qualifier-seq U".
+    if (const ArrayType *AType = Context.getAsArrayType(T))
+      T = QualType(AType, 0);
+  }
 
   // C++2a [dcl.fct]p4:
   //   A parameter with volatile-qualified type is deprecated
diff --git a/clang/test/AST/ByteCode/constexpr.c b/clang/test/AST/ByteCode/constexpr.c
index af96bf3a06f37..a4a8bde266e76 100644
--- a/clang/test/AST/ByteCode/constexpr.c
+++ b/clang/test/AST/ByteCode/constexpr.c
@@ -82,7 +82,7 @@ constexpr TheA V19[3] = {};
 constexpr TheV V20[3] = {};
 // both-error@-1 {{constexpr variable cannot have type 'const TheV[3]' (aka 'const volatile short[3]')}}
 constexpr TheR V21[3] = {};
-// both-error@-1 {{constexpr variable cannot have type 'const TheR[3]' (aka 'float *restrict const[3]')}}
+// both-error@-1 {{constexpr variable cannot have type 'const TheR[3]' (aka 'float *const restrict[3]')}}
 
 struct HasA {
   TheA f;
diff --git a/clang/test/Sema/constexpr.c b/clang/test/Sema/constexpr.c
index 3dcb0b3a7d95f..005e980563cc7 100644
--- a/clang/test/Sema/constexpr.c
+++ b/clang/test/Sema/constexpr.c
@@ -81,7 +81,7 @@ constexpr TheA V19[3] = {};
 constexpr TheV V20[3] = {};
 // expected-error@-1 {{constexpr variable cannot have type 'const TheV[3]' (aka 'const volatile short[3]')}}
 constexpr TheR V21[3] = {};
-// expected-error@-1 {{constexpr variable cannot have type 'const TheR[3]' (aka 'float *restrict const[3]')}}
+// expected-error@-1 {{constexpr variable cannot have type 'const TheR[3]' (aka 'float *const restrict[3]')}}
 
 struct HasA {
   TheA f;
diff --git a/clang/test/SemaCXX/constexpr-implicit-const-97005.cpp b/clang/test/SemaCXX/constexpr-implicit-const-97005.cpp
new file mode 100644
index 0000000000000..78e9b11f4d61c
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-implicit-const-97005.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ast-dump %s | FileCheck %s
+
+bool aaa;
+constexpr const unsigned char ccc[] = { 5 };
+constexpr const unsigned char ddd[1] = { 0 };
+auto bbb = (aaa ? ddd : ccc);
+
+// CHECK: DeclRefExpr {{.*}} 'const unsigned char[1]' {{.*}} 'ddd' 'const unsigned char[1]'

@efriedma-quic
Copy link
Collaborator

Non-canonical types, are, as the name suggests, non-canonical: they can have all kinds of type sugar, and type qualifiers at all levels. Please don't try to change that; just fix getCommonSugaredType so it can tolerate a const qualifier in this position.

@spavloff
Copy link
Collaborator Author

MR #131649 implements an alternative fix to the problem.

@spavloff spavloff closed this Mar 17, 2025
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang crashed in ASTContext::getCommonSugaredType
3 participants