Only run the jump-checker if there's a branch-protected scope *and* there's

a switch or goto somewhere in the function.  Indirect gotos trigger the
jump-checker regardless, because the conditions there are slightly more
elaborate and it's too marginal a case to be worth optimizing.

Turns off the jump-checker in a lot of cases in C++.  rdar://problem/7702918

llvm-svn: 109962

Author: rjmccall

clang/lib/Sema/Sema.cpp

@@ -31,7 +31,10 @@ using namespace clang;
 FunctionScopeInfo::~FunctionScopeInfo() { }
 
 void FunctionScopeInfo::Clear(unsigned NumErrors) {
-  NeedsScopeChecking = false;
+  HasBranchProtectedScope = false;
+  HasBranchIntoScope = false;
+  HasIndirectGoto = false;
+  
   LabelMap.clear();
   SwitchStack.clear();
   Returns.clear();

clang/lib/Sema/Sema.h

@@ -116,11 +116,15 @@ struct FunctionScopeInfo {
   /// a block.
   bool IsBlockInfo;
 
-  /// \brief Set true when a function, method contains a VLA or ObjC try block,
-  /// which introduce scopes that need to be checked for goto conditions.  If a
-  /// function does not contain this, then it need not have the jump checker run
-  /// on it.
-  bool NeedsScopeChecking;
+  /// \brief Whether this function contains a VLA, @try, try, C++
+  /// initializer, or anything else that can't be jumped past.
+  bool HasBranchProtectedScope;
+
+  /// \brief Whether this function contains any switches or direct gotos.
+  bool HasBranchIntoScope;
+
+  /// \brief Whether this function contains any indirect gotos.
+  bool HasIndirectGoto;
 
   /// \brief The number of errors that had occurred before starting this
   /// function or block.
@@ -139,9 +143,17 @@ struct FunctionScopeInfo {
   /// block, if there is any chance of applying the named return value
   /// optimization.
   llvm::SmallVector<ReturnStmt *, 4> Returns;
+
+  bool NeedsScopeChecking() const {
+    return HasIndirectGoto ||
+          (HasBranchProtectedScope && HasBranchIntoScope);
+  }
 
   FunctionScopeInfo(unsigned NumErrors)
-    : IsBlockInfo(false), NeedsScopeChecking(false),
+    : IsBlockInfo(false),
+      HasBranchProtectedScope(false),
+      HasBranchIntoScope(false),
+      HasIndirectGoto(false),
       NumErrorsAtStartOfFunction(NumErrors) { }
 
   virtual ~FunctionScopeInfo();
@@ -706,13 +718,28 @@ class Sema : public Action {
 
   /// \brief Determine whether the current function or block needs scope
   /// checking.
-  bool &FunctionNeedsScopeChecking() {
-    if (FunctionScopes.empty())
-      return TopFunctionScope.NeedsScopeChecking;
+  bool FunctionNeedsScopeChecking() {
+    if (!FunctionScopes.empty())
+      return FunctionScopes.back()->NeedsScopeChecking();
+    return false;
+  }
 
-    return FunctionScopes.back()->NeedsScopeChecking;
+  void setFunctionHasBranchIntoScope() {
+    if (!FunctionScopes.empty())
+      FunctionScopes.back()->HasBranchIntoScope = true;
   }
 
+  void setFunctionHasBranchProtectedScope() {
+    if (!FunctionScopes.empty())
+      FunctionScopes.back()->HasBranchProtectedScope = true;
+  }
+
+  void setFunctionHasIndirectGoto() {
+    if (!FunctionScopes.empty())
+      FunctionScopes.back()->HasIndirectGoto = true;
+  }
+  
+
   bool hasAnyErrorsInThisFunction() const;
 
   /// \brief Retrieve the current block, if any.

clang/lib/Sema/SemaDecl.cpp

@@ -2380,7 +2380,7 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // then it shall have block scope.
   QualType T = NewTD->getUnderlyingType();
   if (T->isVariablyModifiedType()) {
-    FunctionNeedsScopeChecking() = true;
+    setFunctionHasBranchProtectedScope();
 
     if (S->getFnParent() == 0) {
       bool SizeIsNegative;
@@ -2794,12 +2794,8 @@ void Sema::CheckVariableDeclaration(VarDecl *NewVD,
   bool isVM = T->isVariablyModifiedType();
   if (isVM || NewVD->hasAttr<CleanupAttr>() ||
       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()))
-    FunctionNeedsScopeChecking() = true;
+    setFunctionHasBranchProtectedScope();
 
   if ((isVM && NewVD->hasLinkage()) ||
       (T->isVariableArrayType() && NewVD->hasGlobalStorage())) {

clang/lib/Sema/SemaStmt.cpp

@@ -441,6 +441,8 @@ Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ExprArg Cond,
     if (!CondExpr)
       return StmtError();
   }
+
+  setFunctionHasBranchIntoScope();
 
   SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, CondExpr);
   getSwitchStack().push_back(SS);
@@ -962,6 +964,8 @@ Sema::ActOnGotoStmt(SourceLocation GotoLoc, SourceLocation LabelLoc,
   // Look up the record for this label identifier.
   LabelStmt *&LabelDecl = getLabelMap()[LabelII];
 
+  setFunctionHasBranchIntoScope();
+
   // If we haven't seen this label yet, create a forward reference.
   if (LabelDecl == 0)
     LabelDecl = new (Context) LabelStmt(LabelLoc, LabelII, 0);
@@ -982,6 +986,9 @@ Sema::ActOnIndirectGotoStmt(SourceLocation GotoLoc, SourceLocation StarLoc,
     if (DiagnoseAssignmentResult(ConvTy, StarLoc, DestTy, ETy, E, AA_Passing))
       return StmtError();
   }
+
+  setFunctionHasIndirectGoto();
+
   return Owned(new (Context) IndirectGotoStmt(GotoLoc, StarLoc, E));
 }
 
@@ -1504,7 +1511,7 @@ Sema::ActOnObjCAtFinallyStmt(SourceLocation AtLoc, StmtArg Body) {
 Action::OwningStmtResult
 Sema::ActOnObjCAtTryStmt(SourceLocation AtLoc, StmtArg Try, 
                          MultiStmtArg CatchStmts, StmtArg Finally) {
-  FunctionNeedsScopeChecking() = true;
+  setFunctionHasBranchProtectedScope();
   unsigned NumCatchStmts = CatchStmts.size();
   return Owned(ObjCAtTryStmt::Create(Context, AtLoc, Try.takeAs<Stmt>(),
                                      (Stmt **)CatchStmts.release(),
@@ -1549,7 +1556,7 @@ Sema::ActOnObjCAtThrowStmt(SourceLocation AtLoc, ExprArg Throw,
 Action::OwningStmtResult
 Sema::ActOnObjCAtSynchronizedStmt(SourceLocation AtLoc, ExprArg SynchExpr,
                                   StmtArg SynchBody) {
-  FunctionNeedsScopeChecking() = true;
+  setFunctionHasBranchProtectedScope();
 
   // Make sure the expression type is an ObjC pointer or "void *".
   Expr *SyncExpr = static_cast<Expr*>(SynchExpr.get());
@@ -1658,13 +1665,14 @@ Sema::ActOnCXXTryBlock(SourceLocation TryLoc, StmtArg TryBlock,
     }
   }
 
+  setFunctionHasBranchProtectedScope();
+
   // FIXME: We should detect handlers that cannot catch anything because an
   // earlier handler catches a superclass. Need to find a method that is not
   // quadratic for this.
   // Neither of these are explicitly forbidden, but every compiler detects them
   // and warns.
 
-  FunctionNeedsScopeChecking() = true;
   RawHandlers.release();
   return Owned(CXXTryStmt::Create(Context, TryLoc,
                                   static_cast<Stmt*>(TryBlock.release()),

clang/test/Analysis/misc-ps.m

@@ -298,6 +298,7 @@ void rdar_6777209(char *p) {
 typedef void *Opcode;
 Opcode pr_4033_getOpcode();
 void pr_4033(void) {
+  void *lbl = &&next_opcode;
 next_opcode:
   {
     Opcode op = pr_4033_getOpcode();

clang/test/CodeGen/statements.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wreturn-type < %s -emit-llvm
+// RUN: %clang_cc1 -Wreturn-type %s -emit-llvm
 
 void test1(int x) {
 switch (x) {
@@ -31,5 +31,10 @@ static long y = &&baz;
 }
 
 // PR3869
-int test5(long long b) { goto *b; }
+int test5(long long b) {
+  static void *lbls[] = { &&lbl };
+  goto *b;
+ lbl:
+  return 0;
+}