Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Nov 11, 2024

Treat member function calls to ref() and incrementCheckedPtrCount() as trivial so that a function which returns RefPtr/Ref out of a raw reference / pointer is also considered trivial.

Treat member function calls to ref() and incrementCheckedPtrCount() as trivial
so that a function which returns RefPtr/Ref out of a raw reference / pointer is
also considered trivial.
@rniwa rniwa requested a review from t-rasmud November 11, 2024 08:38
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Nov 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

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

Author: Ryosuke Niwa (rniwa)

Changes

Treat member function calls to ref() and incrementCheckedPtrCount() as trivial so that a function which returns RefPtr/Ref out of a raw reference / pointer is also considered trivial.


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

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+4)
  • (modified) clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp (+19)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp (+4)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 31bebdb07dbdc2..08fce7e0fa564a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -498,6 +498,10 @@ class TrivialFunctionAnalysisVisitor
     if (!Callee)
       return false;
 
+    auto Name = safeGetName(Callee);
+    if (Name == "ref" || Name == "incrementCheckedPtrCount")
+      return true;
+
     std::optional<bool> IsGetterOfRefCounted = isGetterOfSafePtr(Callee);
     if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
       return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
index 34ff0c70e230ea..072bceedcf9610 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
@@ -365,3 +365,22 @@ namespace call_with_explicit_temporary_obj {
     CheckedPtr { provide() }->method();
   }
 }
+
+namespace call_with_checked_ptr {
+
+  class Foo : public CheckedObj {
+  public:
+    CheckedPtr<CheckedObj> obj1() { return m_obj; }
+    CheckedRef<CheckedObj> obj2() { return *m_obj; }
+  private:
+    CheckedObj* m_obj;
+  };
+
+  Foo* getFoo();
+
+  void bar() {
+    getFoo()->obj1()->method();
+    getFoo()->obj2()->method();
+  }
+
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index e1dacdd9e25b6d..d654d963a4faef 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -366,6 +366,8 @@ class RefCounted {
   void trivial65() {
     __libcpp_verbose_abort("%s", "aborting");
   }
+  RefPtr<RefCounted> trivial66() { return children[0]; }
+  Ref<RefCounted> trivial67() { return *children[0]; }
 
   static RefCounted& singleton() {
     static RefCounted s_RefCounted;
@@ -550,6 +552,8 @@ class UnrelatedClass {
     getFieldTrivial().trivial63(); // no-warning
     getFieldTrivial().trivial64(); // no-warning
     getFieldTrivial().trivial65(); // no-warning
+    getFieldTrivial().trivial66()->trivial6(); // no-warning
+    getFieldTrivial().trivial67()->trivial6(); // no-warning
 
     RefCounted::singleton().trivial18(); // no-warning
     RefCounted::singleton().someFunction(); // no-warning

@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Treat member function calls to ref() and incrementCheckedPtrCount() as trivial so that a function which returns RefPtr/Ref out of a raw reference / pointer is also considered trivial.


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

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+4)
  • (modified) clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp (+19)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp (+4)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 31bebdb07dbdc2..08fce7e0fa564a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -498,6 +498,10 @@ class TrivialFunctionAnalysisVisitor
     if (!Callee)
       return false;
 
+    auto Name = safeGetName(Callee);
+    if (Name == "ref" || Name == "incrementCheckedPtrCount")
+      return true;
+
     std::optional<bool> IsGetterOfRefCounted = isGetterOfSafePtr(Callee);
     if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
       return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
index 34ff0c70e230ea..072bceedcf9610 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-ptr.cpp
@@ -365,3 +365,22 @@ namespace call_with_explicit_temporary_obj {
     CheckedPtr { provide() }->method();
   }
 }
+
+namespace call_with_checked_ptr {
+
+  class Foo : public CheckedObj {
+  public:
+    CheckedPtr<CheckedObj> obj1() { return m_obj; }
+    CheckedRef<CheckedObj> obj2() { return *m_obj; }
+  private:
+    CheckedObj* m_obj;
+  };
+
+  Foo* getFoo();
+
+  void bar() {
+    getFoo()->obj1()->method();
+    getFoo()->obj2()->method();
+  }
+
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index e1dacdd9e25b6d..d654d963a4faef 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -366,6 +366,8 @@ class RefCounted {
   void trivial65() {
     __libcpp_verbose_abort("%s", "aborting");
   }
+  RefPtr<RefCounted> trivial66() { return children[0]; }
+  Ref<RefCounted> trivial67() { return *children[0]; }
 
   static RefCounted& singleton() {
     static RefCounted s_RefCounted;
@@ -550,6 +552,8 @@ class UnrelatedClass {
     getFieldTrivial().trivial63(); // no-warning
     getFieldTrivial().trivial64(); // no-warning
     getFieldTrivial().trivial65(); // no-warning
+    getFieldTrivial().trivial66()->trivial6(); // no-warning
+    getFieldTrivial().trivial67()->trivial6(); // no-warning
 
     RefCounted::singleton().trivial18(); // no-warning
     RefCounted::singleton().someFunction(); // no-warning

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

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

LGTM!

@rniwa
Copy link
Contributor Author

rniwa commented Nov 14, 2024

Thanks for the review!

@rniwa rniwa merged commit 73b577c into llvm:main Nov 14, 2024
11 checks passed
@rniwa rniwa deleted the treat-ref-incrementCheckedPtrCount-as-trivial branch November 14, 2024 00:28
rniwa added a commit to rniwa/llvm-project that referenced this pull request Feb 3, 2025
…al (llvm#115695)

Treat member function calls to ref() and incrementCheckedPtrCount() as
trivial so that a function which returns RefPtr/Ref out of a raw
reference / pointer is also considered trivial.
devincoughlin pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
…al (llvm#115695)

Treat member function calls to ref() and incrementCheckedPtrCount() as
trivial so that a function which returns RefPtr/Ref out of a raw
reference / pointer is also considered 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.

3 participants