From: Tom Care Date: Thu, 9 Sep 2010 02:04:52 +0000 (+0000) Subject: Simplified reachability checking in IdempotentOperationChecker and added a helper... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b062795a65ef6ef27c88103126def46193fcf2f1;p=clang Simplified reachability checking in IdempotentOperationChecker and added a helper function for path display. - Created private class CFGReachabilityAnalysis, which provides cached reachability lookups in the CFG - Simplified PathWasCompletelyAnalyzed to use the new reachability class - Added getLastRelevantNodes function for future use with path displaying in BugReporter git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@113465 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/IdempotentOperationChecker.cpp b/lib/Checker/IdempotentOperationChecker.cpp index 2e85013a76..3d454bed9a 100644 --- a/lib/Checker/IdempotentOperationChecker.cpp +++ b/lib/Checker/IdempotentOperationChecker.cpp @@ -62,45 +62,64 @@ using namespace clang; namespace { class IdempotentOperationChecker : public CheckerVisitor { +public: + static void *getTag(); + void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B); + void PostVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B); + void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B, GRExprEngine &Eng); + +private: + // Our assumption about a particular operation. + enum Assumption { Possible = 0, Impossible, Equal, LHSis1, RHSis1, LHSis0, + RHSis0 }; + + void UpdateAssumption(Assumption &A, const Assumption &New); + + // False positive reduction methods + static bool isSelfAssign(const Expr *LHS, const Expr *RHS); + static bool isUnused(const Expr *E, AnalysisContext *AC); + static bool isTruncationExtensionAssignment(const Expr *LHS, + const Expr *RHS); + bool PathWasCompletelyAnalyzed(const CFG *C, + const CFGBlock *CB, + const GRCoreEngine &CE); + static bool CanVary(const Expr *Ex, + AnalysisContext *AC); + static bool isConstantOrPseudoConstant(const DeclRefExpr *DR, + AnalysisContext *AC); + static bool containsNonLocalVarDecl(const Stmt *S); + const ExplodedNodeSet getLastRelevantNodes(const CFGBlock *Begin, + const ExplodedNode *N); + + // Hash table and related data structures + struct BinaryOperatorData { + BinaryOperatorData() : assumption(Possible), analysisContext(0) {} + + Assumption assumption; + AnalysisContext *analysisContext; + ExplodedNodeSet explodedNodes; // Set of ExplodedNodes that refer to a + // BinaryOperator + }; + typedef llvm::DenseMap + AssumptionMap; + AssumptionMap hash; + + // A class that performs reachability queries for CFGBlocks. Several internal + // checks in this checker require reachability information. The requests all + // tend to have a common destination, so we lazily do a predecessor search + // from the destination node and cache the results to prevent work + // duplication. + class CFGReachabilityAnalysis { + typedef llvm::SmallSet ReachableSet; + typedef llvm::DenseMap ReachableMap; + ReachableSet analyzed; + ReachableMap reachable; public: - static void *getTag(); - void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B); - void PostVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B); - void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B, GRExprEngine &Eng); - + inline bool isReachable(const CFGBlock *Src, const CFGBlock *Dst); private: - // Our assumption about a particular operation. - enum Assumption { Possible = 0, Impossible, Equal, LHSis1, RHSis1, LHSis0, - RHSis0 }; - - void UpdateAssumption(Assumption &A, const Assumption &New); - - // False positive reduction methods - static bool isSelfAssign(const Expr *LHS, const Expr *RHS); - static bool isUnused(const Expr *E, AnalysisContext *AC); - static bool isTruncationExtensionAssignment(const Expr *LHS, - const Expr *RHS); - static bool PathWasCompletelyAnalyzed(const CFG *C, - const CFGBlock *CB, - const GRCoreEngine &CE); - static bool CanVary(const Expr *Ex, - AnalysisContext *AC); - static bool isConstantOrPseudoConstant(const DeclRefExpr *DR, - AnalysisContext *AC); - static bool containsNonLocalVarDecl(const Stmt *S); - - // Hash table and related data structures - struct BinaryOperatorData { - BinaryOperatorData() : assumption(Possible), analysisContext(0) {} - - Assumption assumption; - AnalysisContext *analysisContext; - ExplodedNodeSet explodedNodes; // Set of ExplodedNodes that refer to a - // BinaryOperator - }; - typedef llvm::DenseMap - AssumptionMap; - AssumptionMap hash; + void MapReachability(const CFGBlock *Dst); + }; + CFGReachabilityAnalysis CRA; }; } @@ -530,50 +549,23 @@ bool IdempotentOperationChecker::PathWasCompletelyAnalyzed( const CFG *C, const CFGBlock *CB, const GRCoreEngine &CE) { - std::deque WorkList; - llvm::SmallSet Aborted; - llvm::SmallSet Visited; - - // Create a set of all aborted blocks + // Test for reachability from any aborted blocks to this block typedef GRCoreEngine::BlocksAborted::const_iterator AbortedIterator; for (AbortedIterator I = CE.blocks_aborted_begin(), E = CE.blocks_aborted_end(); I != E; ++I) { const BlockEdge &BE = I->first; // The destination block on the BlockEdge is the first block that was not - // analyzed. - Aborted.insert(BE.getDst()->getBlockID()); - } - - // Save the entry block ID for early exiting - unsigned EntryBlockID = C->getEntry().getBlockID(); - - // Create initial node - WorkList.push_back(CB); - - while (!WorkList.empty()) { - const CFGBlock *Head = WorkList.front(); - WorkList.pop_front(); - Visited.insert(Head->getBlockID()); - - // If we found the entry block, then there exists a path from the target - // node to the entry point of this function -> the path was completely - // analyzed. - if (Head->getBlockID() == EntryBlockID) - return true; - - // If any of the aborted blocks are on the path to the beginning, then all - // paths to this block were not analyzed. - if (Aborted.count(Head->getBlockID())) + // analyzed. If we can reach this block from the aborted block, then this + // block was not completely analyzed. + if (CRA.isReachable(BE.getDst(), CB)) return false; - - // Add the predecessors to the worklist unless we have already visited them - for (CFGBlock::const_pred_iterator I = Head->pred_begin(); - I != Head->pred_end(); ++I) - if (!Visited.count((*I)->getBlockID())) - WorkList.push_back(*I); } + // Verify that this block is reachable from the entry block + if (!CRA.isReachable(&C->getEntry(), CB)) + return false; + // If we get to this point, there is no connection to the entry block or an // aborted block. This path is unreachable and we can report the error. return true; @@ -700,3 +692,94 @@ bool IdempotentOperationChecker::containsNonLocalVarDecl(const Stmt *S) { return false; } + +// Returns the successor nodes of N whose CFGBlocks cannot reach N's CFGBlock. +// This effectively gives us a set of points in the ExplodedGraph where +// subsequent execution could not affect the idempotent operation on this path. +// This is useful for displaying paths after the point of the error, providing +// an example of how this idempotent operation cannot change. +const ExplodedNodeSet IdempotentOperationChecker::getLastRelevantNodes( + const CFGBlock *Begin, const ExplodedNode *N) { + std::deque WorkList; + llvm::SmallPtrSet Visited; + ExplodedNodeSet Result; + + WorkList.push_back(N); + + while (!WorkList.empty()) { + const ExplodedNode *Head = WorkList.front(); + WorkList.pop_front(); + Visited.insert(Head); + + const ProgramPoint &PP = Head->getLocation(); + if (const BlockEntrance *BE = dyn_cast(&PP)) { + // Get the CFGBlock and test the reachability + const CFGBlock *CB = BE->getBlock(); + + // If we cannot reach the beginning CFGBlock from this block, then we are + // finished + if (!CRA.isReachable(CB, Begin)) { + Result.Add(const_cast(Head)); + continue; + } + } + + // Add unvisited children to the worklist + for (ExplodedNode::const_succ_iterator I = Head->succ_begin(), + E = Head->succ_end(); I != E; ++I) + if (!Visited.count(*I)) + WorkList.push_back(*I); + } + + // Return the ExplodedNodes that were found + return Result; +} + +bool IdempotentOperationChecker::CFGReachabilityAnalysis::isReachable( + const CFGBlock *Src, + const CFGBlock *Dst) { + const unsigned DstBlockID = Dst->getBlockID(); + + // If we haven't analyzed the destination node, run the analysis now + if (!analyzed.count(DstBlockID)) { + MapReachability(Dst); + analyzed.insert(DstBlockID); + } + + // Return the cached result + return reachable[DstBlockID].count(Src->getBlockID()); +} + +// Maps reachability to a common node by walking the predecessors of the +// destination node. +void IdempotentOperationChecker::CFGReachabilityAnalysis::MapReachability( + const CFGBlock *Dst) { + std::deque WorkList; + // Maintain a visited list to ensure we don't get stuck on cycles + llvm::SmallSet Visited; + ReachableSet &DstReachability = reachable[Dst->getBlockID()]; + + // Start searching from the destination node, since we commonly will perform + // multiple queries relating to a destination node. + WorkList.push_back(Dst); + + bool firstRun = true; + while (!WorkList.empty()) { + const CFGBlock *Head = WorkList.front(); + WorkList.pop_front(); + Visited.insert(Head->getBlockID()); + + // Update reachability information for this node -> Dst + if (!firstRun) + // Don't insert Dst -> Dst unless it was a predecessor of itself + DstReachability.insert(Head->getBlockID()); + else + firstRun = false; + + // Add the predecessors to the worklist unless we have already visited them + for (CFGBlock::const_pred_iterator I = Head->pred_begin(); + I != Head->pred_end(); ++I) + if (!Visited.count((*I)->getBlockID())) + WorkList.push_back(*I); + } +}