https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/141293
>From bd7daa355f5c1924475c0818dd4963af7bbffcb1 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Fri, 23 May 2025 13:53:36 -0700 Subject: [PATCH 1/3] [alpha.webkit.NoUnretainedMemberChecker] Recocgnize NS_REQUIRES_PROPERTY_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. --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 1 + .../Checkers/WebKit/PtrTypesSemantics.h | 2 ++ .../WebKit/RawPtrRefMemberChecker.cpp | 5 +++++ .../Checkers/WebKit/objc-mock-types.h | 1 + .../Checkers/WebKit/unretained-members-arc.mm | 17 +++++++++++++++++ .../Checkers/WebKit/unretained-members.mm | 19 +++++++++++++++++++ 6 files changed, 45 insertions(+) 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 >From 0d7c086b725ff01578d594b964e08627c072ebe5 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Fri, 23 May 2025 14:20:39 -0700 Subject: [PATCH 2/3] Emit a warning for @property(assign) and @property(unsafe_unretained) under ARC. --- .../Checkers/WebKit/RawPtrRefMemberChecker.cpp | 16 +++++++++------- .../Checkers/WebKit/unretained-members-arc.mm | 13 +++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index acd23b6eae7ea..650bc41693e54 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -38,7 +38,7 @@ class RawPtrRefMemberChecker RawPtrRefMemberChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} - virtual std::optional<bool> isUnsafePtr(QualType) const = 0; + virtual std::optional<bool> isUnsafePtr(QualType, bool ignoreARC = false) const = 0; virtual const char *typeName() const = 0; virtual const char *invariant() const = 0; @@ -175,11 +175,13 @@ class RawPtrRefMemberChecker return; if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) { - if (!RTC || !RTC->defaultSynthProperties() || ID->isObjCRequiresPropertyDefs()) + if (!RTC || !RTC->defaultSynthProperties() || + ID->isObjCRequiresPropertyDefs()) return; } - auto IsUnsafePtr = isUnsafePtr(QT); + bool ignoreARC = !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; + auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC); if (!IsUnsafePtr || !*IsUnsafePtr) return; @@ -279,7 +281,7 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "reference-countable type") {} - std::optional<bool> isUnsafePtr(QualType QT) const final { + std::optional<bool> isUnsafePtr(QualType QT, bool) const final { return isUncountedPtr(QT.getCanonicalType()); } @@ -296,7 +298,7 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "checked-pointer capable type") {} - std::optional<bool> isUnsafePtr(QualType QT) const final { + std::optional<bool> isUnsafePtr(QualType QT, bool) const final { return isUncheckedPtr(QT.getCanonicalType()); } @@ -316,8 +318,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { RTC = RetainTypeChecker(); } - std::optional<bool> isUnsafePtr(QualType QT) const final { - return RTC->isUnretained(QT); + std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final { + return RTC->isUnretained(QT, ignoreARC); } const char *typeName() const final { return "retainable type"; } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm index ca2679900bebe..54d661639da60 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm @@ -65,6 +65,19 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {} } // namespace ptr_to_ptr_to_retained +@interface AnotherObject : NSObject { + NSString *ns_string; + CFStringRef cf_string; + // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} +} +@property(nonatomic, strong) NSString *prop_string1; +@property(nonatomic, assign) NSString *prop_string2; +// expected-warning@-1{{Property 'prop_string2' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, unsafe_unretained) NSString *prop_string3; +// expected-warning@-1{{Property 'prop_string3' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, readonly) NSString *prop_string4; +@end + NS_REQUIRES_PROPERTY_DEFINITIONS @interface NoSynthObject : NSObject { NSString *ns_string; >From 07f4f7d4cfb006586d3d3a8dfb7aafc66deb6441 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Fri, 23 May 2025 14:28:26 -0700 Subject: [PATCH 3/3] Fix formatting --- .../Checkers/WebKit/RawPtrRefMemberChecker.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 650bc41693e54..aa707823beb5b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -38,7 +38,8 @@ class RawPtrRefMemberChecker RawPtrRefMemberChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} - virtual std::optional<bool> isUnsafePtr(QualType, bool ignoreARC = false) const = 0; + virtual std::optional<bool> isUnsafePtr(QualType, + bool ignoreARC = false) const = 0; virtual const char *typeName() const = 0; virtual const char *invariant() const = 0; @@ -180,7 +181,8 @@ class RawPtrRefMemberChecker return; } - bool ignoreARC = !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; + bool ignoreARC = + !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC); if (!IsUnsafePtr || !*IsUnsafePtr) return; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits