[Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators...

If an iterator is represented by a derived C++ class but its comparison operator
is for its base the iterator checkers cannot recognize the iterators compared.
This results in false positives in very straightforward cases (range error when
dereferencing an iterator after disclosing that it is equal to the past-the-end
iterator).

To overcome this problem we always use the region of the topmost base class for
iterators stored in a region. A new method called getMostDerivedObjectRegion()
was added to the MemRegion class to get this region.

Differential Revision: https://reviews.llvm.org/D54466

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@348244 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 0b0e32b8e324e99713531e949301999d096b5a8d..a4ddb58eb158f822acf1209816c0eea360c7f88a 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -118,6 +118,10 @@ public:
 
   const MemRegion *getBaseRegion() const;
 
+  /// Recursively retrieve the region of the most derived class instance of
+  /// regions of C++ base class instances.
+  const MemRegion *getMostDerivedObjectRegion() const;
+
   /// Check if the region is a subregion of the given region.
   /// Each region is a subregion of itself.
   virtual bool isSubRegionOf(const MemRegion *R) const;

diff --git a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
index 7a717513429dac621e149244337b78bfbf615960..e221252e5921533cb6fd8b543aac6c701205b603 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -1089,9 +1089,7 @@ void IteratorChecker::verifyRandomIncrOrDecr(CheckerContext &C,
 void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
                                   const MemRegion *Cont) const {
   // Verify match between a container and the container of an iterator
-  while (const auto *CBOR = Cont->getAs<CXXBaseObjectRegion>()) {
-    Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
@@ -1125,9 +1123,7 @@ void IteratorChecker::handleBegin(CheckerContext &C, const Expr *CE,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has a begin symbol then use it. Otherwise first
   // create a new one.
@@ -1151,9 +1147,7 @@ void IteratorChecker::handleEnd(CheckerContext &C, const Expr *CE,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has an end symbol then use it. Otherwise first
   // create a new one.
@@ -1174,9 +1168,7 @@ void IteratorChecker::handleEnd(CheckerContext &C, const Expr *CE,
 void IteratorChecker::assignToContainer(CheckerContext &C, const Expr *CE,
                                         const SVal &RetVal,
                                         const MemRegion *Cont) const {
-  while (const auto *CBOR = Cont->getAs<CXXBaseObjectRegion>()) {
-    Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   auto &SymMgr = C.getSymbolManager();
@@ -1194,9 +1186,7 @@ void IteratorChecker::handleAssign(CheckerContext &C, const SVal &Cont,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // Assignment of a new value to a container always invalidates all its
   // iterators
@@ -1211,9 +1201,7 @@ void IteratorChecker::handleAssign(CheckerContext &C, const SVal &Cont,
   if (!OldCont.isUndef()) {
     const auto *OldContReg = OldCont.getAsRegion();
     if (OldContReg) {
-      while (const auto *CBOR = OldContReg->getAs<CXXBaseObjectRegion>()) {
-        OldContReg = CBOR->getSuperRegion();
-      }
+      OldContReg = OldContReg->getMostDerivedObjectRegion();
       const auto OldCData = getContainerData(State, OldContReg);
       if (OldCData) {
         if (const auto OldEndSym = OldCData->getEnd()) {
@@ -1273,9 +1261,7 @@ void IteratorChecker::handleClear(CheckerContext &C, const SVal &Cont) const {
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // The clear() operation invalidates all the iterators, except the past-end
   // iterators of list-like containers
@@ -1302,9 +1288,7 @@ void IteratorChecker::handlePushBack(CheckerContext &C,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // For deque-like containers invalidate all iterator positions
   auto State = C.getState();
@@ -1341,9 +1325,7 @@ void IteratorChecker::handlePopBack(CheckerContext &C, const SVal &Cont) const {
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto CData = getContainerData(State, ContReg);
@@ -1381,9 +1363,7 @@ void IteratorChecker::handlePushFront(CheckerContext &C,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // For deque-like containers invalidate all iterator positions
   auto State = C.getState();
@@ -1416,9 +1396,7 @@ void IteratorChecker::handlePopFront(CheckerContext &C,
   if (!ContReg)
     return;
 
-  while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
-    ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto CData = getContainerData(State, ContReg);
@@ -2015,7 +1993,8 @@ ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
 
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
                                             const SVal &Val) {
-  if (const auto Reg = Val.getAsRegion()) {
+  if (auto Reg = Val.getAsRegion()) {
+    Reg = Reg->getMostDerivedObjectRegion();
     return State->get<IteratorRegionMap>(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
     return State->get<IteratorSymbolMap>(Sym);
@@ -2028,7 +2007,8 @@ const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
                                             RegionOrSymbol RegOrSym) {
   if (RegOrSym.is<const MemRegion *>()) {
-    return State->get<IteratorRegionMap>(RegOrSym.get<const MemRegion *>());
+    auto Reg = RegOrSym.get<const MemRegion *>()->getMostDerivedObjectRegion();
+    return State->get<IteratorRegionMap>(Reg);
   } else if (RegOrSym.is<SymbolRef>()) {
     return State->get<IteratorSymbolMap>(RegOrSym.get<SymbolRef>());
   }
@@ -2037,7 +2017,8 @@ const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 
 ProgramStateRef setIteratorPosition(ProgramStateRef State, const SVal &Val,
                                     const IteratorPosition &Pos) {
-  if (const auto Reg = Val.getAsRegion()) {
+  if (auto Reg = Val.getAsRegion()) {
+    Reg = Reg->getMostDerivedObjectRegion();
     return State->set<IteratorRegionMap>(Reg, Pos);
   } else if (const auto Sym = Val.getAsSymbol()) {
     return State->set<IteratorSymbolMap>(Sym, Pos);
@@ -2051,8 +2032,8 @@ ProgramStateRef setIteratorPosition(ProgramStateRef State,
                                     RegionOrSymbol RegOrSym,
                                     const IteratorPosition &Pos) {
   if (RegOrSym.is<const MemRegion *>()) {
-    return State->set<IteratorRegionMap>(RegOrSym.get<const MemRegion *>(),
-                                         Pos);
+    auto Reg = RegOrSym.get<const MemRegion *>()->getMostDerivedObjectRegion();
+    return State->set<IteratorRegionMap>(Reg, Pos);
   } else if (RegOrSym.is<SymbolRef>()) {
     return State->set<IteratorSymbolMap>(RegOrSym.get<SymbolRef>(), Pos);
   }
@@ -2060,7 +2041,8 @@ ProgramStateRef setIteratorPosition(ProgramStateRef State,
 }
 
 ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) {
-  if (const auto Reg = Val.getAsRegion()) {
+  if (auto Reg = Val.getAsRegion()) {
+    Reg = Reg->getMostDerivedObjectRegion();
     return State->remove<IteratorRegionMap>(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
     return State->remove<IteratorSymbolMap>(Sym);

diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index 221b9176def90e21854ed4207b9c2a1059642335..da368de322637f2dbd265f6c4a1bba443034f69d 100644 (file)
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1175,6 +1175,15 @@ const MemRegion *MemRegion::getBaseRegion() const {
   return R;
 }
 
+// getgetMostDerivedObjectRegion gets the region of the root class of a C++
+// class hierarchy.
+const MemRegion *MemRegion::getMostDerivedObjectRegion() const {
+  const MemRegion *R = this;
+  while (const auto *BR = dyn_cast<CXXBaseObjectRegion>(R))
+    R = BR->getSuperRegion();
+  return R;
+}
+
 bool MemRegion::isSubRegionOf(const MemRegion *) const {
   return false;
 }

diff --git a/test/Analysis/iterator-range.cpp b/test/Analysis/iterator-range.cpp
index 78cdb095c4d163e94a87af14a1279d3effe472fa..1a3207367ca3506035d5ed8e3217111c9ecaa9d3 100644 (file)
--- a/test/Analysis/iterator-range.cpp
+++ b/test/Analysis/iterator-range.cpp
@@ -200,3 +200,40 @@ void bad_move_push_back(std::list<int> &L1, std::list<int> &L2, int n) {
   ++i0;
   *++i0; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+struct simple_iterator_base {
+  simple_iterator_base();
+  simple_iterator_base(const simple_iterator_base& rhs);
+  simple_iterator_base &operator=(const simple_iterator_base& rhs);
+  virtual ~simple_iterator_base();
+  bool friend operator==(const simple_iterator_base &lhs,
+                         const simple_iterator_base &rhs);
+  bool friend operator!=(const simple_iterator_base &lhs,
+                         const simple_iterator_base &rhs);
+private:
+  int *ptr;
+};
+
+struct simple_derived_iterator: public simple_iterator_base {
+  int& operator*();
+  int* operator->();
+  simple_iterator_base &operator++();
+  simple_iterator_base operator++(int);
+  simple_iterator_base &operator--();
+  simple_iterator_base operator--(int);
+};
+
+struct simple_container {
+  typedef simple_derived_iterator iterator;
+
+  iterator begin();
+  iterator end();
+};
+
+void good_derived(simple_container c) {
+  auto i0 = c.end();
+  if (i0 != c.end()) {
+    clang_analyzer_warnIfReached();
+    *i0; // no-warning
+  }
+}