From: Anna Zaks Date: Tue, 18 Oct 2011 23:06:04 +0000 (+0000) Subject: [analyzer] NodeBuilder Refactoring: Subclass BranchNodeBuilder from NodeBuilder. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a19f4af7a94835ce4693bfe12d6270754e79eb56;p=clang [analyzer] NodeBuilder Refactoring: Subclass BranchNodeBuilder from NodeBuilder. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@142444 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index ea7776c61f..2b83ea2df5 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -30,6 +30,8 @@ class CheckerContext { const ProgramPoint Location; const ProgramState *ST; const unsigned size; + // TODO: Use global context. + NodeBuilderContext Ctx; NodeBuilder NB; public: bool *respondsToCallback; @@ -48,7 +50,8 @@ public: 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())); } diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 5849cc1545..22f5d3d951 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -27,6 +27,8 @@ class ProgramPointTag; 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 @@ public: 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 @@ class NodeBuilder { typedef llvm::SmallPtrSet 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 @@ class NodeBuilder { 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 @@ public: } // \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 @@ public: } }; -class BranchNodeBuilder: public CommonNodeBuilder { - const CFGBlock *Src; +class BranchNodeBuilder: public NodeBuilder { const CFGBlock *DstT; const CFGBlock *DstF; - typedef SmallVector 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; diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h index fdfed3da54..070f52e0c7 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h @@ -53,6 +53,7 @@ class ExplodedGraph; class ExplodedNode : public llvm::FoldingSetNode { friend class ExplodedGraph; friend class CoreEngine; + friend class NodeBuilder; friend class StmtNodeBuilder; friend class BranchNodeBuilder; friend class IndirectGotoNodeBuilder; diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 9bc470fdde..b57bc4be5e 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -154,7 +154,9 @@ public: /// 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. diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h index ae212bcf5d..6f480359bf 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h @@ -27,6 +27,7 @@ class Stmt; namespace ento { template class GenericNodeBuilder; +struct NodeBuilderContext; class AnalysisManager; class ExplodedNodeSet; class ExplodedNode; @@ -65,7 +66,9 @@ public: /// 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. diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index b860b34ff3..8663893f2e 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -61,9 +61,9 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, 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")); diff --git a/lib/StaticAnalyzer/Core/CoreEngine.cpp b/lib/StaticAnalyzer/Core/CoreEngine.cpp index 5f2f4ea7f1..0078922d69 100644 --- a/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -417,14 +417,15 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { 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 @@ void CoreEngine::generateNode(const ProgramPoint &Loc, 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 @@ GenericNodeBuilderImpl::generateNodeImpl(const ProgramState *state, 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 @@ StmtNodeBuilder::generateNodeInternal(const ProgramPoint &Loc, } // 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, diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index ac9cf0b08d..cc170bc1e6 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -940,11 +940,17 @@ static SVal RecoverCastedSymbol(ProgramStateManager& StateMgr, } 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 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term, 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 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term, 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 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term, else builder.markInfeasible(false); } + + // Enqueue the results into the work list. + Engine.enqueue(builder); } /// processIndirectGoto - Called by CoreEngine. Used to generate successor