baloghadamsoftware created this revision.
Extension of the mismatched iterator checker for constructors taking range of
first..last (first and last must be iterators of the same container) and also
for comparisons of iterators of different containers (one does not compare
iterators of different containers, since the set of iterators is partially
ordered, there are no relations between iterators of different containers,
except that they are always non-equal).
https://reviews.llvm.org/D32860
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
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<int> &v) {
+ std::vector<int> new_v(v.cbegin(), v.cend()); // no-warning
+}
+
void good_find(std::vector<int> &v, int n) {
std::find(v.cbegin(), v.cend(), n); // no-warning
}
@@ -21,6 +25,14 @@
std::find(i0, v1.cend(), n); // no-warning
}
+void good_comparison(std::vector<int> &v) {
+ if (v.cbegin() == v.cend()) {} // no-warning
+}
+
+void bad_ctor(std::vector<int> &v1, std::vector<int> &v2) {
+ std::vector<int> new_v(v1.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
+}
+
void bad_find(std::vector<int> &v1, std::vector<int> &v2, int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
@@ -35,3 +47,9 @@
v1 = std::move(v2);
std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_comparison(std::vector<int> &v1, std::vector<int> &v2) {
+ if (v1.cbegin() != v2.cend()) { // expected-warning{{Iterator access mismatched}}
+ *v1.cbegin();
+ }
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -131,6 +131,7 @@
class IteratorChecker
: public Checker<check::PreCall, check::PostCall,
+ check::PreStmt<CXXConstructExpr>,
check::PreStmt<CXXOperatorCallExpr>,
check::PostStmt<MaterializeTemporaryExpr>,
check::LiveSymbols, check::DeadSymbols,
@@ -163,6 +164,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;
@@ -188,7 +191,10 @@
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 CXXConstructExpr *CCE, CheckerContext &C) const;
+ void checkPostStmt(const DeclStmt *DS, CheckerContext &C) const;
void checkPostStmt(const MaterializeTemporaryExpr *MTE,
CheckerContext &C) const;
void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
@@ -218,6 +224,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);
@@ -341,6 +348,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<CXXInstanceCall>(&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<CXXConstructorCall>(&Call)) {
// The main purpose of iterators is to abstract away from different
@@ -501,6 +530,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);
@@ -811,6 +865,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<CXXBaseObjectRegion>()) {
+ 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, C, N);
+ }
+}
+
void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter1,
const SVal &Iter2) const {
// Verify match between the containers of two iterators
@@ -1048,6 +1120,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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
