Implement CFG construction for __try / __except / __leave.

[thakis]

Summary

This makes -Wunreachable-code work for programs containing SEH (except for __finally, which is still missing for now).

__try is modeled like try (but simpler since it can only have a single __except or __finally), __except is fairly similar to catch (but simpler, since it can't contain declarations). __leave is implemented similarly to break / continue.

Use the existing addTryDispatchBlock infrastructure (which FindUnreachableCode() in ReachableCode.cpp uses via cfg->try_blocks_begin()) to mark things in the __except blocks as reachable.

Re-use TryTerminatedBlock. This means we add EH edges from calls to the __try block, but not from all other statements. While this is incomplete, it matches LLVM's SEH codegen support. Also, in practice, BuildOpts.AddEHEdges is always false in practice from what I can tell, so we never even insert the call EH edges either.

[rnk]

Don't add any EH edges to the CFG for SEH. In practice, BuildOpts.AddEHEdges is always false in practice from what I can tell, and with SEH every single stmt would have to get an EH edge.

Since we can't mix C++ EH and SEH, do you think it would be better to reuse the TryTerminatedBlock chain so that we get edges from every call to the __except? That's the approximation of SEH that we actually support in LLVM anyway.

lib/Analysis/CFG.cpp

2586

Looks like a // got re-wrapped in the comment

test/Sema/warn-unreachable-ms.c

24

typo empty

[thakis]

Just use TryTerminatedBlock

[thakis]

lib/Analysis/CFG.cpp

448

(this is now a no-op and only a clang-formatting of the existing ctor code. I can omit this if you want.)

[rnk]

Looks functionally correct

test/Sema/warn-unreachable-ms.c

43

Can we add a test to exercise that this builds the right CFG?

__try {
  __try {
    f();
  } __except(1) {
    __leave; // should exit outer try
  }
  __leave;
  f(); // expected-warning{{never be executed}}
} __except(1) {
}

[thakis]

add test from rnk

[rnk]

Looks good!

test/Sema/warn-unreachable-ms.c

43

Sure. Did you intentionally put two leaves in there, or do you only want the one in the inner except?

I think both are required to trigger the warning in case f() doesn't throw, but I could be wrong.

[rnk]

test/Sema/warn-unreachable-ms.c

43

I woke up this morning and realized what you meant. Is there another way we can check that leave in except exits the outer __try?

lib/Analysis/CFG.cpp

 ///
 class CFGBuilder {
   typedef BlockScopePosPair JumpTarget;
   typedef BlockScopePosPair JumpSource;
 
   ASTContext *Context;
   std::unique_ptr<CFG> cfg;
 
-  CFGBlock *Block;
-  CFGBlock *Succ;
+  CFGBlock *Block;  // Current block.
+  CFGBlock *Succ;  // Block after the current block.
   JumpTarget ContinueJumpTarget;
   JumpTarget BreakJumpTarget;
+  JumpTarget SEHLeaveJumpTarget;
   CFGBlock *SwitchTerminatedBlock;
   CFGBlock *DefaultCaseBlock;
+
+  // This can point either to a try or a __try block. The frontend forbids
+  // mixing both kinds in one function, so having one for both is enough.
   CFGBlock *TryTerminatedBlock;
 
   // Current position in local scope.
   LocalScope::const_iterator ScopePos;
 
   // LabelMap records the mapping from Label expressions to their jump targets.
   typedef llvm::DenseMap<LabelDecl*, JumpTarget> LabelMapTy;
   LabelMapTy LabelMap;
class CFGBuilder {
 
   // Caches boolean evaluations of expressions to avoid multiple re-evaluations
   // during construction of branches for chained logical operators.
   typedef llvm::DenseMap<Expr *, TryResult> CachedBoolEvalsTy;
   CachedBoolEvalsTy CachedBoolEvals;
 
 public:
   explicit CFGBuilder(ASTContext *astContext,
-                      const CFG::BuildOptions &buildOpts) 
+                      const CFG::BuildOptions &buildOpts)
       : Context(astContext), cfg(new CFG()), // crew a new CFG
-      Block(nullptr), Succ(nullptr),
-      SwitchTerminatedBlock(nullptr), DefaultCaseBlock(nullptr),
-      TryTerminatedBlock(nullptr), badCFG(false), BuildOpts(buildOpts),
-      switchExclusivelyCovered(false), switchCond(nullptr),
-      cachedEntry(nullptr), lastLookup(nullptr) {}
+        Block(nullptr), Succ(nullptr), SwitchTerminatedBlock(nullptr),
+        DefaultCaseBlock(nullptr), TryTerminatedBlock(nullptr), badCFG(false),
+        BuildOpts(buildOpts), switchExclusivelyCovered(false),
+        switchCond(nullptr), cachedEntry(nullptr), lastLookup(nullptr) {}
 
   // buildCFG - Used by external clients to construct the CFG.
   std::unique_ptr<CFG> buildCFG(const Decl *D, Stmt *Statement);
 
   bool alwaysAdd(const Stmt *stmt);
   
 private:
   // Visitors to walk an AST and construct the CFG.
private:
   CFGBlock *VisitObjCAtCatchStmt(ObjCAtCatchStmt *S);
   CFGBlock *VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *S);
   CFGBlock *VisitObjCAtThrowStmt(ObjCAtThrowStmt *S);
   CFGBlock *VisitObjCAtTryStmt(ObjCAtTryStmt *S);
   CFGBlock *VisitObjCAutoreleasePoolStmt(ObjCAutoreleasePoolStmt *S);
   CFGBlock *VisitObjCForCollectionStmt(ObjCForCollectionStmt *S);
   CFGBlock *VisitPseudoObjectExpr(PseudoObjectExpr *E);
   CFGBlock *VisitReturnStmt(ReturnStmt *R);
+  CFGBlock *VisitSEHExceptStmt(SEHExceptStmt *S);
+  CFGBlock *VisitSEHFinallyStmt(SEHFinallyStmt *S);
+  CFGBlock *VisitSEHLeaveStmt(SEHLeaveStmt *S);
+  CFGBlock *VisitSEHTryStmt(SEHTryStmt *S);
   CFGBlock *VisitStmtExpr(StmtExpr *S, AddStmtChoice asc);
   CFGBlock *VisitSwitchStmt(SwitchStmt *S);
   CFGBlock *VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
                                           AddStmtChoice asc);
   CFGBlock *VisitUnaryOperator(UnaryOperator *U, AddStmtChoice asc);
   CFGBlock *VisitWhileStmt(WhileStmt *W);
 
   CFGBlock *Visit(Stmt *S, AddStmtChoice asc = AddStmtChoice::NotAlwaysAdd);
case Stmt::OpaqueValueExprClass:
       return Block;
 
     case Stmt::PseudoObjectExprClass:
       return VisitPseudoObjectExpr(cast<PseudoObjectExpr>(S));
 
     case Stmt::ReturnStmtClass:
       return VisitReturnStmt(cast<ReturnStmt>(S));
 
+    case Stmt::SEHExceptStmtClass:
+      return VisitSEHExceptStmt(cast<SEHExceptStmt>(S));
+
+    case Stmt::SEHFinallyStmtClass:
+      return VisitSEHFinallyStmt(cast<SEHFinallyStmt>(S));
+
+    case Stmt::SEHLeaveStmtClass:
+      return VisitSEHLeaveStmt(cast<SEHLeaveStmt>(S));
+
+    case Stmt::SEHTryStmtClass:
+      return VisitSEHTryStmt(cast<SEHTryStmt>(S));
+
     case Stmt::UnaryExprOrTypeTraitExprClass:
       return VisitUnaryExprOrTypeTraitExpr(cast<UnaryExprOrTypeTraitExpr>(S),
                                            asc);
 
     case Stmt::StmtExprClass:
       return VisitStmtExpr(cast<StmtExpr>(S), asc);
 
     case Stmt::SwitchStmtClass:
CFGBlock *CFGBuilder::VisitReturnStmt(ReturnStmt *R) {
   if (!Block->hasNoReturnElement())
     addSuccessor(Block, &cfg->getExit());
 
   // Add the return statement to the block.  This may create new blocks if R
   // contains control-flow (short-circuit operations).
   return VisitStmt(R, AddStmtChoice::AlwaysAdd);
 }
 
+CFGBlock *CFGBuilder::VisitSEHExceptStmt(SEHExceptStmt *ES) {
+  // SEHExceptStmt are treated like labels, so they are the first statement in a
+  // block.
+
+  // Save local scope position because in case of exception variable ScopePos
+  // won't be restored when traversing AST.
+  SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
+
+  addStmt(ES->getBlock());
+  CFGBlock *SEHExceptBlock = Block;
+  if (!SEHExceptBlock)
+    SEHExceptBlock = createBlock();
+
+  appendStmt(SEHExceptBlock, ES);
+
+  // Also add the SEHExceptBlock as a label, like with regular labels.
+  SEHExceptBlock->setLabel(ES);
+
+  // Bail out if the CFG is bad.
+  if (badCFG)
+    return nullptr;
+
+  // We set Block to NULL to allow lazy creation of a new block (if necessary).
+  Block = nullptr;
+
+  return SEHExceptBlock;
+}
+
+CFGBlock *CFGBuilder::VisitSEHFinallyStmt(SEHFinallyStmt *FS) {
+  return VisitCompoundStmt(FS->getBlock());
+}
+
+CFGBlock *CFGBuilder::VisitSEHLeaveStmt(SEHLeaveStmt *LS) {
+  // "__leave" is a control-flow statement.  Thus we stop processing the current
+  // block.
+  if (badCFG)
+    return nullptr;
+
+  // Now create a new block that ends with the __leave statement.
+  Block = createBlock(false);
+  Block->setTerminator(LS);
+
+  // If there is no target for the __leave, then we are looking at an incomplete
+  // AST.  This means that the CFG cannot be constructed.
+  if (SEHLeaveJumpTarget.block) {
+    addAutomaticObjHandling(ScopePos, SEHLeaveJumpTarget.scopePosition, LS);
+    addSuccessor(Block, SEHLeaveJumpTarget.block);
+  } else
+    badCFG = true;
+
+  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;
+
+  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);
+
+  // 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);
+  }
+  if (PrevSEHTryTerminatedBlock)
+    addSuccessor(NewTryTerminatedBlock, PrevSEHTryTerminatedBlock);
+  else
+    addSuccessor(NewTryTerminatedBlock, &cfg->getExit());
+
+  // The code after the try 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);
+  SEHLeaveJumpTarget = JumpTarget(SEHTrySuccessor, ScopePos);
+
+  assert(Terminator->getTryBlock() && "__try must contain a non-NULL body");
+  Block = nullptr;
+  return addStmt(Terminator->getTryBlock());
+}
+
 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.
     LabelBlock = createBlock(); // scopes that only contains NullStmts.
 
void VisitSwitchStmt(SwitchStmt *Terminator) {
     OS << "switch ";
     Terminator->getCond()->printPretty(OS, Helper, Policy);
   }
 
   void VisitCXXTryStmt(CXXTryStmt *CS) {
     OS << "try ...";
   }
 
+  void VisitSEHTryStmt(SEHTryStmt *CS) {
+    OS << "__try ...";
+  }
+
   void VisitAbstractConditionalOperator(AbstractConditionalOperator* C) {
     if (Stmt *Cond = C->getCond())
       Cond->printPretty(OS, Helper, Policy);
     OS << " ? ... : ...";
   }
 
   void VisitChooseExpr(ChooseExpr *C) {
     OS << "__builtin_choose_expr( ";
if (Stmt *Label = const_cast<Stmt*>(B.getLabel())) {
     else if (CXXCatchStmt *CS = dyn_cast<CXXCatchStmt>(Label)) {
       OS << "catch (";
       if (CS->getExceptionDecl())
         CS->getExceptionDecl()->print(OS, PrintingPolicy(Helper.getLangOpts()),
                                       0);
       else
         OS << "...";
       OS << ")";
-
+    } else if (SEHExceptStmt *ES = dyn_cast<SEHExceptStmt>(Label)) {
+      OS << "__except (";
+      ES->getFilterExpr()->printPretty(OS, &Helper,
+                                       PrintingPolicy(Helper.getLangOpts()), 0);
+      OS << ")";
     } else
       llvm_unreachable("Invalid label statement in CFGBlock.");
 
     OS << ":\n";
   }
 
   // Iterate through the statements in the block and print them.
   unsigned j = 1;

test/Sema/warn-unreachable-ms.c

+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fsyntax-only -verify -fms-extensions -Wunreachable-code
+
+void f();
+
+void g1() {
+  __try {
+    f();
+    __leave;
+    f();  // expected-warning{{will never be executed}}
+  } __except(1) {
+    f();
+  }
+
+  // Completely empty.
+  __try {
+  } __except(1) {
+  }
+
+  __try {
+    f();
+    return;
+  } __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) {
+    }
+    f();
+    __leave;
+    f(); // expected-warning{{will never be executed}}
+  } __except(1) {
+    f();
+  }
+}
+
+void g3() {
+  __try {
+    __try {
+      f();
+    } __except (1) {
+      __leave; // should exit outer try
+    }
+    __leave;
+    f(); // expected-warning{{never be executed}}
+  } __except (1) {
+  }
+}