Skip to content

Commit 48bb5f4

Browse files
committed
[clang] Add early exit when checking for const init of arrays.
Before this commit, on code like: struct S { ... }; S arr[10000000]; while checking if arr is constexpr, clang would reserve memory for arr before running constructor for S. If S turned out to not have a valid constexpr c-tor, clang would still try to initialize each element (and, in case the c-tor was trivial, even skipping the constexpr step limit), only to discard that whole APValue later, since the first element generated a diagnostic. With this change, we start by allocating just 1 element in the array to try out the c-tor and take an early exit if any diagnostics are generated, avoiding possibly large memory allocation and a lot of work initializing to-be-discarded APValues. Fixes 51712 and 51843. In the future we may want to be smarter about large possibly-constexrp arrays and maybe make the allocation lazy. Differential Revision: https://reviews.llvm.org/D113120
1 parent e64c766 commit 48bb5f4

File tree

2 files changed

+51
-13
lines changed

2 files changed

+51
-13
lines changed

clang/lib/AST/ExprConstant.cpp

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10596,28 +10596,55 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,
1059610596
bool HadZeroInit = Value->hasValue();
1059710597

1059810598
if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
10599-
unsigned N = CAT->getSize().getZExtValue();
10599+
unsigned FinalSize = CAT->getSize().getZExtValue();
1060010600

1060110601
// Preserve the array filler if we had prior zero-initialization.
1060210602
APValue Filler =
1060310603
HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
1060410604
: APValue();
1060510605

10606-
*Value = APValue(APValue::UninitArray(), N, N);
10607-
10608-
if (HadZeroInit)
10609-
for (unsigned I = 0; I != N; ++I)
10610-
Value->getArrayInitializedElt(I) = Filler;
10606+
*Value = APValue(APValue::UninitArray(), 0, FinalSize);
10607+
if (FinalSize == 0)
10608+
return true;
1061110609

10612-
// Initialize the elements.
1061310610
LValue ArrayElt = Subobject;
1061410611
ArrayElt.addArray(Info, E, CAT);
10615-
for (unsigned I = 0; I != N; ++I)
10616-
if (!VisitCXXConstructExpr(E, ArrayElt, &Value->getArrayInitializedElt(I),
10617-
CAT->getElementType()) ||
10618-
!HandleLValueArrayAdjustment(Info, E, ArrayElt,
10619-
CAT->getElementType(), 1))
10620-
return false;
10612+
// We do the whole initialization in two passes, first for just one element,
10613+
// then for the whole array. It's possible we may find out we can't do const
10614+
// init in the first pass, in which case we avoid allocating a potentially
10615+
// large array. We don't do more passes because expanding array requires
10616+
// copying the data, which is wasteful.
10617+
for (const unsigned N : {1u, FinalSize}) {
10618+
unsigned OldElts = Value->getArrayInitializedElts();
10619+
if (OldElts == N)
10620+
break;
10621+
10622+
// Expand the array to appropriate size.
10623+
APValue NewValue(APValue::UninitArray(), N, FinalSize);
10624+
for (unsigned I = 0; I < OldElts; ++I)
10625+
NewValue.getArrayInitializedElt(I).swap(
10626+
Value->getArrayInitializedElt(I));
10627+
Value->swap(NewValue);
10628+
10629+
if (HadZeroInit)
10630+
for (unsigned I = OldElts; I < N; ++I)
10631+
Value->getArrayInitializedElt(I) = Filler;
10632+
10633+
// Initialize the elements.
10634+
for (unsigned I = OldElts; I < N; ++I) {
10635+
if (!VisitCXXConstructExpr(E, ArrayElt,
10636+
&Value->getArrayInitializedElt(I),
10637+
CAT->getElementType()) ||
10638+
!HandleLValueArrayAdjustment(Info, E, ArrayElt,
10639+
CAT->getElementType(), 1))
10640+
return false;
10641+
// When checking for const initilization any diagnostic is considered
10642+
// an error.
10643+
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
10644+
!Info.keepEvaluatingAfterFailure())
10645+
return false;
10646+
}
10647+
}
1062110648

1062210649
return true;
1062310650
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// REQUIRES: shell
2+
// UNSUPPORTED: win32
3+
// RUN: ulimit -v 1048576
4+
// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
5+
// expected-no-diagnostics
6+
7+
// This used to require too much memory and crash with OOM.
8+
struct {
9+
int a, b, c, d;
10+
} arr[1<<30];
11+

0 commit comments

Comments
 (0)