Skip to content

[clang][analyzer] Improve the modeling of insert in MismatchedIteratorChecker #132596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 23, 2025

Conversation

flovent
Copy link
Contributor

@flovent flovent commented Mar 23, 2025

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.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Mar 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 23, 2025

@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&lt; class InputIt &gt;
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<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}}
 }

@llvmbot
Copy link
Member

llvmbot commented Mar 23, 2025

@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&lt; class InputIt &gt;
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<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}}
 }

Copy link
Contributor

@steakhal steakhal left a comment

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.

Copy link
Contributor

@steakhal steakhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thabk you!

@steakhal steakhal merged commit 2fe7585 into llvm:main Mar 23, 2025
6 of 11 checks passed
@flovent flovent deleted the issue-132010-fix branch March 24, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-analyzer-alpha.cplusplus.MismatchedIterator false positive with container insertion
3 participants