[PATCH] D28771: [Analyzer] Various fixes for the IteratorPastEnd checker

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 20 09:13:40 PST 2017

NoQ added a comment.

Thanks for the update, most welcome!

Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:234
+        if (Call.getNumArgs() >= 1) {
+          const auto &InstCall = static_cast<const CXXInstanceCall &>(Call);
+          handleRandomIncrOrDecr(C, Func->getOverloadedOperator(),
We usually write these as
const auto *InstCall = cast<CXXInstanceCall>(&Call);
Mostly because our custom `cast<>` has an assertion inside.

Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:514
+    CheckerContext &C, OverloadedOperatorKind Op, const SVal &RetVal,
+    const SVal &LVal, const SVal &RVal, QualType RValType,
+    bool PreCheck) const {
I'd suggest renaming the parameters to "LHS" and "RHS" or something like that, so that not to confuse those with "lvalue" and "rvalue".

Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:530
+  auto value = RVal;
+  if (auto loc = value.getAs<Loc>()) {
+    value = State->getRawSVal(*loc);
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*&`?).

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.
I think incrementing `end()` by `0` is not a bug (?)


More information about the cfe-commits mailing list