From d703305e404d3bb7f506e6f8ab0772d415263c48 Mon Sep 17 00:00:00 2001
From: Adam Balogh <adam.balogh@ericsson.com>
Date: Wed, 13 Mar 2019 13:55:11 +0000
Subject: [PATCH] [Analyzer] Skip symbolic regions based on conjured symbols in
 comparison of the containers of iterators

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

llvm-svn: 356049
---
 .../StaticAnalyzer/Checkers/IteratorChecker.cpp    | 49 ++++++++++++++++++++--
 clang/test/Analysis/mismatched-iterator.cpp        | 14 +++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
index 8649a32..38e9f22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ b/clang/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/clang/test/Analysis/mismatched-iterator.cpp b/clang/test/Analysis/mismatched-iterator.cpp
index 756d095..26a71c3 100644
--- a/clang/test/Analysis/mismatched-iterator.cpp
+++ b/clang/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
+}
-- 
2.7.4