Skip to content

Commit b31fae2

Browse files
rniwarniwa
committed
[alpha.webkit.NoUnretainedMemberChecker] Recognize NS_REQUIRES_PROPERTY_DEFINITIONS (llvm#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 3618534 commit b31fae2

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
@@ -30,6 +30,7 @@ class RawPtrRefMemberChecker
3030
private:
3131
BugType Bug;
3232
mutable BugReporter *BR;
33+
mutable llvm::DenseSet<const ObjCIvarDecl *> IvarDeclsToIgnore;
3334

3435
protected:
3536
mutable std::optional<RetainTypeChecker> RTC;
@@ -38,7 +39,8 @@ class RawPtrRefMemberChecker
3839
RawPtrRefMemberChecker(const char *description)
3940
: Bug(this, description, "WebKit coding guidelines") {}
4041

41-
virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
42+
virtual std::optional<bool> isUnsafePtr(QualType,
43+
bool ignoreARC = false) const = 0;
4244
virtual const char *typeName() const = 0;
4345
virtual const char *invariant() const = 0;
4446

@@ -141,6 +143,8 @@ class RawPtrRefMemberChecker
141143
return;
142144
}
143145
if (auto *ID = dyn_cast<ObjCImplementationDecl>(CD)) {
146+
for (auto *PropImpl : ID->property_impls())
147+
visitPropImpl(CD, PropImpl);
144148
for (auto *Ivar : ID->ivars())
145149
visitIvarDecl(CD, Ivar);
146150
return;
@@ -151,6 +155,10 @@ class RawPtrRefMemberChecker
151155
const ObjCIvarDecl *Ivar) const {
152156
if (BR->getSourceManager().isInSystemHeader(Ivar->getLocation()))
153157
return;
158+
159+
if (IvarDeclsToIgnore.contains(Ivar))
160+
return;
161+
154162
auto QT = Ivar->getType();
155163
const Type *IvarType = QT.getTypePtrOrNull();
156164
if (!IvarType)
@@ -160,6 +168,8 @@ class RawPtrRefMemberChecker
160168
if (!IsUnsafePtr || !*IsUnsafePtr)
161169
return;
162170

171+
IvarDeclsToIgnore.insert(Ivar);
172+
163173
if (auto *MemberCXXRD = IvarType->getPointeeCXXRecordDecl())
164174
reportBug(Ivar, IvarType, MemberCXXRD, CD);
165175
else if (auto *ObjCDecl = getObjCDecl(IvarType))
@@ -170,13 +180,15 @@ class RawPtrRefMemberChecker
170180
const ObjCPropertyDecl *PD) const {
171181
if (BR->getSourceManager().isInSystemHeader(PD->getLocation()))
172182
return;
173-
auto QT = PD->getType();
174-
const Type *PropType = QT.getTypePtrOrNull();
175-
if (!PropType)
176-
return;
177183

178-
auto IsUnsafePtr = isUnsafePtr(QT);
179-
if (!IsUnsafePtr || !*IsUnsafePtr)
184+
if (const ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(CD)) {
185+
if (!RTC || !RTC->defaultSynthProperties() ||
186+
ID->isObjCRequiresPropertyDefs())
187+
return;
188+
}
189+
190+
auto [IsUnsafe, PropType] = isPropImplUnsafePtr(PD);
191+
if (!IsUnsafe)
180192
return;
181193

182194
if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl())
@@ -185,6 +197,47 @@ class RawPtrRefMemberChecker
185197
reportBug(PD, PropType, ObjCDecl, CD);
186198
}
187199

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

278-
std::optional<bool> isUnsafePtr(QualType QT) const final {
331+
std::optional<bool> isUnsafePtr(QualType QT, bool) const final {
279332
return isUncountedPtr(QT.getCanonicalType());
280333
}
281334

@@ -292,7 +345,7 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
292345
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
293346
"checked-pointer capable type") {}
294347

295-
std::optional<bool> isUnsafePtr(QualType QT) const final {
348+
std::optional<bool> isUnsafePtr(QualType QT, bool) const final {
296349
return isUncheckedPtr(QT.getCanonicalType());
297350
}
298351

@@ -312,8 +365,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
312365
RTC = RetainTypeChecker();
313366
}
314367

315-
std::optional<bool> isUnsafePtr(QualType QT) const final {
316-
return RTC->isUnretained(QT);
368+
std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final {
369+
return RTC->isUnretained(QT, ignoreARC);
317370
}
318371

319372
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)