From: Nico Weber <nicolasweber@gmx.de>
Date: Mon, 9 Mar 2015 04:27:56 +0000 (+0000)
Subject: Warn when jumping out of a __finally block via goto.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8880714d42677267f7aee51a630476fcfb822e1a;p=clang

Warn when jumping out of a __finally block via goto.

This only warns on direct gotos and indirect gotos with a unique label
(`goto *&&label;`).  Jumping out ith a true indirect goto is already an error.

This isn't O(1), but goto statements are less common than continue, break, and
return.  Also, the GetDeepestCommonScope() call in the same function does the
same amount of work, so this isn't worse than what's there in a complexity
sense, and it should be pretty fast in practice.

This is the last piece that was missing in r231623.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@231628 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp
index 03fcc9482e..aac28bedfa 100644
--- a/lib/Sema/JumpDiagnostics.cpp
+++ b/lib/Sema/JumpDiagnostics.cpp
@@ -789,6 +789,18 @@ void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
   // Common case: exactly the same scope, which is fine.
   if (FromScope == ToScope) return;
 
+  // Warn on gotos out of __finally blocks.
+  if (isa<GotoStmt>(From) || isa<IndirectGotoStmt>(From)) {
+    // If FromScope > ToScope, FromScope is more nested and the jump goes to a
+    // less nested scope.  Check if it crosses a __finally along the way.
+    for (unsigned I = FromScope; I > ToScope; I = Scopes[I].ParentScope) {
+      if (Scopes[I].InDiag == diag::note_protected_by_seh_finally) {
+        S.Diag(From->getLocStart(), diag::warn_jump_out_of_seh_finally);
+        break;
+      }
+    }
+  }
+
   unsigned CommonScope = GetDeepestCommonScope(FromScope, ToScope);
 
   // It's okay to jump out from a nested scope.
diff --git a/test/SemaCXX/scope-check.cpp b/test/SemaCXX/scope-check.cpp
index c7638aea10..1d2893c7e0 100644
--- a/test/SemaCXX/scope-check.cpp
+++ b/test/SemaCXX/scope-check.cpp
@@ -507,12 +507,12 @@ out_of_finally_try:
 
   __try {
   } __finally {
-    // FIXME: This should warn that jumping out of __finally has undefined
-    // behavior.
-    // FIXME: Once that warns, check that
-    //   __try { __try {} __finally { __leave; } } __except (0) {}
-    // warns in the same way.
-    goto out_of_finally_finally;
+    goto out_of_finally_finally; // expected-warning {{jump out of __finally block has undefined behavior}}
+  }
+
+  __try {
+  } __finally {
+    goto *&&out_of_finally_finally; // expected-warning {{jump out of __finally block has undefined behavior}}
   }
 out_of_finally_finally:
   ;
@@ -548,7 +548,7 @@ void jump_try_finally() {
   from_finally_to_try:
     ;
   } __finally {
-    goto from_finally_to_try; // expected-error {{cannot jump from this goto statement to its label}}
+    goto from_finally_to_try; // expected-error {{cannot jump from this goto statement to its label}} expected-warning {{jump out of __finally block has undefined behavior}}
   }
 }
 
@@ -578,9 +578,7 @@ void nested() {
   __try {
     __try {
     } __finally {
-      // FIXME: This should warn that jumping out of __finally has undefined
-      // behavior.
-      goto after_outer_except;
+      goto after_outer_except; // expected-warning {{jump out of __finally block has undefined behavior}}
     }
   } __except(0) {
   }