Implement CFG construction for __finally.

[thakis]

Summary

This completes CFG construction for SEH AST nodes.

Also tweak test g3 to explicitly check that __leave in an __except leaves the outer __try.

[rnk]

Re: jumps out of try, I wonder if you can tie finally into whatever the CFG does for C++ destructors.

test/Sema/warn-unreachable-ms.c

49

Nice. Would any noreteurn call work here to eliminate the re-run and ifdef?

lib/Analysis/CFG.cpp

CFGBlock *CFGBuilder::VisitSEHLeaveStmt(SEHLeaveStmt *LS) {
   return Block;
 }
 
 CFGBlock *CFGBuilder::VisitSEHTryStmt(SEHTryStmt *Terminator) {
   // "__try"/"__except"/"__finally" is a control-flow statement.  Thus we stop
   // processing the current block.
   CFGBlock *SEHTrySuccessor = nullptr;
 
+  // We create a block for the __try (NewTryTerminatedBlock below).  This block
+  // is entered on exceptional control flow, and for a __try / __except either
+  // goes to the __except block (if the __except catches the exception) or to
+  // the end of the function (or the containing __try if it exists) if this
+  // exception isn't caught. The contents of the __try block are emitted as
+  // a regular block not connected to NewTryTerminatedBlock.  The __except
+  // block goes to the block after the __try / __except.
+  // For a __try / __finally, the __try goes to the __finally on exceptional
+  // control flow.  The __finally block connects to the block after they
+  // __try / __finally (for regular control flow) and to the end of the
+  // function (or the containing __try if it exists) on exceptional control
+  // flow.
+
   if (Block) {
     if (badCFG)
       return nullptr;
     SEHTrySuccessor = Block;
   } else SEHTrySuccessor = Succ;
 
-  // FIXME: Implement __finally support.
-  if (Terminator->getFinallyHandler())
-    return NYS();
-
   CFGBlock *PrevSEHTryTerminatedBlock = TryTerminatedBlock;
 
   // Create a new block that will contain the __try statement.
   CFGBlock *NewTryTerminatedBlock = createBlock(false);
+  CFGBlock *EndOfTry = NewTryTerminatedBlock;
 
   // Add the terminator in the __try block.
   NewTryTerminatedBlock->setTerminator(Terminator);
 
-  if (SEHExceptStmt *Except = Terminator->getExceptHandler()) {
-    // The code after the try is the implicit successor if there's an __except.
-    Succ = SEHTrySuccessor;
-    Block = nullptr;
-    CFGBlock *ExceptBlock = VisitSEHExceptStmt(Except);
-    if (!ExceptBlock)
-      return nullptr;
-    // Add this block to the list of successors for the block with the try
-    // statement.
-    addSuccessor(NewTryTerminatedBlock, ExceptBlock);
-  }
+  // The code after the try is the implicit successor of an __except,
+  // and one implicit successor of a __finally.
+  bool AlreadyHasExitEdge = false;
+  CFGBlock *ExceptOrFinallyBlock;
+  if (SEHFinallyStmt *Finally = Terminator->getFinallyHandler()) {
+    AlreadyHasExitEdge = SEHTrySuccessor == &cfg->getExit();
+    ExceptOrFinallyBlock = VisitSEHFinallyStmt(Finally);
+    if (ExceptOrFinallyBlock) {
+      // With a __finally, the code in the __finally block should run after the
+      // code in the __try.
+      SEHTrySuccessor = ExceptOrFinallyBlock;
+      // With a __finally, we jump to the end-of-function (or enclosing __try)
+      // from the __finally block instead of from the try control block.
+      EndOfTry = ExceptOrFinallyBlock;
+    }
+  } else {
+    assert(Terminator->getExceptHandler());
+    ExceptOrFinallyBlock = VisitSEHExceptStmt(Terminator->getExceptHandler());
+  }
+  addSuccessor(NewTryTerminatedBlock, ExceptOrFinallyBlock);
+
   if (PrevSEHTryTerminatedBlock)
-    addSuccessor(NewTryTerminatedBlock, PrevSEHTryTerminatedBlock);
-  else
-    addSuccessor(NewTryTerminatedBlock, &cfg->getExit());
+    addSuccessor(EndOfTry, PrevSEHTryTerminatedBlock);
+  else if (!AlreadyHasExitEdge)
+    addSuccessor(EndOfTry, &cfg->getExit());
 
-  // The code after the try is the implicit successor.
+  // The code after the try is the implicit successor for an __except,
+  // else the code in the __finally is the implicit successor.
   Succ = SEHTrySuccessor;
 
   // Save the current "__try" context.
   SaveAndRestore<CFGBlock *> save_try(TryTerminatedBlock,
                                       NewTryTerminatedBlock);
   cfg->addTryDispatchBlock(TryTerminatedBlock);
 
   // Save the current value for the __leave target.
-  // All __leaves should go to the code following the __try
-  // (FIXME: or if the __try has a __finally, to the __finally.)
-  SaveAndRestore<JumpTarget> save_break(SEHLeaveJumpTarget);
+  // All __leaves should go to the code following the __try,
+  // or if the __try has a __finally, to the __finally.
+  SaveAndRestore<JumpTarget> save_leave(SEHLeaveJumpTarget);
   SEHLeaveJumpTarget = JumpTarget(SEHTrySuccessor, ScopePos);
 
   assert(Terminator->getTryBlock() && "__try must contain a non-NULL body");
   Block = nullptr;
-  return addStmt(Terminator->getTryBlock());
+  CFGBlock* TryBodyBlock = addStmt(Terminator->getTryBlock());
+  return TryBodyBlock ? TryBodyBlock : SEHTrySuccessor;
 }
 
 CFGBlock *CFGBuilder::VisitLabelStmt(LabelStmt *L) {
   // Get the block of the labeled statement.  Add it to our map.
   addStmt(L->getSubStmt());
   CFGBlock *LabelBlock = Block;
 
   if (!LabelBlock)              // This can happen when the body is empty, i.e.

test/Sema/warn-unreachable-ms.c

 // RUN: %clang_cc1 %s -triple=i686-pc-win32 -fsyntax-only -verify -fms-extensions -Wunreachable-code
+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fsyntax-only -verify -fms-extensions -Wunreachable-code -x c++ -fcxx-exceptions -DWITH_THROW
 
 void f();
 
 void g1() {
   __try {
     f();
     __leave;
     f();  // expected-warning{{will never be executed}}
-  } __except(1) {
+  } __except (1) {
     f();
   }
 
   // Completely empty.
   __try {
-  } __except(1) {
+  } __except (1) {
   }
 
   __try {
     f();
     return;
-  } __except(1) {  // Filter expression should not be marked as unreachable.
+  } __except (1) { // Filter expression should not be marked as unreachable.
     // Empty __except body.
   }
 }
 
 void g2() {
   __try {
     // Nested __try.
     __try {
       f();
       __leave;
       f(); // expected-warning{{will never be executed}}
-    } __except(2) {
+    } __except (2) {
     }
     f();
     __leave;
     f(); // expected-warning{{will never be executed}}
-  } __except(1) {
+  } __except (1) {
     f();
   }
 }
 
+#if defined(WITH_THROW)
 void g3() {
   __try {
     __try {
-      f();
+      throw 1;
     } __except (1) {
       __leave; // should exit outer try
     }
-    __leave;
     f(); // expected-warning{{never be executed}}
   } __except (1) {
   }
 }
+#endif
+
+void g4() {
+  __try {
+    f();
+    __leave;
+    f();  // expected-warning{{will never be executed}}
+  } __finally {
+    f();
+  }
+
+  // Completely empty.
+  __try {
+  } __finally {
+  }
+
+  __try {
+    f();
+    return;  // Shouldn't make __finally body unreachable.
+  } __finally {
+    f();
+  }
+}
+
+void g5() {
+  __try {
+    // Nested __try.
+    __try {
+      f();
+      __leave;
+      f(); // expected-warning{{will never be executed}}
+    } __finally {
+    }
+    f();
+    __leave;
+    f(); // expected-warning{{will never be executed}}
+  } __finally {
+    f();
+  }
+}
+
+// Jumps out of __finally have undefined behavior, but they're useful for
+// checking that the generated CFG looks right.
+
+#if defined(WITH_THROW)
+void g6() {
+  __try {
+    __try {
+      throw 1;
+    } __finally {
+      __leave; // should exit outer try expected-warning {{jump out of __finally block has undefined behavior}}
+    }
+    f(); // expected-warning{{never be executed}}
+  } __finally {
+  }
+}
+#endif
+
+void g7() {
+  f();
+  __try {
+  } __finally {
+    return; // expected-warning {{jump out of __finally block has undefined behavior}}
+  }
+  f(); // expected-warning{{never be executed}}
+}