Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -197,6 +197,7 @@ class IteratorChecker : public Checker, check::PreStmt, check::PostStmt, check::LiveSymbols, check::DeadSymbols, @@ -229,6 +230,8 @@ void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op, const SVal &RetVal, const SVal &LHS, const SVal &RHS) const; + void verifyMatch(CheckerContext &C, const SVal &Iter, + const MemRegion *Cont) const; void verifyMatch(CheckerContext &C, const SVal &Iter1, const SVal &Iter2) const; @@ -237,6 +240,9 @@ void reportMismatchedBug(const StringRef &Message, const SVal &Val1, const SVal &Val2, CheckerContext &C, ExplodedNode *ErrNode) const; + void reportMismatchedBug(const StringRef &Message, const SVal &Val, + const MemRegion *Reg, CheckerContext &C, + ExplodedNode *ErrNode) const; void reportInvalidatedBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const; @@ -255,6 +261,7 @@ void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void checkPreStmt(const CXXConstructExpr *CCE, CheckerContext &C) const; void checkPreStmt(const CXXOperatorCallExpr *COCE, CheckerContext &C) const; void checkPostStmt(const MaterializeTemporaryExpr *MTE, CheckerContext &C) const; @@ -278,6 +285,7 @@ bool isIteratorType(const QualType &Type); bool isIterator(const CXXRecordDecl *CRD); +bool isComparisonOperator(OverloadedOperatorKind OK); bool isBeginCall(const FunctionDecl *Func); bool isEndCall(const FunctionDecl *Func); bool isAssignmentOperator(OverloadedOperatorKind OK); @@ -396,6 +404,28 @@ } else { verifyDereference(C, Call.getArgSVal(0)); } + } else if (ChecksEnabled[CK_MismatchedIteratorChecker] && + isComparisonOperator(Func->getOverloadedOperator())) { + // Check for comparisons of iterators of different containers + if (const auto *InstCall = dyn_cast(&Call)) { + if (Call.getNumArgs() < 1) + return; + + if (!isIteratorType(InstCall->getCXXThisExpr()->getType()) || + !isIteratorType(Call.getArgExpr(0)->getType())) + return; + + verifyMatch(C, InstCall->getCXXThisVal(), Call.getArgSVal(0)); + } else { + if (Call.getNumArgs() < 2) + return; + + if (!isIteratorType(Call.getArgExpr(0)->getType()) || + !isIteratorType(Call.getArgExpr(1)->getType())) + return; + + verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1)); + } } } else if (!isa(&Call)) { // The main purpose of iterators is to abstract away from different @@ -571,6 +601,31 @@ } } +void IteratorChecker::checkPreStmt(const CXXConstructExpr *CCE, + CheckerContext &C) const { + // Check match of first-last iterator pair in a constructor of a container + if (CCE->getNumArgs() < 2) + return; + + const auto *Ctr = CCE->getConstructor(); + if (Ctr->getNumParams() < 2) + return; + + if (Ctr->getParamDecl(0)->getName() != "first" || + Ctr->getParamDecl(1)->getName() != "last") + return; + + if (!isIteratorType(CCE->getArg(0)->getType()) || + !isIteratorType(CCE->getArg(1)->getType())) + return; + + auto State = C.getState(); + const auto *LCtx = C.getPredecessor()->getLocationContext(); + + verifyMatch(C, State->getSVal(CCE->getArg(0), LCtx), + State->getSVal(CCE->getArg(1), LCtx)); +} + void IteratorChecker::checkPreStmt(const CXXOperatorCallExpr *COCE, CheckerContext &C) const { const auto *ThisExpr = COCE->getArg(0); @@ -930,6 +985,24 @@ } } +void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter, + const MemRegion *Cont) const { + // Verify match between a container and the container of an iterator + while (const auto *CBOR = Cont->getAs()) { + Cont = CBOR->getSuperRegion(); + } + + auto State = C.getState(); + const auto *Pos = getIteratorPosition(State, Iter); + if (Pos && Pos->getContainer() != Cont) { + auto *N = C.generateNonFatalErrorNode(State); + if (!N) { + return; + } + reportMismatchedBug("Iterator access mismatched.", Iter, Cont, C, N); + } +} + void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter1, const SVal &Iter2) const { // Verify match between the containers of two iterators @@ -1115,6 +1188,16 @@ C.emitReport(std::move(R)); } +void IteratorChecker::reportMismatchedBug(const StringRef &Message, + const SVal &Val, const MemRegion *Reg, + CheckerContext &C, + ExplodedNode *ErrNode) const { + auto R = llvm::make_unique(*MismatchedBugType, Message, ErrNode); + R->markInteresting(Val); + R->markInteresting(Reg); + C.emitReport(std::move(R)); +} + void IteratorChecker::reportInvalidatedBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const { @@ -1186,6 +1269,11 @@ HasPostIncrOp && HasDerefOp; } +bool isComparisonOperator(OverloadedOperatorKind OK) { + return OK == OO_EqualEqual || OK == OO_ExclaimEqual || OK == OO_Less || + OK == OO_LessEqual || OK == OO_Greater || OK == OO_GreaterEqual; +} + bool isBeginCall(const FunctionDecl *Func) { const auto *IdInfo = Func->getIdentifier(); if (!IdInfo) Index: test/Analysis/mismatched-iterator.cpp =================================================================== --- test/Analysis/mismatched-iterator.cpp +++ test/Analysis/mismatched-iterator.cpp @@ -3,6 +3,10 @@ #include "Inputs/system-header-simulator-cxx.h" +void good_ctor(std::vector &v) { + std::vector new_v(v.cbegin(), v.cend()); // no-warning +} + void good_find(std::vector &v, int n) { std::find(v.cbegin(), v.cend(), n); // no-warning } @@ -34,6 +38,14 @@ std::find(v2.cbegin(), i0, n); // no-warning } +void good_comparison(std::vector &v) { + if (v.cbegin() == v.cend()) {} // no-warning +} + +void bad_ctor(std::vector &v1, std::vector &v2) { + std::vector new_v(v1.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}} +} + void bad_find(std::vector &v1, std::vector &v2, int n) { std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}} } @@ -60,3 +72,9 @@ v1 = std::move(v2); std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}} } + +void bad_comparison(std::vector &v1, std::vector &v2) { + if (v1.cbegin() != v2.cend()) { // expected-warning{{Iterator access mismatched}} + *v1.cbegin(); + } +}