[clang] 48bb5f4 - [clang] Add early exit when checking for const init of arrays.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2021-11-10T18:11:21+01:00
New Revision: 48bb5f4cbe8d5951c1153e469dc6713a122b7fa3

URL: https://github.com/llvm/llvm-project/commit/48bb5f4cbe8d5951c1153e469dc6713a122b7fa3
DIFF: https://github.com/llvm/llvm-project/commit/48bb5f4cbe8d5951c1153e469dc6713a122b7fa3.diff

LOG: [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

Added: 
    clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp

Modified: 
    clang/lib/AST/ExprConstant.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fa0d22064f0eb..a6da3d3fbea2d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10596,28 +10596,55 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,
   bool HadZeroInit = Value->hasValue();
 
   if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-    unsigned N = CAT->getSize().getZExtValue();
+    unsigned FinalSize = CAT->getSize().getZExtValue();
 
     // Preserve the array filler if we had prior zero-initialization.
     APValue Filler =
       HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
                                              : APValue();
 
-    *Value = APValue(APValue::UninitArray(), N, N);
-
-    if (HadZeroInit)
-      for (unsigned I = 0; I != N; ++I)
-        Value->getArrayInitializedElt(I) = Filler;
+    *Value = APValue(APValue::UninitArray(), 0, FinalSize);
+    if (FinalSize == 0)
+      return true;
 
-    // Initialize the elements.
     LValue ArrayElt = Subobject;
     ArrayElt.addArray(Info, E, CAT);
-    for (unsigned I = 0; I != N; ++I)
-      if (!VisitCXXConstructExpr(E, ArrayElt, &Value->getArrayInitializedElt(I),
-                                 CAT->getElementType()) ||
-          !HandleLValueArrayAdjustment(Info, E, ArrayElt,
-                                       CAT->getElementType(), 1))
-        return false;
+    // We do the whole initialization in two passes, first for just one element,
+    // then for the whole array. It's possible we may find out we can't do const
+    // init in the first pass, in which case we avoid allocating a potentially
+    // large array. We don't do more passes because expanding array requires
+    // copying the data, which is wasteful.
+    for (const unsigned N : {1u, FinalSize}) {
+      unsigned OldElts = Value->getArrayInitializedElts();
+      if (OldElts == N)
+        break;
+
+      // Expand the array to appropriate size.
+      APValue NewValue(APValue::UninitArray(), N, FinalSize);
+      for (unsigned I = 0; I < OldElts; ++I)
+        NewValue.getArrayInitializedElt(I).swap(
+            Value->getArrayInitializedElt(I));
+      Value->swap(NewValue);
+
+      if (HadZeroInit)
+        for (unsigned I = OldElts; I < N; ++I)
+          Value->getArrayInitializedElt(I) = Filler;
+
+      // Initialize the elements.
+      for (unsigned I = OldElts; I < N; ++I) {
+        if (!VisitCXXConstructExpr(E, ArrayElt,
+                                   &Value->getArrayInitializedElt(I),
+                                   CAT->getElementType()) ||
+            !HandleLValueArrayAdjustment(Info, E, ArrayElt,
+                                         CAT->getElementType(), 1))
+          return false;
+        // When checking for const initilization any diagnostic is considered
+        // an error.
+        if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+            !Info.keepEvaluatingAfterFailure())
+          return false;
+      }
+    }
 
     return true;
   }

diff  --git a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
new file mode 100644
index 0000000000000..4504c0caed55f
--- /dev/null
+++ b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// UNSUPPORTED: win32
+// RUN: ulimit -v 1048576
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// This used to require too much memory and crash with OOM.
+struct {
+  int a, b, c, d;
+} arr[1<<30];
+