[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

clang/lib/StaticAnalyzer/Checkers/Iterator.cpp

@@ -200,24 +200,31 @@ 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>()) {
+  const auto IntDistOp = Distance.getAs<nonloc::ConcreteInt>();
+  if (!IntDistOp)
+    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()))
+                             IntDist, SymMgr.getType(Pos->getOffset()))
                .getAsSymbol());
   return setIteratorPosition(State, Iter, NewPos);
 }
 
-  return nullptr;
-}
-
 // This function tells the analyzer's engine that symbols produced by our
 // checker, most notably iterator positions, are relatively small.
 // A distance between items in the container should not be very large.

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();