From eaef89b197e640785a1fdb167725359248f77720 Mon Sep 17 00:00:00 2001 From: John McCall Date: Fri, 22 Mar 2013 02:10:40 +0000 Subject: [PATCH] Fix a crash-on-valid where a block capture copy expression was picking up cleanups from earlier in the statement. Also fix a crash-on-invalid where a reference to an invalid decl from an enclosing scope was causing an expression to fail to build, but only *after* a cleanup was registered from that statement, causing an assertion downstream. The crash-on-valid is rdar://13459289. llvm-svn: 177692 --- clang/include/clang/Sema/Sema.h | 1 + clang/lib/Parse/ParseStmt.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 1 + clang/lib/Sema/SemaDeclCXX.cpp | 3 +++ clang/lib/Sema/SemaExpr.cpp | 8 +++++--- clang/lib/Sema/SemaStmt.cpp | 5 +++++ clang/test/CodeGenCXX/blocks.cpp | 25 +++++++++++++++++++++++++ clang/test/SemaCXX/blocks.cpp | 20 +++++++++++++++++++- 8 files changed, 60 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 3f3fcb5..370d7d9 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2637,6 +2637,7 @@ public: } StmtResult ActOnExprStmt(ExprResult Arg); + StmtResult ActOnExprStmtError(); StmtResult ActOnNullStmt(SourceLocation SemiLoc, bool HasLeadingEmptyMacro = false); diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 71a7b2c..5833fc1 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -319,7 +319,7 @@ StmtResult Parser::ParseExprStatement() { SkipUntil(tok::r_brace, /*StopAtSemi=*/true, /*DontConsume=*/true); if (Tok.is(tok::semi)) ConsumeToken(); - return StmtError(); + return Actions.ActOnExprStmtError(); } if (Tok.is(tok::colon) && getCurScope()->isSwitchScope() && diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8213739..5597bf6 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -7916,6 +7916,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { // Regardless, we don't want to ignore array nesting when // constructing this copy. if (type->isStructureOrClassType()) { + EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated); SourceLocation poi = var->getLocation(); Expr *varRef =new (Context) DeclRefExpr(var, false, type, VK_LValue, poi); ExprResult result diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index dd8dad3..52ed28a 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -10288,6 +10288,9 @@ VarDecl *Sema::BuildExceptionDeclaration(Scope *S, if (!Invalid && !ExDeclType->isDependentType()) { if (const RecordType *recordType = ExDeclType->getAs()) { + // Insulate this from anything else we might currently be parsing. + EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated); + // C++ [except.handle]p16: // The object declared in an exception-declaration or, if the // exception-declaration does not specify a name, a temporary (12.2) is diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index e4323c3..26a697c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10698,7 +10698,7 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI, // Introduce a new evaluation context for the initialization, so // that temporaries introduced as part of the capture are retained // to be re-"exported" from the lambda expression itself. - S.PushExpressionEvaluationContext(Sema::PotentiallyEvaluated); + EnterExpressionEvaluationContext scope(S, Sema::PotentiallyEvaluated); // C++ [expr.prim.labda]p12: // An entity captured by a lambda-expression is odr-used (3.2) in @@ -10749,7 +10749,6 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI, if (Subscript.isInvalid()) { S.CleanupVarDeclMarking(); S.DiscardCleanupsInEvaluationContext(); - S.PopExpressionEvaluationContext(); return ExprError(); } @@ -10785,7 +10784,6 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI, // Exit the expression evaluation context used for the capture. S.CleanupVarDeclMarking(); S.DiscardCleanupsInEvaluationContext(); - S.PopExpressionEvaluationContext(); return Result; } @@ -10972,6 +10970,10 @@ bool Sema::tryCaptureVariable(VarDecl *Var, SourceLocation Loc, if (isa(Var)) FinalizeVarWithDestructor(Var, Record); + // Enter a new evaluation context to insulate the copy + // full-expression. + EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated); + // According to the blocks spec, the capture of a variable from // the stack requires a const copy constructor. This is not true // of the copy/move done to move a __block variable to the heap. diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 0c51e44..a2dc616 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -54,6 +54,11 @@ StmtResult Sema::ActOnExprStmt(ExprResult FE) { } +StmtResult Sema::ActOnExprStmtError() { + DiscardCleanupsInEvaluationContext(); + return StmtError(); +} + StmtResult Sema::ActOnNullStmt(SourceLocation SemiLoc, bool HasLeadingEmptyMacro) { return Owned(new (Context) NullStmt(SemiLoc, HasLeadingEmptyMacro)); diff --git a/clang/test/CodeGenCXX/blocks.cpp b/clang/test/CodeGenCXX/blocks.cpp index 2092139..914c069 100644 --- a/clang/test/CodeGenCXX/blocks.cpp +++ b/clang/test/CodeGenCXX/blocks.cpp @@ -228,3 +228,28 @@ namespace test8 { template int X::foo(); } + +// rdar://13459289 +namespace test9 { + struct B { + void *p; + B(); + B(const B&); + ~B(); + }; + + void use_block(void (^)()); + void use_block_2(void (^)(), const B &a); + + // Ensuring that creating a non-trivial capture copy expression + // doesn't end up stealing the block registration for the block we + // just parsed. That block must have captures or else it won't + // force registration. Must occur within a block for some reason. + void test() { + B x; + use_block(^{ + int y; + use_block_2(^{ (void)y; }, x); + }); + } +} diff --git a/clang/test/SemaCXX/blocks.cpp b/clang/test/SemaCXX/blocks.cpp index a635e99..a2672d1 100644 --- a/clang/test/SemaCXX/blocks.cpp +++ b/clang/test/SemaCXX/blocks.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -fblocks -// expected-no-diagnostics void tovoid(void*); @@ -82,3 +81,22 @@ void move_block() { __block MoveOnly mo; } +// Don't crash after failing to build a block due to a capture of an +// invalid declaration. +namespace test5 { + struct B { // expected-note 2 {{candidate constructor}} + void *p; + B(int); // expected-note {{candidate constructor}} + }; + + void use_block(void (^)()); + void use_block_2(void (^)(), const B &a); + + void test() { + B x; // expected-error {{no matching constructor for initialization}} + use_block(^{ + int y; + use_block_2(^{ (void) y; }, x); + }); + } +} -- 2.7.4