diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp --- a/clang/lib/Analysis/CFG.cpp +++ b/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 a C++ try, an Objective-C @try, or an 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)); @@ -3253,8 +3255,7 @@ Succ = SEHTrySuccessor; // Save the current "__try" context. - SaveAndRestore save_try(TryTerminatedBlock, - NewTryTerminatedBlock); + SaveAndRestore SaveTry(TryTerminatedBlock, NewTryTerminatedBlock); cfg->addTryDispatchBlock(TryTerminatedBlock); // Save the current value for the __leave target. @@ -3700,11 +3701,6 @@ return addStmt(S->getSynchExpr()); } -CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) { - // FIXME - return NYS(); -} - CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) { autoCreateBlock(); @@ -3865,16 +3861,37 @@ 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) { - // 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; @@ -3882,14 +3899,78 @@ // 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 I = 0, E = Terminator->getNumCatchStmts(); I != E; ++I) { + // The code after the try is the implicit successor. + Succ = TrySuccessor; + ObjCAtCatchStmt *CS = Terminator->getCatchStmt(I); + 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 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); @@ -4328,7 +4409,8 @@ if (badCFG) return nullptr; TrySuccessor = Block; - } else TrySuccessor = Succ; + } else + TrySuccessor = Succ; CFGBlock *PrevTryTerminatedBlock = TryTerminatedBlock; @@ -4364,7 +4446,7 @@ Succ = TrySuccessor; // Save the current "try" context. - SaveAndRestore save_try(TryTerminatedBlock, NewTryTerminatedBlock); + SaveAndRestore SaveTry(TryTerminatedBlock, NewTryTerminatedBlock); cfg->addTryDispatchBlock(TryTerminatedBlock); assert(Terminator->getTryBlock() && "try must contain a non-NULL body"); @@ -5317,13 +5399,11 @@ 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()) @@ -5639,7 +5719,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"; @@ -5698,6 +5779,13 @@ else OS << "..."; OS << ")"; + } else if (ObjCAtCatchStmt *CS = dyn_cast(Label)) { + OS << "@catch ("; + if (const VarDecl *PD = CS->getCatchParamDecl()) + PD->print(OS, PrintingPolicy(Helper.getLangOpts()), 0); + else + OS << "..."; + OS << ")"; } else if (SEHExceptStmt *ES = dyn_cast(Label)) { OS << "__except ("; ES->getFilterExpr()->printPretty(OS, &Helper, diff --git a/clang/test/Sema/warn-unreachable.c b/clang/test/Sema/warn-unreachable.c --- a/clang/test/Sema/warn-unreachable.c +++ b/clang/test/Sema/warn-unreachable.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -I %S/Inputs +// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -I %S/Inputs %s // RUN: %clang_cc1 -fsyntax-only -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -fdiagnostics-parseable-fixits -I %S/Inputs %s 2>&1 | FileCheck %s #include "warn-unreachable.h" diff --git a/clang/test/Sema/warn-unreachable.m b/clang/test/Sema/warn-unreachable.m new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unreachable.m @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -fsyntax-only -fobjc-exceptions -verify -Wunreachable-code %s + +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 (...) { + } +} diff --git a/clang/test/Sema/warn-unreachable.mm b/clang/test/Sema/warn-unreachable.mm new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unreachable.mm @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -fsyntax-only -fobjc-exceptions -fcxx-exceptions -verify -Wunreachable-code %s + +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 (...) { + } +} diff --git a/clang/test/SemaObjC/try-catch.m b/clang/test/SemaObjC/try-catch.m --- a/clang/test/SemaObjC/try-catch.m +++ b/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() {