Author: Adam Balogh Date: 2020-02-25T14:57:34+01:00 New Revision: 770ad9f55e660e0ec89f61d5579dfafad17ab5f5
URL: https://github.com/llvm/llvm-project/commit/770ad9f55e660e0ec89f61d5579dfafad17ab5f5 DIFF: https://github.com/llvm/llvm-project/commit/770ad9f55e660e0ec89f61d5579dfafad17ab5f5.diff LOG: [Analyzer] Fix for iterator modeling and checkers: handle negative numbers correctly Currently, using negative numbers in iterator operations (additions and subractions) results in advancements with huge positive numbers due to an error. This patch fixes it. Differential Revision: https://reviews.llvm.org/D74760 Added: Modified: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp clang/test/Analysis/iterator-modelling.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp index 64daf358fbe5..e80d8bc32dec 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp @@ -200,22 +200,29 @@ ProgramStateRef advancePosition(ProgramStateRef State, const SVal &Iter, auto &SymMgr = State->getStateManager().getSymbolManager(); auto &SVB = State->getStateManager().getSValBuilder(); + auto &BVF = State->getStateManager().getBasicVals(); assert ((Op == OO_Plus || Op == OO_PlusEqual || Op == OO_Minus || Op == OO_MinusEqual) && "Advance operator must be one of +, -, += and -=."); auto BinOp = (Op == OO_Plus || Op == OO_PlusEqual) ? BO_Add : BO_Sub; - if (const auto IntDist = Distance.getAs<nonloc::ConcreteInt>()) { - // For concrete integers we can calculate the new position - const auto NewPos = - Pos->setTo(SVB.evalBinOp(State, BinOp, - nonloc::SymbolVal(Pos->getOffset()), - *IntDist, SymMgr.getType(Pos->getOffset())) - .getAsSymbol()); - return setIteratorPosition(State, Iter, NewPos); - } + const auto IntDistOp = Distance.getAs<nonloc::ConcreteInt>(); + if (!IntDistOp) + return nullptr; - return nullptr; + // For concrete integers we can calculate the new position + nonloc::ConcreteInt IntDist = *IntDistOp; + + if (IntDist.getValue().isNegative()) { + IntDist = nonloc::ConcreteInt(BVF.getValue(-IntDist.getValue())); + BinOp = (BinOp == BO_Add) ? BO_Sub : BO_Add; + } + const auto NewPos = + Pos->setTo(SVB.evalBinOp(State, BinOp, + nonloc::SymbolVal(Pos->getOffset()), + IntDist, SymMgr.getType(Pos->getOffset())) + .getAsSymbol()); + return setIteratorPosition(State, Iter, NewPos); } // This function tells the analyzer's engine that symbols produced by our diff --git a/clang/test/Analysis/iterator-modelling.cpp b/clang/test/Analysis/iterator-modelling.cpp index b2551939986a..4e40319cedc1 100644 --- a/clang/test/Analysis/iterator-modelling.cpp +++ b/clang/test/Analysis/iterator-modelling.cpp @@ -100,6 +100,16 @@ void plus_equal(const std::vector<int> &v) { clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.begin() + 2}} } +void plus_equal_negative(const std::vector<int> &v) { + auto i = v.end(); + + clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()"); + + i += -2; + + clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.end() - 2}} +} + void minus_equal(const std::vector<int> &v) { auto i = v.end(); @@ -110,6 +120,16 @@ void minus_equal(const std::vector<int> &v) { clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.end() - 2}} } +void minus_equal_negative(const std::vector<int> &v) { + auto i = v.begin(); + + clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()"); + + i -= -2; + + clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.begin() + 2}} +} + void copy(const std::vector<int> &v) { auto i1 = v.end(); @@ -132,6 +152,17 @@ void plus(const std::vector<int> &v) { clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$v.begin() + 2}} } +void plus_negative(const std::vector<int> &v) { + auto i1 = v.end(); + + clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()"); + + auto i2 = i1 + (-2); + + clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}} + clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$v.end() - 2}} +} + void minus(const std::vector<int> &v) { auto i1 = v.end(); @@ -143,6 +174,17 @@ void minus(const std::vector<int> &v) { clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$v.end() - 2}} } +void minus_negative(const std::vector<int> &v) { + auto i1 = v.begin(); + + clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()"); + + auto i2 = i1 - (-2); + + clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}} + clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$v.begin() + 2}} +} + void copy_and_increment1(const std::vector<int> &v) { auto i1 = v.begin(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits