[PATCH] D78289: [analyzer] Stability improvements for IteratorModeling functions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 259521.
ASDenysPetrov edited the summary of this revision.
ASDenysPetrov added a comment.

@baloghadamsoftware
I've added a test for unscoped SVal, but still can't come up with `if (!LPos || 
!RPos)`
Could you, please, suggest me something?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78289/new/

https://reviews.llvm.org/D78289

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/iterator-range.cpp


Index: clang/test/Analysis/iterator-range.cpp
===================================================================
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -810,6 +810,19 @@
   auto j = std::prev(i, 0); // no-warning
 }
 
+// std::prev() with int* for checking Loc value argument
+namespace std {
+template <typename T>
+T prev(T, int *);
+}
+
+void prev_loc_value(const std::vector<int> &V, int o) {
+
+  auto i = return_any_iterator(V.begin());
+  int *offset = &o;
+  auto j = std::prev(i, offset); // no-warning
+}
+
 //
 // Structure member dereference operators
 //
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -402,7 +402,7 @@
   if (!Cont)
     return;
 
-  // At least one of the iterators have recorded positions. If one of them has
+  // At least one of the iterators has recorded positions. If one of them does
   // not then create a new symbol for the offset.
   SymbolRef Sym;
   if (!LPos || !RPos) {
@@ -422,9 +422,14 @@
     RPos = getIteratorPosition(State, RVal);
   }
 
-  // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+  if (!LPos || !RPos)
+    return;
+
+  // We cannot make assumptions on `UnknownVal`. Let us conjure a symbol
   // instead.
   if (RetVal.isUnknown()) {
+    if (!State)
+      return;
     auto &SymMgr = C.getSymbolManager();
     auto *LCtx = C.getLocationContext();
     RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
@@ -532,8 +537,9 @@
     return;
 
   const auto *value = &RHS;
+  SVal val;
   if (auto loc = RHS.getAs<Loc>()) {
-    const auto val = State->getRawSVal(*loc);
+    val = State->getRawSVal(*loc);
     value = &val;
   }
 



Index: clang/test/Analysis/iterator-range.cpp
===================================================================
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -810,6 +810,19 @@
   auto j = std::prev(i, 0); // no-warning
 }
 
+// std::prev() with int* for checking Loc value argument
+namespace std {
+template <typename T>
+T prev(T, int *);
+}
+
+void prev_loc_value(const std::vector<int> &V, int o) {
+
+  auto i = return_any_iterator(V.begin());
+  int *offset = &o;
+  auto j = std::prev(i, offset); // no-warning
+}
+
 //
 // Structure member dereference operators
 //
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -402,7 +402,7 @@
   if (!Cont)
     return;
 
-  // At least one of the iterators have recorded positions. If one of them has
+  // At least one of the iterators has recorded positions. If one of them does
   // not then create a new symbol for the offset.
   SymbolRef Sym;
   if (!LPos || !RPos) {
@@ -422,9 +422,14 @@
     RPos = getIteratorPosition(State, RVal);
   }
 
-  // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+  if (!LPos || !RPos)
+    return;
+
+  // We cannot make assumptions on `UnknownVal`. Let us conjure a symbol
   // instead.
   if (RetVal.isUnknown()) {
+    if (!State)
+      return;
     auto &SymMgr = C.getSymbolManager();
     auto *LCtx = C.getLocationContext();
     RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
@@ -532,8 +537,9 @@
     return;
 
   const auto *value = &RHS;
+  SVal val;
   if (auto loc = RHS.getAs<Loc>()) {
-    const auto val = State->getRawSVal(*loc);
+    val = State->getRawSVal(*loc);
     value = &val;
   }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits