]> granicus.if.org Git - clang/commitdiff
[analyzer] Make NodeBuilder and Pred node loosely coupled
authorAnna Zaks <ganna@apple.com>
Tue, 18 Oct 2011 23:06:44 +0000 (23:06 +0000)
committerAnna Zaks <ganna@apple.com>
Tue, 18 Oct 2011 23:06:44 +0000 (23:06 +0000)
NodeBuilder should not assume it's dealing with a single predecessor. Remove predecessor getters. Modify the BranchNodeBuilder to not be responsible for doing auto-transitions (which depend on a predecessor).

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@142453 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
lib/StaticAnalyzer/Core/CheckerManager.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp

index 51533489e5fbecf30572a5df2978c440c3b005df..2e270000c7e64005c7e1b163f150832c18176efe 100644 (file)
@@ -215,8 +215,9 @@ public:
 class BranchCondition {
   template <typename CHECKER>
   static void _checkBranchCondition(void *checker, const Stmt *condition,
-                                    NodeBuilder &B, ExprEngine &Eng) {
-    ((const CHECKER *)checker)->checkBranchCondition(condition, B, Eng);
+                                    NodeBuilder &B, ExplodedNode *Pred,
+                                    ExprEngine &Eng) {
+    ((const CHECKER *)checker)->checkBranchCondition(condition, B, Pred, Eng);
   }
 
 public:
index 6026aec643aa42ff2264e4c287b1ad6db4f77252..7450df63c47ea87b1457787db3e2589b48f29efa 100644 (file)
@@ -232,7 +232,8 @@ public:
 
   /// \brief Run checkers for branch condition.
   void runCheckersForBranchCondition(const Stmt *condition,
-                                     NodeBuilder &B, ExprEngine &Eng);
+                                     NodeBuilder &B, ExplodedNode *Pred,
+                                     ExprEngine &Eng);
 
   /// \brief Run checkers for live symbols.
   ///
@@ -334,7 +335,8 @@ public:
   typedef CheckerFn<void (EndOfFunctionNodeBuilder &, ExprEngine &)>
       CheckEndPathFunc;
   
-  typedef CheckerFn<void (const Stmt *, NodeBuilder &, ExprEngine &)>
+  typedef CheckerFn<void (const Stmt *, NodeBuilder &, ExplodedNode *Pred,
+                          ExprEngine &)>
       CheckBranchConditionFunc;
   
   typedef CheckerFn<void (SymbolReaper &, CheckerContext &)>
index 7d7aa1417beba4d0a25cdc1b33f17b02494434e0..4892293a428016d4626588e68036dee52741b8f3 100644 (file)
@@ -191,6 +191,8 @@ protected:
   /// is set to false, autotransitions are yet to be generated.
   bool Finalized;
 
+  bool HasGeneratedNodes;
+
   /// \brief The frontier set - a set of nodes which need to be propagated after
   /// the builder dies.
   typedef llvm::SmallPtrSet<ExplodedNode*,5> DeferredTy;
@@ -218,14 +220,14 @@ protected:
 
 public:
   NodeBuilder(ExplodedNode *N, NodeBuilderContext &Ctx, bool F = true)
-    : BuilderPred(N), C(Ctx), Finalized(F) {
+    : BuilderPred(N), C(Ctx), Finalized(F), HasGeneratedNodes(false) {
     assert(!N->isSink());
     Deferred.insert(N);
   }
 
   /// Create a new builder using the parent builder's context.
   NodeBuilder(ExplodedNode *N, const NodeBuilder &ParentBldr, bool F = true)
-    : BuilderPred(N), C(ParentBldr.C), Finalized(F) {
+    : BuilderPred(N), C(ParentBldr.C), Finalized(F), HasGeneratedNodes(false) {
     assert(!N->isSink());
     Deferred.insert(N);
   }
@@ -243,8 +245,9 @@ public:
     return generateNodeImpl(PP, State, Pred, MarkAsSink);
   }
 
+  // TODO: will get removed.
   bool hasGeneratedNodes() const {
-    return (!Deferred.count(BuilderPred));
+    return HasGeneratedNodes;
   }
 
   typedef DeferredTy::iterator iterator;
@@ -269,12 +272,6 @@ public:
                       BuilderPred->getLocationContext()->getCurrentStackFrame(),
                       C.Block->getBlockID());
   }
-
-  // \brief Get the builder's predecessor - the parent to all the other nodes.
-  ExplodedNode *getPredecessor() const { return BuilderPred; }
-
-  // \brief Returns state of the predecessor.
-  const ProgramState *getState() const { return BuilderPred->getState(); }
 };
 
 class CommonNodeBuilder {
@@ -366,7 +363,7 @@ public:
   }
 
   void importNodesFromBuilder(const NodeBuilder &NB) {
-    ExplodedNode *NBPred = const_cast<ExplodedNode*>(NB.getPredecessor());
+    ExplodedNode *NBPred = const_cast<ExplodedNode*>(NB.BuilderPred);
     if (NB.hasGeneratedNodes()) {
       Deferred.erase(NBPred);
       Deferred.insert(NB.Deferred.begin(), NB.Deferred.end());
@@ -378,35 +375,20 @@ class BranchNodeBuilder: public NodeBuilder {
   const CFGBlock *DstT;
   const CFGBlock *DstF;
 
-  bool GeneratedTrue;
-  bool GeneratedFalse;
   bool InFeasibleTrue;
   bool InFeasibleFalse;
 
-  /// Generate default branching transitions of none were generated or
-  /// suppressed.
-  void finalizeResults() {
-    if (Finalized)
-      return;
-    if (!GeneratedTrue) generateNode(BuilderPred->State, true);
-    if (!GeneratedFalse) generateNode(BuilderPred->State, false);
-    Finalized = true;
-  }
-
 public:
   BranchNodeBuilder(ExplodedNode *Pred, NodeBuilderContext &C,
                     const CFGBlock *dstT, const CFGBlock *dstF)
-  : NodeBuilder(Pred, C, false), DstT(dstT), DstF(dstF),
-    GeneratedTrue(false), GeneratedFalse(false),
-    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {
-  }
+  : NodeBuilder(Pred, C), DstT(dstT), DstF(dstF),
+    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {}
 
   /// Create a new builder using the parent builder's context.
   BranchNodeBuilder(ExplodedNode *Pred, BranchNodeBuilder &ParentBldr)
-  : NodeBuilder(Pred, ParentBldr, false), DstT(ParentBldr.DstT),
-    DstF(ParentBldr.DstF), GeneratedTrue(false), GeneratedFalse(false),
-    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {
-  }
+  : NodeBuilder(Pred, ParentBldr), DstT(ParentBldr.DstT),
+    DstF(ParentBldr.DstF),
+    InFeasibleTrue(!DstT), InFeasibleFalse(!DstF) {}
 
   ExplodedNode *generateNode(const ProgramState *State, bool branch,
                              ExplodedNode *Pred = 0);
@@ -417,9 +399,9 @@ public:
 
   void markInfeasible(bool branch) {
     if (branch)
-      InFeasibleTrue = GeneratedTrue = true;
+      InFeasibleTrue = true;
     else
-      InFeasibleFalse = GeneratedFalse = true;
+      InFeasibleFalse = true;
   }
 
   bool isFeasible(bool branch) {
index bc84e2f7d5ddeabd3d6e7b8d0bc02afdfe9382ec..df4580aa906d5c5b8ab061f3531742028f9240ec 100644 (file)
@@ -132,16 +132,20 @@ public:
 
   /// processCFGElement - Called by CoreEngine. Used to generate new successor
   ///  nodes by processing the 'effects' of a CFG element.
-  void processCFGElement(const CFGElement E, StmtNodeBuilder& builder);
+  void processCFGElement(const CFGElement E, StmtNodeBuilder& Bldr,
+                         ExplodedNode *Pred);
 
-  void ProcessStmt(const CFGStmt S, StmtNodeBuilder &builder);
+  void ProcessStmt(const CFGStmt S, StmtNodeBuilder &builder,
+                   ExplodedNode *Pred);
 
-  void ProcessInitializer(const CFGInitializer I, StmtNodeBuilder &builder);
+  void ProcessInitializer(const CFGInitializer I, StmtNodeBuilder &Bldr,
+                          ExplodedNode *Pred);
 
-  void ProcessImplicitDtor(const CFGImplicitDtor D, StmtNodeBuilder &builder);
+  void ProcessImplicitDtor(const CFGImplicitDtor D, StmtNodeBuilder &builder,
+                           ExplodedNode *Pred);
 
   void ProcessAutomaticObjDtor(const CFGAutomaticObjDtor D, 
-                            StmtNodeBuilder &builder);
+                               StmtNodeBuilder &builder, ExplodedNode *Pred);
   void ProcessBaseDtor(const CFGBaseDtor D, StmtNodeBuilder &builder);
   void ProcessMemberDtor(const CFGMemberDtor D, StmtNodeBuilder &builder);
   void ProcessTemporaryDtor(const CFGTemporaryDtor D, 
index 38b7538b6b6be1f0b28d5d85da22ac362a8e9575..8dfc1865bdb40a7e7ef25978d3bc96242cafc92d 100644 (file)
@@ -55,7 +55,8 @@ public:
 
   /// Called by CoreEngine. Used to generate new successor
   /// nodes by processing the 'effects' of a block-level statement.
-  virtual void processCFGElement(const CFGElement E, StmtNodeBuilder& builder)=0;
+  virtual void processCFGElement(const CFGElement E, StmtNodeBuilder& builder,
+                                 ExplodedNode* Pred)=0;
 
   /// Called by CoreEngine when it starts processing a CFGBlock.  The
   /// SubEngine is expected to populate dstNodes with new nodes representing
index 451fa91b3aba8100bd0923424aefa2d3f325901d..d030469459fecebfbe3d4663aa74c79a0f01d961 100644 (file)
@@ -50,26 +50,26 @@ class UndefBranchChecker : public Checker<check::BranchCondition> {
 
 public:
   void checkBranchCondition(const Stmt *Condition, NodeBuilder &Builder,
-                            ExprEngine &Eng) const;
+                            ExplodedNode *Pred, ExprEngine &Eng) const;
 };
 
 }
 
 void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
                                               NodeBuilder &Builder,
+                                              ExplodedNode *Pred,
                                               ExprEngine &Eng) const {
-  const ProgramState *state = Builder.getState();
+  const ProgramState *state = Pred->getState();
   SVal X = state->getSVal(Condition);
   if (X.isUndef()) {
     // TODO: The PP will be generated with the correct tag by the CheckerManager
     // after we migrate the callback to CheckerContext.
     const ProgramPointTag *Tag = 0;
-    ProgramPoint PP = PostCondition(Condition,
-                        Builder.getPredecessor()->getLocationContext(), Tag);
+    ProgramPoint PP = PostCondition(Condition, Pred->getLocationContext(), Tag);
     // Generate a sink node, which implicitly marks both outgoing branches as
     // infeasible.
     ExplodedNode *N = Builder.generateNode(PP, state,
-                                           Builder.getPredecessor(), true);
+                                           Pred, true);
     if (N) {
       if (!BT)
         BT.reset(
index d47033a70e7380d6d9f082adabac77d307dc8685..aa6a51e1aa854c5b5db98eeaeb883341615630f8 100644 (file)
@@ -298,10 +298,11 @@ void CheckerManager::runCheckersForEndPath(EndOfFunctionNodeBuilder &B,
 /// \brief Run checkers for branch condition.
 void CheckerManager::runCheckersForBranchCondition(const Stmt *condition,
                                                    NodeBuilder &B,
+                                                   ExplodedNode *Pred,
                                                    ExprEngine &Eng) {
   for (unsigned i = 0, e = BranchConditionCheckers.size(); i != e; ++i) {
     CheckBranchConditionFunc fn = BranchConditionCheckers[i];
-    fn(condition, B, Eng);
+    fn(condition, B, Pred, Eng);
   }
 }
 
index 0a308670c0c4bf32d4cbf59751b60327bc16f1ff..a40a0dc683c0993da2b4470ac063c9e9237703d8 100644 (file)
@@ -316,7 +316,7 @@ void CoreEngine::HandleBlockEntrance(const BlockEntrance &L,
   if (CFGElement E = L.getFirstElement()) {
     NodeBuilderContext Ctx(*this, L.getBlock());
     StmtNodeBuilder Builder(Pred, 0, Ctx);
-    SubEng.processCFGElement(E, Builder);
+    SubEng.processCFGElement(E, Builder, Pred);
   }
   else
     HandleBlockExit(L.getBlock(), Pred);
@@ -433,7 +433,7 @@ void CoreEngine::HandlePostStmt(const CFGBlock *B, unsigned StmtIdx,
   else {
     NodeBuilderContext Ctx(*this, B);
     StmtNodeBuilder Builder(Pred, StmtIdx, Ctx);
-    SubEng.processCFGElement((*B)[StmtIdx], Builder);
+    SubEng.processCFGElement((*B)[StmtIdx], Builder, Pred);
   }
 }
 
@@ -489,6 +489,8 @@ ExplodedNode* NodeBuilder::generateNodeImpl(const ProgramPoint &Loc,
                                             const ProgramState *State,
                                             ExplodedNode *FromN,
                                             bool MarkAsSink) {
+  HasGeneratedNodes = true;
+
   bool IsNew;
   ExplodedNode *N = C.Eng.G->getNode(Loc, State, &IsNew);
   N->addPredecessor(FromN, *C.Eng.G);
@@ -567,9 +569,6 @@ ExplodedNode *StmtNodeBuilder::MakeNode(ExplodedNodeSet &Dst,
 ExplodedNode *BranchNodeBuilder::generateNode(const ProgramState *State,
                                               bool branch,
                                               ExplodedNode *NodePred) {
-  assert(Finalized == false &&
-         "We cannot create new nodes after the results have been finalized.");
-
   // If the branch has been marked infeasible we should not generate a node.
   if (!isFeasible(branch))
     return NULL;
@@ -579,12 +578,6 @@ ExplodedNode *BranchNodeBuilder::generateNode(const ProgramState *State,
   ProgramPoint Loc = BlockEdge(C.Block, branch ? DstT:DstF,
                                NodePred->getLocationContext());
   ExplodedNode *Succ = generateNodeImpl(Loc, State, NodePred);
-
-  if (branch)
-    GeneratedTrue = true;
-  else
-    GeneratedFalse = true;
-
   return Succ;
 }
 
index 2cce0414c9e6ed83bf9f44004fa791b1273aa6cf..19b1a2fa738febfe51b5aff098429481fe2322c7 100644 (file)
@@ -196,26 +196,29 @@ void ExprEngine::processEndWorklist(bool hasWorkRemaining) {
 }
 
 void ExprEngine::processCFGElement(const CFGElement E, 
-                                  StmtNodeBuilder& builder) {
+                                  StmtNodeBuilder& Bldr,
+                                  ExplodedNode *Pred) {
   switch (E.getKind()) {
     case CFGElement::Invalid:
       llvm_unreachable("Unexpected CFGElement kind.");
     case CFGElement::Statement:
-      ProcessStmt(const_cast<Stmt*>(E.getAs<CFGStmt>()->getStmt()), builder);
+      ProcessStmt(const_cast<Stmt*>(E.getAs<CFGStmt>()->getStmt()), Bldr, Pred);
       return;
     case CFGElement::Initializer:
-      ProcessInitializer(E.getAs<CFGInitializer>()->getInitializer(), builder);
+      ProcessInitializer(E.getAs<CFGInitializer>()->getInitializer(),
+                         Bldr, Pred);
       return;
     case CFGElement::AutomaticObjectDtor:
     case CFGElement::BaseDtor:
     case CFGElement::MemberDtor:
     case CFGElement::TemporaryDtor:
-      ProcessImplicitDtor(*E.getAs<CFGImplicitDtor>(), builder);
+      ProcessImplicitDtor(*E.getAs<CFGImplicitDtor>(), Bldr, Pred);
       return;
   }
 }
 
-void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder) {
+void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder,
+                             ExplodedNode *Pred) {
   // TODO: Use RAII to remove the unnecessary, tagged nodes.
   //RegisterCreatedNodes registerCreatedNodes(getGraph());
 
@@ -232,7 +235,7 @@ void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder) {
   // A tag to track convenience transitions, which can be removed at cleanup.
   static SimpleProgramPointTag cleanupTag("ExprEngine : Clean Node");
   Builder = &builder;
-  EntryNode = builder.getPredecessor();
+  EntryNode = Pred;
 
   const ProgramState *EntryState = EntryNode->getState();
   CleanedState = EntryState;
@@ -320,12 +323,10 @@ void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder) {
 }
 
 void ExprEngine::ProcessInitializer(const CFGInitializer Init,
-                                    StmtNodeBuilder &builder) {
+                                    StmtNodeBuilder &builder,
+                                    ExplodedNode *pred) {
   // We don't set EntryNode and currentStmt. And we don't clean up state.
   const CXXCtorInitializer *BMI = Init.getInitializer();
-
-  ExplodedNode *pred = builder.getPredecessor();
-
   const StackFrameContext *stackFrame = cast<StackFrameContext>(pred->getLocationContext());
   const CXXConstructorDecl *decl = cast<CXXConstructorDecl>(stackFrame->getDecl());
   const CXXThisRegion *thisReg = getCXXThisRegion(decl, stackFrame);
@@ -373,12 +374,13 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
 }
 
 void ExprEngine::ProcessImplicitDtor(const CFGImplicitDtor D,
-                                       StmtNodeBuilder &builder) {
+                                       StmtNodeBuilder &builder,
+                                       ExplodedNode *Pred) {
   Builder = &builder;
 
   switch (D.getKind()) {
   case CFGElement::AutomaticObjectDtor:
-    ProcessAutomaticObjDtor(cast<CFGAutomaticObjDtor>(D), builder);
+    ProcessAutomaticObjDtor(cast<CFGAutomaticObjDtor>(D), builder, Pred);
     break;
   case CFGElement::BaseDtor:
     ProcessBaseDtor(cast<CFGBaseDtor>(D), builder);
@@ -395,8 +397,8 @@ void ExprEngine::ProcessImplicitDtor(const CFGImplicitDtor D,
 }
 
 void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor dtor,
-                                           StmtNodeBuilder &builder) {
-  ExplodedNode *pred = builder.getPredecessor();
+                                         StmtNodeBuilder &builder,
+                                         ExplodedNode *pred) {
   const ProgramState *state = pred->getState();
   const VarDecl *varDecl = dtor.getVarDecl();
 
@@ -949,6 +951,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
   if (!Condition) {
     BranchNodeBuilder NullCondBldr(Pred, BldCtx, DstT, DstF);
     NullCondBldr.markInfeasible(false);
+    NullCondBldr.generateNode(Pred->getState(), true, Pred);
     Engine.enqueue(NullCondBldr);
     return;
   }
@@ -959,7 +962,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
 
   NodeBuilder CheckerBldr(Pred, BldCtx);
   getCheckerManager().runCheckersForBranchCondition(Condition, CheckerBldr,
-                                                    *this);
+                                                    Pred, *this);
 
   for (NodeBuilder::iterator I = CheckerBldr.results_begin(),
                              E = CheckerBldr.results_end(); E != I; ++I) {
@@ -969,7 +972,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
       continue;
 
     BranchNodeBuilder builder(PredI, BldCtx, DstT, DstF);
-    const ProgramState *PrevState = builder.getState();
+    const ProgramState *PrevState = Pred->getState();
     SVal X = PrevState->getSVal(Condition);
 
     if (X.isUnknownOrUndef()) {
@@ -992,8 +995,8 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
     }
     // If the condition is still unknown, give up.
     if (X.isUnknownOrUndef()) {
-      builder.generateNode(MarkBranch(PrevState, Term, true), true);
-      builder.generateNode(MarkBranch(PrevState, Term, false), false);
+      builder.generateNode(MarkBranch(PrevState, Term, true), true, PredI);
+      builder.generateNode(MarkBranch(PrevState, Term, false), false, PredI);
       // Enqueue the results into the work list.
       Engine.enqueue(builder);
       continue;
@@ -1004,7 +1007,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
     // Process the true branch.
     if (builder.isFeasible(true)) {
       if (const ProgramState *state = PrevState->assume(V, true))
-        builder.generateNode(MarkBranch(state, Term, true), true);
+        builder.generateNode(MarkBranch(state, Term, true), true, PredI);
       else
         builder.markInfeasible(true);
     }
@@ -1012,7 +1015,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
     // Process the false branch.
     if (builder.isFeasible(false)) {
       if (const ProgramState *state = PrevState->assume(V, false))
-        builder.generateNode(MarkBranch(state, Term, false), false);
+        builder.generateNode(MarkBranch(state, Term, false), false, PredI);
       else
         builder.markInfeasible(false);
     }