Note tags marking the points where an iterator is incremented or decremented helps finding the root cause of iterator errors, especially out-of-range dereferences and out-of-range increments and decrements. This patch adds such tags.
Diff Detail
Event Timeline
A few nits, otherwise the patch is great!
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
707–708 | Are It1 and It2 used? Why do we default the latter to UndefinedVal? | |
clang/test/Analysis/iterator-modelling.cpp | ||
1–5 ↗ | (On Diff #248447) | Can we prettify this? :) |
clang/test/Analysis/iterator-range.cpp | ||
0–12 | And this? |
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
707–708 | I completely removed them from this patch. |
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
661–662 | Can we assert out the ChangeVal == 0 case or make a better message for it ("unchanged" or something like that)? |
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
661–662 | Another important thing to do here is to track the RHS value via trackExpressionValue(). I.e., why do we think that iterator is incremented by 1 and not by 2? |
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
661–662 | I agree, that is important, but where should I call it? This is the modeling checker, which models the increments and the decrements, but trackExpressionValue() can only be invoked for bug reports. Should I put it into the NoteTag lambda? |
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
661–662 | Yup, it should be possible to invoke trackExpressionValue(), and generally attach more visitors, inside a NoteTag. |
clang/test/Analysis/iterator-modelling.cpp | ||
---|---|---|
434 ↗ | (On Diff #252615) | Strange, that we do not get this note for prev. |
clang/test/Analysis/iterator-modelling.cpp | ||
---|---|---|
434 ↗ | (On Diff #252615) | Mmm, why 0-1? |
clang/test/Analysis/iterator-modelling.cpp | ||
---|---|---|
434 ↗ | (On Diff #252615) | Because it depends whether next() is inlined. |
Can we assert out the ChangeVal == 0 case or make a better message for it ("unchanged" or something like that)?