[PATCH] D70244: [Analyzer] Iterator Checkers: Replace `UnknownVal` in comparison result by a conjured value

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.

Sometimes the return value of a comparison operator call is `UnkownVal`. Since no assumptions can be made on `UnknownVal`, this leeds to keeping impossible execution paths in the exploded graph resulting in poor performance and false positives. To overcome this we replace unknown results of iterator comparisons by conjured symbols.


Repository:
  rC Clang

https://reviews.llvm.org/D70244

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  clang/test/Analysis/invalidated-iterator.cpp
  clang/test/Analysis/iterator-modelling.cpp


Index: clang/test/Analysis/iterator-modelling.cpp
===================================================================
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -1966,7 +1966,7 @@
   clang_analyzer_eval(clang_analyzer_container_end(V) ==
                       clang_analyzer_iterator_position(first)); // expected-warning at -1{{FALSE}} expected-warning at -1{{TRUE}}
   if (V.end() != first) {
-      clang_analyzer_eval(clang_analyzer_container_end(V) ==
-                        clang_analyzer_iterator_position(first)); // expected-warning at -1{{FALSE}} expected-warning at -1 0-1{{TRUE}} FIXME: should only expect FALSE in every case
+    clang_analyzer_eval(clang_analyzer_container_end(V) ==
+                        clang_analyzer_iterator_position(first)); // expected-warning at -1{{FALSE}}
   }
 }
Index: clang/test/Analysis/invalidated-iterator.cpp
===================================================================
--- clang/test/Analysis/invalidated-iterator.cpp
+++ clang/test/Analysis/invalidated-iterator.cpp
@@ -120,4 +120,3 @@
   V.erase(i);
   auto j = V.cbegin(); // no-warning
 }
-
Index: clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -180,7 +180,7 @@
   std::unique_ptr<BugType> InvalidatedBugType;
   std::unique_ptr<BugType> DebugMsgBugType;
 
-  void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+  void handleComparison(CheckerContext &C, const Expr *CE, SVal RetVal,
                         const SVal &LVal, const SVal &RVal,
                         OverloadedOperatorKind Op) const;
   void processComparison(CheckerContext &C, ProgramStateRef State,
@@ -883,7 +883,7 @@
 }
 
 void IteratorChecker::handleComparison(CheckerContext &C, const Expr *CE,
-                                       const SVal &RetVal, const SVal &LVal,
+                                       SVal RetVal, const SVal &LVal,
                                        const SVal &RVal,
                                        OverloadedOperatorKind Op) const {
   // Record the operands and the operator of the comparison for the next
@@ -922,6 +922,16 @@
     RPos = getIteratorPosition(State, RVal);
   }
 
+  // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+  // instead.
+  if (RetVal.isUnknown()) {
+    auto &SymMgr = C.getSymbolManager();
+    auto *LCtx = C.getLocationContext();
+    RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
+        CE, LCtx, C.getASTContext().BoolTy, C.blockCount()));
+    State = State->BindExpr(CE, LCtx, RetVal);
+  }
+
   processComparison(C, State, LPos->getOffset(), RPos->getOffset(), RetVal, Op);
 }
 
@@ -943,7 +953,7 @@
   const auto ConditionVal = RetVal.getAs<DefinedSVal>();
   if (!ConditionVal)
     return;
-  
+
   if (auto StateTrue = relateSymbols(State, Sym1, Sym2, Op == OO_EqualEqual)) {
     StateTrue = StateTrue->assume(*ConditionVal, true);
     C.addTransition(StateTrue);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70244.229313.patch
Type: text/x-patch
Size: 3129 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191114/8c1a40db/attachment.bin>