[Analyzer] Skip symbolic regions based on conjured symbols in comparison of the conta...

Checking whether two regions are the same is a partially decidable problem:
either we know for sure that they are the same or we cannot decide. A typical
case for this are the symbolic regions based on conjured symbols. Two
different conjured symbols are either the same or they are different. Since
we cannot decide this and want to reduce false positives as much as possible
we exclude these regions whenever checking whether two containers are the
same at iterator mismatch check.

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

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

diff --git a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
index 8649a32f054bf6901252c2f6f136d3c0b3fae3f4..38e9f22675f17b6029bf384728eb3ed1b122ecde 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -1098,14 +1098,34 @@ void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
   // Verify match between a container and the container of an iterator
   Cont = Cont->getMostDerivedObjectRegion();
 
+  if (const auto *ContSym = Cont->getSymbolicBase()) {
+    if (isa<SymbolConjured>(ContSym->getSymbol()))
+      return;
+  }
+
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
-  if (Pos && Pos->getContainer() != Cont) {
+  if (!Pos)
+    return;
+
+  const auto *IterCont = Pos->getContainer();
+
+  // Skip symbolic regions based on conjured symbols. Two conjured symbols
+  // may or may not be the same. For example, the same function can return
+  // the same or a different container but we get different conjured symbols
+  // for each call. This may cause false positives so omit them from the check.
+  if (const auto *ContSym = IterCont->getSymbolicBase()) {
+    if (isa<SymbolConjured>(ContSym->getSymbol()))
+      return;
+  }
+
+  if (IterCont != Cont) {
     auto *N = C.generateNonFatalErrorNode(State);
     if (!N) {
       return;
     }
-    reportMismatchedBug("Container accessed using foreign iterator argument.", Iter, Cont, C, N);
+    reportMismatchedBug("Container accessed using foreign iterator argument.",
+                        Iter, Cont, C, N);
   }
 }
 
@@ -1114,8 +1134,31 @@ void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter1,
   // Verify match between the containers of two iterators
   auto State = C.getState();
   const auto *Pos1 = getIteratorPosition(State, Iter1);
+  if (!Pos1)
+    return;
+
+  const auto *IterCont1 = Pos1->getContainer();
+
+  // Skip symbolic regions based on conjured symbols. Two conjured symbols
+  // may or may not be the same. For example, the same function can return
+  // the same or a different container but we get different conjured symbols
+  // for each call. This may cause false positives so omit them from the check.
+  if (const auto *ContSym = IterCont1->getSymbolicBase()) {
+    if (isa<SymbolConjured>(ContSym->getSymbol()))
+      return;
+  }
+
   const auto *Pos2 = getIteratorPosition(State, Iter2);
-  if (Pos1 && Pos2 && Pos1->getContainer() != Pos2->getContainer()) {
+  if (!Pos2)
+    return;
+
+  const auto *IterCont2 = Pos2->getContainer();
+  if (const auto *ContSym = IterCont2->getSymbolicBase()) {
+    if (isa<SymbolConjured>(ContSym->getSymbol()))
+      return;
+  }
+
+  if (IterCont1 != IterCont2) {
     auto *N = C.generateNonFatalErrorNode(State);
     if (!N)
       return;

diff --git a/test/Analysis/mismatched-iterator.cpp b/test/Analysis/mismatched-iterator.cpp
index 756d095443576c50eb9a875e9374e2c551a7566c..26a71c399c7057cfbfc82a011bd616a761d103c5 100644 (file)
--- a/test/Analysis/mismatched-iterator.cpp
+++ b/test/Analysis/mismatched-iterator.cpp
@@ -189,3 +189,17 @@ void bad_comparison(std::vector<int> &v1, std::vector<int> &v2) {
     *v1.cbegin();
   }
 }
+
+std::vector<int> &return_vector_ref();
+
+void ignore_conjured1() {
+  std::vector<int> &v1 = return_vector_ref(), &v2 = return_vector_ref();
+
+  v2.erase(v1.cbegin()); // no-warning
+}
+
+void ignore_conjured2() {
+  std::vector<int> &v1 = return_vector_ref(), &v2 = return_vector_ref();
+
+  if (v1.cbegin() == v2.cbegin()) {} //no-warning
+}