<div dir="ltr">This change doesn't look correct to me. It's possible to observe an array during its own initialization, and if that happens, the array needs to be filled with the array filler up to its declared length. It's not beautiful, but here's a testcase that was broken by this change: <a href="https://godbolt.org/z/5zof71v36" target="_blank">https://godbolt.org/z/5zof71v36</a><div><br></div><div>extern const struct A a;<br>constexpr const A &get() { return a; }<br>struct A {<br> struct B {<br> constexpr B() : n(get().b[4].n) {}<br> int n;<br> };<br> B b[5];<br>};<br>constexpr A a = A();<br></div><div><br></div><div>Looks like it should be relatively straightforward to create the small version of the array with the filler in place.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 29 Dec 2021 at 04:07, Sam McCall via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Sam McCall<br>
Date: 2021-12-29T13:07:30+01:00<br>
New Revision: 9dc4af327b12dfbcf90fde1641cd649c6814bf98<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/9dc4af327b12dfbcf90fde1641cd649c6814bf98" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9dc4af327b12dfbcf90fde1641cd649c6814bf98</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/9dc4af327b12dfbcf90fde1641cd649c6814bf98.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9dc4af327b12dfbcf90fde1641cd649c6814bf98.diff</a><br>
<br>
LOG: Re-land "[clang] Add early exit when checking for const init of arrays."<br>
<br>
This reverts commit 6d09aaecdfe51e13fc64d539aa7c9a790de341d7.<br>
<br>
The test uses ulimit and ran into problems on some bots. Run on linux only.<br>
There's nothing platform-specific about the code we're testing, so this<br>
should be enough to ensure correctness.<br>
<br>
Added: <br>
clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp<br>
<br>
Modified: <br>
clang/lib/AST/ExprConstant.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp<br>
index 469339e8cd624..105cd7a3506dd 100644<br>
--- a/clang/lib/AST/ExprConstant.cpp<br>
+++ b/clang/lib/AST/ExprConstant.cpp<br>
@@ -10680,28 +10680,55 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,<br>
bool HadZeroInit = Value->hasValue();<br>
<br>
if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {<br>
- unsigned N = CAT->getSize().getZExtValue();<br>
+ unsigned FinalSize = CAT->getSize().getZExtValue();<br>
<br>
// Preserve the array filler if we had prior zero-initialization.<br>
APValue Filler =<br>
HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()<br>
: APValue();<br>
<br>
- *Value = APValue(APValue::UninitArray(), N, N);<br>
-<br>
- if (HadZeroInit)<br>
- for (unsigned I = 0; I != N; ++I)<br>
- Value->getArrayInitializedElt(I) = Filler;<br>
+ *Value = APValue(APValue::UninitArray(), 0, FinalSize);<br>
+ if (FinalSize == 0)<br>
+ return true;<br>
<br>
- // Initialize the elements.<br>
LValue ArrayElt = Subobject;<br>
ArrayElt.addArray(Info, E, CAT);<br>
- for (unsigned I = 0; I != N; ++I)<br>
- if (!VisitCXXConstructExpr(E, ArrayElt, &Value->getArrayInitializedElt(I),<br>
- CAT->getElementType()) ||<br>
- !HandleLValueArrayAdjustment(Info, E, ArrayElt, CAT->getElementType(),<br>
- 1))<br>
- return false;<br>
+ // We do the whole initialization in two passes, first for just one element,<br>
+ // then for the whole array. It's possible we may find out we can't do const<br>
+ // init in the first pass, in which case we avoid allocating a potentially<br>
+ // large array. We don't do more passes because expanding array requires<br>
+ // copying the data, which is wasteful.<br>
+ for (const unsigned N : {1u, FinalSize}) {<br>
+ unsigned OldElts = Value->getArrayInitializedElts();<br>
+ if (OldElts == N)<br>
+ break;<br>
+<br>
+ // Expand the array to appropriate size.<br>
+ APValue NewValue(APValue::UninitArray(), N, FinalSize);<br>
+ for (unsigned I = 0; I < OldElts; ++I)<br>
+ NewValue.getArrayInitializedElt(I).swap(<br>
+ Value->getArrayInitializedElt(I));<br>
+ Value->swap(NewValue);<br>
+<br>
+ if (HadZeroInit)<br>
+ for (unsigned I = OldElts; I < N; ++I)<br>
+ Value->getArrayInitializedElt(I) = Filler;<br>
+<br>
+ // Initialize the elements.<br>
+ for (unsigned I = OldElts; I < N; ++I) {<br>
+ if (!VisitCXXConstructExpr(E, ArrayElt,<br>
+ &Value->getArrayInitializedElt(I),<br>
+ CAT->getElementType()) ||<br>
+ !HandleLValueArrayAdjustment(Info, E, ArrayElt,<br>
+ CAT->getElementType(), 1))<br>
+ return false;<br>
+ // When checking for const initilization any diagnostic is considered<br>
+ // an error.<br>
+ if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&<br>
+ !Info.keepEvaluatingAfterFailure())<br>
+ return false;<br>
+ }<br>
+ }<br>
<br>
return true;<br>
}<br>
<br>
diff --git a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp<br>
new file mode 100644<br>
index 0000000000000..859b1ac3fb2be<br>
--- /dev/null<br>
+++ b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp<br>
@@ -0,0 +1,17 @@<br>
+// Only run this test where ulimit is known to work well.<br>
+// (There's nothing really platform-specific being tested, this is just ulimit).<br>
+//<br>
+// REQUIRES: shell<br>
+// REQUIRES: linux<br>
+// UNSUPPORTED: msan<br>
+// UNSUPPORTED: asan<br>
+//<br>
+// RUN: ulimit -v 1048576<br>
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -triple=x86_64 %s<br>
+// expected-no-diagnostics<br>
+<br>
+// This used to require too much memory and crash with OOM.<br>
+struct {<br>
+ int a, b, c, d;<br>
+} arr[1<<30];<br>
+<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>