Author: rsmith Date: Tue Jun 21 15:10:11 2016 New Revision: 273309 URL: http://llvm.org/viewvc/llvm-project?rev=273309&view=rev Log: Refactor scope building in JumpDiagnostics for simplicity. This fixes a (currently theoretical) bug where recursive calls to BuildScopeInformation would do the wrong thing if the type of the statement is one of the kinds with special handling, but this is not currently observable because the relevant recursive calls happen to all be for CompoundStmts. (This becomes visible with the C++17 'constexpr if' feature, where we get a protected scope for the 'then' / 'else' cases of some 'if's, and don't necessarily have a corresponding compound statement.)
Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=273309&r1=273308&r2=273309&view=diff ============================================================================== --- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original) +++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Tue Jun 21 15:10:11 2016 @@ -270,7 +270,8 @@ void JumpScopeChecker::BuildScopeInforma /// coherent VLA scope with a specified parent node. Walk through the /// statements, adding any labels or gotos to LabelAndGotoScopes and recursively /// walking the AST as needed. -void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned &origParentScope) { +void JumpScopeChecker::BuildScopeInformation(Stmt *S, + unsigned &origParentScope) { // If this is a statement, rather than an expression, scopes within it don't // propagate out into the enclosing scope. Otherwise we have to worry // about block literals, which have the lifetime of their enclosing statement. @@ -320,57 +321,186 @@ void JumpScopeChecker::BuildScopeInforma case Stmt::CXXTryStmtClass: { CXXTryStmt *TS = cast<CXXTryStmt>(S); - unsigned newParentScope; - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_cxx_try, - diag::note_exits_cxx_try, - TS->getSourceRange().getBegin())); - if (Stmt *TryBlock = TS->getTryBlock()) - BuildScopeInformation(TryBlock, (newParentScope = Scopes.size()-1)); + { + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_cxx_try, + diag::note_exits_cxx_try, + TS->getSourceRange().getBegin())); + if (Stmt *TryBlock = TS->getTryBlock()) + BuildScopeInformation(TryBlock, NewParentScope); + } // Jump from the catch into the try is not allowed either. for (unsigned I = 0, E = TS->getNumHandlers(); I != E; ++I) { CXXCatchStmt *CS = TS->getHandler(I); + unsigned NewParentScope = Scopes.size(); Scopes.push_back(GotoScope(ParentScope, diag::note_protected_by_cxx_catch, diag::note_exits_cxx_catch, CS->getSourceRange().getBegin())); - BuildScopeInformation(CS->getHandlerBlock(), - (newParentScope = Scopes.size()-1)); + BuildScopeInformation(CS->getHandlerBlock(), NewParentScope); } return; } case Stmt::SEHTryStmtClass: { SEHTryStmt *TS = cast<SEHTryStmt>(S); - unsigned newParentScope; - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_seh_try, - diag::note_exits_seh_try, - TS->getSourceRange().getBegin())); - if (Stmt *TryBlock = TS->getTryBlock()) - BuildScopeInformation(TryBlock, (newParentScope = Scopes.size()-1)); + { + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_seh_try, + diag::note_exits_seh_try, + TS->getSourceRange().getBegin())); + if (Stmt *TryBlock = TS->getTryBlock()) + BuildScopeInformation(TryBlock, NewParentScope); + } // Jump from __except or __finally into the __try are not allowed either. if (SEHExceptStmt *Except = TS->getExceptHandler()) { + unsigned NewParentScope = Scopes.size(); Scopes.push_back(GotoScope(ParentScope, diag::note_protected_by_seh_except, diag::note_exits_seh_except, Except->getSourceRange().getBegin())); - BuildScopeInformation(Except->getBlock(), - (newParentScope = Scopes.size()-1)); + BuildScopeInformation(Except->getBlock(), NewParentScope); } else if (SEHFinallyStmt *Finally = TS->getFinallyHandler()) { + unsigned NewParentScope = Scopes.size(); Scopes.push_back(GotoScope(ParentScope, diag::note_protected_by_seh_finally, diag::note_exits_seh_finally, Finally->getSourceRange().getBegin())); - BuildScopeInformation(Finally->getBlock(), - (newParentScope = Scopes.size()-1)); + BuildScopeInformation(Finally->getBlock(), NewParentScope); } return; } + case Stmt::DeclStmtClass: { + // If this is a declstmt with a VLA definition, it defines a scope from here + // to the end of the containing context. + DeclStmt *DS = cast<DeclStmt>(S); + // The decl statement creates a scope if any of the decls in it are VLAs + // or have the cleanup attribute. + for (auto *I : DS->decls()) + BuildScopeInformation(I, origParentScope); + return; + } + + case Stmt::ObjCAtTryStmtClass: { + // Disallow jumps into any part of an @try statement by pushing a scope and + // walking all sub-stmts in that scope. + ObjCAtTryStmt *AT = cast<ObjCAtTryStmt>(S); + // Recursively walk the AST for the @try part. + { + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_objc_try, + diag::note_exits_objc_try, + AT->getAtTryLoc())); + if (Stmt *TryPart = AT->getTryBody()) + BuildScopeInformation(TryPart, NewParentScope); + } + + // Jump from the catch to the finally or try is not valid. + for (unsigned I = 0, N = AT->getNumCatchStmts(); I != N; ++I) { + ObjCAtCatchStmt *AC = AT->getCatchStmt(I); + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_objc_catch, + diag::note_exits_objc_catch, + AC->getAtCatchLoc())); + // @catches are nested and it isn't + BuildScopeInformation(AC->getCatchBody(), NewParentScope); + } + + // Jump from the finally to the try or catch is not valid. + if (ObjCAtFinallyStmt *AF = AT->getFinallyStmt()) { + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_objc_finally, + diag::note_exits_objc_finally, + AF->getAtFinallyLoc())); + BuildScopeInformation(AF, NewParentScope); + } + + return; + } + + case Stmt::ObjCAtSynchronizedStmtClass: { + // Disallow jumps into the protected statement of an @synchronized, but + // allow jumps into the object expression it protects. + ObjCAtSynchronizedStmt *AS = cast<ObjCAtSynchronizedStmt>(S); + // Recursively walk the AST for the @synchronized object expr, it is + // evaluated in the normal scope. + BuildScopeInformation(AS->getSynchExpr(), ParentScope); + + // Recursively walk the AST for the @synchronized part, protected by a new + // scope. + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_objc_synchronized, + diag::note_exits_objc_synchronized, + AS->getAtSynchronizedLoc())); + BuildScopeInformation(AS->getSynchBody(), NewParentScope); + return; + } + + case Stmt::ObjCAutoreleasePoolStmtClass: { + // Disallow jumps into the protected statement of an @autoreleasepool. + ObjCAutoreleasePoolStmt *AS = cast<ObjCAutoreleasePoolStmt>(S); + // Recursively walk the AST for the @autoreleasepool part, protected by a + // new scope. + unsigned NewParentScope = Scopes.size(); + Scopes.push_back(GotoScope(ParentScope, + diag::note_protected_by_objc_autoreleasepool, + diag::note_exits_objc_autoreleasepool, + AS->getAtLoc())); + BuildScopeInformation(AS->getSubStmt(), NewParentScope); + return; + } + + case Stmt::ExprWithCleanupsClass: { + // Disallow jumps past full-expressions that use blocks with + // non-trivial cleanups of their captures. This is theoretically + // implementable but a lot of work which we haven't felt up to doing. + ExprWithCleanups *EWC = cast<ExprWithCleanups>(S); + for (unsigned i = 0, e = EWC->getNumObjects(); i != e; ++i) { + const BlockDecl *BDecl = EWC->getObject(i); + for (const auto &CI : BDecl->captures()) { + VarDecl *variable = CI.getVariable(); + BuildScopeInformation(variable, BDecl, origParentScope); + } + } + break; + } + + case Stmt::MaterializeTemporaryExprClass: { + // Disallow jumps out of scopes containing temporaries lifetime-extended to + // automatic storage duration. + MaterializeTemporaryExpr *MTE = cast<MaterializeTemporaryExpr>(S); + if (MTE->getStorageDuration() == SD_Automatic) { + SmallVector<const Expr *, 4> CommaLHS; + SmallVector<SubobjectAdjustment, 4> Adjustments; + const Expr *ExtendedObject = + MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments( + CommaLHS, Adjustments); + if (ExtendedObject->getType().isDestructedType()) { + Scopes.push_back(GotoScope(ParentScope, 0, + diag::note_exits_temporary_dtor, + ExtendedObject->getExprLoc())); + origParentScope = Scopes.size()-1; + } + } + break; + } + + case Stmt::CaseStmtClass: + case Stmt::DefaultStmtClass: + case Stmt::LabelStmtClass: + LabelAndGotoScopes[S] = ParentScope; + break; + default: break; } @@ -401,117 +531,6 @@ void JumpScopeChecker::BuildScopeInforma SubStmt = Next; } - // If this is a declstmt with a VLA definition, it defines a scope from here - // to the end of the containing context. - if (DeclStmt *DS = dyn_cast<DeclStmt>(SubStmt)) { - // The decl statement creates a scope if any of the decls in it are VLAs - // or have the cleanup attribute. - for (auto *I : DS->decls()) - BuildScopeInformation(I, ParentScope); - continue; - } - // Disallow jumps into any part of an @try statement by pushing a scope and - // walking all sub-stmts in that scope. - if (ObjCAtTryStmt *AT = dyn_cast<ObjCAtTryStmt>(SubStmt)) { - unsigned newParentScope; - // Recursively walk the AST for the @try part. - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_objc_try, - diag::note_exits_objc_try, - AT->getAtTryLoc())); - if (Stmt *TryPart = AT->getTryBody()) - BuildScopeInformation(TryPart, (newParentScope = Scopes.size()-1)); - - // Jump from the catch to the finally or try is not valid. - for (unsigned I = 0, N = AT->getNumCatchStmts(); I != N; ++I) { - ObjCAtCatchStmt *AC = AT->getCatchStmt(I); - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_objc_catch, - diag::note_exits_objc_catch, - AC->getAtCatchLoc())); - // @catches are nested and it isn't - BuildScopeInformation(AC->getCatchBody(), - (newParentScope = Scopes.size()-1)); - } - - // Jump from the finally to the try or catch is not valid. - if (ObjCAtFinallyStmt *AF = AT->getFinallyStmt()) { - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_objc_finally, - diag::note_exits_objc_finally, - AF->getAtFinallyLoc())); - BuildScopeInformation(AF, (newParentScope = Scopes.size()-1)); - } - - continue; - } - - unsigned newParentScope; - // Disallow jumps into the protected statement of an @synchronized, but - // allow jumps into the object expression it protects. - if (ObjCAtSynchronizedStmt *AS = - dyn_cast<ObjCAtSynchronizedStmt>(SubStmt)) { - // Recursively walk the AST for the @synchronized object expr, it is - // evaluated in the normal scope. - BuildScopeInformation(AS->getSynchExpr(), ParentScope); - - // Recursively walk the AST for the @synchronized part, protected by a new - // scope. - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_objc_synchronized, - diag::note_exits_objc_synchronized, - AS->getAtSynchronizedLoc())); - BuildScopeInformation(AS->getSynchBody(), - (newParentScope = Scopes.size()-1)); - continue; - } - - // Disallow jumps into the protected statement of an @autoreleasepool. - if (ObjCAutoreleasePoolStmt *AS = - dyn_cast<ObjCAutoreleasePoolStmt>(SubStmt)) { - // Recursively walk the AST for the @autoreleasepool part, protected by a - // new scope. - Scopes.push_back(GotoScope(ParentScope, - diag::note_protected_by_objc_autoreleasepool, - diag::note_exits_objc_autoreleasepool, - AS->getAtLoc())); - BuildScopeInformation(AS->getSubStmt(), - (newParentScope = Scopes.size() - 1)); - continue; - } - - // Disallow jumps past full-expressions that use blocks with - // non-trivial cleanups of their captures. This is theoretically - // implementable but a lot of work which we haven't felt up to doing. - if (ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(SubStmt)) { - for (unsigned i = 0, e = EWC->getNumObjects(); i != e; ++i) { - const BlockDecl *BDecl = EWC->getObject(i); - for (const auto &CI : BDecl->captures()) { - VarDecl *variable = CI.getVariable(); - BuildScopeInformation(variable, BDecl, ParentScope); - } - } - } - - // Disallow jumps out of scopes containing temporaries lifetime-extended to - // automatic storage duration. - if (MaterializeTemporaryExpr *MTE = - dyn_cast<MaterializeTemporaryExpr>(SubStmt)) { - if (MTE->getStorageDuration() == SD_Automatic) { - SmallVector<const Expr *, 4> CommaLHS; - SmallVector<SubobjectAdjustment, 4> Adjustments; - const Expr *ExtendedObject = - MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments( - CommaLHS, Adjustments); - if (ExtendedObject->getType().isDestructedType()) { - Scopes.push_back(GotoScope(ParentScope, 0, - diag::note_exits_temporary_dtor, - ExtendedObject->getExprLoc())); - ParentScope = Scopes.size()-1; - } - } - } - // Recursively walk the AST. BuildScopeInformation(SubStmt, ParentScope); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits