On Mar 1, 2010, at 12:59 PM, Chris Lattner wrote: > Author: lattner > Date: Mon Mar 1 14:59:53 2010 > New Revision: 97497 > > URL: http://llvm.org/viewvc/llvm-project?rev=97497&view=rev > Log: > Implement jump checking for initialized c++ variables, implementing > a fixme and PR6451. > > Only perform jump checking if the containing function has no errors, > and add the infrastructure needed to do this. > > On the testcase in the PR, we produce: > > t.cc:6:3: error: illegal goto into protected scope > goto later; > ^ > t.cc:7:5: note: jump bypasses variable initialization > X x; > ^
Very nice, thanks! > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/JumpDiagnostics.cpp > cfe/trunk/lib/Sema/Sema.cpp > cfe/trunk/lib/Sema/Sema.h > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/lib/Sema/SemaDeclObjC.cpp > cfe/trunk/lib/Sema/SemaExpr.cpp > cfe/trunk/test/SemaCXX/statements.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Mar 1 14:59:53 > 2010 > @@ -1592,6 +1592,8 @@ > def err_addr_of_label_in_protected_scope : Error< > "address taken of label in protected scope, jump to it would have " > "unknown effect on scope">; > +def note_protected_by_variable_init : Note< > + "jump bypasses variable initialization">; > def note_protected_by_vla_typedef : Note< > "jump bypasses initialization of VLA typedef">; > def note_protected_by_vla : Note< > > Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original) > +++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Mon Mar 1 14:59:53 2010 > @@ -77,7 +77,7 @@ > > /// GetDiagForGotoScopeDecl - If this decl induces a new goto scope, return a > /// diagnostic that should be emitted if control goes over it. If not, return > 0. > -static unsigned GetDiagForGotoScopeDecl(const Decl *D) { > +static unsigned GetDiagForGotoScopeDecl(const Decl *D, bool isCPlusPlus) { > if (const VarDecl *VD = dyn_cast<VarDecl>(D)) { > if (VD->getType()->isVariablyModifiedType()) > return diag::note_protected_by_vla; > @@ -85,6 +85,9 @@ > return diag::note_protected_by_cleanup; > if (VD->hasAttr<BlocksAttr>()) > return diag::note_protected_by___block; > + if (isCPlusPlus && VD->hasLocalStorage() && VD->hasInit()) > + return diag::note_protected_by_variable_init; > + > } else if (const TypedefDecl *TD = dyn_cast<TypedefDecl>(D)) { > if (TD->getUnderlyingType()->isVariablyModifiedType()) > return diag::note_protected_by_vla_typedef; > @@ -116,18 +119,17 @@ > Stmt *SubStmt = *CI; > if (SubStmt == 0) continue; > > - // FIXME: diagnose jumps past initialization: required in C++, warning > in C. > - // goto L; int X = 4; L: ; > + bool isCPlusPlus = this->S.getLangOptions().CPlusPlus; > > // 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. > + // The decl statement creates a scope if any of the decls in it are > VLAs > + // or have the cleanup attribute. > for (DeclStmt::decl_iterator I = DS->decl_begin(), E = DS->decl_end(); > I != E; ++I) { > // If this decl causes a new scope, push and switch to it. > - if (unsigned Diag = GetDiagForGotoScopeDecl(*I)) { > + if (unsigned Diag = GetDiagForGotoScopeDecl(*I, isCPlusPlus)) { > Scopes.push_back(GotoScope(ParentScope, Diag, (*I)->getLocation())); > ParentScope = Scopes.size()-1; > } > > Modified: cfe/trunk/lib/Sema/Sema.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/Sema.cpp (original) > +++ cfe/trunk/lib/Sema/Sema.cpp Mon Mar 1 14:59:53 2010 > @@ -127,6 +127,8 @@ > if (getLangOptions().CPlusPlus) > FieldCollector.reset(new CXXFieldCollector()); > > + NumErrorsAtStartOfFunction = 0; > + > // Tell diagnostics how to render things from the AST library. > PP.getDiagnostics().SetArgToStringFn(&FormatASTNodeDiagnosticArgument, > &Context); > > Modified: cfe/trunk/lib/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/Sema.h (original) > +++ cfe/trunk/lib/Sema/Sema.h Mon Mar 1 14:59:53 2010 > @@ -147,6 +147,10 @@ > /// return types, if any, in the block body. > QualType ReturnType; > > + /// SavedNumErrorsAtStartOfFunction - This is the value of the > + /// NumErrorsAtStartOfFunction variable at the point when the block > started. > + unsigned SavedNumErrorsAtStartOfFunction; > + > /// SavedFunctionNeedsScopeChecking - This is the value of > /// CurFunctionNeedsScopeChecking at the point when the block started. > bool SavedFunctionNeedsScopeChecking; > @@ -241,6 +245,13 @@ > /// the current full expression. > llvm::SmallVector<CXXTemporary*, 8> ExprTemporaries; > > + /// NumErrorsAtStartOfFunction - This is the number of errors that were > + /// emitted to the diagnostics object at the start of the current > + /// function/method definition. If no additional errors are emitted by the > + /// end of the function, we assume the AST is well formed enough to be > + /// worthwhile to emit control flow diagnostics. > + unsigned NumErrorsAtStartOfFunction; > + > /// CurFunctionNeedsScopeChecking - This is set to true when a function or > /// ObjC method body contains a VLA or an ObjC try block, which introduce > /// scopes that need to be checked for goto conditions. If a function does > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Mar 1 14:59:53 2010 > @@ -850,7 +850,7 @@ > // is normally mapped to an error, but can be controlled with > // -Wtypedef-redefinition. If either the original or the redefinition is > // in a system header, don't emit this for compatibility with GCC. > - if (PP.getDiagnostics().getSuppressSystemWarnings() && > + if (getDiagnostics().getSuppressSystemWarnings() && > (Context.getSourceManager().isInSystemHeader(Old->getLocation()) || > Context.getSourceManager().isInSystemHeader(New->getLocation()))) > return; > @@ -2500,7 +2500,12 @@ > > bool isVM = T->isVariablyModifiedType(); > if (isVM || NewVD->hasAttr<CleanupAttr>() || > - NewVD->hasAttr<BlocksAttr>()) > + NewVD->hasAttr<BlocksAttr>() || > + // FIXME: We need to diagnose jumps passed initialized variables in > C++. > + // However, this turns on the scope checker for everything with a > variable > + // which may impact compile time. See if we can find a better solution > + // to this, perhaps only checking functions that contain gotos in C++? > + (LangOpts.CPlusPlus && NewVD->hasLocalStorage())) > CurFunctionNeedsScopeChecking = true; > > if ((isVM && NewVD->hasLinkage()) || > @@ -4079,6 +4084,7 @@ > FD = cast<FunctionDecl>(D.getAs<Decl>()); > > CurFunctionNeedsScopeChecking = false; > + NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors(); > > // See if this is a redefinition. > // But don't complain if we're in GNU89 mode and the previous definition > @@ -4257,7 +4263,8 @@ > CheckUnreachable(AC); > > // Verify that that gotos and switch cases don't jump into scopes illegally. > - if (CurFunctionNeedsScopeChecking) > + if (CurFunctionNeedsScopeChecking && > + NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) > DiagnoseInvalidJumps(Body); > > // C++ constructors that have function-try-blocks can't have return > @@ -4272,7 +4279,7 @@ > // If any errors have occurred, clear out any temporaries that may have > // been leftover. This ensures that these temporaries won't be picked up for > // deletion in some later function. > - if (PP.getDiagnostics().hasErrorOccurred()) > + if (getDiagnostics().hasErrorOccurred()) > ExprTemporaries.clear(); > > assert(ExprTemporaries.empty() && "Leftover temporaries in function"); > > Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Mon Mar 1 14:59:53 2010 > @@ -50,6 +50,7 @@ > return; > > CurFunctionNeedsScopeChecking = false; > + NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors(); > > // Allow the rest of sema to find private method decl implementations. > if (MDecl->isInstanceMethod()) > > Modified: cfe/trunk/lib/Sema/SemaExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Mar 1 14:59:53 2010 > @@ -6734,8 +6734,10 @@ > BSI->hasBlockDeclRefExprs = false; > BSI->hasPrototype = false; > BSI->SavedFunctionNeedsScopeChecking = CurFunctionNeedsScopeChecking; > + BSI->SavedNumErrorsAtStartOfFunction = NumErrorsAtStartOfFunction; > CurFunctionNeedsScopeChecking = false; > - > + NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors(); > + > BSI->TheDecl = BlockDecl::Create(Context, CurContext, CaretLoc); > CurContext->addDecl(BSI->TheDecl); > PushDeclContext(BlockScope, BSI->TheDecl); > @@ -6849,6 +6851,7 @@ > llvm::OwningPtr<BlockScopeInfo> CC(CurBlock); > > CurFunctionNeedsScopeChecking = CurBlock->SavedFunctionNeedsScopeChecking; > + NumErrorsAtStartOfFunction = CurBlock->SavedNumErrorsAtStartOfFunction; > > // Pop off CurBlock, handle nested blocks. > PopDeclContext(); > @@ -6895,9 +6898,12 @@ > BlockTy = Context.getBlockPointerType(BlockTy); > > // If needed, diagnose invalid gotos and switches in the block. > - if (CurFunctionNeedsScopeChecking) > + if (CurFunctionNeedsScopeChecking && > + NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) > DiagnoseInvalidJumps(static_cast<CompoundStmt*>(body.get())); > + > CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking; > + NumErrorsAtStartOfFunction = BSI->SavedNumErrorsAtStartOfFunction; > > BSI->TheDecl->setBody(body.takeAs<CompoundStmt>()); > > > Modified: cfe/trunk/test/SemaCXX/statements.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/statements.cpp?rev=97497&r1=97496&r2=97497&view=diff > ============================================================================== > --- cfe/trunk/test/SemaCXX/statements.cpp (original) > +++ cfe/trunk/test/SemaCXX/statements.cpp Mon Mar 1 14:59:53 2010 > @@ -1,5 +1,17 @@ > -// RUN: %clang_cc1 %s -fsyntax-only -pedantic > +// RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify > > void foo() { > return foo(); > } > + > +// PR6451 - C++ Jump checking > +struct X { > + X(); > +}; > + > +void test2() { > + goto later; // expected-error {{illegal goto into protected scope}} > + X x; // expected-note {{jump bypasses variable initialization}} > +later: > + ; > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
