Skip to content

[clang][ExprConst] Fix crash on uninitialized array subobject #67817

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 27, 2023
Merged

[clang][ExprConst] Fix crash on uninitialized array subobject #67817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f4db6d
[clang][ExprConst] Fix crash on uninitialized array subobject
hazohelet Sep 29, 2023
fe009c3
Add release note
hazohelet Sep 29, 2023
9fb5dd6
Add test to test the fallbacked output
hazohelet Oct 16, 2023
d4890c1
Address comments from Corentin
hazohelet Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DiagnosticASTKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
13 changes: 9 additions & 4 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if we look back at https://reviews.llvm.org/D146358 you used to get the location which could be invalid and the diagnostic that was based on that is now lost b/c we lost that information b/c in many cases you just pass nullptr explicitly. So Perhaps you should have been encoding that information as well and then understand why that is important here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not worrying about SubobjectLoc that was used for subobject declared here note. https://reviews.llvm.org/D146358 introduced the explicit nullptr args, but it only happens on calls where invalid source location was explicitly passed before that patch. (There was one exception about base classes but it was fixed in https://reviews.llvm.org/D153969. Despite passing null SubobjectDecl there, the passed APValue is known to be isStruct(), so its subobjects do not inherit the nullness of SubobjectDecl)
So, we always had an invalid source location where we now have nullSubobjectDecl; thus no information being lost.

Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
<< /*(name)*/ 1 << SubobjectDecl;
Info.Note(SubobjectDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
} else {
Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
<< /*of type*/ 0 << Type;
}
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/Interp/Interp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) << SubObjDecl;
S.FFDiag(SI, diag::note_constexpr_uninitialized)
<< /*(name)*/ 1 << SubObjDecl;
S.Note(SubObjDecl->getLocation(),
diag::note_constexpr_subobject_declared_here);
}
Expand Down
6 changes: 6 additions & 0 deletions clang/test/SemaCXX/constant-expression-cxx2a.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,3 +1492,9 @@ class B{
class D : B{}; // expected-error {{deleted function '~D' cannot override a non-deleted function}}
// expected-note@-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]);
};
7 changes: 7 additions & 0 deletions clang/test/SemaCXX/eval-crashes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {}
};
}