[cfe-commits] r103536 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/JumpDiagnostics.cpp test/Sema/scope-check.c test/SemaCXX/scope-check.cpp

John McCall rjmccall at apple.com
Tue May 11 17:58:13 PDT 2010


Author: rjmccall
Date: Tue May 11 19:58:13 2010
New Revision: 103536

URL: http://llvm.org/viewvc/llvm-project?rev=103536&view=rev
Log:
When checking scopes for indirect goto, be more permissive (but still safe)
about the permitted scopes.  Specifically:
  1) Permit labels and gotos to appear after a prologue of variable initializations.
  2) Permit indirect gotos to jump out of scopes that don't require cleanup.
  3) Diagnose possible attempts to indirect-jump out of scopes that do require
     cleanup.
This requires a substantial reinvention of the algorithm for checking indirect
goto.  The current algorithm is Omega(M*N), with M = the number of unique
scopes being jumped from and N = the number of unique scopes being jumped to,
with an additional factor that is probably (worst-case) linear in the depth
of scopes.  Thus the entire thing is likely cubic given some truly bizarre
ill-formed code;  on well-formed code the additional factor collapses to
an amortized constant (when amortized over the entire function) and so
the algorithm is quadratic.  Even this requires every label to appear in
its own scope, which would be very unusual for indirect-goto code (and
extremely unlikely for well-formed code);  it is far more likely that
all labels will be in the same scope and so the algorithm becomes linear.
For such a marginal feature, I am fairly happy with this result.

(this is using JumpDiagnostic's definition of scope, where successive
variables in a block appear in their own scope)


Added:
    cfe/trunk/test/SemaCXX/scope-check.cpp
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/JumpDiagnostics.cpp
    cfe/trunk/test/Sema/scope-check.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=103536&r1=103535&r2=103536&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 11 19:58:13 2010
@@ -1754,20 +1754,20 @@
 def err_goto_into_protected_scope : Error<"illegal goto into protected scope">;
 def err_switch_into_protected_scope : Error<
   "illegal switch case into protected scope">;
-def err_indirect_goto_in_protected_scope : Warning<
-  "illegal indirect goto in protected scope, unknown effect on scopes">,
-    InGroup<DiagGroup<"label-address-scope">>;
-def err_addr_of_label_in_protected_scope : Warning<
-  "address taken of label in protected scope, jump to it would have "
-  "unknown effect on scope">, InGroup<DiagGroup<"label-address-scope">>;
+def err_indirect_goto_without_addrlabel : Error<
+  "indirect goto in function with no address-of-label expressions">;
+def warn_indirect_goto_in_protected_scope : Warning<
+  "indirect goto might cross protected scopes">,
+  InGroup<DiagGroup<"label-address-scope">>;
+def note_indirect_goto_target : Note<"possible target of indirect goto">;
 def note_protected_by_variable_init : Note<
   "jump bypasses variable initialization">;
+def note_protected_by_cleanup : Note<
+  "jump bypasses initialization of variable with __attribute__((cleanup))">;
 def note_protected_by_vla_typedef : Note<
   "jump bypasses initialization of VLA typedef">;
 def note_protected_by_vla : Note<
   "jump bypasses initialization of variable length array">;
-def note_protected_by_cleanup : Note<
-  "jump bypasses initialization of declaration with __attribute__((cleanup))">;
 def note_protected_by_objc_try : Note<
   "jump bypasses initialization of @try block">;
 def note_protected_by_objc_catch : Note<
@@ -1783,6 +1783,25 @@
 def note_protected_by___block : Note<
   "jump bypasses setup of __block variable">;
 
+def note_exits_cleanup : Note<
+  "jump exits scope of variable with __attribute__((cleanup))">;
+def note_exits_dtor : Note<
+  "jump exits scope of variable with non-trivial destructor">;
+def note_exits___block : Note<
+  "jump exits scope of __block variable">;
+def note_exits_objc_try : Note<
+  "jump exits @try block">;
+def note_exits_objc_catch : Note<
+  "jump exits @catch block">;
+def note_exits_objc_finally : Note<
+  "jump exits @finally block">;
+def note_exits_objc_synchronized : Note<
+  "jump exits @synchronized block">;
+def note_exits_cxx_try : Note<
+  "jump exits try block">;
+def note_exits_cxx_catch : Note<
+  "jump exits catch block">;
+
 def err_func_returning_array_function : Error<
   "function cannot return %select{array|function}0 type %1">;
 def err_field_declared_as_function : Error<"field %0 declared as a function">;

Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=103536&r1=103535&r2=103536&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original)
+++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Tue May 11 19:58:13 2010
@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/BitVector.h"
 #include "Sema.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/StmtObjC.h"
@@ -39,24 +40,36 @@
     /// the parent scope is the function body.
     unsigned ParentScope;
 
-    /// Diag - The diagnostic to emit if there is a jump into this scope.
-    unsigned Diag;
+    /// InDiag - The diagnostic to emit if there is a jump into this scope.
+    unsigned InDiag;
+
+    /// OutDiag - The diagnostic to emit if there is an indirect jump out
+    /// of this scope.  Direct jumps always clean up their current scope
+    /// in an orderly way.
+    unsigned OutDiag;
 
     /// Loc - Location to emit the diagnostic.
     SourceLocation Loc;
 
-    GotoScope(unsigned parentScope, unsigned diag, SourceLocation L)
-    : ParentScope(parentScope), Diag(diag), Loc(L) {}
+    GotoScope(unsigned parentScope, unsigned InDiag, unsigned OutDiag,
+              SourceLocation L)
+      : ParentScope(parentScope), InDiag(InDiag), OutDiag(OutDiag), Loc(L) {}
   };
 
   llvm::SmallVector<GotoScope, 48> Scopes;
   llvm::DenseMap<Stmt*, unsigned> LabelAndGotoScopes;
   llvm::SmallVector<Stmt*, 16> Jumps;
+
+  llvm::SmallVector<IndirectGotoStmt*, 4> IndirectJumps;
+  llvm::SmallVector<LabelStmt*, 4> IndirectJumpTargets;
 public:
   JumpScopeChecker(Stmt *Body, Sema &S);
 private:
   void BuildScopeInformation(Stmt *S, unsigned ParentScope);
   void VerifyJumps();
+  void VerifyIndirectJumps();
+  void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope,
+                            LabelStmt *Target, unsigned TargetScope);
   void CheckJump(Stmt *From, Stmt *To,
                  SourceLocation DiagLoc, unsigned JumpDiag);
 };
@@ -65,7 +78,7 @@
 
 JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s) : S(s) {
   // Add a scope entry for function scope.
-  Scopes.push_back(GotoScope(~0U, ~0U, SourceLocation()));
+  Scopes.push_back(GotoScope(~0U, ~0U, ~0U, SourceLocation()));
 
   // Build information for the top level compound statement, so that we have a
   // defined scope record for every "goto" and label.
@@ -73,29 +86,47 @@
 
   // Check that all jumps we saw are kosher.
   VerifyJumps();
+  VerifyIndirectJumps();
 }
 
 /// 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, bool isCPlusPlus) {
+static std::pair<unsigned,unsigned>
+    GetDiagForGotoScopeDecl(const Decl *D, bool isCPlusPlus) {
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+    unsigned InDiag = 0, OutDiag = 0;
     if (VD->getType()->isVariablyModifiedType())
-      return diag::note_protected_by_vla;
-    if (VD->hasAttr<CleanupAttr>())
-      return diag::note_protected_by_cleanup;
-    if (VD->hasAttr<BlocksAttr>())
-      return diag::note_protected_by___block;
-    // FIXME: In C++0x, we have to check more conditions than "did we
-    // just give it an initializer?". See 6.7p3.
-    if (isCPlusPlus && VD->hasLocalStorage() && VD->hasInit())
-      return diag::note_protected_by_variable_init;
+      InDiag = diag::note_protected_by_vla;
+
+    if (VD->hasAttr<BlocksAttr>()) {
+      InDiag = diag::note_protected_by___block;
+      OutDiag = diag::note_exits___block;
+    } else if (VD->hasAttr<CleanupAttr>()) {
+      InDiag = diag::note_protected_by_cleanup;
+      OutDiag = diag::note_exits_cleanup;
+    } else if (isCPlusPlus) {
+      // FIXME: In C++0x, we have to check more conditions than "did we
+      // just give it an initializer?". See 6.7p3.
+      if (VD->hasLocalStorage() && VD->hasInit())
+        InDiag = diag::note_protected_by_variable_init;
+
+      CanQualType T = VD->getType()->getCanonicalTypeUnqualified();
+      while (CanQual<ArrayType> AT = T->getAs<ArrayType>())
+        T = AT->getElementType();
+      if (CanQual<RecordType> RT = T->getAs<RecordType>())
+        if (!cast<CXXRecordDecl>(RT->getDecl())->hasTrivialDestructor())
+          OutDiag = diag::note_exits_dtor;
+    }
     
-  } else if (const TypedefDecl *TD = dyn_cast<TypedefDecl>(D)) {
+    return std::make_pair(InDiag, OutDiag);    
+  }
+
+  if (const TypedefDecl *TD = dyn_cast<TypedefDecl>(D)) {
     if (TD->getUnderlyingType()->isVariablyModifiedType())
-      return diag::note_protected_by_vla_typedef;
+      return std::make_pair((unsigned) diag::note_protected_by_vla_typedef, 0);
   }
 
-  return 0;
+  return std::make_pair(0U, 0U);
 }
 
 
@@ -106,14 +137,32 @@
 void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
 
   // If we found a label, remember that it is in ParentScope scope.
-  if (isa<LabelStmt>(S) || isa<DefaultStmt>(S) || isa<CaseStmt>(S)) {
+  switch (S->getStmtClass()) {
+  case Stmt::LabelStmtClass:
+  case Stmt::DefaultStmtClass:
+  case Stmt::CaseStmtClass:
     LabelAndGotoScopes[S] = ParentScope;
-  } else if (isa<GotoStmt>(S) || isa<SwitchStmt>(S) ||
-             isa<IndirectGotoStmt>(S) || isa<AddrLabelExpr>(S)) {
+    break;
+
+  case Stmt::AddrLabelExprClass:
+    IndirectJumpTargets.push_back(cast<AddrLabelExpr>(S)->getLabel());
+    break;
+
+  case Stmt::IndirectGotoStmtClass:
+    LabelAndGotoScopes[S] = ParentScope;
+    IndirectJumps.push_back(cast<IndirectGotoStmt>(S));
+    break;
+
+  case Stmt::GotoStmtClass:
+  case Stmt::SwitchStmtClass:
     // Remember both what scope a goto is in as well as the fact that we have
     // it.  This makes the second scan not have to walk the AST again.
     LabelAndGotoScopes[S] = ParentScope;
     Jumps.push_back(S);
+    break;
+
+  default:
+    break;
   }
 
   for (Stmt::child_iterator CI = S->child_begin(), E = S->child_end(); CI != E;
@@ -131,8 +180,11 @@
       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, isCPlusPlus)) {
-          Scopes.push_back(GotoScope(ParentScope, Diag, (*I)->getLocation()));
+        std::pair<unsigned,unsigned> Diags
+          = GetDiagForGotoScopeDecl(*I, isCPlusPlus);
+        if (Diags.first || Diags.second) {
+          Scopes.push_back(GotoScope(ParentScope, Diags.first, Diags.second,
+                                     (*I)->getLocation()));
           ParentScope = Scopes.size()-1;
         }
 
@@ -149,7 +201,9 @@
     // walking all sub-stmts in that scope.
     if (ObjCAtTryStmt *AT = dyn_cast<ObjCAtTryStmt>(SubStmt)) {
       // Recursively walk the AST for the @try part.
-      Scopes.push_back(GotoScope(ParentScope,diag::note_protected_by_objc_try,
+      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, Scopes.size()-1);
@@ -159,6 +213,7 @@
         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(), Scopes.size()-1);
@@ -168,6 +223,7 @@
       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, Scopes.size()-1);
       }
@@ -186,6 +242,7 @@
       // scope.
       Scopes.push_back(GotoScope(ParentScope,
                                  diag::note_protected_by_objc_synchronized,
+                                 diag::note_exits_objc_synchronized,
                                  AS->getAtSynchronizedLoc()));
       BuildScopeInformation(AS->getSynchBody(), Scopes.size()-1);
       continue;
@@ -194,7 +251,9 @@
     // Disallow jumps into any part of a C++ try statement. This is pretty
     // much the same as for Obj-C.
     if (CXXTryStmt *TS = dyn_cast<CXXTryStmt>(SubStmt)) {
-      Scopes.push_back(GotoScope(ParentScope, diag::note_protected_by_cxx_try,
+      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, Scopes.size()-1);
@@ -204,6 +263,7 @@
         CXXCatchStmt *CS = TS->getHandler(I);
         Scopes.push_back(GotoScope(ParentScope,
                                    diag::note_protected_by_cxx_catch,
+                                   diag::note_exits_cxx_catch,
                                    CS->getSourceRange().getBegin()));
         BuildScopeInformation(CS->getHandlerBlock(), Scopes.size()-1);
       }
@@ -229,54 +289,163 @@
       continue;
     }
 
-    if (SwitchStmt *SS = dyn_cast<SwitchStmt>(Jump)) {
-      for (SwitchCase *SC = SS->getSwitchCaseList(); SC;
-           SC = SC->getNextSwitchCase()) {
-        assert(LabelAndGotoScopes.count(SC) && "Case not visited?");
-        CheckJump(SS, SC, SC->getLocStart(),
-                  diag::err_switch_into_protected_scope);
-      }
-      continue;
+    SwitchStmt *SS = cast<SwitchStmt>(Jump);
+    for (SwitchCase *SC = SS->getSwitchCaseList(); SC;
+         SC = SC->getNextSwitchCase()) {
+      assert(LabelAndGotoScopes.count(SC) && "Case not visited?");
+      CheckJump(SS, SC, SC->getLocStart(),
+                diag::err_switch_into_protected_scope);
     }
+  }
+}
 
-    unsigned DiagnosticScope;
+/// VerifyIndirectJumps - Verify whether any possible indirect jump might
+/// cross a protection boundary.
+///
+/// An indirect jump is "trivial" if it bypasses no initializations
+/// and no teardowns.  More formally, the jump from A to B is trivial
+/// if the path out from A to DCA(A,B) is trivial and the path in from
+/// DCA(A,B) to B is trivial, where DCA(A,B) is the deepest common
+/// ancestor of A and B.
+/// A path in is trivial if none of the entered scopes have an InDiag.
+/// A path out is trivial is none of the exited scopes have an OutDiag.
+/// Jump-triviality is transitive but asymmetric.
+void JumpScopeChecker::VerifyIndirectJumps() {
+  if (IndirectJumps.empty()) return;
+
+  // If there aren't any address-of-label expressions in this function,
+  // complain about the first indirect goto.
+  if (IndirectJumpTargets.empty()) {
+    S.Diag(IndirectJumps[0]->getGotoLoc(),
+           diag::err_indirect_goto_without_addrlabel);
+    return;
+  }
 
-    // We don't know where an indirect goto goes, require that it be at the
-    // top level of scoping.
-    if (IndirectGotoStmt *IG = dyn_cast<IndirectGotoStmt>(Jump)) {
-      assert(LabelAndGotoScopes.count(Jump) &&
-             "Jump didn't get added to scopes?");
-      unsigned GotoScope = LabelAndGotoScopes[IG];
-      if (GotoScope == 0) continue;  // indirect jump is ok.
-      S.Diag(IG->getGotoLoc(), diag::err_indirect_goto_in_protected_scope);
-      DiagnosticScope = GotoScope;
-    } else {
-      // We model &&Label as a jump for purposes of scope tracking.  We actually
-      // don't care *where* the address of label is, but we require the *label
-      // itself* to be in scope 0.  If it is nested inside of a VLA scope, then
-      // it is possible for an indirect goto to illegally enter the VLA scope by
-      // indirectly jumping to the label.
-      assert(isa<AddrLabelExpr>(Jump) && "Unknown jump type");
-      LabelStmt *TheLabel = cast<AddrLabelExpr>(Jump)->getLabel();
-
-      assert(LabelAndGotoScopes.count(TheLabel) &&
-             "Referenced label didn't get added to scopes?");
-      unsigned LabelScope = LabelAndGotoScopes[TheLabel];
-      if (LabelScope == 0) continue; // Addr of label is ok.
+  // Build a vector of source scopes.  This serves to unique source
+  // scopes as well as to eliminate redundant lookups into
+  // LabelAndGotoScopes.
+  typedef std::pair<unsigned, IndirectGotoStmt*> JumpScope;
+  llvm::SmallVector<JumpScope, 32> JumpScopes;
+  {
+    llvm::DenseMap<unsigned, IndirectGotoStmt*> JumpScopesMap;
+    for (llvm::SmallVectorImpl<IndirectGotoStmt*>::iterator
+           I = IndirectJumps.begin(), E = IndirectJumps.end(); I != E; ++I) {
+      IndirectGotoStmt *IG = *I;
+      assert(LabelAndGotoScopes.count(IG) &&
+             "indirect jump didn't get added to scopes?");
+      unsigned IGScope = LabelAndGotoScopes[IG];
+      IndirectGotoStmt *&Entry = JumpScopesMap[IGScope];
+      if (!Entry) Entry = IG;
+    }
+    JumpScopes.reserve(JumpScopesMap.size());
+    for (llvm::DenseMap<unsigned, IndirectGotoStmt*>::iterator
+           I = JumpScopesMap.begin(), E = JumpScopesMap.end(); I != E; ++I)
+      JumpScopes.push_back(*I);
+  }
 
-      S.Diag(Jump->getLocStart(), diag::err_addr_of_label_in_protected_scope);
-      DiagnosticScope = LabelScope;
+  // Find a representative label from each protection scope.
+  llvm::DenseMap<unsigned, LabelStmt*> TargetScopes;
+  for (llvm::SmallVectorImpl<LabelStmt*>::iterator
+         I = IndirectJumpTargets.begin(), E = IndirectJumpTargets.end();
+       I != E; ++I) {
+    LabelStmt *TheLabel = *I;
+    assert(LabelAndGotoScopes.count(TheLabel) &&
+           "Referenced label didn't get added to scopes?");
+    unsigned LabelScope = LabelAndGotoScopes[TheLabel];
+    LabelStmt *&Target = TargetScopes[LabelScope];
+    if (!Target) Target = TheLabel;
+  }
+
+  llvm::BitVector Reachable(Scopes.size(), false);
+  for (llvm::DenseMap<unsigned,LabelStmt*>::iterator
+         TI = TargetScopes.begin(), TE = TargetScopes.end(); TI != TE; ++TI) {
+    unsigned TargetScope = TI->first;
+    LabelStmt *TargetLabel = TI->second;
+
+    Reachable.reset();
+
+    // Mark all the enclosing scopes from which you can safely jump
+    // into the target scope.
+    unsigned Min = TargetScope;
+    while (true) {
+      Reachable.set(Min);
+
+      // Don't go beyond the outermost scope.
+      if (Min == 0) break;
+
+      // Don't go further if we couldn't trivially enter this scope.
+      if (Scopes[Min].InDiag) break;
+
+      Min = Scopes[Min].ParentScope;
     }
 
-    // Report all the things that would be skipped over by this &&label or
-    // indirect goto.
-    while (DiagnosticScope != 0) {
-      S.Diag(Scopes[DiagnosticScope].Loc, Scopes[DiagnosticScope].Diag);
-      DiagnosticScope = Scopes[DiagnosticScope].ParentScope;
+    // Walk through all the jump sites, checking that they can trivially
+    // reach this label scope.
+    for (llvm::SmallVectorImpl<JumpScope>::iterator
+           I = JumpScopes.begin(), E = JumpScopes.end(); I != E; ++I) {
+      unsigned Scope = I->first;
+
+      // Walk out the "scope chain" for this scope, looking for a scope
+      // we've marked reachable.
+      bool IsReachable = false;
+      while (true) {
+        if (Reachable.test(Scope)) {
+          // If we find something reachable, mark all the scopes we just
+          // walked through as reachable.
+          for (unsigned S = I->first; S != Scope; S = Scopes[S].ParentScope)
+            Reachable.set(S);
+          IsReachable = true;
+          break;
+        }
+
+        // Don't walk out if we've reached the top-level scope or we've
+        // gotten shallower than the shallowest reachable scope.
+        if (Scope == 0 || Scope < Min) break;
+
+        // Don't walk out through an out-diagnostic.
+        if (Scopes[Scope].OutDiag) break;
+
+        Scope = Scopes[Scope].ParentScope;
+      }
+
+      // Only diagnose if we didn't find something.
+      if (IsReachable) continue;
+
+      DiagnoseIndirectJump(I->second, I->first, TargetLabel, TargetScope);
     }
   }
 }
 
+void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump,
+                                            unsigned JumpScope,
+                                            LabelStmt *Target,
+                                            unsigned TargetScope) {
+  assert(JumpScope != TargetScope);
+
+  S.Diag(Jump->getGotoLoc(), diag::warn_indirect_goto_in_protected_scope);
+  S.Diag(Target->getIdentLoc(), diag::note_indirect_goto_target);
+
+  // Collect everything in the target scope chain.
+  llvm::DenseSet<unsigned> TargetScopeChain;
+  for (unsigned SI = TargetScope; SI != 0; SI = Scopes[SI].ParentScope)
+    TargetScopeChain.insert(SI);
+  TargetScopeChain.insert(0);
+
+  // Walk out the scopes containing the indirect goto until we find a
+  // common ancestor with the target label.
+  unsigned Common = JumpScope;
+  while (!TargetScopeChain.count(Common)) {
+    // FIXME: this isn't necessarily a problem!  Not every protected
+    // scope requires destruction.
+    S.Diag(Scopes[Common].Loc, Scopes[Common].OutDiag);
+    Common = Scopes[Common].ParentScope;
+  }
+
+  // Now walk into the scopes containing the label whose address was taken.
+  for (unsigned SI = TargetScope; SI != Common; SI = Scopes[SI].ParentScope)
+    S.Diag(Scopes[SI].Loc, Scopes[SI].InDiag);
+}
+
 /// CheckJump - Validate that the specified jump statement is valid: that it is
 /// jumping within or out of its current scope, not into a deeper one.
 void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To,
@@ -303,9 +472,10 @@
     TestScope = Scopes[TestScope].ParentScope;
   }
 
-  // If we get here, then we know we have invalid code.  Diagnose the bad jump,
-  // and then emit a note at each VLA being jumped out of.
-  S.Diag(DiagLoc, JumpDiag);
+  // If we get here, then either we have invalid code or we're jumping in
+  // past some cleanup blocks.  It may seem strange to have a declaration
+  // with a trivial constructor and a non-trivial destructor, but it's
+  // possible.
 
   // Eliminate the common prefix of the jump and the target.  Start by
   // linearizing both scopes, reversing them as we go.
@@ -318,14 +488,23 @@
     ToScopes.push_back(TestScope);
 
   // Remove any common entries (such as the top-level function scope).
-  while (!FromScopes.empty() && FromScopes.back() == ToScopes.back()) {
+  while (!FromScopes.empty() && (FromScopes.back() == ToScopes.back())) {
     FromScopes.pop_back();
     ToScopes.pop_back();
   }
 
+  // Ignore any cleanup blocks on the way in.
+  while (!ToScopes.empty()) {
+    if (Scopes[ToScopes.back()].InDiag) break;
+    ToScopes.pop_back();
+  }
+  if (ToScopes.empty()) return;
+
+  S.Diag(DiagLoc, JumpDiag);
+
   // Emit diagnostics for whatever is left in ToScopes.
   for (unsigned i = 0, e = ToScopes.size(); i != e; ++i)
-    S.Diag(Scopes[ToScopes[i]].Loc, Scopes[ToScopes[i]].Diag);
+    S.Diag(Scopes[ToScopes[i]].Loc, Scopes[ToScopes[i]].InDiag);
 }
 
 void Sema::DiagnoseInvalidJumps(Stmt *Body) {

Modified: cfe/trunk/test/Sema/scope-check.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/scope-check.c?rev=103536&r1=103535&r2=103536&view=diff
==============================================================================
--- cfe/trunk/test/Sema/scope-check.c (original)
+++ cfe/trunk/test/Sema/scope-check.c Tue May 11 19:58:13 2010
@@ -133,20 +133,20 @@
 void test9(int n, void *P) {
   int Y;
   int Z = 4;
-  goto *P;  // ok.
+  goto *P;  // expected-warning {{indirect goto might cross protected scopes}}
 
 L2: ;
-  int a[n]; // expected-note 2 {{jump bypasses initialization of variable length array}}
+  int a[n]; // expected-note {{jump bypasses initialization of variable length array}}
 
-L3:
+L3:         // expected-note {{possible target of indirect goto}}
 L4:  
-  goto *P;  // expected-warning {{illegal indirect goto in protected scope, unknown effect on scopes}}
+  goto *P;
   goto L3;  // ok
   goto L4;  // ok
   
   void *Ptrs[] = {
-    &&L2,   // Ok.
-    &&L3   // expected-warning {{address taken of label in protected scope, jump to it would have unknown effect on scope}}
+    &&L2,
+    &&L3
   };
 }
 
@@ -193,3 +193,9 @@
   };
 }
 
+void test13(int n, void *p) {
+  int vla[n];
+  goto *p;
+ a0: ;
+  static void *ps[] = { &&a0 };
+}

Added: cfe/trunk/test/SemaCXX/scope-check.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/scope-check.cpp?rev=103536&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/scope-check.cpp (added)
+++ cfe/trunk/test/SemaCXX/scope-check.cpp Tue May 11 19:58:13 2010
@@ -0,0 +1,123 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks %s -Wno-unreachable-code
+
+namespace test0 {
+  struct D { ~D(); };
+
+  int f(bool b) {
+    if (b) {
+      D d;
+      goto end;
+    }
+
+  end:
+    return 1;
+  }
+}
+
+namespace test1 {
+  struct C { C(); };
+
+  int f(bool b) {
+    if (b)
+      goto foo; // expected-error {{illegal goto into protected scope}}
+    C c; // expected-note {{jump bypasses variable initialization}}
+  foo:
+    return 1;
+  }
+}
+
+namespace test2 {
+  struct C { C(); };
+
+  int f(void **ip) {
+    static void *ips[] = { &&lbl1, &&lbl2 };
+
+    C c;
+    goto *ip;
+  lbl1:
+    return 0;
+  lbl2:
+    return 1;
+  }
+}
+
+namespace test3 {
+  struct C { C(); };
+
+  int f(void **ip) {
+    static void *ips[] = { &&lbl1, &&lbl2 };
+
+    goto *ip;
+  lbl1: {
+    C c;
+    return 0;
+  }
+  lbl2:
+    return 1;
+  }
+}
+
+namespace test4 {
+  struct C { C(); };
+  struct D { ~D(); };
+
+  int f(void **ip) {
+    static void *ips[] = { &&lbl1, &&lbl2 };
+
+    C c0;
+
+    goto *ip; // expected-warning {{indirect goto might cross protected scopes}}
+    C c1; // expected-note {{jump bypasses variable initialization}}
+  lbl1: // expected-note {{possible target of indirect goto}}
+    return 0;
+  lbl2:
+    return 1;
+  }
+}
+
+namespace test5 {
+  struct C { C(); };
+  struct D { ~D(); };
+
+  int f(void **ip) {
+    static void *ips[] = { &&lbl1, &&lbl2 };
+    C c0;
+
+    goto *ip;
+  lbl1: // expected-note {{possible target of indirect goto}}
+    return 0;
+  lbl2:
+    if (ip[1]) {
+      D d; // expected-note {{jump exits scope of variable with non-trivial destructor}}
+      ip += 2;
+      goto *ip; // expected-warning {{indirect goto might cross protected scopes}}
+    }
+    return 1;
+  }
+}
+
+namespace test6 {
+  struct C { C(); };
+
+  unsigned f(unsigned s0, unsigned s1, void **ip) {
+    static void *ips[] = { &&lbl1, &&lbl2, &&lbl3, &&lbl4 };
+    C c0;
+
+    goto *ip;
+  lbl1:
+    s0++;
+    goto *++ip;
+  lbl2:
+    s0 -= s1;
+    goto *++ip;
+  lbl3: {
+    unsigned tmp = s0;
+    s0 = s1;
+    s1 = tmp;
+    goto *++ip;
+  }
+  lbl4:
+    return s0;
+  }
+}
+





More information about the cfe-commits mailing list