Author: Ryosuke Niwa Date: 2025-06-09T20:55:50-07:00 New Revision: 0123ee51ef3290466c6d743aac2932b23655aa04
URL: https://github.com/llvm/llvm-project/commit/0123ee51ef3290466c6d743aac2932b23655aa04 DIFF: https://github.com/llvm/llvm-project/commit/0123ee51ef3290466c6d743aac2932b23655aa04.diff LOG: [alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS (#143408) 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 as well as when explicitly synthesizing a unsafe raw pointer property. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp clang/test/Analysis/Checkers/WebKit/objc-mock-types.h clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm clang/test/Analysis/Checkers/WebKit/unretained-members.mm Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index cd33476344a34..72199af2f80a5 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 b1350b9093021..8faf6a219450a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -28,6 +28,7 @@ class RawPtrRefMemberChecker private: BugType Bug; mutable BugReporter *BR; + mutable llvm::DenseSet<const ObjCIvarDecl *> IvarDeclsToIgnore; protected: mutable std::optional<RetainTypeChecker> RTC; @@ -36,7 +37,8 @@ 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; @@ -138,6 +140,8 @@ class RawPtrRefMemberChecker return; } if (auto *ID = dyn_cast<ObjCImplementationDecl>(CD)) { + for (auto *PropImpl : ID->property_impls()) + visitPropImpl(CD, PropImpl); for (auto *Ivar : ID->ivars()) visitIvarDecl(CD, Ivar); return; @@ -148,6 +152,10 @@ class RawPtrRefMemberChecker const ObjCIvarDecl *Ivar) const { if (BR->getSourceManager().isInSystemHeader(Ivar->getLocation())) return; + + if (IvarDeclsToIgnore.contains(Ivar)) + return; + auto QT = Ivar->getType(); const Type *IvarType = QT.getTypePtrOrNull(); if (!IvarType) @@ -157,6 +165,8 @@ class RawPtrRefMemberChecker if (!IsUnsafePtr || !*IsUnsafePtr) return; + IvarDeclsToIgnore.insert(Ivar); + if (auto *MemberCXXRD = IvarType->getPointeeCXXRecordDecl()) reportBug(Ivar, IvarType, MemberCXXRD, CD); else if (auto *ObjCDecl = getObjCDecl(IvarType)) @@ -167,13 +177,15 @@ class RawPtrRefMemberChecker const ObjCPropertyDecl *PD) const { if (BR->getSourceManager().isInSystemHeader(PD->getLocation())) return; - auto QT = PD->getType(); - const Type *PropType = QT.getTypePtrOrNull(); - if (!PropType) - return; - auto IsUnsafePtr = isUnsafePtr(QT); - if (!IsUnsafePtr || !*IsUnsafePtr) + if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) { + if (!RTC || !RTC->defaultSynthProperties() || + ID->isObjCRequiresPropertyDefs()) + return; + } + + auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PD); + if (!IsUnsafe) return; if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl()) @@ -182,6 +194,47 @@ class RawPtrRefMemberChecker reportBug(PD, PropType, ObjCDecl, CD); } + void visitPropImpl(const ObjCContainerDecl *CD, + const ObjCPropertyImplDecl *PID) const { + if (BR->getSourceManager().isInSystemHeader(PID->getLocation())) + return; + + if (PID->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize) + return; + + auto *PropDecl = PID->getPropertyDecl(); + if (auto *IvarDecl = PID->getPropertyIvarDecl()) { + if (IvarDeclsToIgnore.contains(IvarDecl)) + return; + IvarDeclsToIgnore.insert(IvarDecl); + } + auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PropDecl); + if (!IsUnsafe) + return; + + if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl()) + reportBug(PropDecl, PropType, MemberCXXRD, CD); + else if (auto *ObjCDecl = getObjCDecl(PropType)) + reportBug(PropDecl, PropType, ObjCDecl, CD); + } + + std::pair<bool, const Type *> + isPropImplUnsafePtr(const ObjCPropertyDecl *PD) const { + if (!PD) + return {false, nullptr}; + + auto QT = PD->getType(); + const Type *PropType = QT.getTypePtrOrNull(); + if (!PropType) + return {false, nullptr}; + + // "assign" property doesn't retain even under ARC so treat it as unsafe. + bool ignoreARC = + !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; + auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC); + return {IsUnsafePtr && *IsUnsafePtr, PropType}; + } + bool shouldSkipDecl(const RecordDecl *RD) const { if (!RD->isThisDeclarationADefinition()) return true; @@ -272,7 +325,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()); } @@ -289,7 +342,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()); } @@ -309,8 +362,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/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..00e6e6ec1dcfa 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm @@ -64,3 +64,39 @@ 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; + 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; +@property(nonatomic, assign) NSString *prop_string3; +// expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, unsafe_unretained) NSString *prop_string4; +// expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@end + +@implementation NoSynthObject +- (NSString *)prop_string1 { + return nil; +} +@synthesize prop_string2; +@synthesize prop_string3; +@synthesize prop_string4; +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index 0cb4c4ac0f6a0..46f65dfa603ad 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -99,3 +99,28 @@ @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; +// expected-warning@-1{{Property 'prop_string2' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}} +@property(nonatomic, assign) NSString *prop_string3; +// expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, unsafe_unretained) NSString *prop_string4; +// expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@end + +@implementation NoSynthObject +- (NSString *)prop_string1 { + return nil; +} +@synthesize prop_string2; +@synthesize prop_string3; +@synthesize prop_string4; +@end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
