[cfe-commits] r142444 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp lib/StaticAnalyzer/Core/CoreEngine.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp

Anna Zaks ganna at apple.com
Tue Oct 18 16:06:04 PDT 2011


Author: zaks
Date: Tue Oct 18 18:06:04 2011
New Revision: 142444

URL: http://llvm.org/viewvc/llvm-project?rev=142444&view=rev
Log:
[analyzer] NodeBuilder Refactoring: Subclass BranchNodeBuilder from NodeBuilder.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
    cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Tue Oct 18 18:06:04 2011
@@ -30,6 +30,8 @@
   const ProgramPoint Location;
   const ProgramState *ST;
   const unsigned size;
+  // TODO: Use global context.
+  NodeBuilderContext Ctx;
   NodeBuilder NB;
 public:
   bool *respondsToCallback;
@@ -48,7 +50,8 @@
       Location(loc),
       ST(st),
       size(Dst.size()),
-      NB(builder.Eng, pred),
+      Ctx(builder.Eng, pred, builder.getBlock()),
+      NB(Ctx),
       respondsToCallback(respondsToCB) {
     assert(!(ST && ST != Pred->getState()));
   }

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h Tue Oct 18 18:06:04 2011
@@ -27,6 +27,8 @@
   
 namespace ento {
 
+class NodeBuilder;
+
 //===----------------------------------------------------------------------===//
 /// CoreEngine - Implements the core logic of the graph-reachability
 ///   analysis. It traverses the CFG and generates the ExplodedGraph.
@@ -161,14 +163,25 @@
   BlocksAborted::const_iterator blocks_aborted_end() const {
     return blocksAborted.end();
   }
+
+  /// Enqueue the results of the node builder onto the work list.
+  void enqueue(NodeBuilder &NB);
+};
+
+struct NodeBuilderContext {
+  CoreEngine &Eng;
+  ExplodedNode *Pred;
+  const CFGBlock *Block;
+  NodeBuilderContext(CoreEngine &E, ExplodedNode *N, const CFGBlock *B)
+    : Eng(E), Pred(N), Block(B) { assert(B); assert(!N->isSink()); }
 };
 
 /// This is the simplest builder which generates nodes in the ExplodedGraph.
 class NodeBuilder {
+protected:
   friend class StmtNodeBuilder;
 
-  CoreEngine& Eng;
-  ExplodedNode *Pred;
+  NodeBuilderContext &C;
   bool Finalized;
 
   /// \brief The frontier set - a set of nodes which need to be propagated after
@@ -176,13 +189,19 @@
   typedef llvm::SmallPtrSet<ExplodedNode*,5> DeferredTy;
   DeferredTy Deferred;
 
-  BlockCounter getBlockCounter() const { return Eng.WList->getBlockCounter(); }
+  BlockCounter getBlockCounter() const { return C.Eng.WList->getBlockCounter();}
+
+  bool checkResults() {
+    if (!Finalized)
+      return false;
+    for (DeferredTy::iterator I=Deferred.begin(), E=Deferred.end(); I!=E; ++I)
+      if ((*I)->isSink())
+        return false;
+    return true;
+  }
 
   virtual void finalizeResults() {
     if (!Finalized) {
-      // TODO: Remove assert after refactoring is complete?
-      for (DeferredTy::iterator I=Deferred.begin(), E=Deferred.end(); I!=E; ++I)
-        assert(!(*I)->isSink());
       Finalized = true;
     }
   }
@@ -193,8 +212,10 @@
                                  bool MarkAsSink = false);
 
 public:
-  NodeBuilder(CoreEngine& e, ExplodedNode *pred);
-  ~NodeBuilder() {}
+  NodeBuilder(NodeBuilderContext &Ctx)
+    : C(Ctx), Finalized(false) { Deferred.insert(C.Pred); }
+
+  virtual ~NodeBuilder() {}
 
   /// \brief Generates a node in the ExplodedGraph.
   ///
@@ -208,17 +229,33 @@
   }
 
   // \brief Get the builder's predecessor - the parent to all the other nodes.
-  const ExplodedNode* getPred() const { return Pred; }
+  const ExplodedNode *getPred() const { return C.Pred; }
 
   bool hasGeneratedNodes() const {
-    return (!Deferred.count(Pred));
+    return (!Deferred.count(C.Pred));
   }
 
   typedef DeferredTy::iterator iterator;
   /// \brief Iterators through the results frontier.
-  inline iterator results_begin() { finalizeResults(); return Deferred.begin();}
-  inline iterator results_end() { finalizeResults(); return Deferred.end(); }
+  inline iterator results_begin() {
+    finalizeResults();
+    assert(checkResults());
+    return Deferred.begin();
+  }
+  inline iterator results_end() {
+    finalizeResults();
+    return Deferred.end();
+  }
 
+  /// \brief Returns the number of times the current basic block has been
+  /// visited on the exploded graph path.
+  unsigned getCurrentBlockCount() const {
+    return getBlockCounter().getNumVisited(
+                         C.Pred->getLocationContext()->getCurrentStackFrame(),
+                         C.Block->getBlockID());
+  }
+
+  ExplodedNode *getPredecessor() const { return C.Pred; }
 };
 
 class CommonNodeBuilder {
@@ -351,38 +388,40 @@
   }
 };
 
-class BranchNodeBuilder: public CommonNodeBuilder {
-  const CFGBlock *Src;
+class BranchNodeBuilder: public NodeBuilder {
   const CFGBlock *DstT;
   const CFGBlock *DstF;
 
-  typedef SmallVector<ExplodedNode*,3> DeferredTy;
-  DeferredTy Deferred;
-
   bool GeneratedTrue;
   bool GeneratedFalse;
   bool InFeasibleTrue;
   bool InFeasibleFalse;
 
+  void finalizeResults() {
+    if (Finalized)
+      return;
+    if (!GeneratedTrue) generateNode(C.Pred->State, true);
+    if (!GeneratedFalse) generateNode(C.Pred->State, false);
+    Finalized = true;
+  }
+
 public:
-  BranchNodeBuilder(const CFGBlock *src, const CFGBlock *dstT, 
-                      const CFGBlock *dstF, ExplodedNode *pred, CoreEngine* e)
-  : CommonNodeBuilder(e, pred), Src(src), DstT(dstT), DstF(dstF),
+  BranchNodeBuilder(NodeBuilderContext &C, const CFGBlock *dstT,
+                    const CFGBlock *dstF)
+  : NodeBuilder(C), DstT(dstT), DstF(dstF),
     GeneratedTrue(false), GeneratedFalse(false),
-    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {}
-
-  ~BranchNodeBuilder();
-
-  ExplodedNode *getPredecessor() const { return Pred; }
-
-  const ExplodedGraph& getGraph() const { return *Eng.G; }
+    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {
+  }
 
-  /// This function generates a new ExplodedNode but not a new
-  /// branch(block edge).
+  /// This function generate a new ExplodedNode but not a new
+  /// branch(block edge). Creates a transition from the Builder's top
+  /// predecessor.
   ExplodedNode *generateNode(const Stmt *Condition, const ProgramState *State,
-                             const ProgramPointTag *Tag = 0);
+                             const ProgramPointTag *Tag = 0,
+                             bool MarkAsSink = false);
 
-  ExplodedNode *generateNode(const ProgramState *State, bool branch);
+  ExplodedNode *generateNode(const ProgramState *State, bool branch,
+                             ExplodedNode *Pred = 0);
 
   const CFGBlock *getTargetBlock(bool branch) const {
     return branch ? DstT : DstF;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h Tue Oct 18 18:06:04 2011
@@ -53,6 +53,7 @@
 class ExplodedNode : public llvm::FoldingSetNode {
   friend class ExplodedGraph;
   friend class CoreEngine;
+  friend class NodeBuilder;
   friend class StmtNodeBuilder;
   friend class BranchNodeBuilder;
   friend class IndirectGotoNodeBuilder;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Tue Oct 18 18:06:04 2011
@@ -154,7 +154,9 @@
   /// ProcessBranch - Called by CoreEngine.  Used to generate successor
   ///  nodes by processing the 'effects' of a branch condition.
   void processBranch(const Stmt *Condition, const Stmt *Term, 
-                     BranchNodeBuilder& builder);
+                     NodeBuilderContext& BuilderCtx,
+                     const CFGBlock *DstT,
+                     const CFGBlock *DstF);
 
   /// processIndirectGoto - Called by CoreEngine.  Used to generate successor
   ///  nodes by processing the 'effects' of a computed goto jump.

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h Tue Oct 18 18:06:04 2011
@@ -27,6 +27,7 @@
 namespace ento {
   
 template <typename PP> class GenericNodeBuilder;
+struct NodeBuilderContext;
 class AnalysisManager;
 class ExplodedNodeSet;
 class ExplodedNode;
@@ -65,7 +66,9 @@
   /// Called by CoreEngine.  Used to generate successor
   ///  nodes by processing the 'effects' of a branch condition.
   virtual void processBranch(const Stmt *Condition, const Stmt *Term,
-                             BranchNodeBuilder& builder) = 0;
+                             NodeBuilderContext& BuilderCtx,
+                             const CFGBlock *DstT,
+                             const CFGBlock *DstF) = 0;
 
   /// Called by CoreEngine.  Used to generate successor
   /// nodes by processing the 'effects' of a computed goto jump.

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp Tue Oct 18 18:06:04 2011
@@ -61,9 +61,9 @@
   const ProgramState *state = Builder.getState();
   SVal X = state->getSVal(Condition);
   if (X.isUndef()) {
-    ExplodedNode *N = Builder.generateNode(Condition, state);
+    // Generate a sink node.
+    ExplodedNode *N = Builder.generateNode(Condition, state, 0, true);
     if (N) {
-      N->markAsSink();
       if (!BT)
         BT.reset(
                new BuiltinBug("Branch condition evaluates to a garbage value"));

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp Tue Oct 18 18:06:04 2011
@@ -417,14 +417,15 @@
 void CoreEngine::HandleBranch(const Stmt *Cond, const Stmt *Term, 
                                 const CFGBlock * B, ExplodedNode *Pred) {
   assert(B->succ_size() == 2);
-  BranchNodeBuilder Builder(B, *(B->succ_begin()), *(B->succ_begin()+1),
-                            Pred, this);
-  SubEng.processBranch(Cond, Term, Builder);
+  NodeBuilderContext Ctx(*this, Pred, B);
+  SubEng.processBranch(Cond, Term, Ctx,
+                       *(B->succ_begin()), *(B->succ_begin()+1));
 }
 
 void CoreEngine::HandlePostStmt(const CFGBlock *B, unsigned StmtIdx, 
                                   ExplodedNode *Pred) {
-  assert (!B->empty());
+  assert(B);
+  assert(!B->empty());
 
   if (StmtIdx == B->size())
     HandleBlockExit(B, Pred);
@@ -454,6 +455,13 @@
   if (IsNew) WList->enqueue(Node);
 }
 
+void CoreEngine::enqueue(NodeBuilder &NB) {
+  for (NodeBuilder::iterator I = NB.results_begin(),
+                               E = NB.results_end(); I != E; ++I) {
+    WList->enqueue(*I);
+  }
+}
+
 ExplodedNode *
 GenericNodeBuilderImpl::generateNodeImpl(const ProgramState *state,
                                          ExplodedNode *pred,
@@ -475,23 +483,17 @@
   return 0;
 }
 
-NodeBuilder::NodeBuilder(CoreEngine& e, ExplodedNode *N)
-  : Eng(e), Pred(N), Finalized(false) {
-  assert(!N->isSink());
-  Deferred.insert(N);
-}
-
 ExplodedNode* NodeBuilder::generateNodeImpl(const ProgramPoint &Loc,
-                                            const ProgramState *State,
-                                            ExplodedNode *Pred,
-                                            bool MarkAsSink) {
+                                              const ProgramState *State,
+                                              ExplodedNode *FromN,
+                                              bool MarkAsSink) {
   assert(Finalized == false &&
          "We cannot create new nodes after the results have been finalized.");
-                                              
+
   bool IsNew;
-  ExplodedNode *N = Eng.G->getNode(Loc, State, &IsNew);
-  N->addPredecessor(Pred, *Eng.G);
-  Deferred.erase(Pred);
+  ExplodedNode *N = C.Eng.G->getNode(Loc, State, &IsNew);
+  N->addPredecessor(FromN, *C.Eng.G);
+  Deferred.erase(FromN);
 
   if (MarkAsSink)
     N->markAsSink();
@@ -601,61 +603,41 @@
 }
 
 // This function generate a new ExplodedNode but not a new branch(block edge).
+// Creates a transition from the Builder's top predecessor.
 ExplodedNode *BranchNodeBuilder::generateNode(const Stmt *Condition,
                                               const ProgramState *State,
-                                              const ProgramPointTag *Tag) {
-  bool IsNew;
-  ExplodedNode *Succ 
-    = Eng.G->getNode(PostCondition(Condition, Pred->getLocationContext(), Tag),
-                     State, &IsNew);
-  
-  Succ->addPredecessor(Pred, *Eng.G);
-  
-  Pred = Succ;
-  
-  if (IsNew) 
-    return Succ;
-  
-  return NULL;
+                                              const ProgramPointTag *Tag,
+                                              bool MarkAsSink) {
+  ProgramPoint PP = PostCondition(Condition, C.Pred->getLocationContext(), Tag);
+  ExplodedNode *N = generateNodeImpl(PP, State, C.Pred, MarkAsSink);
+  assert(N);
+  // TODO: This needs to go - we should not change Pred!!!
+  C.Pred = N;
+  return N;
 }
 
 ExplodedNode *BranchNodeBuilder::generateNode(const ProgramState *State,
-                                              bool branch) {
+                                              bool branch,
+                                              ExplodedNode *NodePred) {
 
   // If the branch has been marked infeasible we should not generate a node.
   if (!isFeasible(branch))
     return NULL;
 
-  bool IsNew;
-
-  ExplodedNode *Succ =
-    Eng.G->getNode(BlockEdge(Src,branch ? DstT:DstF,Pred->getLocationContext()),
-                   State, &IsNew);
-
-  Succ->addPredecessor(Pred, *Eng.G);
+  if (!NodePred)
+    NodePred = C.Pred;
+  ProgramPoint Loc = BlockEdge(C.Block, branch ? DstT:DstF,
+                               NodePred->getLocationContext());
+  ExplodedNode *Succ = generateNodeImpl(Loc, State, NodePred);
 
   if (branch)
     GeneratedTrue = true;
   else
     GeneratedFalse = true;
 
-  if (IsNew) {
-    Deferred.push_back(Succ);
-    return Succ;
-  }
-
-  return NULL;
-}
-
-BranchNodeBuilder::~BranchNodeBuilder() {
-  if (!GeneratedTrue) generateNode(Pred->State, true);
-  if (!GeneratedFalse) generateNode(Pred->State, false);
-
-  for (DeferredTy::iterator I=Deferred.begin(), E=Deferred.end(); I!=E; ++I)
-    if (!(*I)->isSink()) Eng.WList->enqueue(*I);
+  return Succ;
 }
 
-
 ExplodedNode*
 IndirectGotoNodeBuilder::generateNode(const iterator &I,
                                       const ProgramState *St,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=142444&r1=142443&r2=142444&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Oct 18 18:06:04 2011
@@ -940,11 +940,17 @@
 }
 
 void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
-                                 BranchNodeBuilder& builder) {
+                               NodeBuilderContext& BldCtx,
+                               const CFGBlock *DstT,
+                               const CFGBlock *DstF) {
+
+  BranchNodeBuilder builder(BldCtx, DstT, DstF);
 
   // Check for NULL conditions; e.g. "for(;;)"
   if (!Condition) {
-    builder.markInfeasible(false);
+    BranchNodeBuilder NullCondBldr(BldCtx, DstT, DstF);
+    NullCondBldr.markInfeasible(false);
+    Engine.enqueue(NullCondBldr);
     return;
   }
 
@@ -952,11 +958,9 @@
                                 Condition->getLocStart(),
                                 "Error evaluating branch");
 
-  getCheckerManager().runCheckersForBranchCondition(Condition, builder, *this);
-
-  // If the branch condition is undefined, return;
-  if (!builder.isFeasible(true) && !builder.isFeasible(false))
-    return;
+  //TODO: This should take the NodeBuiolder.
+  getCheckerManager().runCheckersForBranchCondition(Condition, builder,
+                                                    *this);
 
   const ProgramState *PrevState = builder.getState();
   SVal X = PrevState->getSVal(Condition);
@@ -982,6 +986,8 @@
     if (X.isUnknownOrUndef()) {
       builder.generateNode(MarkBranch(PrevState, Term, true), true);
       builder.generateNode(MarkBranch(PrevState, Term, false), false);
+      // Enqueue the results into the work list.
+      Engine.enqueue(builder);
       return;
     }
   }
@@ -1003,6 +1009,9 @@
     else
       builder.markInfeasible(false);
   }
+
+  // Enqueue the results into the work list.
+  Engine.enqueue(builder);
 }
 
 /// processIndirectGoto - Called by CoreEngine.  Used to generate successor





More information about the cfe-commits mailing list