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 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 CFPointees; llvm::DenseSet 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..ccef7570e18b2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -30,6 +30,7 @@ class RawPtrRefMemberChecker private: BugType Bug; mutable BugReporter *BR; + mutable llvm::DenseSet IvarDeclsToIgnore; protected: mutable std::optional RTC; @@ -38,7 +39,8 @@ class RawPtrRefMemberChecker RawPtrRefMemberChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} - virtual std::optional isUnsafePtr(QualType) const = 0; + virtual std::optional isUnsafePtr(QualType, + bool ignoreARC = false) const = 0; virtual const char *typeName() const = 0; virtual const char *invariant() const = 0; @@ -142,6 +144,8 @@ class RawPtrRefMemberChecker if (auto *ID = dyn_cast(CD)) { for (auto *Ivar : ID->ivars()) visitIvarDecl(CD, Ivar); + for (auto *PropImpl : ID->property_impls()) + visitPropImpl(CD, PropImpl); return; } } @@ -150,6 +154,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) @@ -159,6 +167,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)) @@ -169,13 +179,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(CD)) { + if (!RTC || !RTC->defaultSynthProperties() || + ID->isObjCRequiresPropertyDefs()) + return; + } + + auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PD); + if (!IsUnsafe) return; if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl()) @@ -184,6 +196,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 + 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; @@ -274,7 +327,7 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "reference-countable type") {} - std::optional isUnsafePtr(QualType QT) const final { + std::optional isUnsafePtr(QualType QT, bool) const final { return isUncountedPtr(QT.getCanonicalType()); } @@ -291,7 +344,7 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "checked-pointer capable type") {} - std::optional isUnsafePtr(QualType QT) const final { + std::optional isUnsafePtr(QualType QT, bool) const final { return isUncheckedPtr(QT.getCanonicalType()); } @@ -311,8 +364,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { RTC = RetainTypeChecker(); } - std::optional isUnsafePtr(QualType QT) const final { - return RTC->isUnretained(QT); + std::optional 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) {} }; } // 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..d0e489654e7b8 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; +@property(nonatomic, assign) NSString *prop_string3; +@property(nonatomic, unsafe_unretained) NSString *prop_string4; +@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'}} +@synthesize prop_string3; +// expected-warning@-1{{Instance variable 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@synthesize prop_string4; +// expected-warning@-1{{Instance variable 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@end