From 6d97e5e4b7abdae710c2548b51f4ed0298e86d80 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Mon, 1 Mar 2010 20:59:53 +0000 Subject: [PATCH] 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; ^ git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97497 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 2 ++ lib/Sema/JumpDiagnostics.cpp | 14 ++++++++------ lib/Sema/Sema.cpp | 2 ++ lib/Sema/Sema.h | 11 +++++++++++ lib/Sema/SemaDecl.cpp | 15 +++++++++++---- lib/Sema/SemaDeclObjC.cpp | 1 + lib/Sema/SemaExpr.cpp | 10 ++++++++-- test/SemaCXX/statements.cpp | 14 +++++++++++++- 8 files changed, 56 insertions(+), 13 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index abc89b41f4..9f77655052 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1592,6 +1592,8 @@ def err_indirect_goto_in_protected_scope : Error< 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< diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp index 2b37e9df2c..7cf207f77a 100644 --- a/lib/Sema/JumpDiagnostics.cpp +++ b/lib/Sema/JumpDiagnostics.cpp @@ -77,7 +77,7 @@ JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s) : S(s) { /// 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(D)) { if (VD->getType()->isVariablyModifiedType()) return diag::note_protected_by_vla; @@ -85,6 +85,9 @@ static unsigned GetDiagForGotoScopeDecl(const Decl *D) { return diag::note_protected_by_cleanup; if (VD->hasAttr()) 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(D)) { if (TD->getUnderlyingType()->isVariablyModifiedType()) return diag::note_protected_by_vla_typedef; @@ -116,18 +119,17 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) { 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(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; } diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 38c842eede..fb7d1991fb 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -127,6 +127,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, if (getLangOptions().CPlusPlus) FieldCollector.reset(new CXXFieldCollector()); + NumErrorsAtStartOfFunction = 0; + // Tell diagnostics how to render things from the AST library. PP.getDiagnostics().SetArgToStringFn(&FormatASTNodeDiagnosticArgument, &Context); diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 3072dfeaf2..2d6fd2629b 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -147,6 +147,10 @@ struct BlockScopeInfo : FunctionScopeInfo { /// 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 @@ public: /// the current full expression. llvm::SmallVector 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 diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 47c787b0c4..be4a591f29 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -850,7 +850,7 @@ void Sema::MergeTypeDefDecl(TypedefDecl *New, LookupResult &OldDecls) { // 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 @@ void Sema::CheckVariableDeclaration(VarDecl *NewVD, bool isVM = T->isVariablyModifiedType(); if (isVM || NewVD->hasAttr() || - NewVD->hasAttr()) + NewVD->hasAttr() || + // 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 @@ Sema::DeclPtrTy Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, DeclPtrTy D) { FD = cast(D.getAs()); 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 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, 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 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, // 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"); diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index 939ccab35d..0ec793360b 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -50,6 +50,7 @@ void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, DeclPtrTy D) { return; CurFunctionNeedsScopeChecking = false; + NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors(); // Allow the rest of sema to find private method decl implementations. if (MDecl->isInstanceMethod()) diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 0d1614abe9..966cbc72a2 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6734,8 +6734,10 @@ void Sema::ActOnBlockStart(SourceLocation CaretLoc, Scope *BlockScope) { 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 @@ void Sema::ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope) { llvm::OwningPtr CC(CurBlock); CurFunctionNeedsScopeChecking = CurBlock->SavedFunctionNeedsScopeChecking; + NumErrorsAtStartOfFunction = CurBlock->SavedNumErrorsAtStartOfFunction; // Pop off CurBlock, handle nested blocks. PopDeclContext(); @@ -6895,9 +6898,12 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc, BlockTy = Context.getBlockPointerType(BlockTy); // If needed, diagnose invalid gotos and switches in the block. - if (CurFunctionNeedsScopeChecking) + if (CurFunctionNeedsScopeChecking && + NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) DiagnoseInvalidJumps(static_cast(body.get())); + CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking; + NumErrorsAtStartOfFunction = BSI->SavedNumErrorsAtStartOfFunction; BSI->TheDecl->setBody(body.takeAs()); diff --git a/test/SemaCXX/statements.cpp b/test/SemaCXX/statements.cpp index 36982582fa..852086ed9a 100644 --- a/test/SemaCXX/statements.cpp +++ b/test/SemaCXX/statements.cpp @@ -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: + ; +} -- 2.40.0