[cfe-commits] r52443 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRCoreEngine.h include/clang/Analysis/ProgramPoint.h lib/Analysis/BugReporter.cpp lib/Analysis/GRCoreEngine.cpp lib/Analysis/GRExprEngine.cpp
Ted Kremenek
kremenek at apple.com
Tue Jun 17 22:34:08 PDT 2008
Author: kremenek
Date: Wed Jun 18 00:34:07 2008
New Revision: 52443
URL: http://llvm.org/viewvc/llvm-project?rev=52443&view=rev
Log:
Added a new ProgramPoint: PostPurgeDeadSymbols. This new program point distinguishes between the cases when we just evaluated the transfer function of a Stmt* (PostStmt) or performed a load (PostLoad). This solves a caching bug observed in a recent bug report.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/GRCoreEngine.h
cfe/trunk/include/clang/Analysis/ProgramPoint.h
cfe/trunk/lib/Analysis/BugReporter.cpp
cfe/trunk/lib/Analysis/GRCoreEngine.cpp
cfe/trunk/lib/Analysis/GRExprEngine.cpp
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRCoreEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRCoreEngine.h?rev=52443&r1=52442&r2=52443&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRCoreEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRCoreEngine.h Wed Jun 18 00:34:07 2008
@@ -149,20 +149,25 @@
unsigned getCurrentBlockCount() const {
return getBlockCounter().getNumVisited(B.getBlockID());
}
-
- ExplodedNodeImpl* generateNodeImpl(Stmt* S, void* State,
- ExplodedNodeImpl* Pred,
- bool isLoad = false);
-
- inline ExplodedNodeImpl* generateNodeImpl(Stmt* S, void* State,
- bool isLoad = false) {
+
+ ExplodedNodeImpl*
+ generateNodeImpl(Stmt* S, void* State, ExplodedNodeImpl* Pred,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind);
+
+ ExplodedNodeImpl*
+ generateNodeImpl(Stmt* S, void* State,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind) {
ExplodedNodeImpl* N = getLastNode();
assert (N && "Predecessor of new node is infeasible.");
- return generateNodeImpl(S, State, N, isLoad);
+ return generateNodeImpl(S, State, N, K);
}
+ /// getStmt - Return the current block-level expression associated with
+ /// this builder.
Stmt* getStmt() const { return B[Idx]; }
+ /// getBlock - Return the CFGBlock associated with the block-level expression
+ /// of this builder.
CFGBlock* getBlock() const { return &B; }
};
@@ -182,6 +187,7 @@
GRStmtNodeBuilder(GRStmtNodeBuilderImpl& nb) : NB(nb),
CallExprAuditBeg(0), CallExprAuditEnd(0),
ObjCMsgExprAuditBeg(0), ObjCMsgExprAuditEnd(0),
+ PurgingDeadSymbols(false),
BuildSinks(false), HasGeneratedNode(false) {
CleanedState = getLastNode()->getState();
@@ -203,14 +209,22 @@
return static_cast<NodeTy*>(NB.getLastNode());
}
- NodeTy* generateNode(Stmt* S, StateTy* St, NodeTy* Pred, bool isLoad = false){
+ NodeTy*
+ generateNode(Stmt* S, StateTy* St, NodeTy* Pred,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind) {
+
HasGeneratedNode = true;
- return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, Pred, isLoad));
+ if (PurgingDeadSymbols) K = ProgramPoint::PostPurgeDeadSymbolsKind;
+ return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, Pred, K));
}
- NodeTy* generateNode(Stmt* S, StateTy* St, bool isLoad = false) {
+ NodeTy*
+ generateNode(Stmt* S, StateTy* St,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind) {
+
HasGeneratedNode = true;
- return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, isLoad));
+ if (PurgingDeadSymbols) K = ProgramPoint::PostPurgeDeadSymbolsKind;
+ return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, K));
}
GRBlockCounter getBlockCounter() const {
@@ -274,6 +288,7 @@
return N;
}
+ bool PurgingDeadSymbols;
bool BuildSinks;
bool HasGeneratedNode;
};
@@ -338,7 +353,7 @@
return getPredecessor()->getState();
}
- inline NodeTy* generateNode(StateTy* St, bool branch) {
+ NodeTy* generateNode(StateTy* St, bool branch) {
return static_cast<NodeTy*>(NB.generateNodeImpl(St, branch));
}
@@ -350,7 +365,7 @@
return NB.getTargetBlock(branch);
}
- inline void markInfeasible(bool branch) {
+ void markInfeasible(bool branch) {
NB.markInfeasible(branch);
}
};
@@ -393,8 +408,8 @@
ExplodedNodeImpl* generateNodeImpl(const Iterator& I, void* State,
bool isSink);
- inline Expr* getTarget() const { return E; }
- inline void* getState() const { return Pred->State; }
+ Expr* getTarget() const { return E; }
+ void* getState() const { return Pred->State; }
};
template<typename STATE>
@@ -410,16 +425,16 @@
typedef GRIndirectGotoNodeBuilderImpl::Iterator iterator;
- inline iterator begin() { return NB.begin(); }
- inline iterator end() { return NB.end(); }
+ iterator begin() { return NB.begin(); }
+ iterator end() { return NB.end(); }
- inline Expr* getTarget() const { return NB.getTarget(); }
+ Expr* getTarget() const { return NB.getTarget(); }
- inline NodeTy* generateNode(const iterator& I, StateTy* St, bool isSink=false){
+ NodeTy* generateNode(const iterator& I, StateTy* St, bool isSink=false){
return static_cast<NodeTy*>(NB.generateNodeImpl(I, St, isSink));
}
- inline StateTy* getState() const {
+ StateTy* getState() const {
return static_cast<StateTy*>(NB.getState());
}
};
@@ -459,8 +474,8 @@
ExplodedNodeImpl* generateCaseStmtNodeImpl(const Iterator& I, void* State);
ExplodedNodeImpl* generateDefaultCaseNodeImpl(void* State, bool isSink);
- inline Expr* getCondition() const { return Condition; }
- inline void* getState() const { return Pred->State; }
+ Expr* getCondition() const { return Condition; }
+ void* getState() const { return Pred->State; }
};
template<typename STATE>
@@ -476,20 +491,20 @@
typedef GRSwitchNodeBuilderImpl::Iterator iterator;
- inline iterator begin() { return NB.begin(); }
- inline iterator end() { return NB.end(); }
+ iterator begin() { return NB.begin(); }
+ iterator end() { return NB.end(); }
- inline Expr* getCondition() const { return NB.getCondition(); }
+ Expr* getCondition() const { return NB.getCondition(); }
- inline NodeTy* generateCaseStmtNode(const iterator& I, StateTy* St) {
+ NodeTy* generateCaseStmtNode(const iterator& I, StateTy* St) {
return static_cast<NodeTy*>(NB.generateCaseStmtNodeImpl(I, St));
}
- inline NodeTy* generateDefaultCaseNode(StateTy* St, bool isSink = false) {
+ NodeTy* generateDefaultCaseNode(StateTy* St, bool isSink = false) {
return static_cast<NodeTy*>(NB.generateDefaultCaseNodeImpl(St, isSink));
}
- inline StateTy* getState() const {
+ StateTy* getState() const {
return static_cast<StateTy*>(NB.getState());
}
};
Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=52443&r1=52442&r2=52443&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed Jun 18 00:34:07 2008
@@ -25,9 +25,10 @@
class ProgramPoint {
public:
- enum Kind { BlockEntranceKind=0, PostStmtKind=1, PostLoadKind=2,
- BlockExitKind=3, BlockEdgeSrcKind=4, BlockEdgeDstKind=5,
- BlockEdgeAuxKind=6 };
+ enum Kind { BlockEntranceKind=0,
+ PostStmtKind=1, PostLoadKind=2, PostPurgeDeadSymbolsKind=3,
+ BlockExitKind=4, BlockEdgeSrcKind=5, BlockEdgeDstKind=6,
+ BlockEdgeAuxKind=7 };
protected:
uintptr_t Data;
@@ -110,7 +111,7 @@
static bool classof(const ProgramPoint* Location) {
unsigned k = Location->getKind();
- return k == PostStmtKind || k == PostLoadKind;
+ return k >= PostStmtKind && k <= PostPurgeDeadSymbolsKind;
}
};
@@ -123,6 +124,15 @@
}
};
+class PostPurgeDeadSymbols : public PostStmt {
+public:
+ PostPurgeDeadSymbols(const Stmt* S) : PostStmt(S, PostPurgeDeadSymbolsKind) {}
+
+ static bool classof(const ProgramPoint* Location) {
+ return Location->getKind() == PostPurgeDeadSymbolsKind;
+ }
+};
+
class BlockEdge : public ProgramPoint {
typedef std::pair<CFGBlock*,CFGBlock*> BPair;
public:
Modified: cfe/trunk/lib/Analysis/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BugReporter.cpp?rev=52443&r1=52442&r2=52443&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BugReporter.cpp (original)
+++ cfe/trunk/lib/Analysis/BugReporter.cpp Wed Jun 18 00:34:07 2008
@@ -21,7 +21,7 @@
#include "clang/AST/Expr.h"
#include "clang/Analysis/ProgramPoint.h"
#include "clang/Analysis/PathDiagnostic.h"
-#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/DenseMap.h"
#include <sstream>
using namespace clang;
@@ -204,43 +204,86 @@
G = new ExplodedGraph<ValueState>(GTrim->getCFG(), GTrim->getCodeDecl(),
GTrim->getContext());
-
- ExplodedNode<ValueState> *Last = 0, *First = 0;
-
- // Sometimes TrimGraph can contain a cycle because there are multiple BFS
- // paths with the same length. We mark the nodes we visit so that we
- // don't get stuck in a cycle.
- llvm::DenseSet<void*> Visited;
+ // Sometimes TrimGraph can contain a cycle. Perform a reverse DFS
+ // to the root node, and then construct a new graph that contains only
+ // a single path.
+ llvm::DenseMap<void*,unsigned> Visited;
+ llvm::SmallVector<ExplodedNode<ValueState>*, 10> WS;
+ WS.push_back(N);
+ unsigned cnt = 0;
+ ExplodedNode<ValueState>* Root = 0;
+
+ while (!WS.empty()) {
+ ExplodedNode<ValueState>* Node = WS.back();
+ WS.pop_back();
+
+ if (Visited.find(Node) != Visited.end())
+ continue;
+
+ Visited[Node] = cnt++;
+
+ if (Node->pred_empty()) {
+ Root = Node;
+ break;
+ }
+
+ for (ExplodedNode<ValueState>::pred_iterator I=Node->pred_begin(),
+ E=Node->pred_end(); I!=E; ++I)
+ WS.push_back(*I);
+ }
- while (N) {
- Visited.insert(N);
+ assert (Root);
+
+ // Now walk from the root down the DFS path, always taking the successor
+ // with the lowest number.
+ ExplodedNode<ValueState> *Last = 0, *First = 0;
- ExplodedNode<ValueState>* NewN =
- G->getNode(N->getLocation(), N->getState());
+ for ( N = Root ;;) {
- if (!First) First = NewN;
- if (Last) Last->addPredecessor(NewN);
+ // Lookup the number associated with the current node.
+ llvm::DenseMap<void*,unsigned>::iterator I=Visited.find(N);
+ assert (I != Visited.end());
- Last = NewN;
+ // Create the equivalent node in the new graph with the same state
+ // and location.
+ ExplodedNode<ValueState>* NewN =
+ G->getNode(N->getLocation(), N->getState());
+
+ // Link up the new node with the previous node.
+ if (Last)
+ NewN->addPredecessor(Last);
- if (N->pred_empty())
+ Last = NewN;
+
+ // Are we at the final node?
+ if (I->second == 0) {
+ First = NewN;
break;
-
- ExplodedNode<ValueState>::pred_iterator PI = N->pred_begin();
- ExplodedNode<ValueState>::pred_iterator PE = N->pred_end();
+ }
+
+ // Find the next successor node. We choose the node that is marked
+ // with the lowest DFS number.
+ ExplodedNode<ValueState>::succ_iterator SI = N->succ_begin();
+ ExplodedNode<ValueState>::succ_iterator SE = N->succ_end();
N = 0;
- // Look for the first predecessor that we have not already visited.
+ for (unsigned MinVal = 0; SI != SE; ++SI) {
- for (; PI != PE; ++PI)
- if (!Visited.count(*PI)) {
- N = *PI;
- break;
+ I = Visited.find(*SI);
+
+ if (I == Visited.end())
+ continue;
+
+ if (!N || I->second < MinVal) {
+ N = *SI;
+ MinVal = I->second;
}
-
- assert (N && "All predecessors involved in a cycle!");
+ }
+
+ assert (N);
}
-
+
+ assert (First);
return std::make_pair(G, First);
}
Modified: cfe/trunk/lib/Analysis/GRCoreEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRCoreEngine.cpp?rev=52443&r1=52442&r2=52443&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRCoreEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRCoreEngine.cpp Wed Jun 18 00:34:07 2008
@@ -317,12 +317,30 @@
Eng.WList->Enqueue(Succ, B, Idx+1);
}
+static inline ProgramPoint GetPostLoc(Stmt* S, ProgramPoint::Kind K) {
+ switch (K) {
+ default:
+ assert(false && "Invalid PostXXXKind.");
+
+ case ProgramPoint::PostStmtKind:
+ return PostStmt(S);
+
+ case ProgramPoint::PostLoadKind:
+ return PostLoad(S);
+
+ case ProgramPoint::PostPurgeDeadSymbolsKind:
+ return PostPurgeDeadSymbols(S);
+ }
+}
+
ExplodedNodeImpl*
GRStmtNodeBuilderImpl::generateNodeImpl(Stmt* S, void* State,
- ExplodedNodeImpl* Pred, bool isLoad) {
+ ExplodedNodeImpl* Pred,
+ ProgramPoint::Kind K) {
bool IsNew;
- ProgramPoint Loc = isLoad ? PostLoad(S) : PostStmt(S);
+ ProgramPoint Loc = GetPostLoc(S, K);
+
ExplodedNodeImpl* N = Eng.G->getNodeImpl(Loc, State, &IsNew);
N->addPredecessor(Pred);
Deferred.erase(Pred);
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=52443&r1=52442&r2=52443&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Wed Jun 18 00:34:07 2008
@@ -214,6 +214,9 @@
SaveAndRestore<bool> OldSink(Builder->BuildSinks);
SaveOr OldHasGen(Builder->HasGeneratedNode);
+ SaveAndRestore<bool> OldPurgeDeadSymbols(Builder->PurgingDeadSymbols);
+ Builder->PurgingDeadSymbols = true;
+
TF->EvalDeadSymbols(Tmp, *this, *Builder, EntryNode, S,
CleanedState, DeadSymbols);
@@ -965,7 +968,10 @@
// Check for loads/stores from/to undefined values.
if (location.isUndef()) {
- if (NodeTy* Succ = Builder->generateNode(Ex, St, Pred, isLoad)) {
+ ProgramPoint::Kind K =
+ isLoad ? ProgramPoint::PostLoadKind : ProgramPoint::PostStmtKind;
+
+ if (NodeTy* Succ = Builder->generateNode(Ex, St, Pred, K)) {
Succ->markAsSink();
UndefDeref.insert(Succ);
}
@@ -1000,7 +1006,10 @@
// We don't use "MakeNode" here because the node will be a sink
// and we have no intention of processing it later.
- NodeTy* NullNode = Builder->generateNode(Ex, StNull, Pred, isLoad);
+ ProgramPoint::Kind K =
+ isLoad ? ProgramPoint::PostLoadKind : ProgramPoint::PostStmtKind;
+
+ NodeTy* NullNode = Builder->generateNode(Ex, StNull, Pred, K);
if (NullNode) {
@@ -2437,6 +2446,7 @@
break;
case ProgramPoint::PostLoadKind:
+ case ProgramPoint::PostPurgeDeadSymbolsKind:
case ProgramPoint::PostStmtKind: {
const PostStmt& L = cast<PostStmt>(Loc);
Stmt* S = L.getStmt();
More information about the cfe-commits
mailing list