[clang] Implement CFG construction for @try and @catch

[thakis]

Summary

@finally is still not implemented.

With this, clang can emit -Wreturn-type warnings for functions containing
@try/@catch (but not yet @finally), and -Wunreachable-code also works for those
functions.

The implementation is similar to D36914.

Part of PR46693.

[thakis]

clang-format

[hans]

lgtm, just some nits.

clang/lib/Analysis/CFG.cpp

485

One "to" too much in "to either to a ...".
Should probably be *an* SEH __try (at least in my head, that S is pronounced "ess").

3888

ultra nit: period.

3895

does the fixme still stand?

3941

Why h? Could this be a range for instead? Otherwise for (i = .., e = ..., i != e) is the classic llvm style.

5790

maybe

if (const VarDecl *PD = CS->getCatchParamDecl())
  PD->print(...)

clang/test/Sema/warn-unreachable.m

1

Not important, but I think it's much more common to have %s at the end. Same for the other test file.

[thakis]

Thanks, all done.

Looks like there's no range access to a try's catch blocks, so no for-range loop for now. Maybe I'll add range accessors for those later.

clang/lib/Analysis/CFG.cpp

3895

No, I think this is done now. Removed, thanks.

5790

oh yes, that's much nicer. thanks, done.

[comanea]

I believe there is an issue that this change introduced. It is described here. https://bugs.llvm.org/show_bug.cgi?id=52473
If anyone could provide some insight, it would be much appreciated.

[thakis]

In D112287#3146956, @comanea wrote:

I believe there is an issue that this change introduced. It is described here. https://bugs.llvm.org/show_bug.cgi?id=52473
If anyone could provide some insight, it would be much appreciated.

Thanks for the report. I think D114660 might fix this.

(I'd say that on the bug too, but the bug tracker is currently read-only.)

clang/lib/Analysis/CFG.cpp

class CFGBuilder {
   CFGBlock *Succ = nullptr;
 
   JumpTarget ContinueJumpTarget;
   JumpTarget BreakJumpTarget;
   JumpTarget SEHLeaveJumpTarget;
   CFGBlock *SwitchTerminatedBlock = nullptr;
   CFGBlock *DefaultCaseBlock = nullptr;
 
-  // 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.
+  // This can point to either to a C++ try, an Objective-C @try, or a SEH __try.
+  // try and @try can be mixed and generally work the same.
+  // The frontend forbids mixing SEH __try with either try or @try.
+  // So having one for all three is enough.
   CFGBlock *TryTerminatedBlock = nullptr;
 
   // Current position in local scope.
   LocalScope::const_iterator ScopePos;
 
   // LabelMap records the mapping from Label expressions to their jump targets.
   using LabelMapTy = llvm::DenseMap<LabelDecl *, JumpTarget>;
   LabelMapTy LabelMap;
switch (S->getStmtClass()) {
 
     case Stmt::NullStmtClass:
       return Block;
 
     case Stmt::ObjCAtCatchStmtClass:
       return VisitObjCAtCatchStmt(cast<ObjCAtCatchStmt>(S));
 
     case Stmt::ObjCAutoreleasePoolStmtClass:
       return VisitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(S));
 
     case Stmt::ObjCAtSynchronizedStmtClass:
       return VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S));
 
     case Stmt::ObjCAtThrowStmtClass:
       return VisitObjCAtThrowStmt(cast<ObjCAtThrowStmt>(S));
 
     case Stmt::ObjCAtTryStmtClass:
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);
+  SaveAndRestore<CFGBlock *> SaveTry(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);
 
CFGBlock *CFGBuilder::VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *S) {
   // Add the @synchronized to the CFG.
   autoCreateBlock();
   appendStmt(Block, S);
 
   // Inline the sync expression.
   return addStmt(S->getSynchExpr());
 }
 
-CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) {
-  // FIXME
-  return NYS();
-}
-
 CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
   autoCreateBlock();
 
   // Add the PseudoObject as the last thing.
   appendStmt(Block, E);
 
   CFGBlock *lastBlock = Block;
 
CFGBlock *CFGBuilder::VisitWhileStmt(WhileStmt *W) {
   // to this block.  NULL out Block to force lazy creation of another block.
   Block = nullptr;
 
   // Return the condition block, which is the dominating block for the loop.
   Succ = EntryConditionBlock;
   return EntryConditionBlock;
 }
 
-CFGBlock *CFGBuilder::VisitObjCAtCatchStmt(ObjCAtCatchStmt *S) {
-  // FIXME: For now we pretend that @catch and the code it contains does not
-  //  exit.
-  return Block;
+CFGBlock *CFGBuilder::VisitObjCAtCatchStmt(ObjCAtCatchStmt *CS) {
+  // ObjCAtCatchStmt 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);
+
+  if (CS->getCatchBody())
+    addStmt(CS->getCatchBody());
+
+  CFGBlock *CatchBlock = Block;
+  if (!CatchBlock)
+    CatchBlock = createBlock();
+
+  appendStmt(CatchBlock, CS);
+
+  // Also add the ObjCAtCatchStmt as a label, like with regular labels.
+  CatchBlock->setLabel(CS);
+
+  // 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 CatchBlock;
 }
 
 CFGBlock *CFGBuilder::VisitObjCAtThrowStmt(ObjCAtThrowStmt *S) {
   // FIXME: This isn't complete.  We basically treat @throw like a return
   //  statement.
 
   // If we were in the middle of a block we stop processing that block.
   if (badCFG)
     return nullptr;
 
   // Create the new block.
   Block = createBlock(false);
 
-  // The Exit block is the only successor.
+  if (TryTerminatedBlock)
+    // The current try statement is the only successor.
+    addSuccessor(Block, TryTerminatedBlock);
+  else
+    // otherwise the Exit block is the only successor.
     addSuccessor(Block, &cfg->getExit());
 
   // Add the statement to the block.  This may create new blocks if S contains
   // control-flow (short-circuit operations).
   return VisitStmt(S, AddStmtChoice::AlwaysAdd);
 }
 
+CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *Terminator) {
+  // "@try"/"@catch" is a control-flow statement.  Thus we stop processing the
+  // current block.
+  CFGBlock *TrySuccessor = nullptr;
+
+  if (Block) {
+    if (badCFG)
+      return nullptr;
+    TrySuccessor = Block;
+  } else
+    TrySuccessor = Succ;
+
+  // FIXME: Implement @finally support.
+  if (Terminator->getFinallyStmt())
+    return NYS();
+
+  CFGBlock *PrevTryTerminatedBlock = 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);
+
+  bool HasCatchAll = false;
+  for (unsigned h = 0; h < Terminator->getNumCatchStmts(); ++h) {
+    // The code after the try is the implicit successor.
+    Succ = TrySuccessor;
+    ObjCAtCatchStmt *CS = Terminator->getCatchStmt(h);
+    if (CS->hasEllipsis()) {
+      HasCatchAll = true;
+    }
+    Block = nullptr;
+    CFGBlock *CatchBlock = VisitObjCAtCatchStmt(CS);
+    if (!CatchBlock)
+      return nullptr;
+    // Add this block to the list of successors for the block with the try
+    // statement.
+    addSuccessor(NewTryTerminatedBlock, CatchBlock);
+  }
+
+  // FIXME: This needs updating when @finally support is added.
+  if (!HasCatchAll) {
+    if (PrevTryTerminatedBlock)
+      addSuccessor(NewTryTerminatedBlock, PrevTryTerminatedBlock);
+    else
+      addSuccessor(NewTryTerminatedBlock, &cfg->getExit());
+  }
+
+  // The code after the try is the implicit successor.
+  Succ = TrySuccessor;
+
+  // Save the current "try" context.
+  SaveAndRestore<CFGBlock *> SaveTry(TryTerminatedBlock, NewTryTerminatedBlock);
+  cfg->addTryDispatchBlock(TryTerminatedBlock);
+
+  assert(Terminator->getTryBody() && "try must contain a non-NULL body");
+  Block = nullptr;
+  return addStmt(Terminator->getTryBody());
+}
+
 CFGBlock *CFGBuilder::VisitObjCMessageExpr(ObjCMessageExpr *ME,
                                            AddStmtChoice asc) {
   findConstructionContextsForArguments(ME);
 
   autoCreateBlock();
   appendObjCMessage(Block, ME);
 
   return VisitChildren(ME);
CFGBlock *CFGBuilder::VisitCXXTryStmt(CXXTryStmt *Terminator) {
   // "try"/"catch" is a control-flow statement.  Thus we stop processing the
   // current block.
   CFGBlock *TrySuccessor = nullptr;
 
   if (Block) {
     if (badCFG)
       return nullptr;
     TrySuccessor = Block;
-  } else TrySuccessor = Succ;
+  } else
+    TrySuccessor = Succ;
 
   CFGBlock *PrevTryTerminatedBlock = 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 (!HasCatchAll) {
     else
       addSuccessor(NewTryTerminatedBlock, &cfg->getExit());
   }
 
   // The code after the try is the implicit successor.
   Succ = TrySuccessor;
 
   // Save the current "try" context.
-  SaveAndRestore<CFGBlock*> save_try(TryTerminatedBlock, NewTryTerminatedBlock);
+  SaveAndRestore<CFGBlock *> SaveTry(TryTerminatedBlock, NewTryTerminatedBlock);
   cfg->addTryDispatchBlock(TryTerminatedBlock);
 
   assert(Terminator->getTryBlock() && "try must contain a non-NULL body");
   Block = nullptr;
   return addStmt(Terminator->getTryBlock());
 }
 
 CFGBlock *CFGBuilder::VisitCXXCatchStmt(CXXCatchStmt *CS) {
if (Stmt *C = D->getCond())
       C->printPretty(OS, Helper, Policy);
   }
 
   void VisitSwitchStmt(SwitchStmt *Terminator) {
     OS << "switch ";
     Terminator->getCond()->printPretty(OS, Helper, Policy);
   }
 
-  void VisitCXXTryStmt(CXXTryStmt *CS) {
-    OS << "try ...";
-  }
+  void VisitCXXTryStmt(CXXTryStmt *) { OS << "try ..."; }
 
-  void VisitSEHTryStmt(SEHTryStmt *CS) {
-    OS << "__try ...";
-  }
+  void VisitObjCAtTryStmt(ObjCAtTryStmt *) { 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) {
case CFGElement::Kind::MemberDtor: {
     const Type *T = FD->getType()->getBaseElementTypeUnsafe();
     OS << "this->" << FD->getName();
     OS << ".~" << T->getAsCXXRecordDecl()->getName() << "()";
     OS << " (Member object destructor)\n";
     break;
   }
 
   case CFGElement::Kind::TemporaryDtor: {
-    const CXXBindTemporaryExpr *BT = E.castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
+    const CXXBindTemporaryExpr *BT =
+        E.castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
     OS << "~";
     BT->getType().print(OS, PrintingPolicy(Helper.getLangOpts()));
     OS << "() (Temporary object destructor)\n";
     break;
   }
   }
 }
 
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 (ObjCAtCatchStmt *CS = dyn_cast<ObjCAtCatchStmt>(Label)) {
+      OS << "@catch (";
+      if (CS->getCatchParamDecl())
+        CS->getCatchParamDecl()->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.");
 

clang/test/Sema/warn-unreachable.m

+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -verify -Wunreachable-code
+
+void f();
+
+void g1() {
+  @try {
+    f();
+    @throw @"";
+    f();  // expected-warning{{will never be executed}}
+  } @catch(id i) {
+    f();
+  }
+
+  // Completely empty.
+  @try {
+  } @catch(...) {
+  }
+
+  @try {
+    f();
+    return;
+  } @catch(id i = nil) {  // Catch block should not be marked as unreachable.
+    // Empty @catch body.
+  }
+}
+
+void g2() {
+  @try {
+    // Nested @try.
+    @try {
+      f();
+      @throw @"";
+      f(); // expected-warning{{will never be executed}}
+    } @catch(...) {
+    }
+    f();
+    @throw @"";
+    f(); // expected-warning{{will never be executed}}
+  } @catch(...) {
+    f();
+  }
+}
+
+void g3() {
+  @try {
+    @try {
+      f();
+    } @catch (...) {
+      @throw @""; // should exit outer try
+    }
+    @throw @"";
+    f(); // expected-warning{{never be executed}}
+  } @catch (...) {
+  }
+}

clang/test/Sema/warn-unreachable.mm

+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -fcxx-exceptions -verify -Wunreachable-code
+
+void f();
+
+void g3() {
+  try {
+    @try {
+      f();
+      throw 4; // caught by @catch, not by outer c++ catch.
+      f(); // expected-warning {{will never be executed}}
+    } @catch (...) {
+    }
+    f(); // not-unreachable
+  } catch (...) {
+  }
+}

clang/test/SemaObjC/try-catch.m

 @interface XCRefactoringTransformation : NSObject {}
 @end
 
 @implementation XCRefactoringTransformation
 - (NSDictionary *)setUpInfoForTransformKey:(NSString *)transformKey outError:(NSError **)outError {
     @try {}
     // the exception name is optional (weird)
     @catch (NSException *) {}
-}
+} // expected-warning {{non-void function does not return a value}}
+
+- (NSDictionary *)anotherFunction {
+    @try {}
+    @finally {}
+} // FIXME: This should warn about a missing return too.
 @end
 
 int foo() {
   struct s { int a, b; } agg, *pagg;
 
   @throw 42; // expected-error {{@throw requires an Objective-C object type ('int' invalid)}}
   @throw agg; // expected-error {{@throw requires an Objective-C object type ('struct s' invalid)}}
   @throw pagg; // expected-error {{@throw requires an Objective-C object type ('struct s *' invalid)}}
   @throw; // expected-error {{@throw (rethrow) used outside of a @catch block}}
 }