Pārlūkot izejas kodu

[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



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356049 91177308-0d34-0410-b5e6-96231b3b80d8
Adam Balogh 6 gadi atpakaļ
vecāks
revīzija
ec9dd97583

+ 46 - 3
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
   // Verify match between a container and the container of an iterator
   Cont = Cont->getMostDerivedObjectRegion();
   Cont = Cont->getMostDerivedObjectRegion();
 
 
+  if (const auto *ContSym = Cont->getSymbolicBase()) {
+    if (isa<SymbolConjured>(ContSym->getSymbol()))
+      return;
+  }
+
   auto State = C.getState();
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
   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);
     auto *N = C.generateNonFatalErrorNode(State);
     if (!N) {
     if (!N) {
       return;
       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
   // Verify match between the containers of two iterators
   auto State = C.getState();
   auto State = C.getState();
   const auto *Pos1 = getIteratorPosition(State, Iter1);
   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);
   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);
     auto *N = C.generateNonFatalErrorNode(State);
     if (!N)
     if (!N)
       return;
       return;

+ 14 - 0
test/Analysis/mismatched-iterator.cpp

@@ -189,3 +189,17 @@ void bad_comparison(std::vector<int> &v1, std::vector<int> &v2) {
     *v1.cbegin();
     *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
+}