Skip to content

[alpha.webkit.UnretainedCallArgsChecker] Add the support for RetainPtrArc #135532

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 3 commits into from
Apr 16, 2025

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Apr 13, 2025

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the support for it.

…rArc

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the support for it.
@rniwa rniwa requested a review from t-rasmud April 13, 2025 08:32
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Apr 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 13, 2025

@llvm/pr-subscribers-clang

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

Author: Ryosuke Niwa (rniwa)

Changes

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the support for it.


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

4 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+11-6)
  • (modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+5)
  • (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm (+11)
  • (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm (+11)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 781b0de5abd2f..4971b2c6a1190 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -119,7 +119,9 @@ bool isRefType(const std::string &Name) {
          Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
 }
 
-bool isRetainPtr(const std::string &Name) { return Name == "RetainPtr"; }
+bool isRetainPtr(const std::string &Name) {
+  return Name == "RetainPtr" || Name == "RetainPtrArc";
+}
 
 bool isCheckedPtr(const std::string &Name) {
   return Name == "CheckedPtr" || Name == "CheckedRef";
@@ -157,7 +159,8 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
 bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
   const std::string &FunctionName = safeGetName(F);
   return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
-         FunctionName == "adoptCF" || FunctionName == "retainPtr";
+         FunctionName == "adoptCF" || FunctionName == "retainPtr" ||
+         FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc";
 }
 
 bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
@@ -190,7 +193,9 @@ bool isRefOrCheckedPtrType(const clang::QualType T) {
 }
 
 bool isRetainPtrType(const clang::QualType T) {
-  return isPtrOfType(T, [](auto Name) { return Name == "RetainPtr"; });
+  return isPtrOfType(T, [](auto Name) {
+    return Name == "RetainPtr" || Name == "RetainPtrArc";
+  });
 }
 
 bool isOwnerPtrType(const clang::QualType T) {
@@ -374,7 +379,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
          method == "impl"))
       return true;
 
-    if (className == "RetainPtr" && method == "get")
+    if (isRetainPtr(className) && method == "get")
       return true;
 
     // Ref<T> -> T conversion
@@ -395,7 +400,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
       }
     }
 
-    if (className == "RetainPtr") {
+    if (isRetainPtr(className)) {
       if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
         auto QT = maybeRefToRawOperator->getConversionType();
         auto *T = QT.getTypePtrOrNull();
@@ -429,7 +434,7 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
 bool isRetainPtr(const CXXRecordDecl *R) {
   assert(R);
   if (auto *TmplR = R->getTemplateInstantiationPattern())
-    return safeGetName(TmplR) == "RetainPtr";
+    return isRetainPtr(safeGetName(TmplR));
   return false;
 }
 
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 3f075ca0a6e5b..76d3d187368ae 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -17,6 +17,7 @@ typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef;
 typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef;
 typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
 typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
+typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef;
 
 #define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
 #define CF_CONSUMED __attribute__((cf_consumed))
@@ -150,6 +151,10 @@ namespace WTF {
 
 void WTFCrash(void);
 
+#if __has_feature(objc_arc)
+#define RetainPtr RetainPtrArc
+#endif
+
 template<typename T> class RetainPtr;
 template<typename T> RetainPtr<T> adoptNS(T*);
 template<typename T> RetainPtr<T> adoptCF(T);
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
index f1f4d912663aa..4207c1836079f 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
@@ -5,6 +5,8 @@
 SomeObj *provide();
 CFMutableArrayRef provide_cf();
 void someFunction();
+CGImageRef provideImage();
+NSString *stringForImage(CGImageRef);
 
 namespace raw_ptr {
 
@@ -36,4 +38,13 @@ - (SomeObj *)getSomeObj {
 - (void)doWorkOnSomeObj {
     [[self getSomeObj] doWork];
 }
+
+- (CGImageRef)createImage {
+  return provideImage();
+}
+
+- (NSString *)convertImage {
+  RetainPtr<CGImageRef> image = [self createImage];
+  return stringForImage(image.get());
+}
 @end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index dd21864300387..e63af08c21205 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -9,6 +9,9 @@
 CFMutableArrayRef provide_cf();
 void consume_cf(CFMutableArrayRef);
 
+CGImageRef provideImage();
+NSString *stringForImage(CGImageRef);
+
 void some_function();
 
 namespace simple {
@@ -430,4 +433,12 @@ - (void)doWorkOnSomeObj {
     [[self getSomeObj] doWork];
 }
 
+- (CGImageRef)createImage {
+  return provideImage();
+}
+
+- (NSString *)convertImage {
+  RetainPtr<CGImageRef> image = [self createImage];
+  return stringForImage(image.get());
+}
 @end

Copy link
Contributor

@t-rasmud t-rasmud 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 16, 2025

Thanks for the review!

@rniwa rniwa merged commit 517605c into llvm:main Apr 16, 2025
10 of 11 checks passed
@rniwa rniwa deleted the webkit-retain-ptr-arc branch April 16, 2025 03:00
rniwa added a commit to rniwa/llvm-project that referenced this pull request Apr 16, 2025
…rArc (llvm#135532)

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the
support for it.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…rArc (llvm#135532)

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the
support for it.
rniwa added a commit to rniwa/llvm-project that referenced this pull request Apr 22, 2025
…rArc (llvm#135532)

WebKit uses #define to rename RetainPtr to RetainPtrArc so add the
support for it.
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