From: Richard Smith Date: Sat, 3 Feb 2018 00:44:57 +0000 (+0000) Subject: Fix crash when trying to pack-expand a GNU statement expression. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=64e12bfe9c3e83e35bee59146577a4e951bee09f;p=clang Fix crash when trying to pack-expand a GNU statement expression. We could in principle support such pack expansion, using techniques similar to what we do for pack expansion of lambdas, but it's not clear it's worthwhile. For now at least, cleanly reject these cases rather than crashing. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@324160 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h index 3aa34ae0e0..5f96f59563 100644 --- a/include/clang/Sema/ScopeInfo.h +++ b/include/clang/Sema/ScopeInfo.h @@ -55,13 +55,17 @@ namespace sema { /// parsed. class CompoundScopeInfo { public: - CompoundScopeInfo() - : HasEmptyLoopBodies(false) { } + CompoundScopeInfo(bool IsStmtExpr) + : HasEmptyLoopBodies(false), IsStmtExpr(IsStmtExpr) { } /// \brief Whether this compound stamement contains `for' or `while' loops /// with empty bodies. bool HasEmptyLoopBodies; + /// \brief Whether this compound statement corresponds to a GNU statement + /// expression. + bool IsStmtExpr; + void setHasEmptyLoopBodies() { HasEmptyLoopBodies = true; } diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 7349edff61..2614a64d4a 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1339,7 +1339,7 @@ public: getCurFunction()->recordUseOfWeak(E, IsRead); } - void PushCompoundScope(); + void PushCompoundScope(bool IsStmtExpr); void PopCompoundScope(); sema::CompoundScopeInfo &getCurCompoundScope() const; @@ -3667,7 +3667,7 @@ public: StmtResult ActOnNullStmt(SourceLocation SemiLoc, bool HasLeadingEmptyMacro = false); - void ActOnStartOfCompoundStmt(); + void ActOnStartOfCompoundStmt(bool IsStmtExpr); void ActOnFinishOfCompoundStmt(); StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R, ArrayRef Elts, bool isStmtExpr); @@ -3675,8 +3675,8 @@ public: /// \brief A RAII object to enter scope of a compound statement. class CompoundScopeRAII { public: - CompoundScopeRAII(Sema &S): S(S) { - S.ActOnStartOfCompoundStmt(); + CompoundScopeRAII(Sema &S, bool IsStmtExpr = false) : S(S) { + S.ActOnStartOfCompoundStmt(IsStmtExpr); } ~CompoundScopeRAII() { diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp index f3edf7ca51..e9f2029fb7 100644 --- a/lib/Parse/ParseOpenMP.cpp +++ b/lib/Parse/ParseOpenMP.cpp @@ -1080,21 +1080,18 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( StmtResult AssociatedStmt; if (HasAssociatedStatement) { // The body is a block scope like in Lambdas and Blocks. - Sema::CompoundScopeRAII CompoundScope(Actions); Actions.ActOnOpenMPRegionStart(DKind, getCurScope()); - Actions.ActOnStartOfCompoundStmt(); - // Parse statement - AssociatedStmt = ParseStatement(); - Actions.ActOnFinishOfCompoundStmt(); + // FIXME: We create a bogus CompoundStmt scope to hold the contents of + // the captured region. Code elsewhere assumes that any FunctionScopeInfo + // should have at least one compound statement scope within it. + AssociatedStmt = (Sema::CompoundScopeRAII(Actions), ParseStatement()); AssociatedStmt = Actions.ActOnOpenMPRegionEnd(AssociatedStmt, Clauses); } else if (DKind == OMPD_target_update || DKind == OMPD_target_enter_data || DKind == OMPD_target_exit_data) { - Sema::CompoundScopeRAII CompoundScope(Actions); Actions.ActOnOpenMPRegionStart(DKind, getCurScope()); - Actions.ActOnStartOfCompoundStmt(); - AssociatedStmt = - Actions.ActOnCompoundStmt(Loc, Loc, llvm::None, /*isStmtExpr=*/false); - Actions.ActOnFinishOfCompoundStmt(); + AssociatedStmt = (Sema::CompoundScopeRAII(Actions), + Actions.ActOnCompoundStmt(Loc, Loc, llvm::None, + /*isStmtExpr=*/false)); AssociatedStmt = Actions.ActOnOpenMPRegionEnd(AssociatedStmt, Clauses); } Directive = Actions.ActOnOpenMPExecutableDirective( diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 09ae9d7f22..b63c69db48 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -954,7 +954,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { if (T.consumeOpen()) return StmtError(); - Sema::CompoundScopeRAII CompoundScope(Actions); + Sema::CompoundScopeRAII CompoundScope(Actions, isStmtExpr); // Parse any pragmas at the beginning of the compound statement. ParseCompoundStatementLeadingPragmas(); diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index e07f60989d..45fd2d3249 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -1393,8 +1393,8 @@ void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, delete Scope; } -void Sema::PushCompoundScope() { - getCurFunction()->CompoundScopes.push_back(CompoundScopeInfo()); +void Sema::PushCompoundScope(bool IsStmtExpr) { + getCurFunction()->CompoundScopes.push_back(CompoundScopeInfo(IsStmtExpr)); } void Sema::PopCompoundScope() { diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 1ebc36716a..93907038e0 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -337,8 +337,8 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { DiagRuntimeBehavior(Loc, nullptr, PDiag(DiagID) << R1 << R2); } -void Sema::ActOnStartOfCompoundStmt() { - PushCompoundScope(); +void Sema::ActOnStartOfCompoundStmt(bool IsStmtExpr) { + PushCompoundScope(IsStmtExpr); } void Sema::ActOnFinishOfCompoundStmt() { diff --git a/lib/Sema/SemaTemplateVariadic.cpp b/lib/Sema/SemaTemplateVariadic.cpp index d81837dad5..7aa0e317df 100644 --- a/lib/Sema/SemaTemplateVariadic.cpp +++ b/lib/Sema/SemaTemplateVariadic.cpp @@ -314,8 +314,18 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc, // later. SmallVector LambdaParamPackReferences; for (unsigned N = FunctionScopes.size(); N; --N) { - if (sema::LambdaScopeInfo *LSI = - dyn_cast(FunctionScopes[N-1])) { + sema::FunctionScopeInfo *Func = FunctionScopes[N-1]; + // We do not permit pack expansion that would duplicate a statement + // expression, not even within a lambda. + // FIXME: We could probably support this for statement expressions that do + // not contain labels, and for pack expansions that expand both the stmt + // expr and the enclosing lambda. + if (std::any_of( + Func->CompoundScopes.begin(), Func->CompoundScopes.end(), + [](sema::CompoundScopeInfo &CSI) { return CSI.IsStmtExpr; })) + break; + + if (auto *LSI = dyn_cast(Func)) { if (N == FunctionScopes.size()) { for (auto &Param : Unexpanded) { auto *PD = dyn_cast_or_null( diff --git a/test/SemaTemplate/stmt-expr.cpp b/test/SemaTemplate/stmt-expr.cpp new file mode 100644 index 0000000000..2516a5220c --- /dev/null +++ b/test/SemaTemplate/stmt-expr.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -verify %s + +// FIXME: We could in principle support cases like this (particularly, cases +// where the statement-expression contains no labels). +template void f1() { + int arr[] = { + ({ + T(); // expected-error {{unexpanded parameter pack}} + }) ... // expected-error {{does not contain any unexpanded parameter packs}} + }; +} + +// FIXME: The error for this isn't ideal; it'd be preferable to say that pack +// expansion of a statement expression is not permitted. +template void f2() { + [] { + int arr[] = { + T() + ({ + foo: + T t; // expected-error {{unexpanded parameter pack}} + goto foo; + 0; + }) ... + }; + }; +} + +template void f3() { + ({ + int arr[] = { + [] { + foo: + T t; // OK, expanded within compound statement + goto foo; + return 0; + } ... + }; + }); +}