NoQ added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:530
+ auto value = RVal;
+ if (auto loc = value.getAs<Loc>()) {
+ value = State->getRawSVal(*loc);
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > Is there a test case for this hack?
> >
> > I'd also consider inspecting the AST (probably before passing the values to
> > `handleRandomIncrOrDecr()`) and making the decision based on that. Because
> > even though this pattern ("if a value is a loc and we expect a nonloc, do
> > an extra dereference") is present in many places in the analyzer, in most
> > of these places it doesn't work correctly (what if we try to discriminate
> > between `int*` and `int*&`?).
> I just want to get the sign of the integer value (if it is available). It
> turned out that I cannot do comparison between loc and nonloc. (Strange,
> because I can do anything else). After I created this hack, the Analyzer did
> not crash anymore on the llvm/clang code.
>
> I do not fully understand what I should fix here and how? In this particular
> place we expect some integer, thus no int* or int*&.
Loc value, essentially, *is* a pointer or reference value. If you're getting a
Loc, then your expectations of an integer are not met in the actual code. In
this case you *want* to know why they are not met, otherwise you may avoid the
crash, but do incorrect things and run into false positives. So i'd rather have
this investigated carefully.
You say that you are crashing otherwise - and then it should be trivial for you
to attach a debugger and `dump()` the expression for which you expect to take
the integer value, and see why it suddenly has a pointer type in a particular
case. From that you'd easily see what to do.
Also, crashes are often easy to auto-reduce using tools like `creduce`. Unlike
false positives, which may turn into true positives during reduction.
If you still don't see the reason why your workaround is necessary and what
exactly it does, could you attach a preprocessed file and an analyzer runline
for the crash, so that we could have a look together?
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:553
+
+ // When increasing by positive or decreasing by negative an iterator past its
+ // end, then it is a bug. We check for bugs before the operator call.
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > I think incrementing `end()` by `0` is not a bug (?)
> I think it is not a bug, but how to solve it properly? If I chose just
> greaterThanZero, then we have the same problem for decrementing end() by 0.
> Is it worth to create three states here?
Yep, i believe that indeed, we need three states here. There are three possible
cases.
https://reviews.llvm.org/D28771
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
