From: Ted Kremenek Date: Sat, 2 Apr 2011 02:56:23 +0000 (+0000) Subject: Teach IdempotentOperationsChecker about paths aborted because ExprEngine didn't know... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=422ab7a49a9a4252dbc6350e49d7a5708337b9c7;p=clang Teach IdempotentOperationsChecker about paths aborted because ExprEngine didn't know how to handle a specific Expr type. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@128761 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index a968385b3c..41d7b9eafd 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -48,6 +48,10 @@ class CoreEngine { public: typedef std::vector > BlocksExhausted; + + typedef std::vector > + BlocksAborted; + private: SubEngine& SubEng; @@ -68,6 +72,10 @@ private: /// The locations where we stopped doing work because we visited a location /// too many times. BlocksExhausted blocksExhausted; + + /// The locations where we stopped because the engine aborted analysis, + /// usually because it could not reason about something. + BlocksAborted blocksAborted; void generateNode(const ProgramPoint& Loc, const GRState* State, ExplodedNode* Pred); @@ -122,17 +130,32 @@ public: ExplodedNodeSet &Dst); // Functions for external checking of whether we have unfinished work - bool wasBlockAborted() const { return !blocksExhausted.empty(); } - bool hasWorkRemaining() const { return wasBlockAborted() || WList->hasWork(); } - + bool wasBlockAborted() const { return !blocksAborted.empty(); } + bool wasBlocksExhausted() const { return !blocksExhausted.empty(); } + bool hasWorkRemaining() const { return wasBlocksExhausted() || + WList->hasWork() || + wasBlockAborted(); } + + /// Inform the CoreEngine that a basic block was aborted because + /// it could not be completely analyzed. + void addAbortedBlock(const ExplodedNode *node, const CFGBlock *block) { + blocksAborted.push_back(std::make_pair(block, node)); + } + WorkList *getWorkList() const { return WList; } - BlocksExhausted::const_iterator blocks_aborted_begin() const { + BlocksExhausted::const_iterator blocks_exhausted_begin() const { return blocksExhausted.begin(); } - BlocksExhausted::const_iterator blocks_aborted_end() const { + BlocksExhausted::const_iterator blocks_exhausted_end() const { return blocksExhausted.end(); } + BlocksAborted::const_iterator blocks_aborted_begin() const { + return blocksAborted.begin(); + } + BlocksAborted::const_iterator blocks_aborted_end() const { + return blocksAborted.end(); + } }; class StmtNodeBuilder { diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 3708bb8e11..ab8e4bcdde 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -213,11 +213,9 @@ public: const SymbolManager& getSymbolManager() const { return SymMgr; } // Functions for external checking of whether we have unfinished work - bool wasBlockAborted() const { return Engine.wasBlockAborted(); } + bool wasBlocksExhausted() const { return Engine.wasBlocksExhausted(); } bool hasEmptyWorkList() const { return !Engine.getWorkList()->hasWork(); } - bool hasWorkRemaining() const { - return wasBlockAborted() || Engine.getWorkList()->hasWork(); - } + bool hasWorkRemaining() const { return Engine.hasWorkRemaining(); } const CoreEngine &getCoreEngine() const { return Engine; } diff --git a/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp b/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp index 07d56b5f6e..983427afb6 100644 --- a/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp @@ -88,8 +88,8 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G, } output << " -> Total CFGBlocks: " << total << " | Unreachable CFGBlocks: " - << unreachable << " | Aborted Block: " - << (Eng.wasBlockAborted() ? "yes" : "no") + << unreachable << " | Exhausted Block: " + << (Eng.wasBlocksExhausted() ? "yes" : "no") << " | Empty WorkList: " << (Eng.hasEmptyWorkList() ? "yes" : "no"); @@ -97,10 +97,10 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G, D->getLocation()); // Emit warning for each block we bailed out on - typedef CoreEngine::BlocksExhausted::const_iterator AbortedIterator; + typedef CoreEngine::BlocksExhausted::const_iterator ExhaustedIterator; const CoreEngine &CE = Eng.getCoreEngine(); - for (AbortedIterator I = CE.blocks_aborted_begin(), - E = CE.blocks_aborted_end(); I != E; ++I) { + for (ExhaustedIterator I = CE.blocks_exhausted_begin(), + E = CE.blocks_exhausted_end(); I != E; ++I) { const BlockEdge &BE = I->first; const CFGBlock *Exit = BE.getDst(); const CFGElement &CE = Exit->front(); diff --git a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp index 6190b02379..3c8d45ee2d 100644 --- a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp @@ -534,9 +534,9 @@ IdempotentOperationChecker::pathWasCompletelyAnalyzed(AnalysisContext *AC, CFGReverseBlockReachabilityAnalysis *CRA = AC->getCFGReachablityAnalysis(); // Test for reachability from any aborted blocks to this block - typedef CoreEngine::BlocksExhausted::const_iterator AbortedIterator; - for (AbortedIterator I = CE.blocks_aborted_begin(), - E = CE.blocks_aborted_end(); I != E; ++I) { + typedef CoreEngine::BlocksExhausted::const_iterator ExhaustedIterator; + for (ExhaustedIterator I = CE.blocks_exhausted_begin(), + E = CE.blocks_exhausted_end(); I != E; ++I) { const BlockEdge &BE = I->first; // The destination block on the BlockEdge is the first block that was not @@ -550,6 +550,15 @@ IdempotentOperationChecker::pathWasCompletelyAnalyzed(AnalysisContext *AC, if (destBlock == CB || CRA->isReachable(destBlock, CB)) return false; } + + // Test for reachability from blocks we just gave up on. + typedef CoreEngine::BlocksAborted::const_iterator AbortedIterator; + for (AbortedIterator I = CE.blocks_aborted_begin(), + E = CE.blocks_aborted_end(); I != E; ++I) { + const CFGBlock *destBlock = I->first; + if (destBlock == CB || CRA->isReachable(destBlock, CB)) + return false; + } // For the items still on the worklist, see if they are in blocks that // can eventually reach 'CB'. diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index 8197db60a0..3826a12226 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -444,7 +444,8 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred, { SaveAndRestore OldSink(Builder->BuildSinks); Builder->BuildSinks = true; - MakeNode(Dst, S, Pred, GetState(Pred)); + const ExplodedNode *node = MakeNode(Dst, S, Pred, GetState(Pred)); + Engine.addAbortedBlock(node, Builder->getBlock()); break; } diff --git a/test/Analysis/idempotent-operations.cpp b/test/Analysis/idempotent-operations.cpp index f9240eb61a..9d22909ed3 100644 --- a/test/Analysis/idempotent-operations.cpp +++ b/test/Analysis/idempotent-operations.cpp @@ -13,3 +13,22 @@ void false1() { test(five * a); // expected-warning {{The right operand to '*' is always 0}} b = 4; } + +// Test not flagging idempotent operations because we aborted the analysis +// of a path because of an unsupported construct. +struct RDar9219143_Foo { + ~RDar9219143_Foo(); + operator bool() const; +}; + +RDar9219143_Foo foo(); +unsigned RDar9219143_bar(); +void RDar9219143_test() { + unsigned i, e; + for (i = 0, e = RDar9219143_bar(); i != e; ++i) + if (foo()) + break; + if (i == e) // no-warning + return; +} +