[clang] [alpha.webkit.UncountedCallArgsChecker] Allow trivial operator++ (PR #91102)

[Ryosuke Niwa via cfe-commits]

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/91102

>From b3ae3d7a2a8885777b691e7fde237f5745e764a1 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 4 May 2024 20:29:03 -0700
Subject: [PATCH 1/2] [alpha.webkit.UncountedCallArgsChecker] Allow trivial
 operator++

This PR adds the support for trivial operator++ implementations.
T& operator++() and T operator++(int) are trivial if the calle is trivial.

Also allow incrementing and decrementing of a POD member variable.
---
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 17 ++++++++++-
 .../Checkers/WebKit/uncounted-obj-arg.cpp     | 28 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 6901dbb415bf76..c219172477a621 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -316,10 +316,15 @@ class TrivialFunctionAnalysisVisitor
 
     if (UO->isIncrementOp() || UO->isDecrementOp()) {
       // Allow increment or decrement of a POD type.
-      if (auto *RefExpr = dyn_cast<DeclRefExpr>(UO->getSubExpr())) {
+      auto *SubExpr = UO->getSubExpr();
+      if (auto *RefExpr = dyn_cast<DeclRefExpr>(SubExpr)) {
         if (auto *Decl = dyn_cast<VarDecl>(RefExpr->getDecl()))
           return Decl->isLocalVarDeclOrParm() &&
                  Decl->getType().isPODType(Decl->getASTContext());
+      } else if (auto *ME = dyn_cast<MemberExpr>(SubExpr)) {
+        if (auto *Decl = ME->getMemberDecl())
+          return Visit(ME->getBase()) &&
+                 Decl->getType().isPODType(Decl->getASTContext());
       }
     }
     // Other operators are non-trivial.
@@ -405,6 +410,16 @@ class TrivialFunctionAnalysisVisitor
     return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
   }
 
+  bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
+    if (!checkArguments(OCE))
+      return false;
+    auto *Callee = OCE->getCalleeDecl();
+    if (!Callee)
+      return false;
+    // Recursively descend into the callee to confirm that it's trivial as well.
+    return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
+  }
+
   bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) {
     if (auto *Expr = E->getExpr()) {
       if (!Visit(Expr))
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 63a68a994a5c64..2816a9cb823649 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -137,11 +137,26 @@ class Number {
   Number(int v) : v(v) { }
   Number(double);
   Number operator+(const Number&);
+  Number& operator++() { ++v; return *this; }
+  Number operator++(int) { Number returnValue(v); ++v; return returnValue; }
   const int& value() const { return v; }
+  void someMethod();
+
 private:
   int v;
 };
 
+class ComplexNumber {
+public:
+  ComplexNumber() : real(0), complex(0) { }
+  ComplexNumber& operator++() { real.someMethod(); return *this; }
+  ComplexNumber operator++(int);
+
+private:
+  Number real;
+  Number complex;
+};
+
 class RefCounted {
 public:
   void ref() const;
@@ -208,6 +223,9 @@ class RefCounted {
   unsigned trivial32() { return sizeof(int); }
   unsigned trivial33() { return ~0xff; }
   template <unsigned v> unsigned trivial34() { return v; }
+  void trivial35() { v++; }
+  void trivial36() { ++(*number); }
+  void trivial37() { (*number)++; }
 
   static RefCounted& singleton() {
     static RefCounted s_RefCounted;
@@ -282,9 +300,12 @@ class RefCounted {
 
   int nonTrivial13() { return ~otherFunction(); }
   int nonTrivial14() { int r = 0xff; r |= otherFunction(); return r; }
+  void nonTrivial15() { ++complex; }
+  void nonTrivial16() { complex++; }
 
   unsigned v { 0 };
   Number* number { nullptr };
+  ComplexNumber complex;
   Enum enumValue { Enum::Value1 };
 };
 
@@ -340,6 +361,9 @@ class UnrelatedClass {
     getFieldTrivial().trivial32(); // no-warning
     getFieldTrivial().trivial33(); // no-warning
     getFieldTrivial().trivial34<7>(); // no-warning
+    getFieldTrivial().trivial35(); // no-warning
+    getFieldTrivial().trivial36(); // no-warning
+    getFieldTrivial().trivial37(); // no-warning
 
     RefCounted::singleton().trivial18(); // no-warning
     RefCounted::singleton().someFunction(); // no-warning
@@ -374,6 +398,10 @@ class UnrelatedClass {
     // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
     getFieldTrivial().nonTrivial14();
     // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial15();
+    // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial16();
+    // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
 

>From b625bebb95bfc224dc299579fd6da1e480f6796c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 5 May 2024 12:33:38 -0700
Subject: [PATCH 2/2] Also treat any __builtin_ functions as trivial.

---
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
 clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp      | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index c219172477a621..5837872847bb67 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -369,7 +369,7 @@ class TrivialFunctionAnalysisVisitor
 
     if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
         Name == "WTFReportAssertionFailure" ||
-        Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
+        Name == "compilerFenceForCrash" || Name.find("__builtin") == 0)
       return true;
 
     return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 2816a9cb823649..d4b4916ad7e291 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -226,6 +226,7 @@ class RefCounted {
   void trivial35() { v++; }
   void trivial36() { ++(*number); }
   void trivial37() { (*number)++; }
+  void trivial38() { v++; if (__builtin_expect(!!(number), 1)) (*number)++; }
 
   static RefCounted& singleton() {
     static RefCounted s_RefCounted;
@@ -364,6 +365,7 @@ class UnrelatedClass {
     getFieldTrivial().trivial35(); // no-warning
     getFieldTrivial().trivial36(); // no-warning
     getFieldTrivial().trivial37(); // no-warning
+    getFieldTrivial().trivial38(); // no-warning
 
     RefCounted::singleton().trivial18(); // no-warning
     RefCounted::singleton().someFunction(); // no-warning