baloghadamsoftware added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:530
+ auto value = RVal;
+ if (auto loc = value.getAs<Loc>()) {
+ value = State->getRawSVal(*loc);
----------------
NoQ wrote:
> 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?
Just to be clear: I know why it crashes without the hack: I simply cannot
compare loc and nonloc. Since concrete 0 is nonloc I need another nonloc. I
suppose this happens if an integer reference is passed to the operator +, +=, -
or -=. So I thought that dereferencing it by getting the raw SVal is the
correct thing to do.
================
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.
----------------
NoQ wrote:
> 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.
OK, I will do it.
https://reviews.llvm.org/D28771
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
