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

@@ -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<

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<VarDecl>(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<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 @@ 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.
+    bool isCPlusPlus = this->S.getLangOptions().CPlusPlus;
-    //   goto L; int X = 4;   L: ;
 
     // 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
+      // The decl statement creates a scope if any of the decls in it are VLAs
-      // have the cleanup attribute.
+      // 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;
         }

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);

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<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

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<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 @@ Sema::DeclPtrTy Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, DeclPtrTy D) {
     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 @@ 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");

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())

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<BlockScopeInfo> 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<CompoundStmt*>(body.get()));
+  
   CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking;
+  NumErrorsAtStartOfFunction = BSI->SavedNumErrorsAtStartOfFunction;
 
   BSI->TheDecl->setBody(body.takeAs<CompoundStmt>());
 

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:
+  ;
+}