[clang] [clang][ExprConst] Fix crash on uninitialized array subobject (PR #67817)
Takuya Shimizu via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 26 22:07:27 PDT 2023
https://github.com/hazohelet updated https://github.com/llvm/llvm-project/pull/67817
>From 7f4db6d81f24adb72416e79bde65a1a13d9a82b8 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu <shimizu2486 at gmail.com>
Date: Fri, 29 Sep 2023 23:49:11 +0900
Subject: [PATCH 1/4] [clang][ExprConst] Fix crash on uninitialized array
subobject
https://reviews.llvm.org/D146358 was assuming that all subobjects have
their own name (`SubobjectDecl`), but it was not true for array
elements.
Fixes https://github.com/llvm/llvm-project/issues/67317
---
clang/include/clang/Basic/DiagnosticASTKinds.td | 2 +-
clang/lib/AST/ExprConstant.cpp | 13 +++++++++----
clang/lib/AST/Interp/Interp.cpp | 2 +-
clang/test/SemaCXX/eval-crashes.cpp | 7 +++++++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index b70cf1071d865da..492f6b270ca5201 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -69,7 +69,7 @@ def note_consteval_address_accessible : Note<
"%select{pointer|reference}0 to a consteval declaration "
"is not a constant expression">;
def note_constexpr_uninitialized : Note<
- "subobject %0 is not initialized">;
+ "subobject %select{of type |}0%1 is not initialized">;
def note_constexpr_uninitialized_base : Note<
"constructor of base class %0 is not called">;
def note_constexpr_static_local : Note<
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 320e2ef12c38db3..131624e4e82db6e 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2411,10 +2411,15 @@ static bool CheckEvaluationResult(CheckEvaluationResultKind CERK,
const FieldDecl *SubobjectDecl,
CheckedTemporaries &CheckedTemps) {
if (!Value.hasValue()) {
- assert(SubobjectDecl && "SubobjectDecl shall be non-null");
- Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << SubobjectDecl;
- Info.Note(SubobjectDecl->getLocation(),
- diag::note_constexpr_subobject_declared_here);
+ if (SubobjectDecl) {
+ Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
+ << true << SubobjectDecl;
+ Info.Note(SubobjectDecl->getLocation(),
+ diag::note_constexpr_subobject_declared_here);
+ } else {
+ // FIXME: We should add a test to check the output of this case.
+ Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << false << Type;
+ }
return false;
}
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 1ebbadc375f38c8..50ee4192712fcbe 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -439,7 +439,7 @@ bool CheckPure(InterpState &S, CodePtr OpPC, const CXXMethodDecl *MD) {
static void DiagnoseUninitializedSubobject(InterpState &S, const SourceInfo &SI,
const FieldDecl *SubObjDecl) {
assert(SubObjDecl && "Subobject declaration does not exist");
- S.FFDiag(SI, diag::note_constexpr_uninitialized) << SubObjDecl;
+ S.FFDiag(SI, diag::note_constexpr_uninitialized) << true << SubObjDecl;
S.Note(SubObjDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
}
diff --git a/clang/test/SemaCXX/eval-crashes.cpp b/clang/test/SemaCXX/eval-crashes.cpp
index 3e59ad31c559da8..ac04b113f99b7aa 100644
--- a/clang/test/SemaCXX/eval-crashes.cpp
+++ b/clang/test/SemaCXX/eval-crashes.cpp
@@ -54,3 +54,10 @@ namespace pr33140_10 {
int a(const int &n = 0);
bool b() { return a() == a(); }
}
+
+namespace GH67317 {
+struct array {
+ int (&data)[2];
+ array() : data(*new int[1][2]) {}
+};
+}
>From fe009c3eeac2283df7b4b966d91b2a2bfb824f56 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu <shimizu2486 at gmail.com>
Date: Sat, 30 Sep 2023 00:03:29 +0900
Subject: [PATCH 2/4] Add release note
---
clang/docs/ReleaseNotes.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 82550232947f743..8eca67677f1d82c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -529,6 +529,8 @@ Bug Fixes in This Version
``thread_local`` instead of ``_Thread_local``.
Fixes (`#70068 <https://github.com/llvm/llvm-project/issues/70068>`_) and
(`#69167 <https://github.com/llvm/llvm-project/issues/69167>`_)
+- Fix crash from constexpr evaluator evaluating uninitialized arrays as rvalue.
+ Fixes (`#67317 <https://github.com/llvm/llvm-project/issues/67317>`_)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>From 9fb5dd6c3b3f0e456e6a8deb99caa97fbb2f8cc0 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu <shimizu2486 at gmail.com>
Date: Mon, 16 Oct 2023 13:43:02 +0900
Subject: [PATCH 3/4] Add test to test the fallbacked output
---
clang/lib/AST/ExprConstant.cpp | 1 -
clang/test/SemaCXX/constant-expression-cxx2a.cpp | 6 ++++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 131624e4e82db6e..a5d113d5fe7cc60 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2417,7 +2417,6 @@ static bool CheckEvaluationResult(CheckEvaluationResultKind CERK,
Info.Note(SubobjectDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
} else {
- // FIXME: We should add a test to check the output of this case.
Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << false << Type;
}
return false;
diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 09f17d5b3894998..e4d97dcb73562d6 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1492,3 +1492,9 @@ class B{
class D : B{}; // expected-error {{deleted function '~D' cannot override a non-deleted function}}
// expected-note at -1 {{destructor of 'D' is implicitly deleted because base class 'B' has an inaccessible destructor}}
}
+
+namespace GH67317 {
+ constexpr unsigned char a = // expected-error {{constexpr variable 'a' must be initialized by a constant expression}} \
+ // expected-note {{subobject of type 'const unsigned char' is not initialized}}
+ __builtin_bit_cast(unsigned char, *new char[3][1]);
+};
>From d4890c10ac66f1421bd5332692206729597d4a5c Mon Sep 17 00:00:00 2001
From: Takuya Shimizu <shimizu2486 at gmail.com>
Date: Mon, 16 Oct 2023 15:02:04 +0900
Subject: [PATCH 4/4] Address comments from Corentin
---
clang/lib/AST/ExprConstant.cpp | 5 +++--
clang/lib/AST/Interp/Interp.cpp | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index a5d113d5fe7cc60..5947805f9576ff8 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2413,11 +2413,12 @@ static bool CheckEvaluationResult(CheckEvaluationResultKind CERK,
if (!Value.hasValue()) {
if (SubobjectDecl) {
Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
- << true << SubobjectDecl;
+ << /*(name)*/ 1 << SubobjectDecl;
Info.Note(SubobjectDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
} else {
- Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << false << Type;
+ Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
+ << /*of type*/ 0 << Type;
}
return false;
}
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 50ee4192712fcbe..144b674451e353c 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -439,7 +439,8 @@ bool CheckPure(InterpState &S, CodePtr OpPC, const CXXMethodDecl *MD) {
static void DiagnoseUninitializedSubobject(InterpState &S, const SourceInfo &SI,
const FieldDecl *SubObjDecl) {
assert(SubObjDecl && "Subobject declaration does not exist");
- S.FFDiag(SI, diag::note_constexpr_uninitialized) << true << SubObjDecl;
+ S.FFDiag(SI, diag::note_constexpr_uninitialized)
+ << /*(name)*/ 1 << SubObjDecl;
S.Note(SubObjDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
}
More information about the cfe-commits
mailing list