[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from literal as +1 #132350

rniwa · 2025-03-21T07:14:00Z

This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

…iteral as +1 This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

llvmbot · 2025-03-21T07:14:33Z

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

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

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

4 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+12)
(modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+4)
(modified) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm (+6)
(modified) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm (+6)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 4ce3262e90e13..1f9a4712cedf6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -202,6 +202,12 @@ class RetainPtrCtorAdoptChecker
 
   bool isAllocInit(const Expr *E) const {
     auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
+    if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
+      if (unsigned ExprCount = POE->getNumSemanticExprs()) {
+        auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
+        ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
+      }
+    }
     if (!ObjCMsgExpr)
       return false;
     auto Selector = ObjCMsgExpr->getSelector();
@@ -247,6 +253,12 @@ class RetainPtrCtorAdoptChecker
   enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
   IsOwnedResult isOwned(const Expr *E) const {
     while (1) {
+      if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
+        if (unsigned SemanticExprCount = POE->getNumSemanticExprs()) {
+          E = POE->getSemanticExpr(SemanticExprCount - 1);
+          continue;
+        }
+      }
       if (isa<CXXNullPtrLiteralExpr>(E))
         return IsOwnedResult::NotOwned;
       if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 5bd265596a0b4..fc798cfc587b4 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -1,6 +1,8 @@
 @class NSString;
 @class NSArray;
 @class NSMutableArray;
+@class NSDictionary;
+@class NSMutableDictionary;
 #define nil ((id)0)
 #define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T)))
 #define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
@@ -90,12 +92,14 @@ __attribute__((objc_root_class))
 - (NSEnumerator *)keyEnumerator;
 + (id)dictionary;
 + (id)dictionaryWithObject:(id)object forKey:(id <NSCopying>)key;
+- (NSMutableDictionary *)mutableCopy;
 + (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
 @end
 
 @interface NSArray : NSObject <NSCopying, NSFastEnumeration>
 - (NSUInteger)count;
 - (NSEnumerator *)objectEnumerator;
+- (NSMutableArray *)mutableCopy;
 + (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
 @end
 
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
index dd7208a534ea1..e4bbd5e2fc77b 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
@@ -97,3 +97,9 @@ void create_member_init() {
 RetainPtr<id> return_bridge_cast() {
   return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
 }
+
+void mutable_copy() {
+  RetainPtr<NSMutableDictionary> mutableArray = adoptNS(@{
+    @"Content-Type": @"text/html",
+  }.mutableCopy);
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
index 79e0bdb7c577b..8f6783288a093 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -97,3 +97,9 @@ void create_member_init() {
 RetainPtr<id> return_bridge_cast() {
   return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
 }
+
+void mutable_copy() {
+  RetainPtr<NSMutableDictionary> mutableArray = adoptNS(@{
+    @"Content-Type": @"text/html",
+  }.mutableCopy);
+}

llvmbot · 2025-03-21T07:14:34Z

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

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

4 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+12)
(modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+4)
(modified) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm (+6)
(modified) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm (+6)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 4ce3262e90e13..1f9a4712cedf6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -202,6 +202,12 @@ class RetainPtrCtorAdoptChecker
 
   bool isAllocInit(const Expr *E) const {
     auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
+    if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
+      if (unsigned ExprCount = POE->getNumSemanticExprs()) {
+        auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
+        ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
+      }
+    }
     if (!ObjCMsgExpr)
       return false;
     auto Selector = ObjCMsgExpr->getSelector();
@@ -247,6 +253,12 @@ class RetainPtrCtorAdoptChecker
   enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
   IsOwnedResult isOwned(const Expr *E) const {
     while (1) {
+      if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
+        if (unsigned SemanticExprCount = POE->getNumSemanticExprs()) {
+          E = POE->getSemanticExpr(SemanticExprCount - 1);
+          continue;
+        }
+      }
       if (isa<CXXNullPtrLiteralExpr>(E))
         return IsOwnedResult::NotOwned;
       if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 5bd265596a0b4..fc798cfc587b4 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -1,6 +1,8 @@
 @class NSString;
 @class NSArray;
 @class NSMutableArray;
+@class NSDictionary;
+@class NSMutableDictionary;
 #define nil ((id)0)
 #define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T)))
 #define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
@@ -90,12 +92,14 @@ __attribute__((objc_root_class))
 - (NSEnumerator *)keyEnumerator;
 + (id)dictionary;
 + (id)dictionaryWithObject:(id)object forKey:(id <NSCopying>)key;
+- (NSMutableDictionary *)mutableCopy;
 + (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
 @end
 
 @interface NSArray : NSObject <NSCopying, NSFastEnumeration>
 - (NSUInteger)count;
 - (NSEnumerator *)objectEnumerator;
+- (NSMutableArray *)mutableCopy;
 + (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
 @end
 
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
index dd7208a534ea1..e4bbd5e2fc77b 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
@@ -97,3 +97,9 @@ void create_member_init() {
 RetainPtr<id> return_bridge_cast() {
   return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
 }
+
+void mutable_copy() {
+  RetainPtr<NSMutableDictionary> mutableArray = adoptNS(@{
+    @"Content-Type": @"text/html",
+  }.mutableCopy);
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
index 79e0bdb7c577b..8f6783288a093 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -97,3 +97,9 @@ void create_member_init() {
 RetainPtr<id> return_bridge_cast() {
   return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
 }
+
+void mutable_copy() {
+  RetainPtr<NSMutableDictionary> mutableArray = adoptNS(@{
+    @"Content-Type": @"text/html",
+  }.mutableCopy);
+}

t-rasmud · 2025-04-08T22:07:13Z

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm

+  RetainPtr<NSMutableDictionary> mutableArray = adoptNS(@{
+    @"Content-Type": @"text/html",
+  }.mutableCopy);
+}


This is a question for my understanding: Is the behavior expected to be same/different for CFArrayCreateMutable?

Right but I didn't add a test for it separately since CFArrayCreateMutable contains Create in its name and it's a regular function call so the regular Create function rule applies there.

Got it, thank you for explaining.

t-rasmud · 2025-04-08T22:07:48Z

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm

+
+void string_copy(NSString *str) {
+  RetainPtr<NSString> copy = adoptNS(str.copy);
+}


Can we also add a test for RetainPtr<NSMutableDictionary>?

There is a test case for RetainPtr<NSMutableDictionary> right above it. Or are you saying that we should add a test case which takes NSMutableDictionary* as a function argument?

Sorry, I meant RetainPtr<NSArray>

ah, sure, I can add one.

t-rasmud

LGTM!

rniwa · 2025-04-09T17:32:59Z

Thanks for the review!

…iteral as +1 (llvm#132350) This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from l…

65c1d80

…iteral as +1 This PR adds the support for recognizing the return value of copy / mutableCopy as +1. isAllocInit and isOwned now traverses PseudoObjectExpr to its last semantic expression.

llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Mar 21, 2025

rniwa requested a review from t-rasmud March 21, 2025 07:14

Add a test case for accessing copy property on NSString

f86ed5c

t-rasmud reviewed Apr 8, 2025

View reviewed changes

Add test cases for making a mutable copy of NSArray.

8beff97

t-rasmud approved these changes Apr 9, 2025

View reviewed changes

rniwa merged commit c8d49e9 into llvm:main Apr 9, 2025
11 checks passed

rniwa deleted the fix-mutable-copy-from-literal branch April 9, 2025 17:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from literal as +1 #132350

[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from literal as +1 #132350

rniwa commented Mar 21, 2025

llvmbot commented Mar 21, 2025

llvmbot commented Mar 21, 2025

t-rasmud Apr 8, 2025

rniwa Apr 8, 2025

t-rasmud Apr 8, 2025

t-rasmud Apr 8, 2025

rniwa Apr 8, 2025

t-rasmud Apr 8, 2025

rniwa Apr 8, 2025

t-rasmud left a comment

rniwa commented Apr 9, 2025

[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from literal as +1 #132350

[alpha.webkit.RetainPtrCtorAdoptChecker] Recognize mutableCopy from literal as +1 #132350

Conversation

rniwa commented Mar 21, 2025

llvmbot commented Mar 21, 2025

llvmbot commented Mar 21, 2025

t-rasmud Apr 8, 2025

Choose a reason for hiding this comment

rniwa Apr 8, 2025

Choose a reason for hiding this comment

t-rasmud Apr 8, 2025

Choose a reason for hiding this comment

t-rasmud Apr 8, 2025

Choose a reason for hiding this comment

rniwa Apr 8, 2025

Choose a reason for hiding this comment

t-rasmud Apr 8, 2025

Choose a reason for hiding this comment

rniwa Apr 8, 2025

Choose a reason for hiding this comment

t-rasmud left a comment

Choose a reason for hiding this comment

rniwa commented Apr 9, 2025