Skip to content

Commit 0123ee5

Browse files
rniwarniwa
authored
[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.
1 parent 040e9e0 commit 0123ee5

File tree

6 files changed

+129
-11
lines changed

6 files changed

+129
-11
lines changed

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ std::optional<bool> isUnchecked(const QualType T) {
236236
void RetainTypeChecker::visitTranslationUnitDecl(
237237
const TranslationUnitDecl *TUD) {
238238
IsARCEnabled = TUD->getLangOpts().ObjCAutoRefCount;
239+
DefaultSynthProperties = TUD->getLangOpts().ObjCDefaultSynthProperties;
239240
}
240241

241242
void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,14 @@ class RetainTypeChecker {
7676
llvm::DenseSet<const RecordType *> CFPointees;
7777
llvm::DenseSet<const Type *> RecordlessTypes;
7878
bool IsARCEnabled{false};
79+
bool DefaultSynthProperties{true};
7980

8081
public:
8182
void visitTranslationUnitDecl(const TranslationUnitDecl *);
8283
void visitTypedef(const TypedefDecl *);
8384
bool isUnretained(const QualType, bool ignoreARC = false);
8485
bool isARCEnabled() const { return IsARCEnabled; }
86+
bool defaultSynthProperties() const { return DefaultSynthProperties; }
8587
};
8688

8789
/// \returns true if \p Class is NS or CF objects AND not retained, false if

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

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class RawPtrRefMemberChecker
2828
private:
2929
BugType Bug;
3030
mutable BugReporter *BR;
31+
mutable llvm::DenseSet<const ObjCIvarDecl *> IvarDeclsToIgnore;
3132

3233
protected:
3334
mutable std::optional<RetainTypeChecker> RTC;
@@ -36,7 +37,8 @@ class RawPtrRefMemberChecker
3637
RawPtrRefMemberChecker(const char *description)
3738
: Bug(this, description, "WebKit coding guidelines") {}
3839

39-
virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
40+
virtual std::optional<bool> isUnsafePtr(QualType,
41+
bool ignoreARC = false) const = 0;
4042
virtual const char *typeName() const = 0;
4143
virtual const char *invariant() const = 0;
4244

@@ -138,6 +140,8 @@ class RawPtrRefMemberChecker
138140
return;
139141
}
140142
if (auto *ID = dyn_cast<ObjCImplementationDecl>(CD)) {
143+
for (auto *PropImpl : ID->property_impls())
144+
visitPropImpl(CD, PropImpl);
141145
for (auto *Ivar : ID->ivars())
142146
visitIvarDecl(CD, Ivar);
143147
return;
@@ -148,6 +152,10 @@ class RawPtrRefMemberChecker
148152
const ObjCIvarDecl *Ivar) const {
149153
if (BR->getSourceManager().isInSystemHeader(Ivar->getLocation()))
150154
return;
155+
156+
if (IvarDeclsToIgnore.contains(Ivar))
157+
return;
158+
151159
auto QT = Ivar->getType();
152160
const Type *IvarType = QT.getTypePtrOrNull();
153161
if (!IvarType)
@@ -157,6 +165,8 @@ class RawPtrRefMemberChecker
157165
if (!IsUnsafePtr || !*IsUnsafePtr)
158166
return;
159167

168+
IvarDeclsToIgnore.insert(Ivar);
169+
160170
if (auto *MemberCXXRD = IvarType->getPointeeCXXRecordDecl())
161171
reportBug(Ivar, IvarType, MemberCXXRD, CD);
162172
else if (auto *ObjCDecl = getObjCDecl(IvarType))
@@ -167,13 +177,15 @@ class RawPtrRefMemberChecker
167177
const ObjCPropertyDecl *PD) const {
168178
if (BR->getSourceManager().isInSystemHeader(PD->getLocation()))
169179
return;
170-
auto QT = PD->getType();
171-
const Type *PropType = QT.getTypePtrOrNull();
172-
if (!PropType)
173-
return;
174180

175-
auto IsUnsafePtr = isUnsafePtr(QT);
176-
if (!IsUnsafePtr || !*IsUnsafePtr)
181+
if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
182+
if (!RTC || !RTC->defaultSynthProperties() ||
183+
ID->isObjCRequiresPropertyDefs())
184+
return;
185+
}
186+
187+
auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PD);
188+
if (!IsUnsafe)
177189
return;
178190

179191
if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl())
@@ -182,6 +194,47 @@ class RawPtrRefMemberChecker
182194
reportBug(PD, PropType, ObjCDecl, CD);
183195
}
184196

197+
void visitPropImpl(const ObjCContainerDecl *CD,
198+
const ObjCPropertyImplDecl *PID) const {
199+
if (BR->getSourceManager().isInSystemHeader(PID->getLocation()))
200+
return;
201+
202+
if (PID->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize)
203+
return;
204+
205+
auto *PropDecl = PID->getPropertyDecl();
206+
if (auto *IvarDecl = PID->getPropertyIvarDecl()) {
207+
if (IvarDeclsToIgnore.contains(IvarDecl))
208+
return;
209+
IvarDeclsToIgnore.insert(IvarDecl);
210+
}
211+
auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PropDecl);
212+
if (!IsUnsafe)
213+
return;
214+
215+
if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl())
216+
reportBug(PropDecl, PropType, MemberCXXRD, CD);
217+
else if (auto *ObjCDecl = getObjCDecl(PropType))
218+
reportBug(PropDecl, PropType, ObjCDecl, CD);
219+
}
220+
221+
std::pair<bool, const Type *>
222+
isPropImplUnsafePtr(const ObjCPropertyDecl *PD) const {
223+
if (!PD)
224+
return {false, nullptr};
225+
226+
auto QT = PD->getType();
227+
const Type *PropType = QT.getTypePtrOrNull();
228+
if (!PropType)
229+
return {false, nullptr};
230+
231+
// "assign" property doesn't retain even under ARC so treat it as unsafe.
232+
bool ignoreARC =
233+
!PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign;
234+
auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC);
235+
return {IsUnsafePtr && *IsUnsafePtr, PropType};
236+
}
237+
185238
bool shouldSkipDecl(const RecordDecl *RD) const {
186239
if (!RD->isThisDeclarationADefinition())
187240
return true;
@@ -272,7 +325,7 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
272325
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
273326
"reference-countable type") {}
274327

275-
std::optional<bool> isUnsafePtr(QualType QT) const final {
328+
std::optional<bool> isUnsafePtr(QualType QT, bool) const final {
276329
return isUncountedPtr(QT.getCanonicalType());
277330
}
278331

@@ -289,7 +342,7 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
289342
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
290343
"checked-pointer capable type") {}
291344

292-
std::optional<bool> isUnsafePtr(QualType QT) const final {
345+
std::optional<bool> isUnsafePtr(QualType QT, bool) const final {
293346
return isUncheckedPtr(QT.getCanonicalType());
294347
}
295348

@@ -309,8 +362,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
309362
RTC = RetainTypeChecker();
310363
}
311364

312-
std::optional<bool> isUnsafePtr(QualType QT) const final {
313-
return RTC->isUnretained(QT);
365+
std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final {
366+
return RTC->isUnretained(QT, ignoreARC);
314367
}
315368

316369
const char *typeName() const final { return "retainable type"; }

‎clang/test/Analysis/Checkers/WebKit/objc-mock-types.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef;
2222
#define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
2323
#define CF_CONSUMED __attribute__((cf_consumed))
2424
#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
25+
#define NS_REQUIRES_PROPERTY_DEFINITIONS __attribute__((objc_requires_property_definitions))
2526

2627
extern const CFAllocatorRef kCFAllocatorDefault;
2728
typedef struct _NSZone NSZone;

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,39 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
6464
};
6565

6666
} // namespace ptr_to_ptr_to_retained
67+
68+
@interface AnotherObject : NSObject {
69+
NSString *ns_string;
70+
CFStringRef cf_string;
71+
// expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
72+
}
73+
@property(nonatomic, strong) NSString *prop_string1;
74+
@property(nonatomic, assign) NSString *prop_string2;
75+
// expected-warning@-1{{Property 'prop_string2' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
76+
@property(nonatomic, unsafe_unretained) NSString *prop_string3;
77+
// expected-warning@-1{{Property 'prop_string3' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
78+
@property(nonatomic, readonly) NSString *prop_string4;
79+
@end
80+
81+
NS_REQUIRES_PROPERTY_DEFINITIONS
82+
@interface NoSynthObject : NSObject {
83+
NSString *ns_string;
84+
CFStringRef cf_string;
85+
// expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
86+
}
87+
@property(nonatomic, readonly, strong) NSString *prop_string1;
88+
@property(nonatomic, readonly, strong) NSString *prop_string2;
89+
@property(nonatomic, assign) NSString *prop_string3;
90+
// expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
91+
@property(nonatomic, unsafe_unretained) NSString *prop_string4;
92+
// expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
93+
@end
94+
95+
@implementation NoSynthObject
96+
- (NSString *)prop_string1 {
97+
return nil;
98+
}
99+
@synthesize prop_string2;
100+
@synthesize prop_string3;
101+
@synthesize prop_string4;
102+
@end

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,28 @@ @interface AnotherObject : NSObject {
9999
@property(nonatomic, strong) NSString *prop_string;
100100
// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
101101
@end
102+
103+
NS_REQUIRES_PROPERTY_DEFINITIONS
104+
@interface NoSynthObject : NSObject {
105+
NSString *ns_string;
106+
// expected-warning@-1{{Instance variable 'ns_string' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
107+
CFStringRef cf_string;
108+
// expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}}
109+
}
110+
@property(nonatomic, readonly, strong) NSString *prop_string1;
111+
@property(nonatomic, readonly, strong) NSString *prop_string2;
112+
// expected-warning@-1{{Property 'prop_string2' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}}
113+
@property(nonatomic, assign) NSString *prop_string3;
114+
// expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
115+
@property(nonatomic, unsafe_unretained) NSString *prop_string4;
116+
// expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
117+
@end
118+
119+
@implementation NoSynthObject
120+
- (NSString *)prop_string1 {
121+
return nil;
122+
}
123+
@synthesize prop_string2;
124+
@synthesize prop_string3;
125+
@synthesize prop_string4;
126+
@end

0 commit comments

Comments
 (0)