[clang] [llvm] [alpha.webkit.NoUncheckedPtrMemberChecker] Introduce member variable checker for CheckedPtr/CheckedRef (PR #108352)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 12 02:55:15 PDT 2024


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

>From ac0447762c98da3cfb41a6b462034e3ab410bc33 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 12 Sep 2024 02:13:12 -0700
Subject: [PATCH 1/4] [alpha.webkit.NoUncheckedPtrMemberChecker] Introduce
 member variable checker for CheckedPtr/CheckedRef

This PR introduces new WebKit checker to warn a member variable that is a raw reference or
a raw pointer to an object, which is capable of creating a CheckedRef/CheckedPtr.
---
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  4 +
 .../StaticAnalyzer/Checkers/CMakeLists.txt    |  2 +-
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 37 ++++++--
 .../Checkers/WebKit/PtrTypesSemantics.h       |  7 ++
 ...Checker.cpp => RawPtrRefMemberChecker.cpp} | 88 ++++++++++++++++---
 .../Analysis/Checkers/WebKit/mock-types.h     | 48 ++++++++++
 .../Checkers/WebKit/unchecked-members.cpp     | 53 +++++++++++
 .../lib/StaticAnalyzer/Checkers/BUILD.gn      |  2 +-
 8 files changed, 219 insertions(+), 22 deletions(-)
 rename clang/lib/StaticAnalyzer/Checkers/WebKit/{NoUncountedMembersChecker.cpp => RawPtrRefMemberChecker.cpp} (66%)
 create mode 100644 clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 585246547b3dce..4759f680fb4ff7 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1771,6 +1771,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
 
 let ParentPackage = WebKitAlpha in {
 
+def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
+  HelpText<"Check for no unchecked member variables.">,
+  Documentation<NotDocumented>;
+
 def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">,
   HelpText<"Check uncounted call arguments.">,
   Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 414282d58f779f..6da3665ab9a4df 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -132,7 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   VLASizeChecker.cpp
   ValistChecker.cpp
   VirtualCallChecker.cpp
-  WebKit/NoUncountedMembersChecker.cpp
+  WebKit/RawPtrRefMemberChecker.cpp
   WebKit/ASTUtils.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index f48b2fd9dca71b..09298102993f99 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -53,7 +53,9 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch) {
   return hasPublicMethodInBaseClass(R, NameToMatch) ? R : nullptr;
 }
 
-std::optional<bool> isRefCountable(const CXXRecordDecl* R)
+std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
+                                         const char *IncMethodName,
+                                         const char *DecMethodName)
 {
   assert(R);
 
@@ -61,8 +63,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
   if (!R)
     return std::nullopt;
 
-  bool hasRef = hasPublicMethodInBaseClass(R, "ref");
-  bool hasDeref = hasPublicMethodInBaseClass(R, "deref");
+  bool hasRef = hasPublicMethodInBaseClass(R, IncMethodName);
+  bool hasDeref = hasPublicMethodInBaseClass(R, DecMethodName);
   if (hasRef && hasDeref)
     return true;
 
@@ -71,8 +73,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
 
   bool AnyInconclusiveBase = false;
   const auto hasPublicRefInBase =
-      [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
-        auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
+      [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
+        auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
         if (!hasRefInBase) {
           AnyInconclusiveBase = true;
           return false;
@@ -87,8 +89,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
 
   Paths.clear();
   const auto hasPublicDerefInBase =
-      [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
-        auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
+      [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
+        auto hasDerefInBase = clang::hasPublicMethodInBase(Base, DecMethodName);
         if (!hasDerefInBase) {
           AnyInconclusiveBase = true;
           return false;
@@ -103,11 +105,23 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
   return hasRef && hasDeref;
 }
 
+std::optional<bool> isRefCountable(const clang::CXXRecordDecl *R) {
+  return isSmartPtrCompatible(R, "ref", "deref");
+}
+
+std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl *R) {
+  return isSmartPtrCompatible(R, "incrementPtrCount", "decrementPtrCount");
+}
+
 bool isRefType(const std::string &Name) {
   return Name == "Ref" || Name == "RefAllowingPartiallyDestroyed" ||
          Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
 }
 
+bool isCheckedPtr(const std::string &Name) {
+  return Name == "CheckedPtr" || Name == "CheckedRef";
+}
+
 bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
   assert(F);
   const std::string &FunctionName = safeGetName(F);
@@ -218,6 +232,15 @@ bool isRefCounted(const CXXRecordDecl *R) {
   return false;
 }
 
+bool isCheckedPtr(const CXXRecordDecl *R) {
+  assert(R);
+  if (auto *TmplR = R->getTemplateInstantiationPattern()) {
+    const auto &ClassName = safeGetName(TmplR);
+    return isCheckedPtr(ClassName);
+  }
+  return false;
+}
+
 bool isPtrConversion(const FunctionDecl *F) {
   assert(F);
   if (isCtorOfRefCounted(F))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 2932e62ad06e4b..08f9be49970394 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -40,9 +40,16 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch);
 /// inconclusive.
 std::optional<bool> isRefCountable(const clang::CXXRecordDecl* Class);
 
+/// \returns true if \p Class is checked-pointer compatible, false if not,
+/// std::nullopt if inconclusive.
+std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl* Class);
+
 /// \returns true if \p Class is ref-counted, false if not.
 bool isRefCounted(const clang::CXXRecordDecl *Class);
 
+/// \returns true if \p Class is a CheckedPtr / CheckedRef, false if not.
+bool isCheckedPtr(const clang::CXXRecordDecl *Class);
+
 /// \returns true if \p Class is ref-countable AND not ref-counted, false if
 /// not, std::nullopt if inconclusive.
 std::optional<bool> isUncounted(const clang::QualType T);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
similarity index 66%
rename from clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
rename to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 69a0eb3086ab72..455bb27e0207e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -1,4 +1,4 @@
-//=======- NoUncountedMembersChecker.cpp -------------------------*- C++ -*-==//
+//=======- RawPtrRefMemberChecker.cpp ----------------------------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -25,18 +25,20 @@ using namespace ento;
 
 namespace {
 
-class NoUncountedMemberChecker
+class RawPtrRefMemberChecker
     : public Checker<check::ASTDecl<TranslationUnitDecl>> {
 private:
   BugType Bug;
   mutable BugReporter *BR;
 
 public:
-  NoUncountedMemberChecker()
-      : Bug(this,
-            "Member variable is a raw-pointer/reference to reference-countable "
-            "type",
-            "WebKit coding guidelines") {}
+  RawPtrRefMemberChecker(const char *description)
+      : Bug(this, description, "WebKit coding guidelines") {}
+
+  virtual std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *) const = 0;
+  virtual bool isPtrCls(const clang::CXXRecordDecl *) const = 0;
+  virtual const char* typeName() const = 0;
+  virtual const char* invariantName() const = 0;
 
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
                     BugReporter &BRArg) const {
@@ -46,8 +48,8 @@ class NoUncountedMemberChecker
     // visit template instantiations or lambda classes. We
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
-      const NoUncountedMemberChecker *Checker;
-      explicit LocalVisitor(const NoUncountedMemberChecker *Checker)
+      const RawPtrRefMemberChecker *Checker;
+      explicit LocalVisitor(const RawPtrRefMemberChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
       }
@@ -77,7 +79,7 @@ class NoUncountedMemberChecker
       if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
         // If we don't see the definition we just don't know.
         if (MemberCXXRD->hasDefinition()) {
-            std::optional<bool> isRCAble = isRefCountable(MemberCXXRD);
+            std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
             if (isRCAble && *isRCAble)
                 reportBug(Member, MemberType, MemberCXXRD, RD);
         }
@@ -114,7 +116,7 @@ class NoUncountedMemberChecker
     // a member but we trust them to handle it correctly.
     auto CXXRD = llvm::dyn_cast_or_null<CXXRecordDecl>(RD);
     if (CXXRD)
-      return isRefCounted(CXXRD);
+      return isPtrCls(CXXRD);
 
     return false;
   }
@@ -135,9 +137,13 @@ class NoUncountedMemberChecker
     printQuotedQualifiedName(Os, ClassCXXRD);
     Os << " is a "
        << (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
-       << " to ref-countable type ";
+       << " to "
+       << typeName()
+       << " ";
     printQuotedQualifiedName(Os, MemberCXXRD);
-    Os << "; member variables must be ref-counted.";
+    Os << "; "
+       << invariantName()
+       << ".";
 
     PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
                                  BR->getSourceManager());
@@ -146,6 +152,53 @@ class NoUncountedMemberChecker
     BR->emitReport(std::move(Report));
   }
 };
+
+class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
+public:
+  NoUncountedMemberChecker()
+      : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
+                               "reference-countable type") {}
+
+  std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const final {
+    return isRefCountable(R);
+  }
+
+  bool isPtrCls(const clang::CXXRecordDecl *R) const final {
+    return isRefCounted(R);
+  }
+
+  const char* typeName() const final {
+    return "ref-countable type";
+  }
+
+  const char* invariantName() const final {
+    return "member variables must be ref-counted";
+  }
+};
+
+class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
+public:
+  NoUncheckedPtrMemberChecker()
+      : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
+                               "checked-pointer capable type") {}
+
+  std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const final {
+    return isCheckedPtrCapable(R);
+  }
+
+  bool isPtrCls(const clang::CXXRecordDecl *R) const final {
+    return isCheckedPtr(R);
+  }
+
+  const char* typeName() const final {
+    return "CheckedPtr capable type";
+  }
+
+  const char* invariantName() const final {
+    return "member variables must be a CheckedPtr or CheckedRef";
+  }
+};
+
 } // namespace
 
 void ento::registerNoUncountedMemberChecker(CheckerManager &Mgr) {
@@ -156,3 +209,12 @@ bool ento::shouldRegisterNoUncountedMemberChecker(
     const CheckerManager &Mgr) {
   return true;
 }
+
+void ento::registerNoUncheckedPtrMemberChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<NoUncheckedPtrMemberChecker>();
+}
+
+bool ento::shouldRegisterNoUncheckedPtrMemberChecker(
+    const CheckerManager &Mgr) {
+  return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index c427b22fd683e5..933b4c5e62a79c 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -108,4 +108,52 @@ struct RefCountable {
 
 template <typename T> T *downcast(T *t) { return t; }
 
+template <typename T> struct CheckedRef {
+private:
+  T *t;
+
+public:
+  CheckedRef() : t{} {};
+  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; }
+  T *operator->() { return t; }
+  operator const T &() const { return *t; }
+  operator T &() { return *t; }
+};
+
+template <typename T> struct CheckedPtr {
+private:
+  T *t;
+
+public:
+  CheckedPtr() : t(nullptr) {}
+  CheckedPtr(T *t)
+    : t(t) {
+    if (t)
+      t->incrementPtrCount();
+  }
+  CheckedPtr(Ref<T>&& o)
+    : t(o.leakRef())
+  { }
+  ~CheckedPtr() {
+    if (t)
+      t->decrementPtrCount();
+  }
+  T *get() { return t; }
+  T *operator->() { return t; }
+  const T *operator->() const { return t; }
+  T &operator*() { return *t; }
+  CheckedPtr &operator=(T *) { return *this; }
+  operator bool() const { return t; }
+};
+
+class CheckedObj {
+public:
+  void incrementPtrCount();
+  void decrementPtrCount();
+};
+
 #endif
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
new file mode 100644
index 00000000000000..4450d5cab60f0a
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+namespace members {
+
+  struct Foo {
+  private:
+    CheckedObj* a = nullptr;
+// expected-warning at -1{{Member variable 'a' in 'members::Foo' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+    CheckedObj& b;
+// expected-warning at -1{{Member variable 'b' in 'members::Foo' is a reference to CheckedPtr capable type 'CheckedObj'}}
+
+    [[clang::suppress]]
+    CheckedObj* a_suppressed = nullptr;
+
+    [[clang::suppress]]
+    CheckedObj& b_suppressed;
+
+    CheckedPtr<CheckedObj> c;
+    CheckedRef<CheckedObj> d;
+
+  public:
+    Foo();
+  };
+
+  template <typename S>
+  struct FooTmpl {
+    S* e;
+// expected-warning at -1{{Member variable 'e' in 'members::FooTmpl<CheckedObj>' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+  };
+
+  void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
+
+} // namespace members
+
+namespace ignore_unions {
+
+  union Foo {
+    CheckedObj* a;
+    CheckedPtr<CheckedObj> c;
+    CheckedRef<CheckedObj> d;
+  };
+
+  template<class T>
+  union FooTmpl {
+    T* a;
+  };
+
+  void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
+
+} // namespace ignore_unions
diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
index 3b640ae41b9f62..7a6c360e88c14e 100644
--- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
@@ -141,7 +141,7 @@ static_library("Checkers") {
     "VforkChecker.cpp",
     "VirtualCallChecker.cpp",
     "WebKit/ASTUtils.cpp",
-    "WebKit/NoUncountedMembersChecker.cpp",
+    "WebKit/RawPtrRefMemberChecker.cpp",
     "WebKit/PtrTypesSemantics.cpp",
     "WebKit/RefCntblBaseVirtualDtorChecker.cpp",
     "WebKit/UncountedCallArgsChecker.cpp",

>From 5abb2493cfc67f2bd1dcdd946ed9ecbef8929d2b Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 12 Sep 2024 02:32:28 -0700
Subject: [PATCH 2/4] Update warnings to explicitly state which types of
 pointers should be used.

---
 .../StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 455bb27e0207e9..9ea0abbee974bc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -172,7 +172,7 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
   }
 
   const char* invariantName() const final {
-    return "member variables must be ref-counted";
+    return "member variables must be Ref, RefPtr, WeakRef, or WeakPtr";
   }
 };
 
@@ -195,7 +195,7 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
   }
 
   const char* invariantName() const final {
-    return "member variables must be a CheckedPtr or CheckedRef";
+    return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or WeakPtr";
   }
 };
 

>From b0f8992ef7f8642a24bfd4e8422705bf15cc3723 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 12 Sep 2024 02:50:32 -0700
Subject: [PATCH 3/4] Formatting.

---
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 39 ++++++++-------
 .../Checkers/WebKit/PtrTypesSemantics.h       |  4 +-
 .../WebKit/RawPtrRefMemberChecker.cpp         | 47 +++++++++----------
 3 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 09298102993f99..7084a09a03d686 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -55,8 +55,7 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch) {
 
 std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
                                          const char *IncMethodName,
-                                         const char *DecMethodName)
-{
+                                         const char *DecMethodName) {
   assert(R);
 
   R = R->getDefinition();
@@ -72,15 +71,15 @@ std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
   Paths.setOrigin(const_cast<CXXRecordDecl *>(R));
 
   bool AnyInconclusiveBase = false;
-  const auto hasPublicRefInBase =
-      [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
-        auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
-        if (!hasRefInBase) {
-          AnyInconclusiveBase = true;
-          return false;
-        }
-        return (*hasRefInBase) != nullptr;
-      };
+  const auto hasPublicRefInBase = [&](const CXXBaseSpecifier *Base,
+                                      CXXBasePath &) {
+    auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
+    if (!hasRefInBase) {
+      AnyInconclusiveBase = true;
+      return false;
+    }
+    return (*hasRefInBase) != nullptr;
+  };
 
   hasRef = hasRef || R->lookupInBases(hasPublicRefInBase, Paths,
                                       /*LookupInDependent =*/true);
@@ -88,15 +87,15 @@ std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
     return std::nullopt;
 
   Paths.clear();
-  const auto hasPublicDerefInBase =
-      [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
-        auto hasDerefInBase = clang::hasPublicMethodInBase(Base, DecMethodName);
-        if (!hasDerefInBase) {
-          AnyInconclusiveBase = true;
-          return false;
-        }
-        return (*hasDerefInBase) != nullptr;
-      };
+  const auto hasPublicDerefInBase = [&](const CXXBaseSpecifier *Base,
+                                        CXXBasePath &) {
+    auto hasDerefInBase = clang::hasPublicMethodInBase(Base, DecMethodName);
+    if (!hasDerefInBase) {
+      AnyInconclusiveBase = true;
+      return false;
+    }
+    return (*hasDerefInBase) != nullptr;
+  };
   hasDeref = hasDeref || R->lookupInBases(hasPublicDerefInBase, Paths,
                                           /*LookupInDependent =*/true);
   if (AnyInconclusiveBase)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 08f9be49970394..25ec6f252b9702 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -38,11 +38,11 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch);
 
 /// \returns true if \p Class is ref-countable, false if not, std::nullopt if
 /// inconclusive.
-std::optional<bool> isRefCountable(const clang::CXXRecordDecl* Class);
+std::optional<bool> isRefCountable(const clang::CXXRecordDecl *Class);
 
 /// \returns true if \p Class is checked-pointer compatible, false if not,
 /// std::nullopt if inconclusive.
-std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl* Class);
+std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl *Class);
 
 /// \returns true if \p Class is ref-counted, false if not.
 bool isRefCounted(const clang::CXXRecordDecl *Class);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 9ea0abbee974bc..0a8dab737add04 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -35,10 +35,11 @@ class RawPtrRefMemberChecker
   RawPtrRefMemberChecker(const char *description)
       : Bug(this, description, "WebKit coding guidelines") {}
 
-  virtual std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *) const = 0;
+  virtual std::optional<bool>
+  isPtrCompatible(const clang::CXXRecordDecl *) const = 0;
   virtual bool isPtrCls(const clang::CXXRecordDecl *) const = 0;
-  virtual const char* typeName() const = 0;
-  virtual const char* invariantName() const = 0;
+  virtual const char *typeName() const = 0;
+  virtual const char *invariant() const = 0;
 
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
                     BugReporter &BRArg) const {
@@ -79,9 +80,9 @@ class RawPtrRefMemberChecker
       if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
         // If we don't see the definition we just don't know.
         if (MemberCXXRD->hasDefinition()) {
-            std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
-            if (isRCAble && *isRCAble)
-                reportBug(Member, MemberType, MemberCXXRD, RD);
+          std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
+          if (isRCAble && *isRCAble)
+              reportBug(Member, MemberType, MemberCXXRD, RD);
         }
       }
     }
@@ -136,14 +137,10 @@ class RawPtrRefMemberChecker
     Os << " in ";
     printQuotedQualifiedName(Os, ClassCXXRD);
     Os << " is a "
-       << (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
-       << " to "
-       << typeName()
-       << " ";
+       << (isa<PointerType>(MemberType) ? "raw pointer" : "reference") << " to "
+       << typeName() << " ";
     printQuotedQualifiedName(Os, MemberCXXRD);
-    Os << "; "
-       << invariantName()
-       << ".";
+    Os << "; " << invariant() << ".";
 
     PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
                                  BR->getSourceManager());
@@ -159,7 +156,8 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
       : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
                                "reference-countable type") {}
 
-  std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const final {
+  std::optional<bool>
+  isPtrCompatible(const clang::CXXRecordDecl *R) const final {
     return isRefCountable(R);
   }
 
@@ -167,11 +165,9 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
     return isRefCounted(R);
   }
 
-  const char* typeName() const final {
-    return "ref-countable type";
-  }
+  const char *typeName() const final { return "ref-countable type"; }
 
-  const char* invariantName() const final {
+  const char *invariant() const final {
     return "member variables must be Ref, RefPtr, WeakRef, or WeakPtr";
   }
 };
@@ -182,7 +178,8 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
       : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
                                "checked-pointer capable type") {}
 
-  std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const final {
+  std::optional<bool>
+  isPtrCompatible(const clang::CXXRecordDecl *R) const final {
     return isCheckedPtrCapable(R);
   }
 
@@ -190,12 +187,11 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
     return isCheckedPtr(R);
   }
 
-  const char* typeName() const final {
-    return "CheckedPtr capable type";
-  }
+  const char *typeName() const final { return "CheckedPtr capable type"; }
 
-  const char* invariantName() const final {
-    return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or WeakPtr";
+  const char *invariant() const final {
+    return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or "
+           "WeakPtr";
   }
 };
 
@@ -205,8 +201,7 @@ void ento::registerNoUncountedMemberChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<NoUncountedMemberChecker>();
 }
 
-bool ento::shouldRegisterNoUncountedMemberChecker(
-    const CheckerManager &Mgr) {
+bool ento::shouldRegisterNoUncountedMemberChecker(const CheckerManager &Mgr) {
   return true;
 }
 

>From b96394d596d07a92fab03bcb083505d6ea0328ac Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 12 Sep 2024 02:54:49 -0700
Subject: [PATCH 4/4] Formatting 2.

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

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 0a8dab737add04..2ce6bc330e0ca1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -82,7 +82,7 @@ class RawPtrRefMemberChecker
         if (MemberCXXRD->hasDefinition()) {
           std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
           if (isRCAble && *isRCAble)
-              reportBug(Member, MemberType, MemberCXXRD, RD);
+            reportBug(Member, MemberType, MemberCXXRD, RD);
         }
       }
     }



More information about the cfe-commits mailing list