baloghadamsoftware updated this revision to Diff 163736.
baloghadamsoftware added a comment.
Herald added subscribers: Szelethus, mikhail.ramalho.
Checking of constructor parameters moved to `PreCall` check.
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
}
@@ -34,6 +38,14 @@
std::find(v2.cbegin(), i0, 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{{Iterators of different containers used where the same container is expected}}
+}
+
void bad_find(std::vector<int> &v1, std::vector<int> &v2, int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
@@ -60,3 +72,9 @@
v1 = std::move(v2);
std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
+
+void bad_comparison(std::vector<int> &v1, std::vector<int> &v2) {
+ if (v1.cbegin() != v2.cend()) { // expected-warning{{Iterators of different containers used where the same container is expected}}
+ *v1.cbegin();
+ }
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -236,6 +236,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;
@@ -276,6 +279,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);
@@ -397,8 +401,48 @@
} 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)) {
+ } else if (isa<CXXConstructorCall>(&Call)) {
+ // Check match of first-last iterator pair in a constructor of a container
+ if (Call.getNumArgs() < 2)
+ return;
+
+ const auto *Ctr = cast<CXXConstructorDecl>(Call.getDecl());
+ if (Ctr->getNumParams() < 2)
+ return;
+
+ if (Ctr->getParamDecl(0)->getName() != "first" ||
+ Ctr->getParamDecl(1)->getName() != "last")
+ return;
+
+ if (!isIteratorType(Call.getArgExpr(0)->getType()) ||
+ !isIteratorType(Call.getArgExpr(1)->getType()))
+ return;
+
+ verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));
+ } else {
// The main purpose of iterators is to abstract away from different
// containers and provide a (maybe limited) uniform access to them.
// This implies that any correctly written template function that
@@ -1102,6 +1146,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<BugReport>(*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 {
@@ -1173,6 +1227,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
