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]; + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
