Check the type of Objective-C++ instance variables in WebKit member v…

…ariable checkers. (llvm#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.
rniwa · Feb 19, 2025 · 8a6eb13 · 8a6eb13
1 parent 6adc720
commit 8a6eb13
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 3 deletions.
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -62,6 +62,11 @@ class RawPtrRefMemberChecker
         Checker->visitRecordDecl(RD);
         return true;
       }
+
+      bool VisitObjCContainerDecl(const ObjCContainerDecl *CD) override {
+        Checker->visitObjCDecl(CD);
+        return true;
+      }
     };
 
     LocalVisitor visitor(this);
@@ -88,6 +93,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;
@@ -122,17 +152,21 @@ 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);
 
     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
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+
+__attribute__((objc_root_class))
+@interface NSObject
++ (instancetype) alloc;
+- (instancetype) init;
+- (instancetype)retain;
+- (void)release;
+@end
+
+void doSomeWork();
+
+@interface SomeObjC : NSObject {
+  CheckedObj* _unchecked1;
+// expected-warning@-1{{Instance variable '_unchecked1' in 'SomeObjC' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}  
+  CheckedPtr<CheckedObj> _counted1;
+  [[clang::suppress]] CheckedObj* _unchecked2;
+}
+- (void)doWork;
+@end
+
+@implementation SomeObjC {
+  CheckedObj* _unchecked3;
+// expected-warning@-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();
+}
+
+@end
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members-objc.mm 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))
+@interface NSObject
++ (instancetype) alloc;
+- (instancetype) init;
+- (instancetype)retain;
+- (void)release;
+@end
+
+void doSomeWork();
+
+@interface SomeObjC : NSObject {
+  RefCountable* _uncounted1;
+// expected-warning@-1{{Instance variable '_uncounted1' in 'SomeObjC' is a raw pointer to ref-countable type 'RefCountable'}}  
+  RefPtr<RefCountable> _counted1;
+  [[clang::suppress]] RefCountable* _uncounted2;
+}
+- (void)doWork;
+@end
+
+@implementation SomeObjC {
+  RefCountable* _uncounted3;
+// expected-warning@-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();
+}
+
+@end