baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus, martong, gamesh411.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, steakhal, Charusso, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:256
+  SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext());
+  SVal Result = State->getSVal(UO, C.getLocationContext());
+
----------------
This is the problematic point which is not working. I left the comments 
intentionally in the code.

The problem is that in `postStmt` we are //after//  the operation. Thus the 
value of the operand (`SubExpr`) is not `i` anymore, but the //former value// 
of `i` (a pointer to a symbolic region initially). Instead, the result is `i` 
in case of prefix operators, but also the former value in case of postfix 
operators. This is correct, of course, because here, after the call the value 
of `i` was changed, thus it is not equal to the parameter. However, we need the 
region of `i` here and/or the new value bound to it (e.g. the pointer to an 
element region which is usually the result of a `++` or `--` on a pointer to a 
symbolic region). How to reach that? Of course, in `preStmt` the operand is `i` 
as it should be. The same is true for binary operators `+=` and `-=`. 


Iterators are an abstraction of pointers and in some data structures iterators 
may be implemented by pointers. This patch adds support for iterators 
implemented as pointers in all the iterator checkers (including iterator 
modeling).

This patch is work in progress, modeling of lvalue operations do not work yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82185

Files:
  clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.h
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
  clang/test/Analysis/invalidated-iterator.cpp
  clang/test/Analysis/iterator-modeling.cpp
  clang/test/Analysis/iterator-range.cpp
  clang/test/Analysis/mismatched-iterator.cpp

Index: clang/test/Analysis/mismatched-iterator.cpp
===================================================================
--- clang/test/Analysis/mismatched-iterator.cpp
+++ clang/test/Analysis/mismatched-iterator.cpp
@@ -118,3 +118,15 @@
 
   if (V1.cbegin() == V2.cbegin()) {} //no-warning
 }
+
+template<typename T>
+struct cont_with_ptr_iterator {
+  T *begin() const;
+  T *end() const;
+};
+
+void comparison_ptr_iterator(cont_with_ptr_iterator<int> &C1,
+                             cont_with_ptr_iterator<int> &C2) {
+  if (C1.begin() != C2.end()) {} // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
Index: clang/test/Analysis/iterator-range.cpp
===================================================================
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -854,3 +854,84 @@
   *i; // expected-warning{{Past-the-end iterator dereferenced}}
       // expected-note@-1{{Past-the-end iterator dereferenced}}
 }
+
+template<typename T>
+struct cont_with_ptr_iterator {
+  T* begin() const;
+  T* end() const;
+};
+
+void deref_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  (void) *i; // expected-warning{{Past-the-end iterator dereferenced}}
+             // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void array_deref_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  (void) i[0]; // expected-warning{{Past-the-end iterator dereferenced}}
+               // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void arrow_deref_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  (void) i->n; // expected-warning{{Past-the-end iterator dereferenced}}
+               // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void arrow_star_deref_end_ptr_iterator(const cont_with_ptr_iterator<S> &c,
+                                       int S::*p) {
+  auto i = c.end();
+  (void)(i->*p); // expected-warning{{Past-the-end iterator dereferenced}}
+                 // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void prefix_incr_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  ++i; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+       // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
+}
+
+void postfix_incr_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  i++; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+       // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
+}
+
+void prefix_decr_begin_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.begin();
+  --i; // expected-warning{{Iterator decremented ahead of its valid range}}
+       // expected-note@-1{{Iterator decremented ahead of its valid range}}
+}
+
+void postfix_decr_begin_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.begin();
+  i--; // expected-warning{{Iterator decremented ahead of its valid range}}
+       // expected-note@-1{{Iterator decremented ahead of its valid range}}
+}
+
+void prefix_add_2_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  (void)(i + 2); // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+                 // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
+}
+
+void postfix_add_assign_2_end_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.end();
+  i += 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+          // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
+}
+
+void prefix_minus_2_begin_ptr_iterator(const cont_with_ptr_iterator<S> &c) {
+  auto i = c.begin();
+  (void)(i - 2); // expected-warning{{Iterator decremented ahead of its valid range}}
+                 // expected-note@-1{{Iterator decremented ahead of its valid range}}
+}
+
+void postfix_minus_assign_2_begin_ptr_iterator(
+    const cont_with_ptr_iterator<S> &c) {
+  auto i = c.begin();
+  i -= 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+          // expected-note@-1{{Iterator decremented ahead of its valid range}}
+}
+
Index: clang/test/Analysis/iterator-modeling.cpp
===================================================================
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -18,6 +18,7 @@
 long clang_analyzer_container_end(const Container&);
 template <typename Iterator>
 long clang_analyzer_iterator_position(const Iterator&);
+long clang_analyzer_iterator_position(int*);
 template <typename Iterator>
 void* clang_analyzer_iterator_container(const Iterator&);
 template <typename Iterator>
@@ -1859,6 +1860,115 @@
   }
 }
 
+template<typename T>
+struct cont_with_ptr_iterator {
+  typedef T* iterator;
+  T* begin() const;
+  T* end() const;
+};
+
+void begin_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.begin();
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i) == &c); // expected-warning{{TRUE}}
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.begin()}}
+
+  if (i != c.begin()) {
+    clang_analyzer_warnIfReached();
+  }
+  }
+
+void clang_analyzer_dump(const int*);
+void clang_analyzer_dump(long);
+void clang_analyzer_printState();
+
+void prefix_increment_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+
+  auto j = ++i;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.begin() + 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(j)); //expected-warning{{$c.begin() + 1}}
+}
+
+void prefix_decrement_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.end();
+
+  clang_analyzer_denote(clang_analyzer_container_end(c), "$c.end()");
+
+  auto j = --i;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.end() - 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(j)); //expected-warning{{$c.end() - 1}}
+}
+
+void postfix_increment_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+
+  auto j = i++;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.begin() + 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(j)); //expected-warning{{$c.begin()}}
+}
+
+void postfix_decrement_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.end();
+
+  clang_analyzer_denote(clang_analyzer_container_end(c), "$c.end()");
+
+  auto j = i--;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.end() - 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(j)); //expected-warning{{$c.end()}}
+}
+
+void plus_equal_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+
+  i += 2;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.begin() + 2}}
+}
+
+void minus_equal_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i = c.end();
+
+  clang_analyzer_denote(clang_analyzer_container_end(c), "$c.end()");
+
+  i -= 2;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$c.end() - 2}}
+}
+
+void plus_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i1 = c.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+
+  auto i2 = i1 + 2;
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &c); // expected-warning{{TRUE}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$c.begin() + 2}}
+}
+
+void minus_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
+  auto i1 = c.end();
+
+  clang_analyzer_denote(clang_analyzer_container_end(c), "$c.end()");
+
+  auto i2 = i1 - 2;
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &c); // expected-warning{{TRUE}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); //expected-warning{{$c.end() - 2}}
+}
+
 void clang_analyzer_printState();
 
 void print_state(std::vector<int> &V) {
@@ -1880,3 +1990,4 @@
 // CHECK-NEXT:     "i1 : Valid ; Container == SymRegion{reg_$[[#]]<std::vector<int> & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
 }
+
Index: clang/test/Analysis/invalidated-iterator.cpp
===================================================================
--- clang/test/Analysis/invalidated-iterator.cpp
+++ clang/test/Analysis/invalidated-iterator.cpp
@@ -120,3 +120,80 @@
   V.erase(i);
   auto j = V.cbegin(); // no-warning
 }
+
+template<typename T>
+struct cont_with_ptr_iterator {
+  T *begin() const;
+  T *end() const;
+  T &operator[](size_t);
+  void push_back(const T&);
+  T* erase(T*);
+};
+
+void invalidated_dereference_end_ptr_iterator(cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  (void) *i; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_prefix_increment_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  ++i; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_prefix_decrement_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin() + 1;
+  C.erase(i);
+  --i; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_postfix_increment_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  i++; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_postfix_decrement_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin() + 1;
+  C.erase(i);
+  i--; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_increment_by_2_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  i += 2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_increment_by_2_copy_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  auto j = i + 2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_decrement_by_2_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  i -= 2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_decrement_by_2_copy_end_ptr_iterator(
+    cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  auto j = i - 2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void invalidated_subscript_end_ptr_iterator(cont_with_ptr_iterator<int> &C) {
+  auto i = C.begin();
+  C.erase(i);
+  (void) i[1]; // expected-warning{{Invalidated iterator accessed}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
@@ -28,7 +28,7 @@
 namespace {
 
 class MismatchedIteratorChecker
-  : public Checker<check::PreCall> {
+  : public Checker<check::PreCall, check::PreStmt<BinaryOperator>> {
 
   std::unique_ptr<BugType> MismatchedBugType;
 
@@ -47,6 +47,7 @@
   MismatchedIteratorChecker();
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const;
 
 };
 
@@ -188,6 +189,17 @@
   }
 }
 
+void MismatchedIteratorChecker::checkPreStmt(const BinaryOperator *BO,
+                                             CheckerContext &C) const {
+  if (!isComparisonOperator(BO->getOpcode()))
+    return;
+
+  ProgramStateRef State = C.getState();
+  SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
+  SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
+  verifyMatch(C, LVal, RVal);
+}
+
 void MismatchedIteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
                                             const MemRegion *Cont) const {
   // Verify match between a container and the container of an iterator
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -27,7 +27,10 @@
 namespace {
 
 class IteratorRangeChecker
-  : public Checker<check::PreCall> {
+  : public Checker<check::PreCall, check::PreStmt<UnaryOperator>,
+                   check::PreStmt<BinaryOperator>,
+                   check::PreStmt<ArraySubscriptExpr>,
+                   check::PreStmt<MemberExpr>> {
 
   std::unique_ptr<BugType> OutOfRangeBugType;
 
@@ -46,6 +49,10 @@
   IteratorRangeChecker();
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreStmt(const UnaryOperator *UO, CheckerContext &C) const;
+  void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const;
+  void checkPreStmt(const ArraySubscriptExpr *ASE, CheckerContext &C) const;
+  void checkPreStmt(const MemberExpr *ME, CheckerContext &C) const;
 
   using AdvanceFn = void (IteratorRangeChecker::*)(CheckerContext &, SVal,
                                                    SVal) const;
@@ -134,6 +141,55 @@
   }
 }
 
+void IteratorRangeChecker::checkPreStmt(const UnaryOperator *UO,
+                                        CheckerContext &C) const {
+  if (isa<CXXThisExpr>(UO->getSubExpr()))
+    return;
+
+  ProgramStateRef State = C.getState();
+  UnaryOperatorKind OK = UO->getOpcode();
+  SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext());
+  
+  if (isDereferenceOperator(OK)) {
+    verifyDereference(C, SubVal);
+  } else if (isIncrementOperator(OK)) {
+    verifyIncrement(C, SubVal);
+  } else if (isDecrementOperator(OK)) {
+    verifyDecrement(C, SubVal);
+  }
+}
+
+void IteratorRangeChecker::checkPreStmt(const BinaryOperator *BO,
+                                        CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  BinaryOperatorKind OK = BO->getOpcode();
+  SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
+
+  if (isDereferenceOperator(OK)) {
+    verifyDereference(C, LVal);
+  } else if (isRandomIncrOrDecrOperator(OK)) {
+    SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
+    verifyRandomIncrOrDecr(C, toOverloadedOperator(OK), LVal, RVal);
+  }
+}
+
+void IteratorRangeChecker::checkPreStmt(const ArraySubscriptExpr *ASE,
+                                        CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal LVal = State->getSVal(ASE->getLHS(), C.getLocationContext());
+  verifyDereference(C, LVal);
+}
+
+void IteratorRangeChecker::checkPreStmt(const MemberExpr *ME,
+                                        CheckerContext &C) const {
+  if (!ME->isArrow() || ME->isImplicitAccess())
+    return;
+
+  ProgramStateRef State = C.getState();
+  SVal BaseVal = State->getSVal(ME->getBase(), C.getLocationContext());
+  verifyDereference(C, BaseVal);
+}
+
 void IteratorRangeChecker::verifyDereference(CheckerContext &C,
                                              SVal Val) const {
   auto State = C.getState();
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -83,7 +83,9 @@
 namespace {
 
 class IteratorModeling
-    : public Checker<check::PostCall, check::PostStmt<MaterializeTemporaryExpr>,
+    : public Checker<check::PostCall, check::PostStmt<UnaryOperator>,
+                     check::PostStmt<BinaryOperator>,
+                     check::PostStmt<MaterializeTemporaryExpr>,
                      check::Bind, check::LiveSymbols, check::DeadSymbols> {
 
   using AdvanceFn = void (IteratorModeling::*)(CheckerContext &, const Expr *,
@@ -144,6 +146,8 @@
 
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
+  void checkPostStmt(const UnaryOperator *UO, CheckerContext &C) const;
+  void checkPostStmt(const BinaryOperator *BO, CheckerContext &C) const;
   void checkPostStmt(const CXXConstructExpr *CCE, CheckerContext &C) const;
   void checkPostStmt(const DeclStmt *DS, CheckerContext &C) const;
   void checkPostStmt(const MaterializeTemporaryExpr *MTE,
@@ -153,6 +157,7 @@
 };
 
 bool isSimpleComparisonOperator(OverloadedOperatorKind OK);
+bool isSimpleComparisonOperator(BinaryOperatorKind OK);
 ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val);
 ProgramStateRef relateSymbols(ProgramStateRef State, SymbolRef Sym1,
                               SymbolRef Sym2, bool Equal);
@@ -207,12 +212,15 @@
   }
 
   // Assumption: if return value is an iterator which is not yet bound to a
-  //             container, then look for the first iterator argument, and
-  //             bind the return value to the same container. This approach
-  //             works for STL algorithms.
+  //             container, then look for the first iterator argument of the
+  //             same type as the return value and bind the return value to
+  //             the same container. This approach works for STL algorithms.
   // FIXME: Add a more conservative mode
   for (unsigned i = 0; i < Call.getNumArgs(); ++i) {
-    if (isIteratorType(Call.getArgExpr(i)->getType())) {
+    if (isIteratorType(Call.getArgExpr(i)->getType()) &&
+        Call.getArgExpr(i)->getType().getNonReferenceType().getDesugaredType(
+            C.getASTContext()).getTypePtr() ==
+        Call.getResultType().getDesugaredType(C.getASTContext()).getTypePtr()) {
       if (const auto *Pos = getIteratorPosition(State, Call.getArgSVal(i))) {
         assignToContainer(C, OrigExpr, Call.getReturnValue(),
                           Pos->getContainer());
@@ -227,17 +235,68 @@
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Val);
   if (Pos) {
+    llvm::errs()<<"Binding "<<Val<<" to "<<Loc<<".\n";
     State = setIteratorPosition(State, Loc, *Pos);
     C.addTransition(State);
   } else {
     const auto *OldPos = getIteratorPosition(State, Loc);
     if (OldPos) {
+      llvm::errs()<<"Removing binding of "<<Loc<<" because "<<Val<<" has no binding.\n";
       State = removeIteratorPosition(State, Loc);
       C.addTransition(State);
     }
   }
 }
 
+void IteratorModeling::checkPostStmt(const UnaryOperator *UO,
+                                     CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  UnaryOperatorKind OK = UO->getOpcode();
+  SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext());
+  SVal Result = State->getSVal(UO, C.getLocationContext());
+
+  if (isIncrementOperator(OK)) {
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()))
+      if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+        llvm::errs()<<"   SubExpr LValue: "<<State->getLValue(VD, C.getLocationContext())<<"\n";
+    UO->getSubExpr()->dump();
+    llvm::errs()<<"++ Result: "<<Result<<"\n";
+    if (const auto *ResRegion = Result.getAsRegion())
+      if (const auto *ResVarReg = dyn_cast<VarRegion>(ResRegion))
+        llvm::errs()<<"   Indirect SVal: "<<State->getSVal(Result.castAs<Loc>())<<"\n";
+    llvm::errs()<<"++ SubVal: "<<SubVal<<"\n";
+    handleIncrement(C, Result, SubVal, OK == UO_PostInc);
+  }
+
+  if (isDecrementOperator(OK)) {
+    llvm::errs()<<"-- Result: "<<Result<<"\n";
+    llvm::errs()<<"-- SubVal: "<<SubVal<<"\n";
+    handleDecrement(C, Result, SubVal, OK == UO_PostDec);
+  }
+}
+
+void IteratorModeling::checkPostStmt(const BinaryOperator *BO,
+                                     CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  BinaryOperatorKind OK = BO->getOpcode();
+  SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
+  SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
+  SVal Result = State->getSVal(BO, C.getLocationContext());
+
+  if (isSimpleComparisonOperator(BO->getOpcode())) {
+    OverloadedOperatorKind Op =
+      (BO->getOpcode() == BO_EQ) ? OO_Equal : OO_ExclaimEqual;
+    handleComparison(C, BO, Result, LVal, RVal, Op);
+  }
+
+  if (isRandomIncrOrDecrOperator(OK)) {
+    llvm::errs()<<"Result: "<<Result<<"\n";
+    llvm::errs()<<"LVal: "<<LVal<<"\n";
+    llvm::errs()<<"RVal: "<<RVal<<"\n";
+    handleRandomIncrOrDecr(C, BO, toOverloadedOperator(OK), Result, LVal, RVal);
+  }
+}
+
 void IteratorModeling::checkPostStmt(const MaterializeTemporaryExpr *MTE,
                                      CheckerContext &C) const {
   /* Transfer iterator state to temporary objects */
@@ -622,6 +681,7 @@
       Pos.getContainer()->dumpToStream(Out);
       Out<<" ; Offset == ";
       Pos.getOffset()->dumpToStream(Out);
+      Out << NL;
     }
 
     for (const auto &Reg : RegionMap) {
@@ -632,6 +692,7 @@
       Pos.getContainer()->dumpToStream(Out);
       Out<<" ; Offset == ";
       Pos.getOffset()->dumpToStream(Out);
+      Out << NL;
     }
   }
 }
@@ -642,6 +703,10 @@
   return OK == OO_EqualEqual || OK == OO_ExclaimEqual;
 }
 
+bool isSimpleComparisonOperator(BinaryOperatorKind OK) {
+  return OK == BO_EQ || OK == BO_NE;
+}
+
 ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) {
   if (auto Reg = Val.getAsRegion()) {
     Reg = Reg->getMostDerivedObjectRegion();
Index: clang/lib/StaticAnalyzer/Checkers/Iterator.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/Iterator.h
+++ clang/lib/StaticAnalyzer/Checkers/Iterator.h
@@ -115,9 +115,12 @@
 class IteratorRegionMap {};
 class ContainerMap {};
 
-using IteratorSymbolMapTy = CLANG_ENTO_PROGRAMSTATE_MAP(SymbolRef, IteratorPosition);
-using IteratorRegionMapTy = CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, IteratorPosition);
-using ContainerMapTy = CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, ContainerData);
+using IteratorSymbolMapTy =
+  CLANG_ENTO_PROGRAMSTATE_MAP(SymbolRef, IteratorPosition);
+using IteratorRegionMapTy =
+  CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, IteratorPosition);
+using ContainerMapTy =
+  CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, ContainerData);
 
 } // namespace iterator
 
@@ -144,15 +147,24 @@
 bool isIteratorType(const QualType &Type);
 bool isIterator(const CXXRecordDecl *CRD);
 bool isComparisonOperator(OverloadedOperatorKind OK);
+bool isComparisonOperator(BinaryOperatorKind OK);
 bool isInsertCall(const FunctionDecl *Func);
 bool isEraseCall(const FunctionDecl *Func);
 bool isEraseAfterCall(const FunctionDecl *Func);
 bool isEmplaceCall(const FunctionDecl *Func);
 bool isAccessOperator(OverloadedOperatorKind OK);
+bool isAccessOperator(UnaryOperatorKind OK);
+bool isAccessOperator(BinaryOperatorKind OK);
 bool isDereferenceOperator(OverloadedOperatorKind OK);
+bool isDereferenceOperator(UnaryOperatorKind OK);
+bool isDereferenceOperator(BinaryOperatorKind OK);
 bool isIncrementOperator(OverloadedOperatorKind OK);
+bool isIncrementOperator(UnaryOperatorKind OK);
 bool isDecrementOperator(OverloadedOperatorKind OK);
+bool isDecrementOperator(UnaryOperatorKind OK);
 bool isRandomIncrOrDecrOperator(OverloadedOperatorKind OK);
+bool isRandomIncrOrDecrOperator(BinaryOperatorKind OK);
+OverloadedOperatorKind toOverloadedOperator(BinaryOperatorKind OK);
 const ContainerData *getContainerData(ProgramStateRef State,
                                       const MemRegion *Cont);
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
Index: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
+++ clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
@@ -73,6 +73,11 @@
          OK == OO_LessEqual || OK == OO_Greater || OK == OO_GreaterEqual;
 }
 
+bool isComparisonOperator(BinaryOperatorKind OK) {
+  return OK == BO_EQ || OK == BO_NE || OK == BO_LT ||
+         OK == BO_LE || OK == BO_GT || OK == BO_GE;
+}
+
 bool isInsertCall(const FunctionDecl *Func) {
   const auto *IdInfo = Func->getIdentifier();
   if (!IdInfo)
@@ -128,24 +133,73 @@
          isDecrementOperator(OK) || isRandomIncrOrDecrOperator(OK);
 }
 
+bool isAccessOperator(UnaryOperatorKind OK) {
+  return isDereferenceOperator(OK) || isIncrementOperator(OK) ||
+         isDecrementOperator(OK);
+}
+
+bool isAccessOperator(BinaryOperatorKind OK) {
+  return isDereferenceOperator(OK) || isRandomIncrOrDecrOperator(OK);
+}
+
 bool isDereferenceOperator(OverloadedOperatorKind OK) {
   return OK == OO_Star || OK == OO_Arrow || OK == OO_ArrowStar ||
          OK == OO_Subscript;
 }
 
+bool isDereferenceOperator(UnaryOperatorKind OK) {
+  return OK == UO_Deref;
+}
+
+bool isDereferenceOperator(BinaryOperatorKind OK) {
+  return OK == BO_PtrMemI;
+}
+
 bool isIncrementOperator(OverloadedOperatorKind OK) {
   return OK == OO_PlusPlus;
 }
 
+bool isIncrementOperator(UnaryOperatorKind OK) {
+  return OK == UO_PreInc || OK == UO_PostInc;
+}
+
 bool isDecrementOperator(OverloadedOperatorKind OK) {
   return OK == OO_MinusMinus;
 }
 
+bool isDecrementOperator(UnaryOperatorKind OK) {
+  return OK == UO_PreDec || OK == UO_PostDec;
+}
+
 bool isRandomIncrOrDecrOperator(OverloadedOperatorKind OK) {
   return OK == OO_Plus || OK == OO_PlusEqual || OK == OO_Minus ||
          OK == OO_MinusEqual;
 }
 
+bool isRandomIncrOrDecrOperator(BinaryOperatorKind OK) {
+  return OK == BO_Add || OK == BO_AddAssign ||
+         OK == BO_Sub || OK == BO_SubAssign;
+}
+
+OverloadedOperatorKind toOverloadedOperator(BinaryOperatorKind OK) {
+  switch (OK) {
+  case BO_Add:
+    return OO_Plus;
+    break;
+  case BO_Sub:
+    return OO_Minus;
+    break;
+  case BO_AddAssign:
+    return OO_PlusEqual;
+    break;
+  case BO_SubAssign:
+    return OO_MinusEqual;
+    break;
+  default:
+    llvm_unreachable("Unsupported binary operator");
+  }
+}
+
 const ContainerData *getContainerData(ProgramStateRef State,
                                       const MemRegion *Cont) {
   return State->get<ContainerMap>(Cont);
Index: clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
@@ -26,7 +26,10 @@
 namespace {
 
 class InvalidatedIteratorChecker
-  : public Checker<check::PreCall> {
+  : public Checker<check::PreCall, check::PreStmt<UnaryOperator>,
+                   check::PreStmt<BinaryOperator>,
+                   check::PreStmt<ArraySubscriptExpr>,
+                   check::PreStmt<MemberExpr>> {
 
   std::unique_ptr<BugType> InvalidatedBugType;
 
@@ -37,6 +40,10 @@
   InvalidatedIteratorChecker();
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreStmt(const UnaryOperator *UO, CheckerContext &C) const;
+  void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const;
+  void checkPreStmt(const ArraySubscriptExpr *ASE, CheckerContext &C) const;
+  void checkPreStmt(const MemberExpr *ME, CheckerContext &C) const;
 
 };
 
@@ -65,6 +72,48 @@
   }
 }
 
+void InvalidatedIteratorChecker::checkPreStmt(const UnaryOperator *UO,
+                                              CheckerContext &C) const {
+  if (isa<CXXThisExpr>(UO->getSubExpr()))
+    return;
+
+  ProgramStateRef State = C.getState();
+  UnaryOperatorKind OK = UO->getOpcode();
+  SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext());
+  
+  if (isAccessOperator(OK)) {
+    verifyAccess(C, SubVal);
+  }
+}
+
+void InvalidatedIteratorChecker::checkPreStmt(const BinaryOperator *BO,
+                                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  BinaryOperatorKind OK = BO->getOpcode();
+  SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
+
+  if (isAccessOperator(OK)) {
+    verifyAccess(C, LVal);
+  }
+}
+
+void InvalidatedIteratorChecker::checkPreStmt(const ArraySubscriptExpr *ASE,
+                                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal LVal = State->getSVal(ASE->getLHS(), C.getLocationContext());
+  verifyAccess(C, LVal);
+}
+
+void InvalidatedIteratorChecker::checkPreStmt(const MemberExpr *ME,
+                                              CheckerContext &C) const {
+  if (!ME->isArrow() || ME->isImplicitAccess())
+    return;
+
+  ProgramStateRef State = C.getState();
+  SVal BaseVal = State->getSVal(ME->getBase(), C.getLocationContext());
+  verifyAccess(C, BaseVal);
+}
+
 void InvalidatedIteratorChecker::verifyAccess(CheckerContext &C, const SVal &Val) const {
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Val);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to