r231623 - Warn when jumping out of a __finally block via continue, break, return, __leave.

Nico Weber nicolasweber at gmx.de
Sun Mar 8 19:48:00 PDT 2015


Author: nico
Date: Sun Mar  8 21:47:59 2015
New Revision: 231623

URL: http://llvm.org/viewvc/llvm-project?rev=231623&view=rev
Log:
Warn when jumping out of a __finally block via continue, break, return, __leave.

Since continue, break, return are much more common than __finally, this tries
to keep the work for continue, break, return O(1).  Sema keeps a stack of active
__finally scopes (to do this, ActOnSEHFinally() is split into
ActOnStartSEHFinally() and ActOnFinishSEHFinally()), and the various jump
statements then check if the current __finally scope (if present) is deeper
than then destination scope of the jump.

The same warning for goto statements is still missing.

This is the moral equivalent of MSVC's C4532.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Scope.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/lib/Sema/TreeTransform.h
    cfe/trunk/test/Sema/__try.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Mar  8 21:47:59 2015
@@ -5490,6 +5490,9 @@ def err_mixing_cxx_try_seh_try : Error<
   "cannot use C++ 'try' in the same function as SEH '__try'">;
 def note_conflicting_try_here : Note<
   "conflicting %0 here">;
+def warn_jump_out_of_seh_finally : Warning<
+  "jump out of __finally block has undefined behavior">,
+  InGroup<DiagGroup<"jump-seh-finally">>;
 def warn_non_virtual_dtor : Warning<
   "%0 has virtual functions but non-virtual destructor">,
   InGroup<NonVirtualDtor>, DefaultIgnore;

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Sun Mar  8 21:47:59 2015
@@ -416,6 +416,12 @@ public:
   /// \brief Determine whether this scope is a SEH '__except' block.
   bool isSEHExceptScope() const { return getFlags() & Scope::SEHExceptScope; }
 
+  /// \brief Returns if rhs has a higher scope depth than this.
+  ///
+  /// The caller is responsible for calling this only if one of the two scopes
+  /// is an ancestor of the other.
+  bool Contains(const Scope& rhs) const { return Depth < rhs.Depth; }
+
   /// containedInPrototypeScope - Return true if this or a parent scope
   /// is a FunctionPrototypeScope.
   bool containedInPrototypeScope() const;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Sun Mar  8 21:47:59 2015
@@ -303,6 +303,9 @@ public:
   /// The stack always has at least one element in it.
   SmallVector<MSVtorDispAttr::Mode, 2> VtorDispModeStack;
 
+  /// Stack of active SEH __finally scopes.  Can be empty.
+  SmallVector<Scope*, 2> CurrentSEHFinally;
+
   /// \brief Source location for newly created implicit MSInheritanceAttrs
   SourceLocation ImplicitMSInheritanceAttrLoc;
 
@@ -3293,7 +3296,8 @@ public:
   StmtResult ActOnSEHExceptBlock(SourceLocation Loc,
                                  Expr *FilterExpr,
                                  Stmt *Block);
-  StmtResult ActOnSEHFinallyBlock(SourceLocation Loc, Stmt *Block);
+  void ActOnStartSEHFinallyBlock();
+  StmtResult ActOnFinishSEHFinallyBlock(SourceLocation Loc, Stmt *Block);
   StmtResult ActOnSEHLeaveStmt(SourceLocation Loc, Scope *CurScope);
 
   void DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock);

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Sun Mar  8 21:47:59 2015
@@ -507,7 +507,7 @@ StmtResult Parser::ParseSEHExceptBlock(S
 /// seh-finally-block:
 ///   '__finally' compound-statement
 ///
-StmtResult Parser::ParseSEHFinallyBlock(SourceLocation FinallyBlock) {
+StmtResult Parser::ParseSEHFinallyBlock(SourceLocation FinallyLoc) {
   PoisonIdentifierRAIIObject raii(Ident__abnormal_termination, false),
     raii2(Ident___abnormal_termination, false),
     raii3(Ident_AbnormalTermination, false);
@@ -515,11 +515,14 @@ StmtResult Parser::ParseSEHFinallyBlock(
   if (Tok.isNot(tok::l_brace))
     return StmtError(Diag(Tok, diag::err_expected) << tok::l_brace);
 
+  ParseScope FinallyScope(this, 0);
+  Actions.ActOnStartSEHFinallyBlock();
+
   StmtResult Block(ParseCompoundStatement());
   if(Block.isInvalid())
     return Block;
 
-  return Actions.ActOnSEHFinallyBlock(FinallyBlock,Block.get());
+  return Actions.ActOnFinishSEHFinallyBlock(FinallyLoc, Block.get());
 }
 
 /// Handle __leave

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Sun Mar  8 21:47:59 2015
@@ -2428,6 +2428,14 @@ Sema::ActOnIndirectGotoStmt(SourceLocati
   return new (Context) IndirectGotoStmt(GotoLoc, StarLoc, E);
 }
 
+static void CheckJumpOutOfSEHFinally(Sema &S, SourceLocation Loc,
+                                     const Scope &DestScope) {
+  if (!S.CurrentSEHFinally.empty() &&
+      DestScope.Contains(*S.CurrentSEHFinally.back())) {
+    S.Diag(Loc, diag::warn_jump_out_of_seh_finally);
+  }
+}
+
 StmtResult
 Sema::ActOnContinueStmt(SourceLocation ContinueLoc, Scope *CurScope) {
   Scope *S = CurScope->getContinueParent();
@@ -2435,6 +2443,7 @@ Sema::ActOnContinueStmt(SourceLocation C
     // C99 6.8.6.2p1: A break shall appear only in or as a loop body.
     return StmtError(Diag(ContinueLoc, diag::err_continue_not_in_loop));
   }
+  CheckJumpOutOfSEHFinally(*this, ContinueLoc, *S);
 
   return new (Context) ContinueStmt(ContinueLoc);
 }
@@ -2449,6 +2458,7 @@ Sema::ActOnBreakStmt(SourceLocation Brea
   if (S->isOpenMPLoopScope())
     return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
                      << "break");
+  CheckJumpOutOfSEHFinally(*this, BreakLoc, *S);
 
   return new (Context) BreakStmt(BreakLoc);
 }
@@ -2908,6 +2918,8 @@ Sema::ActOnReturnStmt(SourceLocation Ret
     CurScope->setNoNRVO();
   }
 
+  CheckJumpOutOfSEHFinally(*this, ReturnLoc, *CurScope->getFnParent());
+
   return R;
 }
 
@@ -3406,11 +3418,14 @@ Sema::ActOnSEHExceptBlock(SourceLocation
   return SEHExceptStmt::Create(Context,Loc,FilterExpr,Block);
 }
 
-StmtResult
-Sema::ActOnSEHFinallyBlock(SourceLocation Loc,
-                           Stmt *Block) {
+void Sema::ActOnStartSEHFinallyBlock() {
+  CurrentSEHFinally.push_back(CurScope);
+}
+
+StmtResult Sema::ActOnFinishSEHFinallyBlock(SourceLocation Loc, Stmt *Block) {
   assert(Block);
-  return SEHFinallyStmt::Create(Context,Loc,Block);
+  CurrentSEHFinally.pop_back();
+  return SEHFinallyStmt::Create(Context, Loc, Block);
 }
 
 StmtResult
@@ -3420,6 +3435,7 @@ Sema::ActOnSEHLeaveStmt(SourceLocation L
     SEHTryParent = SEHTryParent->getParent();
   if (!SEHTryParent)
     return StmtError(Diag(Loc, diag::err_ms___leave_not_in___try));
+  CheckJumpOutOfSEHFinally(*this, Loc, *SEHTryParent);
 
   return new (Context) SEHLeaveStmt(Loc);
 }

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Sun Mar  8 21:47:59 2015
@@ -1702,7 +1702,7 @@ public:
   }
 
   StmtResult RebuildSEHFinallyStmt(SourceLocation Loc, Stmt *Block) {
-    return getSema().ActOnSEHFinallyBlock(Loc, Block);
+    return SEHFinallyStmt::Create(getSema().getASTContext(), Loc, Block);
   }
 
   /// \brief Build a new predefined expression.

Modified: cfe/trunk/test/Sema/__try.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/__try.c?rev=231623&r1=231622&r2=231623&view=diff
==============================================================================
--- cfe/trunk/test/Sema/__try.c (original)
+++ cfe/trunk/test/Sema/__try.c Sun Mar  8 21:47:59 2015
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fborland-extensions -DBORLAND -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fborland-extensions -DBORLAND -fsyntax-only -verify -fblocks %s
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify -fblocks %s
 
 #define JOIN2(x,y) x ## y
 #define JOIN(x,y) JOIN2(x,y)
@@ -207,3 +207,77 @@ void test_seh_leave_stmt() {
   }
   __leave; // expected-error{{'__leave' statement not in __try block}}
 }
+
+void test_jump_out_of___finally() {
+  while(1) {
+    __try {
+    } __finally {
+      continue; // expected-warning{{jump out of __finally block has undefined behavior}}
+    }
+  }
+  __try {
+  } __finally {
+    while (1) {
+      continue;
+    }
+  }
+
+  // Check that a deep __finally containing a block with a shallow continue
+  // doesn't trigger the warning.
+  while(1) {{{{
+    __try {
+    } __finally {
+      ^{
+        while(1)
+          continue;
+      }();
+    }
+  }}}}
+
+  while(1) {
+    __try {
+    } __finally {
+      break; // expected-warning{{jump out of __finally block has undefined behavior}}
+    }
+  }
+  switch(1) {
+  case 1:
+    __try {
+    } __finally {
+      break; // expected-warning{{jump out of __finally block has undefined behavior}}
+    }
+  }
+  __try {
+  } __finally {
+    while (1) {
+      break;
+    }
+  }
+
+  __try {
+    __try {
+    } __finally {
+      __leave; // expected-warning{{jump out of __finally block has undefined behavior}}
+    }
+  } __finally {
+  }
+  __try {
+  } __finally {
+    __try {
+      __leave;
+    } __finally {
+    }
+  }
+
+  __try {
+  } __finally {
+    return; // expected-warning{{jump out of __finally block has undefined behavior}}
+  }
+
+  __try {
+  } __finally {
+    ^{
+      return;
+    }();
+  }
+}





More information about the cfe-commits mailing list