[clang] Check the type of Objective-C++ instance variables in WebKit member variable checkers. (PR #127570)

[Ryosuke Niwa via cfe-commits]

https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/127570

Like a C++ member variable, every Objective-C++ instance variable must be a RefPtr, Ref CheckedPtr, or CheckedRef to an object, not a raw pointer or reference.

>From f93a95fc02a9389fda9597c6dffe0f16d8bc2bf1 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Mon, 17 Feb 2025 22:03:58 -0800
Subject: [PATCH] Check the type of Objective-C++ instance variables in WebKit
 member variable checkers.

Like a C++ member variable, every Objective-C++ instance variable must be a RefPtr, Ref
CheckedPtr, or CheckedRef to an object, not a raw pointer or reference.
---
 .../WebKit/RawPtrRefMemberChecker.cpp         | 40 +++++++++++++++++--
 .../Checkers/WebKit/unchecked-members-objc.mm | 35 ++++++++++++++++
 .../Checkers/WebKit/uncounted-members-objc.mm | 35 ++++++++++++++++
 3 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/Checkers/WebKit/unchecked-members-objc.mm
 create mode 100644 clang/test/Analysis/Checkers/WebKit/uncounted-members-objc.mm

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 79f88553feb95..963f59831c8ed 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -61,6 +61,11 @@ class RawPtrRefMemberChecker
         Checker->visitRecordDecl(RD);
         return true;
       }
+
+      bool VisitObjCContainerDecl(const ObjCContainerDecl *CD) override {
+        Checker->visitObjCDecl(CD);
+        return true;
+      }
     };
 
     LocalVisitor visitor(this);
@@ -87,6 +92,31 @@ class RawPtrRefMemberChecker
     }
   }
 
+  void visitObjCDecl(const ObjCContainerDecl *CD) const {
+    if (auto *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
+      for (auto *Ivar : ID->ivars())
+        visitIvarDecl(CD, Ivar);
+      return;
+    }
+    if (auto *ID = dyn_cast<ObjCImplementationDecl>(CD)) {
+      for (auto *Ivar : ID->ivars())
+        visitIvarDecl(CD, Ivar);
+      return;
+    }
+  }
+
+  void visitIvarDecl(const ObjCContainerDecl *CD,
+                     const ObjCIvarDecl *Ivar) const {
+    const Type *IvarType = Ivar->getType().getTypePtrOrNull();
+    if (!IvarType)
+      return;
+    if (auto *IvarCXXRD = IvarType->getPointeeCXXRecordDecl()) {
+      std::optional<bool> IsCompatible = isPtrCompatible(IvarCXXRD);
+      if (IsCompatible && *IsCompatible)
+        reportBug(Ivar, IvarType, IvarCXXRD, CD);
+    }
+  }
+
   bool shouldSkipDecl(const RecordDecl *RD) const {
     if (!RD->isThisDeclarationADefinition())
       return true;
@@ -121,9 +151,10 @@ class RawPtrRefMemberChecker
     return false;
   }
 
-  void reportBug(const FieldDecl *Member, const Type *MemberType,
+  template <typename DeclType, typename ParentDeclType>
+  void reportBug(const DeclType *Member, const Type *MemberType,
                  const CXXRecordDecl *MemberCXXRD,
-                 const RecordDecl *ClassCXXRD) const {
+                 const ParentDeclType *ClassCXXRD) const {
     assert(Member);
     assert(MemberType);
     assert(MemberCXXRD);
@@ -131,7 +162,10 @@ class RawPtrRefMemberChecker
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
 
-    Os << "Member variable ";
+    if (isa<ObjCContainerDecl>(ClassCXXRD))
+      Os << "Instance variable ";
+    else
+      Os << "Member variable ";
     printQuotedName(Os, Member);
     Os << " in ";
     printQuotedQualifiedName(Os, ClassCXXRD);
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members-objc.mm b/clang/test/Analysis/Checkers/WebKit/unchecked-members-objc.mm
new file mode 100644
index 0000000000000..a9a9a367fb9f4
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members-objc.mm
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+
+__attribute__((objc_root_class))
+ at interface NSObject
++ (instancetype) alloc;
+- (instancetype) init;
+- (instancetype)retain;
+- (void)release;
+ at end
+
+void doSomeWork();
+
+ at interface SomeObjC : NSObject {
+  CheckedObj* _unchecked1;
+// expected-warning at -1{{Instance variable '_unchecked1' in 'SomeObjC' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}  
+  CheckedPtr<CheckedObj> _counted1;
+  [[clang::suppress]] CheckedObj* _unchecked2;
+}
+- (void)doWork;
+ at end
+
+ at implementation SomeObjC {
+  CheckedObj* _unchecked3;
+// expected-warning at -1{{Instance variable '_unchecked3' in 'SomeObjC' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+  CheckedPtr<CheckedObj> _counted2;
+  [[clang::suppress]] CheckedObj* _unchecked4;
+}
+
+- (void)doWork {
+  doSomeWork();
+}
+
+ at end
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members-objc.mm b/clang/test/Analysis/Checkers/WebKit/uncounted-members-objc.mm
new file mode 100644
index 0000000000000..83b08a6841d26
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members-objc.mm
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.NoUncountedMemberChecker -verify %s
+
+#include "mock-types.h"
+
+__attribute__((objc_root_class))
+ at interface NSObject
++ (instancetype) alloc;
+- (instancetype) init;
+- (instancetype)retain;
+- (void)release;
+ at end
+
+void doSomeWork();
+
+ at interface SomeObjC : NSObject {
+  RefCountable* _uncounted1;
+// expected-warning at -1{{Instance variable '_uncounted1' in 'SomeObjC' is a raw pointer to ref-countable type 'RefCountable'}}  
+  RefPtr<RefCountable> _counted1;
+  [[clang::suppress]] RefCountable* _uncounted2;
+}
+- (void)doWork;
+ at end
+
+ at implementation SomeObjC {
+  RefCountable* _uncounted3;
+// expected-warning at -1{{Instance variable '_uncounted3' in 'SomeObjC' is a raw pointer to ref-countable type 'RefCountable'}}
+  RefPtr<RefCountable> _counted2;
+  [[clang::suppress]] RefCountable* _uncounted4;
+}
+
+- (void)doWork {
+  doSomeWork();
+}
+
+ at end