[clang] 04f3079 - [clang] Implement CFG construction for @try and @catch
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 26 06:45:39 PDT 2021
Author: Nico Weber
Date: 2021-10-26T09:45:22-04:00
New Revision: 04f30795f1663843b219393040946136786aa591
URL: https://github.com/llvm/llvm-project/commit/04f30795f1663843b219393040946136786aa591
DIFF: https://github.com/llvm/llvm-project/commit/04f30795f1663843b219393040946136786aa591.diff
LOG: [clang] Implement CFG construction for @try and @catch
@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.
Differential Revision: https://reviews.llvm.org/D112287
Added:
clang/test/Sema/warn-unreachable.m
clang/test/Sema/warn-unreachable.mm
Modified:
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-unreachable.c
clang/test/SemaObjC/try-catch.m
Removed:
################################################################################
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 18598e6eba33..1ac960862ddb 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -482,8 +482,10 @@ class CFGBuilder {
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 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
return VisitObjCAtCatchStmt(cast<ObjCAtCatchStmt>(S));
case Stmt::ObjCAutoreleasePoolStmtClass:
- return VisitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(S));
+ return VisitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(S));
case Stmt::ObjCAtSynchronizedStmtClass:
return VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S));
@@ -3253,8 +3255,7 @@ CFGBlock *CFGBuilder::VisitSEHTryStmt(SEHTryStmt *Terminator) {
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.
@@ -3700,11 +3701,6 @@ CFGBlock *CFGBuilder::VisitObjCAtSynchronizedStmt(ObjCAtSynchronizedStmt *S) {
return addStmt(S->getSynchExpr());
}
-CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) {
- // FIXME
- return NYS();
-}
-
CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
autoCreateBlock();
@@ -3865,16 +3861,37 @@ CFGBlock *CFGBuilder::VisitWhileStmt(WhileStmt *W) {
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;
@@ -3882,14 +3899,78 @@ CFGBlock *CFGBuilder::VisitObjCAtThrowStmt(ObjCAtThrowStmt *S) {
// 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<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);
@@ -4328,7 +4409,8 @@ CFGBlock *CFGBuilder::VisitCXXTryStmt(CXXTryStmt *Terminator) {
if (badCFG)
return nullptr;
TrySuccessor = Block;
- } else TrySuccessor = Succ;
+ } else
+ TrySuccessor = Succ;
CFGBlock *PrevTryTerminatedBlock = TryTerminatedBlock;
@@ -4364,7 +4446,7 @@ CFGBlock *CFGBuilder::VisitCXXTryStmt(CXXTryStmt *Terminator) {
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");
@@ -5317,13 +5399,11 @@ class CFGBlockTerminatorPrint
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 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
}
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";
@@ -5698,6 +5779,13 @@ static void print_block(raw_ostream &OS, const CFG* cfg,
else
OS << "...";
OS << ")";
+ } else if (ObjCAtCatchStmt *CS = dyn_cast<ObjCAtCatchStmt>(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<SEHExceptStmt>(Label)) {
OS << "__except (";
ES->getFilterExpr()->printPretty(OS, &Helper,
diff --git a/clang/test/Sema/warn-unreachable.c b/clang/test/Sema/warn-unreachable.c
index cfac36a13186..e0bd54e497ce 100644
--- 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
index 000000000000..c2f4606d4899
--- /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
index 000000000000..bb573b875c56
--- /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
index 5afbbb6c32e8..04d4ff87ed1d 100644
--- a/clang/test/SemaObjC/try-catch.m
+++ b/clang/test/SemaObjC/try-catch.m
@@ -34,7 +34,12 @@ - (NSDictionary *)setUpInfoForTransformKey:(NSString *)transformKey outError:(NS
@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() {
More information about the cfe-commits
mailing list