Skip to content

[clang] Fix a crash from nested ArrayInitLoopExpr #67722

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

Merged
merged 6 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ Bug Fixes in This Version
- Fixes crash when trying to obtain the common sugared type of
`decltype(instantiation-dependent-expr)`.
Fixes (`#67603 <https://github.com/llvm/llvm-project/issues/67603>`_)
- Fixes a crash caused by a multidimensional array being captured by a lambda
(`#67722 <https://github.com/llvm/llvm-project/issues/67722>`_).

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -370,6 +372,9 @@ Bug Fixes to C++ Support
argument. Fixes:
(`#67395 <https://github.com/llvm/llvm-project/issues/67395>`_)

- Fixed a bug causing destructors of constant-evaluated structured bindings
initialized by array elements to be called in the wrong evaluation context.

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10969,6 +10969,16 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) {

bool Success = true;
for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) {
// C++ [class.temporary]/5
// There are four contexts in which temporaries are destroyed at a different
// point than the end of the full-expression. [...] The second context is
// when a copy constructor is called to copy an element of an array while
// the entire array is copied [...]. In either case, if the constructor has
// one or more default arguments, the destruction of every temporary created
// in a default argument is sequenced before the construction of the next
// array element, if any.
FullExpressionRAII Scope(Info);

if (!EvaluateInPlace(Result.getArrayInitializedElt(Index),
Info, Subobject, E->getSubExpr()) ||
!HandleLValueArrayAdjustment(Info, E, Subobject,
Expand All @@ -10977,6 +10987,9 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) {
return false;
Success = false;
}

// Make sure we run the destructors too.
Scope.destroy();
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we don't have to check the return value? It looks like every other use does not discard the return value.

Copy link
Member Author

Choose a reason for hiding this comment

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

The result of the initializer expression should be placed inside a temporary array, which is created on line 10967. We only destroy whatever is left on the stack and we shouldn't need anything from there.

}

return Success;
Expand Down
4 changes: 1 addition & 3 deletions clang/test/AST/Interp/arrays.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
// RUN: %clang_cc1 -verify=ref -DCUR_INTERP %s
// RUN: %clang_cc1 -verify=ref %s

constexpr int m = 3;
constexpr const int *foo[][5] = {
Expand Down Expand Up @@ -355,7 +355,6 @@ namespace ArrayInitLoop {
/// FIXME: The ArrayInitLoop for the decomposition initializer in g() has
/// f(n) as its CommonExpr. We need to evaluate that exactly once and not
/// N times as we do right now.
#ifndef CUR_INTERP
struct X {
int arr[3];
};
Expand All @@ -369,5 +368,4 @@ namespace ArrayInitLoop {
}
static_assert(g() == 6); // expected-error {{failed}} \
// expected-note {{15 == 6}}
#endif
}
37 changes: 35 additions & 2 deletions clang/test/AST/Interp/cxx20.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
// RUN: %clang_cc1 -std=c++20 -verify=ref %s
// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify %s
// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref %s

void test_alignas_operand() {
alignas(8) char dummy;
Expand Down Expand Up @@ -700,3 +700,36 @@ namespace ThreeWayCmp {
static_assert(pa1 <=> pa2 == -1, "");
static_assert(pa2 <=> pa1 == 1, "");
}

// FIXME: Interp should also be able to evaluate this snippet properly.
namespace ConstexprArrayInitLoopExprDestructors
{
struct Highlander {
int *p = 0;
constexpr Highlander() {}
constexpr void set(int *p) { this->p = p; ++*p; if (*p != 1) throw "there can be only one"; } // expected-note {{not valid in a constant expression}}
constexpr ~Highlander() { --*p; }
};

struct X {
int *p;
constexpr X(int *p) : p(p) {}
constexpr X(const X &x, Highlander &&h = Highlander()) : p(x.p) {
h.set(p); // expected-note {{in call to '&Highlander()->set(&n)'}}
}
};

constexpr int f() {
int n = 0;
X x[3] = {&n, &n, &n};
auto [a, b, c] = x; // expected-note {{in call to 'X(x[0], Highlander())'}}
return n;
}

static_assert(f() == 0); // expected-error {{not an integral constant expression}} \
// expected-note {{in call to 'f()'}}

int main() {
return f();
}
}
13 changes: 13 additions & 0 deletions clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s
// RUN: %clang_cc1 -std=c++17 -verify=ref %s

// ref-no-diagnostics
// expected-no-diagnostics

void used_to_crash() {
int s[2][2];

int arr[4];

arr[0] = [s] { return s[0][0]; }();
}