Excessive time spent in `ArrayExprEvaluator::VisitCXXConstructExpr` for large dynamic array allocation

[PerkinsRay]

struct Line_Buffer {
    char line[128];
};
struct DumpHandler {
    DumpHandler() {
        Line_Buffer *A;
        A = new Line_Buffer[1024 * 1024 * 50];
    }
    ~DumpHandler() {}
};
int main(){
    return 0;
}

[PerkinsRay]

version10 and above are affected

[PerkinsRay]

https://godbolt.org/z/3jK5caaEa

[BertalanD]

This does actually compile, but it's very slow. It takes more than 20 seconds on a relatively fast computer; godbolt probably kills the process before it would have a chance to finish.

Most of that time seems to be spent in this loop:

llvm-project/clang/lib/AST/ExprConstant.cpp

Lines 10860 to 10873 in 2b230d6

	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;
	}
	}

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[tbaederr]

I mean, the array has no filler, but the constructor being called is a trivial default constructor. I wonder if the code could check for that and simply call it once and then plaster the result over the entire array. Or is the constructor guaranteed to be called 1024 * 1024 * 50 times? Any opinion oin this, @AaronBallman? (sorry for asking you again, I just need a second opinion)

[AaronBallman]

I mean, the array has no filler, but the constructor being called is a trivial default constructor. I wonder if the code could check for that and simply call it once and then plaster the result over the entire array. Or is the constructor guaranteed to be called 1024 * 1024 * 50 times? Any opinion oin this, @AaronBallman? (sorry for asking you again, I just need a second opinion)

When we call VisitCXXConstructExpr() from there, we're recurse back into the function, and eventually wind up at RecordExprEvaluator::VisistCXXConstructExpr(), which does have special logic for trivial default constructors:

llvm-project/clang/lib/AST/ExprConstant.cpp

Line 10061 in 2b230d6

if (CheckTrivialDefaultConstructor(Info, E->getExprLoc(), FD, ZeroInit)) {

and I can't see anything in there that would vary depending on which array element is being initialized. I think it would be fine to call it once and then reuse that for the entire array (assuming there's no lifetime management issues with the values produced).

[tbaederr]

Potential solution here: https://reviews.llvm.org/D130791

With a local debug build, clang needs 35 seconds after this change and 360 seconds before to compile the reproducer.