[clang][analyzer] Improve the modeling of insert in MismatchedIteratorChecker by flovent · Pull Request #132596 · llvm/llvm-project (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fixes #132010
Associative containers in STL has an unique insert overload member function comparing to un-associative containers(https://en.cppreference.com/w/cpp/container/unordered_set/insert):
template< class InputIt >
void insert( InputIt first, InputIt last );
Add support for this insert overload in MismatchedIteratorChecker, verify if first and last belongs to the same container in this case.
@llvm/pr-subscribers-clang
Author: None (flovent)
Changes
This PR fixs #132010.
Associative containers in STL has an unique insert overload member function comparing to un-associative containers(https://en.cppreference.com/w/cpp/container/unordered_set/insert):
template< class InputIt >
void insert( InputIt first, InputIt last );
Add support for this insert overload in MismatchedIteratorChecker, verify if first and last belongs to the same container in this case.
Full diff: https://github.com/llvm/llvm-project/pull/132596.diff
4 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp (+12-6)
- (modified) clang/test/Analysis/Inputs/system-header-simulator-cxx.h (+4)
- (added) clang/test/Analysis/issue-132010.cpp (+12)
- (modified) clang/test/Analysis/mismatched-iterator.cpp (+10)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp index 82a6228318179..1c101b91f727f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp @@ -91,12 +91,18 @@ void MismatchedIteratorChecker::checkPreCall(const CallEvent &Call, InstCall->getCXXThisVal().getAsRegion()); } } else if (isInsertCall(Func)) {
verifyMatch(C, Call.getArgSVal(0),InstCall->getCXXThisVal().getAsRegion());if (Call.getNumArgs() == 3 &&isIteratorType(Call.getArgExpr(1)->getType()) &&isIteratorType(Call.getArgExpr(2)->getType())) {verifyMatch(C, Call.getArgSVal(1), Call.getArgSVal(2));
if (Call.getNumArgs() == 2 &&isIteratorType(Call.getArgExpr(0)->getType()) &&isIteratorType(Call.getArgExpr(1)->getType())) {verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));} else {verifyMatch(C, Call.getArgSVal(0),InstCall->getCXXThisVal().getAsRegion());if (Call.getNumArgs() == 3 &&isIteratorType(Call.getArgExpr(1)->getType()) &&isIteratorType(Call.getArgExpr(2)->getType())) {verifyMatch(C, Call.getArgSVal(1), Call.getArgSVal(2));
} else if (isEmplaceCall(Func)) { verifyMatch(C, Call.getArgSVal(0),} }
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index a379a47515668..c5aeb0af9d578 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1244,6 +1244,7 @@ template< class Alloc = std::allocator
class unordered_set { public:
unordered_set() {} unordered_set(initializer_list __list) {}
class iterator {
@@ -1260,6 +1261,9 @@ template< Key *val; iterator begin() const { return iterator(val); } iterator end() const { return iterator(val + 1); } +
- template< class InputIt >
- void insert( InputIt first, InputIt last );
};
template diff --git a/clang/test/Analysis/issue-132010.cpp b/clang/test/Analysis/issue-132010.cpp new file mode 100644 index 0000000000000..abdaed57f26b9 --- /dev/null +++ b/clang/test/Analysis/issue-132010.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_analyze_cc1 -analyzer-config aggressive-binary-operation-simplification=true -analyzer-checker=alpha.cplusplus.MismatchedIterator -analyzer-output text -verify %s + +// expected-no-diagnostics + +#include "Inputs/system-header-simulator-cxx.h" + +void f() +{
- std::list l;
- std::unordered_set us;
- us.insert(l.cbegin(), l.cend()); // no warning
+} diff --git a/clang/test/Analysis/mismatched-iterator.cpp b/clang/test/Analysis/mismatched-iterator.cpp index 570e742751ead..325e7764ad7fa 100644 --- a/clang/test/Analysis/mismatched-iterator.cpp +++ b/clang/test/Analysis/mismatched-iterator.cpp @@ -19,6 +19,11 @@ void good_insert4(std::vector &V, int len, int n) { V.insert(V.cbegin(), {n-1, n, n+1}); // no-warning }
+void good_insert5(std::vector &V, int len, int n) {
- std::unordered_set us;
- us.insert(V.cbegin(), V.cend()); // no-warning +}
- void good_insert_find(std::vector &V, int n, int m) { auto i = std::find(V.cbegin(), V.cend(), n); V.insert(i, m); // no-warning @@ -70,6 +75,11 @@ void bad_insert4(std::vector &V1, std::vector &V2, int len, int n) { V2.insert(V1.cbegin(), {n-1, n, n+1}); // expected-warning{{Container accessed using foreign iterator argument}} }
+void bad_insert5(std::vector &V1, std::vector &V2, int len, int n) {
- std::unordered_set us;
- us.insert(V1.cbegin(), V2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}} +}
- void bad_erase1(std::vector &V1, std::vector &V2) { V2.erase(V1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}} }
@llvm/pr-subscribers-clang-static-analyzer-1
Author: None (flovent)
Changes
This PR fixs #132010.
Associative containers in STL has an unique insert overload member function comparing to un-associative containers(https://en.cppreference.com/w/cpp/container/unordered_set/insert):
template< class InputIt >
void insert( InputIt first, InputIt last );
Add support for this insert overload in MismatchedIteratorChecker, verify if first and last belongs to the same container in this case.
Full diff: https://github.com/llvm/llvm-project/pull/132596.diff
4 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp (+12-6)
- (modified) clang/test/Analysis/Inputs/system-header-simulator-cxx.h (+4)
- (added) clang/test/Analysis/issue-132010.cpp (+12)
- (modified) clang/test/Analysis/mismatched-iterator.cpp (+10)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp index 82a6228318179..1c101b91f727f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp @@ -91,12 +91,18 @@ void MismatchedIteratorChecker::checkPreCall(const CallEvent &Call, InstCall->getCXXThisVal().getAsRegion()); } } else if (isInsertCall(Func)) {
verifyMatch(C, Call.getArgSVal(0),InstCall->getCXXThisVal().getAsRegion());if (Call.getNumArgs() == 3 &&isIteratorType(Call.getArgExpr(1)->getType()) &&isIteratorType(Call.getArgExpr(2)->getType())) {verifyMatch(C, Call.getArgSVal(1), Call.getArgSVal(2));
if (Call.getNumArgs() == 2 &&isIteratorType(Call.getArgExpr(0)->getType()) &&isIteratorType(Call.getArgExpr(1)->getType())) {verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));} else {verifyMatch(C, Call.getArgSVal(0),InstCall->getCXXThisVal().getAsRegion());if (Call.getNumArgs() == 3 &&isIteratorType(Call.getArgExpr(1)->getType()) &&isIteratorType(Call.getArgExpr(2)->getType())) {verifyMatch(C, Call.getArgSVal(1), Call.getArgSVal(2));
} else if (isEmplaceCall(Func)) { verifyMatch(C, Call.getArgSVal(0),} }
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index a379a47515668..c5aeb0af9d578 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1244,6 +1244,7 @@ template< class Alloc = std::allocator
class unordered_set { public:
unordered_set() {} unordered_set(initializer_list __list) {}
class iterator {
@@ -1260,6 +1261,9 @@ template< Key *val; iterator begin() const { return iterator(val); } iterator end() const { return iterator(val + 1); } +
- template< class InputIt >
- void insert( InputIt first, InputIt last );
};
template diff --git a/clang/test/Analysis/issue-132010.cpp b/clang/test/Analysis/issue-132010.cpp new file mode 100644 index 0000000000000..abdaed57f26b9 --- /dev/null +++ b/clang/test/Analysis/issue-132010.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_analyze_cc1 -analyzer-config aggressive-binary-operation-simplification=true -analyzer-checker=alpha.cplusplus.MismatchedIterator -analyzer-output text -verify %s + +// expected-no-diagnostics + +#include "Inputs/system-header-simulator-cxx.h" + +void f() +{
- std::list l;
- std::unordered_set us;
- us.insert(l.cbegin(), l.cend()); // no warning
+} diff --git a/clang/test/Analysis/mismatched-iterator.cpp b/clang/test/Analysis/mismatched-iterator.cpp index 570e742751ead..325e7764ad7fa 100644 --- a/clang/test/Analysis/mismatched-iterator.cpp +++ b/clang/test/Analysis/mismatched-iterator.cpp @@ -19,6 +19,11 @@ void good_insert4(std::vector &V, int len, int n) { V.insert(V.cbegin(), {n-1, n, n+1}); // no-warning }
+void good_insert5(std::vector &V, int len, int n) {
- std::unordered_set us;
- us.insert(V.cbegin(), V.cend()); // no-warning +}
- void good_insert_find(std::vector &V, int n, int m) { auto i = std::find(V.cbegin(), V.cend(), n); V.insert(i, m); // no-warning @@ -70,6 +75,11 @@ void bad_insert4(std::vector &V1, std::vector &V2, int len, int n) { V2.insert(V1.cbegin(), {n-1, n, n+1}); // expected-warning{{Container accessed using foreign iterator argument}} }
+void bad_insert5(std::vector &V1, std::vector &V2, int len, int n) {
- std::unordered_set us;
- us.insert(V1.cbegin(), V2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}} +}
- void bad_erase1(std::vector &V1, std::vector &V2) { V2.erase(V1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}} }
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. It looks good with nits.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thabk you!