Fix PR32933: crash on lambda capture of VLA
https://bugs.llvm.org/show_bug.cgi?id=32933
Turns out clang wasn't really handling vla's (*) in C++11's for-range entirely correctly.
For e.g. This would lead to generation of buggy IR:
void foo(int b) {
int vla[b];
b = -1; // This store would affect the '__end = vla + b'
for (int &c : vla)
c = 0;
}
Additionally, code-gen would get confused when VLA's were reference-captured by lambdas, and then used in a for-range, which would result in an attempt to generate IR for '__end = vla + b' within the lambda's body - without any capture of 'b' - hence the assertion.
This patch modifies clang, so that for VLA's it translates the end pointer approximately into:
__end = __begin + sizeof(vla)/sizeof(vla->getElementType())
As opposed to the __end = __begin + b;
I considered passing a magic value into codegen - or having codegen special case the '__end' variable when it referred to a variably-modified type, but I decided against that approach, because it smelled like I would be increasing a complicated form of coupling, that I think would be even harder to maintain than the above approach (which can easily be optimized (-O1) to refer to the run-time bound that was calculated upon array's creation or copied into the lambda's closure object).
(*) why oh why gcc would you enable this by default?! ;)
llvm-svn: 303026
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 5d7eada..33a8f9c 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2268,9 +2268,57 @@
BoundExpr = IntegerLiteral::Create(
Context, CAT->getSize(), Context.getPointerDiffType(), RangeLoc);
else if (const VariableArrayType *VAT =
- dyn_cast<VariableArrayType>(UnqAT))
- BoundExpr = VAT->getSizeExpr();
- else {
+ dyn_cast<VariableArrayType>(UnqAT)) {
+ // For a variably modified type we can't just use the expression within
+ // the array bounds, since we don't want that to be re-evaluated here.
+ // Rather, we need to determine what it was when the array was first
+ // created - so we resort to using sizeof(vla)/sizeof(element).
+ // For e.g.
+ // void f(int b) {
+ // int vla[b];
+ // b = -1; <-- This should not affect the num of iterations below
+ // for (int &c : vla) { .. }
+ // }
+
+ // FIXME: This results in codegen generating IR that recalculates the
+ // run-time number of elements (as opposed to just using the IR Value
+ // that corresponds to the run-time value of each bound that was
+ // generated when the array was created.) If this proves too embarassing
+ // even for unoptimized IR, consider passing a magic-value/cookie to
+ // codegen that then knows to simply use that initial llvm::Value (that
+ // corresponds to the bound at time of array creation) within
+ // getelementptr. But be prepared to pay the price of increasing a
+ // customized form of coupling between the two components - which could
+ // be hard to maintain as the codebase evolves.
+
+ ExprResult SizeOfVLAExprR = ActOnUnaryExprOrTypeTraitExpr(
+ EndVar->getLocation(), UETT_SizeOf,
+ /*isType=*/true,
+ CreateParsedType(VAT->desugar(), Context.getTrivialTypeSourceInfo(
+ VAT->desugar(), RangeLoc))
+ .getAsOpaquePtr(),
+ EndVar->getSourceRange());
+ if (SizeOfVLAExprR.isInvalid())
+ return StmtError();
+
+ ExprResult SizeOfEachElementExprR = ActOnUnaryExprOrTypeTraitExpr(
+ EndVar->getLocation(), UETT_SizeOf,
+ /*isType=*/true,
+ CreateParsedType(VAT->desugar(),
+ Context.getTrivialTypeSourceInfo(
+ VAT->getElementType(), RangeLoc))
+ .getAsOpaquePtr(),
+ EndVar->getSourceRange());
+ if (SizeOfEachElementExprR.isInvalid())
+ return StmtError();
+
+ BoundExpr =
+ ActOnBinOp(S, EndVar->getLocation(), tok::slash,
+ SizeOfVLAExprR.get(), SizeOfEachElementExprR.get());
+ if (BoundExpr.isInvalid())
+ return StmtError();
+
+ } else {
// Can't be a DependentSizedArrayType or an IncompleteArrayType since
// UnqAT is not incomplete and Range is not type-dependent.
llvm_unreachable("Unexpected array type in for-range");