NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Uh-oh, so you're saying that you're actively modeling every single operator `+` 
and `-` on every single integer in the program even when they don't represent 
any iterators at all? How can a raw integer be an iterator to begin with, given 
that you can't dereference an integer? Is this caused by D82185 
<https://reviews.llvm.org/D82185>? Can you defend against such situations by 
checking that the operand is a pointer before doing any operations?

As of now it sounds like the function quits pretty quickly because it doesn't 
find an old iterator position for that integer but i'm pretty worried about us 
doing weird things because we're not checking our types regularly enough.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp:227
   auto Value = RHS;
   if (auto ValAsLoc = RHS.getAs<Loc>()) {
     Value = State->getRawSVal(*ValAsLoc);
----------------
This code, as per my usual concern, is pretty questionable. A `Loc` may be both 
the iterator lvalue or the iterator rvalue and you can't figure this out by 
looking at the value. The information on which the decision to perform this 
dereference is based should be communicated through a different channel.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp:232
+  if (Value.isUnknownOrUndef())
+    //  if (Value.isUnknownOrUndef())
     return;
----------------
This comment is probably unnecessary.


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

https://reviews.llvm.org/D85424

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

Reply via email to