https://github.com/rniwa updated
https://github.com/llvm/llvm-project/pull/141293
>From bcee06004f24f8488bcc8e84170bf3509daa5fd9 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] Recognize
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.
Also emit a warning for @property(assign) and @property(unsafe_unretained)
under ARC.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 1 +
.../Checkers/WebKit/PtrTypesSemantics.h | 2 ++
.../WebKit/RawPtrRefMemberChecker.cpp | 21 +++++++++----
.../Checkers/WebKit/objc-mock-types.h | 1 +
.../Checkers/WebKit/unretained-members-arc.mm | 30 +++++++++++++++++++
.../Checkers/WebKit/unretained-members.mm | 19 ++++++++++++
6 files changed, 68 insertions(+), 6 deletions(-)
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..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) 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;
@@ -174,7 +175,15 @@ class RawPtrRefMemberChecker
if (!PropType)
return;
- auto IsUnsafePtr = isUnsafePtr(QT);
+ if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
+ if (!RTC || !RTC->defaultSynthProperties() ||
+ ID->isObjCRequiresPropertyDefs())
+ return;
+ }
+
+ bool ignoreARC =
+ !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign;
+ auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC);
if (!IsUnsafePtr || !*IsUnsafePtr)
return;
@@ -274,7 +283,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());
}
@@ -291,7 +300,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());
}
@@ -311,8 +320,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..54d661639da60 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
@@ -64,3 +64,33 @@ 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;
+@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 c768954f7bfa3df3b02b4411f9c828198fb0516e Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 6 Jun 2025 13:16:23 -0600
Subject: [PATCH 2/3] Emit a warning for @synthesize when synthesizing an
unsafe raw pointer.
Added test cases for synthesizing `assign` and `unsafe_unretained` properties.
---
.../WebKit/RawPtrRefMemberChecker.cpp | 59 ++++++++++++++++---
.../Checkers/WebKit/unretained-members-arc.mm | 6 ++
.../Checkers/WebKit/unretained-members.mm | 6 ++
3 files changed, 63 insertions(+), 8 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index aa707823beb5b..8a31ab9a23c61 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<const ObjCIvarDecl *> IvarDeclsToIgnore;
protected:
mutable std::optional<RetainTypeChecker> RTC;
@@ -143,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);
return;
}
}
@@ -151,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)
@@ -160,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))
@@ -170,10 +179,6 @@ 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;
if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
if (!RTC || !RTC->defaultSynthProperties() ||
@@ -181,10 +186,8 @@ class RawPtrRefMemberChecker
return;
}
- bool ignoreARC =
- !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign;
- auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC);
- if (!IsUnsafePtr || !*IsUnsafePtr)
+ auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PD);
+ if (!IsUnsafe)
return;
if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl())
@@ -193,6 +196,46 @@ 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;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
index 54d661639da60..00e6e6ec1dcfa 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
@@ -86,6 +86,10 @@ @interface NoSynthObject : NSObject {
}
@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
@@ -93,4 +97,6 @@ - (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 7b31296378034..d0e489654e7b8 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
@@ -109,6 +109,8 @@ @interface NoSynthObject : NSObject {
}
@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
@@ -117,4 +119,8 @@ - (NSString *)prop_string1 {
}
@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
>From 469b56022db915b81449b4f0ca3121363c78de78 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 6 Jun 2025 13:40:41 -0600
Subject: [PATCH 3/3] Fix formatting
---
.../WebKit/RawPtrRefMemberChecker.cpp | 51 ++++++++++---------
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 8a31ab9a23c61..ccef7570e18b2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -197,43 +197,44 @@ class RawPtrRefMemberChecker
}
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);
+ 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 {
+ std::pair<bool, const Type *>
+ isPropImplUnsafePtr(const ObjCPropertyDecl *PD) const {
if (!PD)
- return { false, nullptr };
+ return {false, nullptr};
auto QT = PD->getType();
const Type *PropType = QT.getTypePtrOrNull();
if (!PropType)
- return { false, nullptr };
+ 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 };
+ return {IsUnsafePtr && *IsUnsafePtr, PropType};
}
bool shouldSkipDecl(const RecordDecl *RD) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
