Skip to content

[alpha.webkit.UncountedCallArgsChecker] Avoid emitting warnings for Ref, RefPtr, and their variants. #90153

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 1 commit into from
Apr 26, 2024

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Apr 26, 2024

Skip the analysis of Ref, RefPtr, and their variant classes in UncountedCallArgsChecker since these classes are "trusted" to not do anything dangerous.

…ef, RefPtr, and their variants.

Skip the analysis of Ref, RefPtr, and their variant classes in UncountedCallArgsChecker since these
classes are "trusted" to not do anything dangerous.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Apr 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2024

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

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Skip the analysis of Ref, RefPtr, and their variant classes in UncountedCallArgsChecker since these classes are "trusted" to not do anything dangerous.


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

4 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+6)
  • (modified) clang/test/Analysis/Checkers/WebKit/call-args.cpp (+1-1)
  • (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+52-15)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 9ed8e7cab6abb9..ec1db1cc335807 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -50,6 +50,9 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
 /// class, false if not, std::nullopt if inconclusive.
 std::optional<bool> isUncountedPtr(const clang::Type* T);
 
+/// \returns true if Name is a RefPtr, Ref, or its variant, false if not.
+bool isRefType(const std::string &Name);
+
 /// \returns true if \p F creates ref-countable object from uncounted parameter,
 /// false if not.
 bool isCtorOfRefCounted(const clang::FunctionDecl *F);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 8b41a949fd6734..741f336761589f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -53,6 +53,12 @@ class UncountedCallArgsChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
+      bool TraverseDecl(Decl *D) {
+        if (isa<ClassTemplateDecl>(D) && isRefType(safeGetName(D)))
+          return true;
+        return RecursiveASTVisitor<LocalVisitor>::TraverseDecl(D);
+      }
+
       bool VisitCallExpr(const CallExpr *CE) {
         Checker->visitCallExpr(CE);
         return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index f2e1f9bc5a2464..2a4b6bb1f1063a 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -32,7 +32,7 @@ namespace ref_counted {
   void consume_ref_counted(Ref<RefCountable>) {}
 
   void foo() {
-    consume_refcntbl(provide_ref_counted().get());
+    consume_refcntbl(provide_ref_counted().ptr());
     // no warning
   }
 }
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index aab99197dfa49e..c27ea9baaf3bf5 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -1,24 +1,61 @@
 #ifndef mock_types_1103988513531
 #define mock_types_1103988513531
 
-template <typename T> struct Ref {
-  T *t;
+template<typename T>
+struct RawPtrTraits {
+  using StorageType = T*;
 
-  Ref() : t{} {};
-  Ref(T &t)
-    : t(t) {
-    if (t)
-      t->ref();
+  template<typename U>
+  static T* exchange(StorageType& ptr, U&& newValue)
+  {
+    StorageType oldValue = static_cast<StorageType&&>(ptr);
+    ptr = static_cast<U&&>(newValue);
+    return oldValue;
   }
-  ~Ref() {
-    if (t)
-      t->deref();
+
+  static void swap(StorageType& a, StorageType& b)
+  {
+    StorageType temp = static_cast<StorageType&&>(a);
+    a = static_cast<StorageType&&>(b);
+    b = static_cast<StorageType&&>(temp);
   }
-  T *get() { return t; }
-  T *ptr() { return t; }
-  T *operator->() { return t; }
-  operator const T &() const { return *t; }
-  operator T &() { return *t; }
+  static T* unwrap(const StorageType& ptr) { return ptr; }
+};
+
+template<typename T> struct DefaultRefDerefTraits {
+  static T* refIfNotNull(T* ptr)
+  {
+    if (ptr)
+      ptr->ref();
+    return ptr;
+  }
+
+  static T& ref(T& ref)
+  {
+    ref.ref();
+    return ref;
+  }
+
+  static void derefIfNotNull(T* ptr)
+  {
+    if (ptr)
+      ptr->deref();
+  }
+};
+
+template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> struct Ref {
+  typename PtrTraits::StorageType t;
+
+  Ref() : t{} {};
+  Ref(T &t) : t(RefDerefTraits::refIfNotNull(t)) { }
+  Ref(const Ref& o) : t(RefDerefTraits::refIfNotNull(PtrTraits::unwrap(o.t))) { }
+  ~Ref() { RefDerefTraits::derefIfNotNull(PtrTraits::exchange(t, nullptr)); }
+  T &get() { return *PtrTraits::unwrap(t); }
+  T *ptr() { return PtrTraits::unwrap(t); }
+  T *operator->() { return PtrTraits::unwrap(t); }
+  operator const T &() const { return *PtrTraits::unwrap(t); }
+  operator T &() { return *PtrTraits::unwrap(t); }
+  T* leakRef() { PtrTraits::exchange(t, nullptr); }
 };
 
 template <typename T> struct RefPtr {

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!

@rniwa
Copy link
Contributor Author

rniwa commented Apr 26, 2024

Thanks for the review!

@rniwa rniwa merged commit 3d5e9ab into llvm:main Apr 26, 2024
6 of 7 checks passed
@rniwa rniwa deleted the avoid-warning-in-ref-refptr branch April 26, 2024 21:28
rniwa added a commit to rniwa/llvm-project that referenced this pull request Feb 3, 2025
…ef, RefPtr, and their variants. (llvm#90153)

Skip the analysis of Ref, RefPtr, and their variant classes in
UncountedCallArgsChecker since these classes are "trusted" to not do
anything dangerous.
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