[cfe-commits] r126797 - in /cfe/trunk: lib/Analysis/CFG.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/misc-ps.m test/SemaCXX/array-bounds.cpp
Ted Kremenek
kremenek at apple.com
Tue Mar 1 15:12:55 PST 2011
Author: kremenek
Date: Tue Mar 1 17:12:55 2011
New Revision: 126797
URL: http://llvm.org/viewvc/llvm-project?rev=126797&view=rev
Log:
Teach CFGBuilder to prune trivially unreachable case statements.
Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/misc-ps.m
cfe/trunk/test/SemaCXX/array-bounds.cpp
Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=126797&r1=126796&r2=126797&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Tue Mar 1 17:12:55 2011
@@ -210,6 +210,25 @@
LocalScope::const_iterator scopePosition;
};
+/// TryResult - a class representing a variant over the values
+/// 'true', 'false', or 'unknown'. This is returned by tryEvaluateBool,
+/// and is used by the CFGBuilder to decide if a branch condition
+/// can be decided up front during CFG construction.
+class TryResult {
+ int X;
+public:
+ TryResult(bool b) : X(b ? 1 : 0) {}
+ TryResult() : X(-1) {}
+
+ bool isTrue() const { return X == 1; }
+ bool isFalse() const { return X == 0; }
+ bool isKnown() const { return X >= 0; }
+ void negate() {
+ assert(isKnown());
+ X ^= 0x1;
+ }
+};
+
/// CFGBuilder - This class implements CFG construction from an AST.
/// The builder is stateful: an instance of the builder should be used to only
/// construct a single CFG.
@@ -238,7 +257,7 @@
CFGBlock* SwitchTerminatedBlock;
CFGBlock* DefaultCaseBlock;
CFGBlock* TryTerminatedBlock;
-
+
// Current position in local scope.
LocalScope::const_iterator ScopePos;
@@ -257,12 +276,17 @@
bool badCFG;
CFG::BuildOptions BuildOpts;
+
+ // State to track for building switch statements.
+ bool switchExclusivelyCovered;
+ Expr::EvalResult switchCond;
public:
explicit CFGBuilder() : cfg(new CFG()), // crew a new CFG
Block(NULL), Succ(NULL),
SwitchTerminatedBlock(NULL), DefaultCaseBlock(NULL),
- TryTerminatedBlock(NULL), badCFG(false) {}
+ TryTerminatedBlock(NULL), badCFG(false),
+ switchExclusivelyCovered(false) {}
// buildCFG - Used by external clients to construct the CFG.
CFG* buildCFG(const Decl *D, Stmt *Statement, ASTContext *C,
@@ -387,45 +411,34 @@
B->addSuccessor(S, cfg->getBumpVectorContext());
}
- /// TryResult - a class representing a variant over the values
- /// 'true', 'false', or 'unknown'. This is returned by tryEvaluateBool,
- /// and is used by the CFGBuilder to decide if a branch condition
- /// can be decided up front during CFG construction.
- class TryResult {
- int X;
- public:
- TryResult(bool b) : X(b ? 1 : 0) {}
- TryResult() : X(-1) {}
-
- bool isTrue() const { return X == 1; }
- bool isFalse() const { return X == 0; }
- bool isKnown() const { return X >= 0; }
- void negate() {
- assert(isKnown());
- X ^= 0x1;
- }
- };
+ /// Try and evaluate an expression to an integer constant.
+ bool tryEvaluate(Expr *S, Expr::EvalResult &outResult) {
+ if (!BuildOpts.PruneTriviallyFalseEdges)
+ return false;
+ return !S->isTypeDependent() &&
+ !S->isValueDependent() &&
+ S->Evaluate(outResult, *Context);
+ }
/// tryEvaluateBool - Try and evaluate the Stmt and return 0 or 1
/// if we can evaluate to a known value, otherwise return -1.
TryResult tryEvaluateBool(Expr *S) {
- if (!BuildOpts.PruneTriviallyFalseEdges)
+ Expr::EvalResult Result;
+ if (!tryEvaluate(S, Result))
return TryResult();
+
+ if (Result.Val.isInt())
+ return Result.Val.getInt().getBoolValue();
- Expr::EvalResult Result;
- if (!S->isTypeDependent() && !S->isValueDependent() &&
- S->Evaluate(Result, *Context)) {
- if (Result.Val.isInt())
- return Result.Val.getInt().getBoolValue();
- if (Result.Val.isLValue()) {
- Expr *e = Result.Val.getLValueBase();
- const CharUnits &c = Result.Val.getLValueOffset();
- if (!e && c.isZero())
- return false;
- }
+ if (Result.Val.isLValue()) {
+ Expr *e = Result.Val.getLValueBase();
+ const CharUnits &c = Result.Val.getLValueOffset();
+ if (!e && c.isZero())
+ return false;
}
return TryResult();
}
+
};
// FIXME: Add support for dependent-sized array types in C++?
@@ -2180,6 +2193,17 @@
assert(Terminator->getBody() && "switch must contain a non-NULL body");
Block = NULL;
+ // For pruning unreachable case statements, save the current state
+ // for tracking the condition value.
+ SaveAndRestore<bool> save_switchExclusivelyCovered(switchExclusivelyCovered,
+ false);
+ SaveAndRestore<Expr::EvalResult> save_switchCond(switchCond);
+
+
+ // Determine if the switch condition can be explicitly evaluated.
+ assert(Terminator->getCond() && "switch condition must be non-NULL");
+ tryEvaluate(Terminator->getCond(), switchCond);
+
// If body is not a compound statement create implicit scope
// and add destructors.
if (!isa<CompoundStmt>(Terminator->getBody()))
@@ -2193,11 +2217,11 @@
// If we have no "default:" case, the default transition is to the code
// following the switch body.
- addSuccessor(SwitchTerminatedBlock, DefaultCaseBlock);
+ addSuccessor(SwitchTerminatedBlock,
+ switchExclusivelyCovered ? 0 : DefaultCaseBlock);
// Add the terminator and condition in the switch block.
SwitchTerminatedBlock->setTerminator(Terminator);
- assert(Terminator->getCond() && "switch condition must be non-NULL");
Block = SwitchTerminatedBlock;
Block = addStmt(Terminator->getCond());
@@ -2213,6 +2237,44 @@
return Block;
}
+
+static bool shouldAddCase(bool &switchExclusivelyCovered,
+ const Expr::EvalResult &switchCond,
+ const CaseStmt *CS,
+ ASTContext &Ctx) {
+ bool addCase = false;
+
+ if (!switchExclusivelyCovered) {
+ if (switchCond.Val.isInt()) {
+ // Evaluate the LHS of the case value.
+ Expr::EvalResult V1;
+ CS->getLHS()->Evaluate(V1, Ctx);
+ assert(V1.Val.isInt());
+ const llvm::APSInt &condInt = switchCond.Val.getInt();
+ const llvm::APSInt &lhsInt = V1.Val.getInt();
+
+ if (condInt == lhsInt) {
+ addCase = true;
+ switchExclusivelyCovered = true;
+ }
+ else if (condInt < lhsInt) {
+ if (const Expr *RHS = CS->getRHS()) {
+ // Evaluate the RHS of the case value.
+ Expr::EvalResult V2;
+ RHS->Evaluate(V2, Ctx);
+ assert(V2.Val.isInt());
+ if (V2.Val.getInt() <= condInt) {
+ addCase = true;
+ switchExclusivelyCovered = true;
+ }
+ }
+ }
+ }
+ else
+ addCase = true;
+ }
+ return addCase;
+}
CFGBlock* CFGBuilder::VisitCaseStmt(CaseStmt* CS) {
// CaseStmts are essentially labels, so they are the first statement in a
@@ -2232,9 +2294,12 @@
else
TopBlock = currentBlock;
- addSuccessor(SwitchTerminatedBlock, currentBlock);
- LastBlock = currentBlock;
+ addSuccessor(SwitchTerminatedBlock,
+ shouldAddCase(switchExclusivelyCovered, switchCond,
+ CS, *Context)
+ ? currentBlock : 0);
+ LastBlock = currentBlock;
CS = cast<CaseStmt>(Sub);
Sub = CS->getSubStmt();
}
@@ -2256,7 +2321,10 @@
// Add this block to the list of successors for the block with the switch
// statement.
assert(SwitchTerminatedBlock);
- addSuccessor(SwitchTerminatedBlock, CaseBlock);
+ addSuccessor(SwitchTerminatedBlock,
+ shouldAddCase(switchExclusivelyCovered, switchCond,
+ CS, *Context)
+ ? CaseBlock : 0);
// We set Block to NULL to allow lazy creation of a new block (if necessary)
Block = NULL;
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=126797&r1=126796&r2=126797&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Mar 1 17:12:55 2011
@@ -1043,6 +1043,10 @@
bool defaultIsFeasible = I == EI;
for ( ; I != EI; ++I) {
+ // Successor may be pruned out during CFG construction.
+ if (!I.getBlock())
+ continue;
+
const CaseStmt* Case = I.getCase();
// Evaluate the LHS of the case value.
Modified: cfe/trunk/test/Analysis/misc-ps.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=126797&r1=126796&r2=126797&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.m (original)
+++ cfe/trunk/test/Analysis/misc-ps.m Tue Mar 1 17:12:55 2011
@@ -1269,3 +1269,22 @@
}
}
+void test_switch() {
+ switch (4) {
+ case 1: {
+ int *p = 0;
+ *p = 0xDEADBEEF; // no-warning
+ break;
+ }
+ case 4: {
+ int *p = 0;
+ *p = 0xDEADBEEF; // expected-warning {{null}}
+ break;
+ }
+ default: {
+ int *p = 0;
+ *p = 0xDEADBEEF; // no-warning
+ break;
+ }
+ }
+}
\ No newline at end of file
Modified: cfe/trunk/test/SemaCXX/array-bounds.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/array-bounds.cpp?rev=126797&r1=126796&r2=126797&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/array-bounds.cpp (original)
+++ cfe/trunk/test/SemaCXX/array-bounds.cpp Tue Mar 1 17:12:55 2011
@@ -127,4 +127,23 @@
return sizeof(char) == sizeof(char) ? arr[2] : arr[1]; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}}
}
+void test_switch() {
+ switch (4) {
+ case 1: {
+ int arr[2];
+ arr[2] = 1; // no-warning
+ break;
+ }
+ case 4: {
+ int arr[2]; // expected-note {{array 'arr' declared here}}
+ arr[2] = 1; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}}
+ break;
+ }
+ default: {
+ int arr[2];
+ arr[2] = 1; // no-warning
+ break;
+ }
+ }
+}
More information about the cfe-commits
mailing list