[clang][analyzer] Fix InvalidatedIterator crash caused by overload operator member function with explicit this

[flovent]

This PR fixs #116372.

From this PR #83585, CSA starts to model overload operator member function with explicit this as SimpleFunctionCall rather than CXXMemberOperatorCall (derived from CXXInstanceCall), so CXXInstanceCall only represents a non-static C++ member function call with implicit this.

For this checker, it models operator= for STL containers, which always uses implicit this, so the situation using explicit this can be skipped directly.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: None (flovent)

Changes

This PR fixs #116372.

From this PR #83585, CSA starts to model overload operator member function with explicit this as SimpleFunctionCall rather than CXXMemberOperatorCall (derived from CXXInstanceCall), so CXXInstanceCall only represents a non-static C++ member function call with implicit this.

For this checker, it models operator= for STL containers, which always uses implicit this, so the situation using explicit this can be skipped directly.

---

Full diff: https://github.com/llvm/llvm-project/pull/132581.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp (+5-2)
• (added) clang/test/Analysis/issue-116372.cpp (+21)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 55ed809bfed6c..d850344db6591 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -157,8 +157,11 @@ void ContainerModeling::checkPostCall(const CallEvent &Call,
   if (Func->isOverloadedOperator()) {
     const auto Op = Func->getOverloadedOperator();
     if (Op == OO_Equal) {
-      // Overloaded 'operator=' must be a non-static member function.
-      const auto *InstCall = cast<CXXInstanceCall>(&Call);
+      // Only handle the assignment operator with implicit this
+      const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call);
+      if (!InstCall)
+        return;
+
       if (cast<CXXMethodDecl>(Func)->isMoveAssignmentOperator()) {
         handleAssignment(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
                      Call.getArgSVal(0));
diff --git a/clang/test/Analysis/issue-116372.cpp b/clang/test/Analysis/issue-116372.cpp
new file mode 100644
index 0000000000000..0843cd614d87c
--- /dev/null
+++ b/clang/test/Analysis/issue-116372.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -std=c++23 %s -verify -analyzer-checker=alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true
+
+// expected-no-diagnostics
+
+class ExplicitThis {
+  int f = 0;
+public:
+  ExplicitThis();
+  ExplicitThis(ExplicitThis& other);
+
+  ExplicitThis& operator=(this ExplicitThis& self, ExplicitThis const& other) { // no crash
+    self.f = other.f;
+    return self;
+  }
+
+  ~ExplicitThis();
+};
+
+void func(ExplicitThis& obj1) {
+    obj1 = obj1;
+}

[Xazax-hun]

Looks good to me.

[Xazax-hun]

Maybe a FIXME with a GitHub ticket to properly support explicit could be nice.

[steakhal]

Actually, it shouldn't be too hard to fix this. Only like 5 lines of code I imagine.
At that point, it may be easier to fix it instead of creating a ticket.

[flovent]

I think I may not understand correctly, what should i fix here? explicit this will not appear in STL containers' member function.

[steakhal]

I think it's technically possible to have a conforming STL implementation using "deducing this" in their implementation.
I agree that it's bikshedding. I'd just say let's move on. We don't even need a GH ticket. I think there are far more pressing issues to track other than this tiny marginal edge case.

[steakhal]

I have not much to say. It's an alpha checker, that is known for crashing.
One less crash wouldn't change the situation by much but would certainly push the needle to the right direction.

[steakhal]

Actually, it shouldn't be too hard to fix this. Only like 5 lines of code I imagine.
At that point, it may be easier to fix it instead of creating a ticket.

[flovent]

I have not much to say. It's an alpha checker, that is known for crashing. One less crash wouldn't change the situation by much but would certainly push the needle to the right direction.

Thank you for the information about this checker, i just happen to see the related issue, and try to find out whether other checkers also assumes CXXInstanceCall(cast) for overload operator call somewhere, the result is no, only this one.