Index: clang/lib/Analysis/CFG.cpp =================================================================== --- clang/lib/Analysis/CFG.cpp +++ clang/lib/Analysis/CFG.cpp @@ -482,8 +482,10 @@ 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. @@ -2286,7 +2288,7 @@ return VisitObjCAtCatchStmt(cast(S)); case Stmt::ObjCAutoreleasePoolStmtClass: - return VisitObjCAutoreleasePoolStmt(cast(S)); + return VisitObjCAutoreleasePoolStmt(cast(S)); case Stmt::ObjCAtSynchronizedStmtClass: return VisitObjCAtSynchronizedStmt(cast(S)); @@ -3700,11 +3702,6 @@ return addStmt(S->getSynchExpr()); } -CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) { - // FIXME - return NYS(); -} - CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) { autoCreateBlock(); @@ -3865,10 +3862,35 @@ 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 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) { @@ -3882,14 +3904,77 @@ // Create the new block. Block = createBlock(false); - // The Exit block is the only successor. - addSuccessor(Block, &cfg->getExit()); + 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 save_try(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); @@ -5317,10 +5402,14 @@ Terminator->getCond()->printPretty(OS, Helper, Policy); } - void VisitCXXTryStmt(CXXTryStmt *CS) { + void VisitCXXTryStmt(CXXTryStmt *) { OS << "try ..."; } + void VisitObjCAtTryStmt(ObjCAtTryStmt *) { + OS << "@try ..."; + } + void VisitSEHTryStmt(SEHTryStmt *CS) { OS << "__try ..."; } @@ -5639,7 +5728,8 @@ } case CFGElement::Kind::TemporaryDtor: { - const CXXBindTemporaryExpr *BT = E.castAs().getBindTemporaryExpr(); + const CXXBindTemporaryExpr *BT = + E.castAs().getBindTemporaryExpr(); OS << "~"; BT->getType().print(OS, PrintingPolicy(Helper.getLangOpts())); OS << "() (Temporary object destructor)\n"; @@ -5701,6 +5791,14 @@ else OS << "..."; OS << ")"; + } else if (ObjCAtCatchStmt *CS = dyn_cast(Label)) { + OS << "@catch ("; + if (CS->getCatchParamDecl()) + CS->getCatchParamDecl()->print(OS, PrintingPolicy(Helper.getLangOpts()), + 0); + else + OS << "..."; + OS << ")"; } else if (SEHExceptStmt *ES = dyn_cast(Label)) { OS << "__except ("; ES->getFilterExpr()->printPretty(OS, &Helper, Index: clang/test/Sema/warn-unreachable.m =================================================================== --- /dev/null +++ clang/test/Sema/warn-unreachable.m @@ -0,0 +1,55 @@ +// 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 (...) { + } +} Index: clang/test/Sema/warn-unreachable.mm =================================================================== --- /dev/null +++ clang/test/Sema/warn-unreachable.mm @@ -0,0 +1,16 @@ +// 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 (...) { + } +} Index: clang/test/SemaObjC/try-catch.m =================================================================== --- clang/test/SemaObjC/try-catch.m +++ clang/test/SemaObjC/try-catch.m @@ -34,7 +34,12 @@ @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() {