[clang][analyzer] Improve the modeling of insert in MismatchedIteratorChecker #132596
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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: None (flovent) ChangesThis PR fixs #132010. Associative containers in STL has an unique Add support for this Full diff: https://github.com/llvm/llvm-project/pull/132596.diff 4 Files Affected:
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<Key>
> class unordered_set {
public:
+ unordered_set() {}
unordered_set(initializer_list<Key> __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 <typename T>
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<int> l;
+ std::unordered_set<int> 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<int> &V, int len, int n) {
V.insert(V.cbegin(), {n-1, n, n+1}); // no-warning
}
+void good_insert5(std::vector<int> &V, int len, int n) {
+ std::unordered_set<int> us;
+ us.insert(V.cbegin(), V.cend()); // no-warning
+}
+
void good_insert_find(std::vector<int> &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<int> &V1, std::vector<int> &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<int> &V1, std::vector<int> &V2, int len, int n) {
+ std::unordered_set<int> 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<int> &V1, std::vector<int> &V2) {
V2.erase(V1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}}
}
|
Member
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: None (flovent) ChangesThis PR fixs #132010. Associative containers in STL has an unique Add support for this Full diff: https://github.com/llvm/llvm-project/pull/132596.diff 4 Files Affected:
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<Key>
> class unordered_set {
public:
+ unordered_set() {}
unordered_set(initializer_list<Key> __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 <typename T>
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<int> l;
+ std::unordered_set<int> 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<int> &V, int len, int n) {
V.insert(V.cbegin(), {n-1, n, n+1}); // no-warning
}
+void good_insert5(std::vector<int> &V, int len, int n) {
+ std::unordered_set<int> us;
+ us.insert(V.cbegin(), V.cend()); // no-warning
+}
+
void good_insert_find(std::vector<int> &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<int> &V1, std::vector<int> &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<int> &V1, std::vector<int> &V2, int len, int n) {
+ std::unordered_set<int> 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<int> &V1, std::vector<int> &V2) {
V2.erase(V1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}}
}
|
steakhal
reviewed
Mar 23, 2025
Contributor
steakhal
left a comment
steakhal
left a comment
There was a problem hiding this comment.
Thanks. It looks good with nits.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #132010
Associative containers in STL has an unique
insertoverload member function comparing to un-associative containers(https://en.cppreference.com/w/cpp/container/unordered_set/insert):Add support for this
insertoverload inMismatchedIteratorChecker, verify iffirstandlastbelongs to the same container in this case.