[analyzer] Stability improvements for IteratorModeling functions

[ASDenysPetrov]

Summary

Function path may lead to crash.

Fixed using local variable outside the scope through a pointer.
Fixed minor misspellings.
Added regression test.

This patch covers a bug https://bugs.llvm.org/show_bug.cgi?id=41485

[NoQ]

Tests please!

[ASDenysPetrov]

@baloghadamsoftware
I know you have much expertise here. Do you have some thoughts of how I can test exactly these cases?

[ASDenysPetrov]

@baloghadamsoftware
I've added a test for unscoped SVal, but still can't come up with if (!LPos || !RPos)
Could you, please, suggest me something?

[ASDenysPetrov]

I decided not to add changes of what I cannot cover by tests. So I removed them.

@NoQ @baloghadamsoftware
please, review my changes.

[baloghadamsoftware]

Thanks for fixing this!

clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp

void IteratorModeling::handleComparison(CheckerContext &C, const Expr *CE,
   if (LPos) {
     Cont = LPos->getContainer();
   } else if (RPos) {
     Cont = RPos->getContainer();
   }
   if (!Cont)
     return;
 
-  // At least one of the iterators have recorded positions. If one of them has
+  // At least one of the iterators has recorded positions. If one of them does
   // not then create a new symbol for the offset.
   SymbolRef Sym;
   if (!LPos || !RPos) {
     auto &SymMgr = C.getSymbolManager();
     Sym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
                                C.getASTContext().LongTy, C.blockCount());
     State = assumeNoOverflow(State, Sym, 4);
   }
 
   if (!LPos) {
     State = setIteratorPosition(State, LVal,
                                 IteratorPosition::getPosition(Cont, Sym));
     LPos = getIteratorPosition(State, LVal);
   } else if (!RPos) {
     State = setIteratorPosition(State, RVal,
                                 IteratorPosition::getPosition(Cont, Sym));
     RPos = getIteratorPosition(State, RVal);
   }
 
-  // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+  // We cannot make assumptions 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);
   }
void IteratorModeling::handleRandomIncrOrDecr(CheckerContext &C,
   // position of the iterator
   auto State = C.getState();
 
   const auto *Pos = getIteratorPosition(State, LHS);
   if (!Pos)
     return;
 
   const auto *value = &RHS;
+  SVal val;
   if (auto loc = RHS.getAs<Loc>()) {
-    const auto val = State->getRawSVal(*loc);
+    val = State->getRawSVal(*loc);
     value = &val;
   }
 
   auto &TgtVal = (Op == OO_PlusEqual || Op == OO_MinusEqual) ? LHS : RetVal;
 
   auto NewState =
     advancePosition(State, LHS, Op, *value);
   if (NewState) {

clang/test/Analysis/iterator-range.cpp

void prev_0_ahead_of_end(const std::vector<int> &V) {
   auto j = std::prev(i, 0); // no-warning
 }
 
 void prev_0_end(const std::vector<int> &V) {
   auto i = V.end();
   auto j = std::prev(i, 0); // no-warning
 }
 
+// std::prev() with int* for checking Loc value argument
+namespace std {
+template <typename T>
+T prev(T, int *);
+}
+
+void prev_loc_value(const std::vector<int> &V, int o) {
+
+  auto i = return_any_iterator(V.begin());
+  int *offset = &o;
+  auto j = std::prev(i, offset); // no-warning
+}
+
 //
 // Structure member dereference operators
 //
 
 struct S {
   int n;
 };