[clang] [WebKit Checkers] Allow a guardian CheckedPtr/CheckedRef (PR #110222)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 24 21:53:41 PDT 2024


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

>From 6e842a0135d097ffcb3c5991bc97543179972405 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 27 Sep 2024 02:05:25 -0700
Subject: [PATCH 1/6] [WebKit Checkers] Allow a guardian CheckedPtr/CheckedRef

This PR makes WebKit checkers allow a guardian variable which is CheckedPtr or CheckedRef as in addition to RefPtr or Ref.
---
 .../Checkers/WebKit/ASTUtils.cpp              | 17 ++++---
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 44 ++++++++++++++--
 .../Checkers/WebKit/PtrTypesSemantics.h       | 17 +++++--
 .../WebKit/UncountedCallArgsChecker.cpp       |  2 +
 .../WebKit/UncountedLocalVarsChecker.cpp      |  1 +
 .../Checkers/WebKit/call-args-checked.cpp     | 46 +++++++++++++++++
 .../Analysis/Checkers/WebKit/mock-types.h     | 16 ++++--
 .../Checkers/WebKit/uncounted-local-vars.cpp  | 51 +++++++++++++++++++
 8 files changed, 177 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 394cb26f03cf99..1b7614d3feeca5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -17,6 +17,10 @@
 
 namespace clang {
 
+bool isSafePtr(clang::CXXRecordDecl *Decl) {
+  return isRefCounted(Decl) || isCheckedPtr(Decl);
+}
+
 bool tryToFindPtrOrigin(
     const Expr *E, bool StopAtFirstRefCountedObj,
     std::function<bool(const clang::Expr *, bool)> callback) {
@@ -31,7 +35,7 @@ bool tryToFindPtrOrigin(
     }
     if (auto *tempExpr = dyn_cast<CXXTemporaryObjectExpr>(E)) {
       if (auto *C = tempExpr->getConstructor()) {
-        if (auto *Class = C->getParent(); Class && isRefCounted(Class))
+        if (auto *Class = C->getParent(); Class && isSafePtr(Class))
           return callback(E, true);
         break;
       }
@@ -56,7 +60,8 @@ bool tryToFindPtrOrigin(
       if (StopAtFirstRefCountedObj) {
         if (auto *ConversionFunc =
                 dyn_cast_or_null<FunctionDecl>(cast->getConversionFunction())) {
-          if (isCtorOfRefCounted(ConversionFunc))
+          if (isCtorOfRefCounted(ConversionFunc) ||
+              isCtorOfCheckedPtr(ConversionFunc))
             return callback(E, true);
         }
       }
@@ -68,7 +73,7 @@ bool tryToFindPtrOrigin(
     if (auto *call = dyn_cast<CallExpr>(E)) {
       if (auto *memberCall = dyn_cast<CXXMemberCallExpr>(call)) {
         if (auto *decl = memberCall->getMethodDecl()) {
-          std::optional<bool> IsGetterOfRefCt = isGetterOfRefCounted(decl);
+          std::optional<bool> IsGetterOfRefCt = isGetterOfSafePtr(decl);
           if (IsGetterOfRefCt && *IsGetterOfRefCt) {
             E = memberCall->getImplicitObjectArgument();
             if (StopAtFirstRefCountedObj) {
@@ -87,7 +92,7 @@ bool tryToFindPtrOrigin(
       }
 
       if (auto *callee = call->getDirectCallee()) {
-        if (isCtorOfRefCounted(callee)) {
+        if (isCtorOfRefCounted(callee) || isCtorOfCheckedPtr(callee)) {
           if (StopAtFirstRefCountedObj)
             return callback(E, true);
 
@@ -95,7 +100,7 @@ bool tryToFindPtrOrigin(
           continue;
         }
 
-        if (isRefType(callee->getReturnType()))
+        if (isSafePtrType(callee->getReturnType()))
           return callback(E, true);
 
         if (isSingleton(callee))
@@ -109,7 +114,7 @@ bool tryToFindPtrOrigin(
     }
     if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
       if (auto *Method = ObjCMsgExpr->getMethodDecl()) {
-        if (isRefType(Method->getReturnType()))
+        if (isSafePtrType(Method->getReturnType()))
           return callback(E, true);
       }
     }
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 4d145be808f6d8..b40e470dc71e03 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -135,7 +135,12 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
          || FunctionName == "Identifier";
 }
 
-bool isRefType(const clang::QualType T) {
+bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
+  assert(F);
+  return isCheckedPtr(safeGetName(F));
+}
+
+bool isSafePtrType(const clang::QualType T) {
   QualType type = T;
   while (!type.isNull()) {
     if (auto *elaboratedT = type->getAs<ElaboratedType>()) {
@@ -145,7 +150,7 @@ bool isRefType(const clang::QualType T) {
     if (auto *specialT = type->getAs<TemplateSpecializationType>()) {
       if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) {
         auto name = decl->getNameAsString();
-        return isRefType(name);
+        return isRefType(name) || isCheckedPtr(name);
       }
       return false;
     }
@@ -177,6 +182,12 @@ std::optional<bool> isUncounted(const CXXRecordDecl* Class)
   return (*IsRefCountable);
 }
 
+std::optional<bool> isUnchecked(const CXXRecordDecl *Class) {
+  if (isCheckedPtr(Class))
+    return false; // Cheaper than below
+  return isCheckedPtrCapable(Class);
+}
+
 std::optional<bool> isUncountedPtr(const Type* T)
 {
   assert(T);
@@ -189,7 +200,18 @@ std::optional<bool> isUncountedPtr(const Type* T)
   return false;
 }
 
-std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
+std::optional<bool> isUnsafePtr(const Type *T) {
+  assert(T);
+
+  if (T->isPointerType() || T->isReferenceType()) {
+    if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
+      return isUncounted(CXXRD) || isUnchecked(CXXRD);
+    }
+  }
+  return false;
+}
+
+std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl* M)
 {
   assert(M);
 
@@ -198,6 +220,9 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
     auto className = safeGetName(calleeMethodsClass);
     auto method = safeGetName(M);
 
+    if (isCheckedPtr(className) && (method == "get" || method == "ptr"))
+      return true;
+
     if ((isRefType(className) && (method == "get" || method == "ptr")) ||
         ((className == "String" || className == "AtomString" ||
           className == "AtomStringImpl" || className == "UniqueString" ||
@@ -211,7 +236,16 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
       if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
         if (auto *targetConversionType =
                 maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
-          return isUncountedPtr(targetConversionType);
+          return isUnsafePtr(targetConversionType);
+        }
+      }
+    }
+
+    if (isCheckedPtr(className)) {
+      if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
+        if (auto *targetConversionType =
+                maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
+          return isUnsafePtr(targetConversionType);
         }
       }
     }
@@ -464,7 +498,7 @@ class TrivialFunctionAnalysisVisitor
     if (!Callee)
       return false;
 
-    std::optional<bool> IsGetterOfRefCounted = isGetterOfRefCounted(Callee);
+    std::optional<bool> IsGetterOfRefCounted = isGetterOfSafePtr(Callee);
     if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
       return true;
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 3528c52a7d659d..2b56edddc86364 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -63,18 +63,29 @@ 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 T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
+/// variant, false if not.
+bool isSafePtrType(const clang::QualType T);
 
 /// \returns true if \p F creates ref-countable object from uncounted parameter,
 /// false if not.
 bool isCtorOfRefCounted(const clang::FunctionDecl *F);
 
+/// \returns true if \p F creates ref-countable object from uncounted parameter,
+/// false if not.
+bool isCtorOfCheckedPtr(const clang::FunctionDecl *F);
+
+/// \returns true if \p Name is RefPtr, Ref, or its variant, false if not.
+bool isRefType(const std::string &Name);
+
+/// \returns true if \p Name is CheckedRef or CheckedPtr, false if not.
+bool isCheckedPtr(const std::string &Name);
+
 /// \returns true if \p T is RefPtr, Ref, or its variant, false if not.
 bool isRefType(const clang::QualType T);
 
 /// \returns true if \p M is getter of a ref-counted class, false if not.
-std::optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl* Method);
+std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl* Method);
 
 /// \returns true if \p F is a conversion between ref-countable or ref-counted
 /// pointer types.
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 0ed93ab26bf5ca..084b789a008276 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -96,6 +96,8 @@ class UncountedCallArgsChecker
           auto name = safeGetName(MD);
           if (name == "ref" || name == "deref")
             return;
+          if (name == "incrementPtrCount" || name == "decrementPtrCount")
+            return;
         }
         auto *E = MemberCallExpr->getImplicitObjectArgument();
         QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 9d0a3bb5da7325..c389820297b3e0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -231,6 +231,7 @@ class UncountedLocalVarsChecker
                       if (MaybeGuardianArgCXXRecord) {
                         if (MaybeGuardian->isLocalVarDecl() &&
                             (isRefCounted(MaybeGuardianArgCXXRecord) ||
+                             isCheckedPtr(MaybeGuardianArgCXXRecord) ||
                              isRefcountedStringsHack(MaybeGuardian)) &&
                             isGuardedScopeEmbeddedInGuardianScope(
                                 V, MaybeGuardian))
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
new file mode 100644
index 00000000000000..49b6bfcd7cadfd
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+RefCountableAndCheckable* makeObj();
+CheckedRef<RefCountableAndCheckable> makeObjChecked();
+void someFunction(RefCountableAndCheckable*);
+
+namespace call_args_unchecked_uncounted {
+
+static void foo() {
+  someFunction(makeObj());
+  // expected-warning at -1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}}
+}
+
+} // namespace call_args_checked
+
+namespace call_args_checked {
+
+static void foo() {
+  CheckedPtr<RefCountableAndCheckable> ptr = makeObj();
+  someFunction(ptr.get());
+}
+
+static void bar() {
+  someFunction(CheckedPtr { makeObj() }.get());
+}
+
+static void baz() {
+  someFunction(makeObjChecked().ptr());
+}
+
+} // namespace call_args_checked
+
+namespace call_args_default {
+
+void someFunction(RefCountableAndCheckable* = makeObj());
+// expected-warning at -1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}}
+void otherFunction(RefCountableAndCheckable* = makeObjChecked().ptr());
+
+void foo() {
+  someFunction();
+  otherFunction();
+}
+
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 933b4c5e62a79c..8d8a90f0afae0e 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -114,8 +114,8 @@ template <typename T> struct CheckedRef {
 
 public:
   CheckedRef() : t{} {};
-  CheckedRef(T &t) : t(t) { t->incrementPtrCount(); }
-  CheckedRef(const CheckedRef& o) : t(o.t) { if (t) t->incrementPtrCount(); }
+  CheckedRef(T &t) : t(&t) { t.incrementPtrCount(); }
+  CheckedRef(const CheckedRef &o) : t(o.t) { if (t) t->incrementPtrCount(); }
   ~CheckedRef() { if (t) t->decrementPtrCount(); }
   T &get() { return *t; }
   T *ptr() { return t; }
@@ -135,7 +135,7 @@ template <typename T> struct CheckedPtr {
     if (t)
       t->incrementPtrCount();
   }
-  CheckedPtr(Ref<T>&& o)
+  CheckedPtr(Ref<T> &&o)
     : t(o.leakRef())
   { }
   ~CheckedPtr() {
@@ -156,4 +156,14 @@ class CheckedObj {
   void decrementPtrCount();
 };
 
+class RefCountableAndCheckable {
+public:
+  void incrementPtrCount() const;
+  void decrementPtrCount() const;
+  void ref() const;
+  void deref() const;
+  void method();
+  int trivial() { return 0; }
+};
+
 #endif
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 25776870dd3ae0..dabe699c44283a 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -289,3 +289,54 @@ void foo() {
 }
 
 } // namespace local_assignment_to_global
+
+namespace local_refcountable_checkable_object {
+
+RefCountableAndCheckable* provide_obj();
+
+void local_raw_ptr() {
+  RefCountableAndCheckable* a = nullptr;
+  // expected-warning at -1{{Local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  a = provide_obj();
+  a->method();
+}
+
+void local_checked_ptr() {
+  CheckedPtr<RefCountableAndCheckable> a = nullptr;
+  a = provide_obj();
+  a->method();
+}
+
+void local_var_with_guardian_checked_ptr() {
+  CheckedPtr<RefCountableAndCheckable> a = provide_obj();
+  {
+    auto* b = a.get();
+    b->method();
+  }
+}
+
+void local_var_with_guardian_checked_ptr_with_assignment() {
+  CheckedPtr<RefCountableAndCheckable> a = provide_obj();
+  {
+    RefCountableAndCheckable* b = a.get();
+    // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+    b = provide_obj();
+    b->method();
+  }
+}
+
+void local_var_with_guardian_checked_ref() {
+  CheckedRef<RefCountableAndCheckable> a = *provide_obj();
+  {
+    RefCountableAndCheckable& b = a;
+    b.method();
+  }
+}
+
+void static_var() {
+  static RefCountableAndCheckable* a = nullptr;
+  // expected-warning at -1{{Static local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  a = provide_obj();
+}
+
+} // namespace local_refcountable_checkable_object

>From 04ea90c6d5730380515ed176179e29e834ad7fb2 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 27 Sep 2024 11:38:00 -0700
Subject: [PATCH 2/6] Fix formatting.

---
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 3 +--
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h   | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index b40e470dc71e03..249263d4fc6770 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -211,8 +211,7 @@ std::optional<bool> isUnsafePtr(const Type *T) {
   return false;
 }
 
-std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl* M)
-{
+std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl* M) {
   assert(M);
 
   if (isa<CXXMethodDecl>(M)) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 2b56edddc86364..ed6122c0a8355d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -85,7 +85,7 @@ bool isCheckedPtr(const std::string &Name);
 bool isRefType(const clang::QualType T);
 
 /// \returns true if \p M is getter of a ref-counted class, false if not.
-std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl* Method);
+std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
 
 /// \returns true if \p F is a conversion between ref-countable or ref-counted
 /// pointer types.

>From 7981562d588e9c368c9ff116ee163d5a03aad603 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 27 Sep 2024 11:43:29 -0700
Subject: [PATCH 3/6] Fix formatting 2.

---
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 249263d4fc6770..c9489101f386e5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -211,7 +211,7 @@ std::optional<bool> isUnsafePtr(const Type *T) {
   return false;
 }
 
-std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl* M) {
+std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
   assert(M);
 
   if (isa<CXXMethodDecl>(M)) {

>From e4aa98b17025bf5d159daa2e097b55a937ee1570 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 17 Oct 2024 20:53:16 -0700
Subject: [PATCH 4/6] Fix merge error

---
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 20 ++++---------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index ba920b2c659003..439736b445ac83 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -196,9 +196,7 @@ std::optional<bool> isUncountedPtr(const QualType T) {
   return false;
 }
 
-std::optional<bool> isUnsafePtr(const Type *T) {
-  assert(T);
-
+std::optional<bool> isUnsafePtr(const QualType T) {
   if (T->isPointerType() || T->isReferenceType()) {
     if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
       return isUncounted(CXXRD) || isUnchecked(CXXRD);
@@ -228,23 +226,13 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
     // Ref<T> -> T conversion
     // FIXME: Currently allowing any Ref<T> -> whatever cast.
     if (isRefType(className)) {
-      if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
-        if (auto *targetConversionType =
-                maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
-          return isUnsafePtr(targetConversionType);
-        }
-      }
+      if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
+        return isUnsafePtr(maybeRefToRawOperator->getConversionType());
     }
 
     if (isCheckedPtr(className)) {
-      if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
-        if (auto *targetConversionType =
-                maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
-          return isUnsafePtr(targetConversionType);
-        }
-      }
       if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
-        return isUncountedPtr(maybeRefToRawOperator->getConversionType());
+        return isUnsafePtr(maybeRefToRawOperator->getConversionType());
     }
   }
   return false;

>From a89a4d8fbd587680d22bf7ccf23ef02a8c8191ac Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 17 Oct 2024 21:11:23 -0700
Subject: [PATCH 5/6] Fix more merge errors

---
 clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index a0443fd92e5387..1c0df42cdda663 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -290,7 +290,6 @@ void foo() {
 
 } // namespace local_assignment_to_global
 
-<<<<<<< HEAD
 namespace local_refcountable_checkable_object {
 
 RefCountableAndCheckable* provide_obj();

>From 43975f29e6474d61e5d3370f0f8a2f2f8d532a45 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 24 Oct 2024 21:52:37 -0700
Subject: [PATCH 6/6] Introduce isCtorOfSafePtr and delete unused declaration
 for isRefType.

---
 clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp    | 3 +--
 .../StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 4 ++++
 .../StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h   | 9 +++++----
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 3ee9024530d6b9..9d34dfd3cea636 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -60,8 +60,7 @@ bool tryToFindPtrOrigin(
       if (StopAtFirstRefCountedObj) {
         if (auto *ConversionFunc =
                 dyn_cast_or_null<FunctionDecl>(cast->getConversionFunction())) {
-          if (isCtorOfRefCounted(ConversionFunc) ||
-              isCtorOfCheckedPtr(ConversionFunc))
+          if (isCtorOfSafePtr(ConversionFunc))
             return callback(E, true);
         }
       }
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 439736b445ac83..0a9b53a28028ad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -140,6 +140,10 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
   return isCheckedPtr(safeGetName(F));
 }
 
+bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
+  return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F);
+}
+
 bool isSafePtrType(const clang::QualType T) {
   QualType type = T;
   while (!type.isNull()) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index eaa7e9632fc782..4b41ca96e1df1d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -71,19 +71,20 @@ bool isSafePtrType(const clang::QualType T);
 /// false if not.
 bool isCtorOfRefCounted(const clang::FunctionDecl *F);
 
-/// \returns true if \p F creates ref-countable object from uncounted parameter,
+/// \returns true if \p F creates checked ptr object from uncounted parameter,
 /// false if not.
 bool isCtorOfCheckedPtr(const clang::FunctionDecl *F);
 
+/// \returns true if \p F creates ref-countable or checked ptr object from
+/// uncounted parameter, false if not.
+bool isCtorOfSafePtr(const clang::FunctionDecl *F);
+
 /// \returns true if \p Name is RefPtr, Ref, or its variant, false if not.
 bool isRefType(const std::string &Name);
 
 /// \returns true if \p Name is CheckedRef or CheckedPtr, false if not.
 bool isCheckedPtr(const std::string &Name);
 
-/// \returns true if \p T is RefPtr, Ref, or its variant, false if not.
-bool isRefType(const clang::QualType T);
-
 /// \returns true if \p M is getter of a ref-counted class, false if not.
 std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
 



More information about the cfe-commits mailing list