Author: Adam Balogh Date: 2019-12-11T15:24:06+01:00 New Revision: 855d21a03ae841b7c6c980e92f67bd5b65287fa6
URL: https://github.com/llvm/llvm-project/commit/855d21a03ae841b7c6c980e92f67bd5b65287fa6 DIFF: https://github.com/llvm/llvm-project/commit/855d21a03ae841b7c6c980e92f67bd5b65287fa6.diff LOG: [Analyzer] Iterator Checkers: Replace `UnknownVal` in comparison result by a conjured value Sometimes the return value of a comparison operator call is `UnkownVal`. Since no assumptions can be made on `UnknownVal`, this leeds to keeping impossible execution paths in the exploded graph resulting in poor performance and false positives. To overcome this we replace unknown results of iterator comparisons by conjured symbols. Differential Revision: https://reviews.llvm.org/D70244 Added: Modified: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp clang/test/Analysis/invalidated-iterator.cpp clang/test/Analysis/iterator-modelling.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp index 0a7015e85e93..ab4e8112d677 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp @@ -87,7 +87,7 @@ class IteratorModeling : public Checker<check::PostCall, check::PostStmt<MaterializeTemporaryExpr>, check::Bind, check::LiveSymbols, check::DeadSymbols> { - void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal, + void handleComparison(CheckerContext &C, const Expr *CE, SVal RetVal, const SVal &LVal, const SVal &RVal, OverloadedOperatorKind Op) const; void processComparison(CheckerContext &C, ProgramStateRef State, @@ -499,9 +499,9 @@ void IteratorModeling::checkDeadSymbols(SymbolReaper &SR, } void IteratorModeling::handleComparison(CheckerContext &C, const Expr *CE, - const SVal &RetVal, const SVal &LVal, - const SVal &RVal, - OverloadedOperatorKind Op) const { + SVal RetVal, const SVal &LVal, + const SVal &RVal, + OverloadedOperatorKind Op) const { // Record the operands and the operator of the comparison for the next // evalAssume, if the result is a symbolic expression. If it is a concrete // value (only one branch is possible), then transfer the state between @@ -538,6 +538,16 @@ void IteratorModeling::handleComparison(CheckerContext &C, const Expr *CE, RPos = getIteratorPosition(State, RVal); } + // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol + // instead. + if (RetVal.isUnknown()) { + auto &SymMgr = C.getSymbolManager(); + auto *LCtx = C.getLocationContext(); + RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol( + CE, LCtx, C.getASTContext().BoolTy, C.blockCount())); + State = State->BindExpr(CE, LCtx, RetVal); + } + processComparison(C, State, LPos->getOffset(), RPos->getOffset(), RetVal, Op); } @@ -559,7 +569,7 @@ void IteratorModeling::processComparison(CheckerContext &C, const auto ConditionVal = RetVal.getAs<DefinedSVal>(); if (!ConditionVal) return; - + if (auto StateTrue = relateSymbols(State, Sym1, Sym2, Op == OO_EqualEqual)) { StateTrue = StateTrue->assume(*ConditionVal, true); C.addTransition(StateTrue); diff --git a/clang/test/Analysis/invalidated-iterator.cpp b/clang/test/Analysis/invalidated-iterator.cpp index 4505aedd4e36..a9ccc3b75834 100644 --- a/clang/test/Analysis/invalidated-iterator.cpp +++ b/clang/test/Analysis/invalidated-iterator.cpp @@ -120,4 +120,3 @@ void assignment(std::vector<int> &V) { V.erase(i); auto j = V.cbegin(); // no-warning } - diff --git a/clang/test/Analysis/iterator-modelling.cpp b/clang/test/Analysis/iterator-modelling.cpp index 2cd7b349be81..3c981b27bbbf 100644 --- a/clang/test/Analysis/iterator-modelling.cpp +++ b/clang/test/Analysis/iterator-modelling.cpp @@ -1969,8 +1969,8 @@ void non_std_find(std::vector<int> &V, int e) { clang_analyzer_eval(clang_analyzer_container_end(V) == clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} expected-warning@-1{{TRUE}} if (V.end() != first) { - clang_analyzer_eval(clang_analyzer_container_end(V) == - clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} expected-warning@-1 0-1{{TRUE}} FIXME: should only expect FALSE in every case + clang_analyzer_eval(clang_analyzer_container_end(V) == + clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits