Skip to content
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

[alpha.webkit.UncountedCallArgsChecker] Allow trivial operator++ #91102

Merged
merged 4 commits into from
May 9, 2024

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented May 5, 2024

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.

Also treat any _builtin functions as trivial.

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.
@rniwa rniwa requested a review from haoNoQ May 5, 2024 03:36
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels May 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 5, 2024

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

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

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.


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

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+16-1)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp (+28)
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@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
     getFieldTrivial().nonTrivial14();
     // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial15();
+    // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial16();
+    // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
 

Copy link
Collaborator

@haoNoQ haoNoQ left a comment

Choose a reason for hiding this comment

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

LGTM now thanks!

@rniwa rniwa merged commit a99cb96 into llvm:main May 9, 2024
3 of 4 checks passed
@rniwa rniwa deleted the allow-trivial-operator-plusplus branch May 9, 2024 22:34
rniwa added a commit to rniwa/llvm-project that referenced this pull request May 25, 2024
…m#91102)

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

Also allow incrementing and decrementing of a POD member variable.

Also treat any __builtin_ functions as trivial.
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.

None yet

3 participants