[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS #141293

rniwa · 2025-05-23T20:57:24Z

Allow @Property of a raw pointer when NS_REQUIRES_PROPERTY_DEFINITIONS is specified on the interface since such an interface does not automatically synthesize raw pointer ivars.

llvmbot · 2025-05-23T20:57:59Z

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

Author: Ryosuke Niwa (rniwa)

Changes

Allow @property of a raw pointer when NS_REQUIRES_PROPERTY_DEFINITIONS is specified on the interface since such an interface does not automatically synthesize raw pointer ivars.

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

6 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+1)
(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+2)
(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+5)
(modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+1)
(modified) clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm (+17)
(modified) clang/test/Analysis/Checkers/WebKit/unretained-members.mm (+19)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 4ddd11495f534..f547f86f4782f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -236,6 +236,7 @@ std::optional<bool> isUnchecked(const QualType T) {
 void RetainTypeChecker::visitTranslationUnitDecl(
     const TranslationUnitDecl *TUD) {
   IsARCEnabled = TUD->getLangOpts().ObjCAutoRefCount;
+  DefaultSynthProperties = TUD->getLangOpts().ObjCDefaultSynthProperties;
 }
 
 void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index f9fcfe9878d54..3c9560cb8059b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -76,12 +76,14 @@ class RetainTypeChecker {
   llvm::DenseSet<const RecordType *> CFPointees;
   llvm::DenseSet<const Type *> RecordlessTypes;
   bool IsARCEnabled{false};
+  bool DefaultSynthProperties{true};
 
 public:
   void visitTranslationUnitDecl(const TranslationUnitDecl *);
   void visitTypedef(const TypedefDecl *);
   bool isUnretained(const QualType, bool ignoreARC = false);
   bool isARCEnabled() const { return IsARCEnabled; }
+  bool defaultSynthProperties() const { return DefaultSynthProperties; }
 };
 
 /// \returns true if \p Class is NS or CF objects AND not retained, false if
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 4ea6158c4c410..acd23b6eae7ea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -174,6 +174,11 @@ class RawPtrRefMemberChecker
     if (!PropType)
       return;
 
+    if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
+      if (!RTC || !RTC->defaultSynthProperties() || ID->isObjCRequiresPropertyDefs())
+        return;
+    }
+
     auto IsUnsafePtr = isUnsafePtr(QT);
     if (!IsUnsafePtr || !*IsUnsafePtr)
       return;
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 93e7dfd77b9e9..9e4356a71f1b5 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -22,6 +22,7 @@ typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef;
 #define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
 #define CF_CONSUMED __attribute__((cf_consumed))
 #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define NS_REQUIRES_PROPERTY_DEFINITIONS __attribute__((objc_requires_property_definitions))
 
 extern const CFAllocatorRef kCFAllocatorDefault;
 typedef struct _NSZone NSZone;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
index 3491bc93ed98a..ca2679900bebe 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
@@ -64,3 +64,20 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
   };
 
 } // namespace ptr_to_ptr_to_retained
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthObject : NSObject {
+  NSString *ns_string;
+  CFStringRef cf_string;
+  // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
+}
+@property(nonatomic, readonly, strong) NSString *prop_string1;
+@property(nonatomic, readonly, strong) NSString *prop_string2;
+@end
+
+@implementation NoSynthObject
+- (NSString *)prop_string1 {
+  return nil;
+}
+@synthesize prop_string2;
+@end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
index 0cb4c4ac0f6a0..7b31296378034 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
@@ -99,3 +99,22 @@ @interface AnotherObject : NSObject {
 @property(nonatomic, strong) NSString *prop_string;
 // expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
 @end
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthObject : NSObject {
+  NSString *ns_string;
+  // expected-warning@-1{{Instance variable 'ns_string' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
+  CFStringRef cf_string;
+  // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
+}
+@property(nonatomic, readonly, strong) NSString *prop_string1;
+@property(nonatomic, readonly, strong) NSString *prop_string2;
+@end
+
+@implementation NoSynthObject
+- (NSString *)prop_string1 {
+  return nil;
+}
+@synthesize prop_string2;
+// expected-warning@-1{{Instance variable 'prop_string2' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}}
+@end

llvmbot · 2025-05-23T20:57:59Z

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Allow @property of a raw pointer when NS_REQUIRES_PROPERTY_DEFINITIONS is specified on the interface since such an interface does not automatically synthesize raw pointer ivars.

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

6 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+1)
(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+2)
(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+5)
(modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+1)
(modified) clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm (+17)
(modified) clang/test/Analysis/Checkers/WebKit/unretained-members.mm (+19)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 4ddd11495f534..f547f86f4782f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -236,6 +236,7 @@ std::optional<bool> isUnchecked(const QualType T) {
 void RetainTypeChecker::visitTranslationUnitDecl(
     const TranslationUnitDecl *TUD) {
   IsARCEnabled = TUD->getLangOpts().ObjCAutoRefCount;
+  DefaultSynthProperties = TUD->getLangOpts().ObjCDefaultSynthProperties;
 }
 
 void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index f9fcfe9878d54..3c9560cb8059b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -76,12 +76,14 @@ class RetainTypeChecker {
   llvm::DenseSet<const RecordType *> CFPointees;
   llvm::DenseSet<const Type *> RecordlessTypes;
   bool IsARCEnabled{false};
+  bool DefaultSynthProperties{true};
 
 public:
   void visitTranslationUnitDecl(const TranslationUnitDecl *);
   void visitTypedef(const TypedefDecl *);
   bool isUnretained(const QualType, bool ignoreARC = false);
   bool isARCEnabled() const { return IsARCEnabled; }
+  bool defaultSynthProperties() const { return DefaultSynthProperties; }
 };
 
 /// \returns true if \p Class is NS or CF objects AND not retained, false if
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 4ea6158c4c410..acd23b6eae7ea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -174,6 +174,11 @@ class RawPtrRefMemberChecker
     if (!PropType)
       return;
 
+    if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
+      if (!RTC || !RTC->defaultSynthProperties() || ID->isObjCRequiresPropertyDefs())
+        return;
+    }
+
     auto IsUnsafePtr = isUnsafePtr(QT);
     if (!IsUnsafePtr || !*IsUnsafePtr)
       return;
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 93e7dfd77b9e9..9e4356a71f1b5 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -22,6 +22,7 @@ typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef;
 #define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
 #define CF_CONSUMED __attribute__((cf_consumed))
 #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define NS_REQUIRES_PROPERTY_DEFINITIONS __attribute__((objc_requires_property_definitions))
 
 extern const CFAllocatorRef kCFAllocatorDefault;
 typedef struct _NSZone NSZone;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
index 3491bc93ed98a..ca2679900bebe 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
@@ -64,3 +64,20 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
   };
 
 } // namespace ptr_to_ptr_to_retained
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthObject : NSObject {
+  NSString *ns_string;
+  CFStringRef cf_string;
+  // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
+}
+@property(nonatomic, readonly, strong) NSString *prop_string1;
+@property(nonatomic, readonly, strong) NSString *prop_string2;
+@end
+
+@implementation NoSynthObject
+- (NSString *)prop_string1 {
+  return nil;
+}
+@synthesize prop_string2;
+@end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
index 0cb4c4ac0f6a0..7b31296378034 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
@@ -99,3 +99,22 @@ @interface AnotherObject : NSObject {
 @property(nonatomic, strong) NSString *prop_string;
 // expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
 @end
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthObject : NSObject {
+  NSString *ns_string;
+  // expected-warning@-1{{Instance variable 'ns_string' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
+  CFStringRef cf_string;
+  // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
+}
+@property(nonatomic, readonly, strong) NSString *prop_string1;
+@property(nonatomic, readonly, strong) NSString *prop_string2;
+@end
+
+@implementation NoSynthObject
+- (NSString *)prop_string1 {
+  return nil;
+}
+@synthesize prop_string2;
+// expected-warning@-1{{Instance variable 'prop_string2' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}}
+@end

github-actions · 2025-05-23T21:00:33Z

✅ With the latest revision this PR passed the C/C++ code formatter.

…TY_DEFINITIONS Allow @Property of a raw pointer when NS_REQUIRES_PROPERTY_DEFINITIONS is specified on the interface since such an interface does not automatically synthesize raw pointer ivars. Also emit a warning for @Property(assign) and @Property(unsafe_unretained) under ARC.

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp

clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm

clang/test/Analysis/Checkers/WebKit/unretained-members.mm

ziqingluo-90 · 2025-06-06T07:06:25Z

Overall, I think the change aligns with what you described. I've just left a few questions to clarify my understanding.

Added test cases for synthesizing `assign` and `unsafe_unretained` properties.

ziqingluo-90 · 2025-06-07T08:47:52Z

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp

@@ -142,6 +144,8 @@ class RawPtrRefMemberChecker
    if (auto *ID = dyn_cast<ObjCImplementationDecl>(CD)) {
      for (auto *Ivar : ID->ivars())
        visitIvarDecl(CD, Ivar);
+      for (auto *PropImpl : ID->property_impls())
+        visitPropImpl(CD, PropImpl);


visitPropImpl populates IvarDeclsToIgnore. Should it be called before visitIvarDecl?

oh, that must be why my warning is emitted in a seemingly wrong place for one of the test cases. Will fix.

rniwa · 2025-06-09T16:36:14Z

There was a typo in the branch name so closing this in favor of a new PR: #143408

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

rniwa changed the title ~~[alpha.webkit.NoUnretainedMemberChecker] Recocgnize NS_REQUIRES_PROPERTY_DEFINITIONS~~ [alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS May 23, 2025

rniwa force-pushed the allow-objc-property-erquires-definitions branch from 07f4f7d to bcee060 Compare May 23, 2025 21:30

rniwa requested a review from t-rasmud May 24, 2025 00:20

ziqingluo-90 reviewed Jun 6, 2025

View reviewed changes

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp Outdated Show resolved Hide resolved

ziqingluo-90 reviewed Jun 6, 2025

View reviewed changes

clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm Show resolved Hide resolved

ziqingluo-90 reviewed Jun 6, 2025

View reviewed changes

clang/test/Analysis/Checkers/WebKit/unretained-members.mm Show resolved Hide resolved

rniwa added 2 commits June 6, 2025 13:28

Emit a warning for @synthesize when synthesizing an unsafe raw pointer.

c768954

Added test cases for synthesizing `assign` and `unsafe_unretained` properties.

Fix formatting

469b560

ziqingluo-90 reviewed Jun 7, 2025

View reviewed changes

rniwa closed this Jun 9, 2025

rniwa deleted the allow-objc-property-erquires-definitions branch June 9, 2025 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS #141293

[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS #141293

Uh oh!

rniwa commented May 23, 2025

Uh oh!

llvmbot commented May 23, 2025

Uh oh!

llvmbot commented May 23, 2025

Uh oh!

github-actions bot commented May 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ziqingluo-90 commented Jun 6, 2025

Uh oh!

ziqingluo-90 Jun 7, 2025

Uh oh!

rniwa Jun 9, 2025

Uh oh!

rniwa commented Jun 9, 2025

Uh oh!

Uh oh!

[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS #141293

[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS #141293

Uh oh!

Conversation

rniwa commented May 23, 2025

Uh oh!

llvmbot commented May 23, 2025

Uh oh!

llvmbot commented May 23, 2025

Uh oh!

github-actions bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ziqingluo-90 commented Jun 6, 2025

Uh oh!

ziqingluo-90 Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

rniwa Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

rniwa commented Jun 9, 2025

Uh oh!

Uh oh!

github-actions bot commented May 23, 2025 •

edited

Loading