[cfe-commits] r113465 - /cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
Tom Care
tcare at apple.com
Wed Sep 8 19:04:52 PDT 2010
Author: tcare
Date: Wed Sep 8 21:04:52 2010
New Revision: 113465
URL: http://llvm.org/viewvc/llvm-project?rev=113465&view=rev
Log:
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
Modified:
cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
Modified: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=113465&r1=113464&r2=113465&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Wed Sep 8 21:04:52 2010
@@ -62,45 +62,64 @@
namespace {
class IdempotentOperationChecker
: public CheckerVisitor<IdempotentOperationChecker> {
+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<const BinaryOperator *, BinaryOperatorData>
+ 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<unsigned, 32> ReachableSet;
+ typedef llvm::DenseMap<unsigned, ReachableSet> 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<const BinaryOperator *, BinaryOperatorData>
- AssumptionMap;
- AssumptionMap hash;
+ void MapReachability(const CFGBlock *Dst);
+ };
+ CFGReachabilityAnalysis CRA;
};
}
@@ -530,50 +549,23 @@
const CFG *C,
const CFGBlock *CB,
const GRCoreEngine &CE) {
- std::deque<const CFGBlock *> WorkList;
- llvm::SmallSet<unsigned, 8> Aborted;
- llvm::SmallSet<unsigned, 128> 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 @@
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<const ExplodedNode *> WorkList;
+ llvm::SmallPtrSet<const ExplodedNode *, 32> 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<BlockEntrance>(&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<ExplodedNode *>(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<const CFGBlock *> WorkList;
+ // Maintain a visited list to ensure we don't get stuck on cycles
+ llvm::SmallSet<unsigned, 32> 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);
+ }
+}
More information about the cfe-commits
mailing list