Skip to content

Commit fd32611

Browse files
rniwaRyosuke Niwa
authored andcommitted
[WebKit checkers] Treat Objective-C message send return value as safe (llvm#133605)
Objective-C selectors are supposed to return autoreleased object. Treat these return values as safe.
1 parent ea79d25 commit fd32611

File tree

6 files changed

+52
-2
lines changed

6 files changed

+52
-2
lines changed

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class RawPtrRefCallArgsChecker
4747
virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
4848
virtual bool isSafePtr(const CXXRecordDecl *Record) const = 0;
4949
virtual bool isSafePtrType(const QualType type) const = 0;
50+
virtual bool isSafeExpr(const Expr *) const { return false; }
5051
virtual const char *ptrKind() const = 0;
5152

5253
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -236,6 +237,8 @@ class RawPtrRefCallArgsChecker
236237
return true;
237238
if (EFA.isACallToEnsureFn(ArgOrigin))
238239
return true;
240+
if (isSafeExpr(ArgOrigin))
241+
return true;
239242
return false;
240243
});
241244
}
@@ -472,6 +475,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
472475
return isRetainPtrType(type);
473476
}
474477

478+
bool isSafeExpr(const Expr *E) const final {
479+
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
480+
isa<ObjCMessageExpr>(E);
481+
}
482+
475483
const char *ptrKind() const final { return "unretained"; }
476484
};
477485

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ class RawPtrRefLocalVarsChecker
182182
virtual std::optional<bool> isUnsafePtr(const QualType T) const = 0;
183183
virtual bool isSafePtr(const CXXRecordDecl *) const = 0;
184184
virtual bool isSafePtrType(const QualType) const = 0;
185+
virtual bool isSafeExpr(const Expr *) const { return false; }
185186
virtual const char *ptrKind() const = 0;
186187

187188
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -306,6 +307,9 @@ class RawPtrRefLocalVarsChecker
306307
if (EFA.isACallToEnsureFn(InitArgOrigin))
307308
return true;
308309

310+
if (isSafeExpr(InitArgOrigin))
311+
return true;
312+
309313
if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
310314
if (auto *MaybeGuardian =
311315
dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
@@ -432,6 +436,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
432436
bool isSafePtrType(const QualType type) const final {
433437
return isRetainPtrType(type);
434438
}
439+
bool isSafeExpr(const Expr *E) const final {
440+
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
441+
isa<ObjCMessageExpr>(E);
442+
}
435443
const char *ptrKind() const final { return "unretained"; }
436444
};
437445

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ __attribute__((objc_root_class))
7171
+ (Class) superclass;
7272
- (instancetype) init;
7373
- (instancetype)retain;
74+
- (instancetype)autorelease;
7475
- (void)release;
7576
- (BOOL)isKindOfClass:(Class)aClass;
7677
@end
@@ -221,6 +222,10 @@ template <typename T> struct RetainPtr {
221222
operator PtrType() const { return t; }
222223
operator bool() const { return t; }
223224

225+
#if !__has_feature(objc_arc)
226+
PtrType autorelease() { [[clang::suppress]] return [t autorelease]; }
227+
#endif
228+
224229
private:
225230
CFTypeRef toCFTypeRef(id ptr) { return (__bridge CFTypeRef)ptr; }
226231
CFTypeRef toCFTypeRef(const void* ptr) { return (CFTypeRef)ptr; }

clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ void foo() {
1818

1919
@interface AnotherObj : NSObject
2020
- (void)foo:(SomeObj *)obj;
21+
- (SomeObj *)getSomeObj;
2122
@end
2223

2324
@implementation AnotherObj
@@ -27,4 +28,12 @@ - (void)foo:(SomeObj*)obj {
2728
CFArrayAppendValue(provide_cf(), nullptr);
2829
// expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe [alpha.webkit.UnretainedCallArgsChecker]}}
2930
}
31+
32+
- (SomeObj *)getSomeObj {
33+
return provide();
34+
}
35+
36+
- (void)doWorkOnSomeObj {
37+
[[self getSomeObj] doWork];
38+
}
3039
@end

clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ void idcf(CFTypeRef obj) {
404404
@interface TestObject : NSObject
405405
- (void)doWork:(NSString *)msg, ...;
406406
- (void)doWorkOnSelf;
407+
- (SomeObj *)getSomeObj;
407408
@end
408409

409410
@implementation TestObject
@@ -420,4 +421,12 @@ - (void)doWorkOnSelf {
420421
[self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get()];
421422
}
422423

424+
- (SomeObj *)getSomeObj {
425+
return RetainPtr<SomeObj *>(provide()).autorelease();
426+
}
427+
428+
- (void)doWorkOnSomeObj {
429+
[[self getSomeObj] doWork];
430+
}
431+
423432
@end

clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ void bar(SomeObj *) {}
1414
} // namespace raw_ptr
1515

1616
namespace pointer {
17+
SomeObj *provide();
1718
void foo_ref() {
18-
SomeObj *bar = [[SomeObj alloc] init];
19+
SomeObj *bar = provide();
1920
// expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
2021
[bar doWork];
2122
}
@@ -387,6 +388,7 @@ unsigned ccf(CFTypeRef obj) {
387388
} // ptr_conversion
388389

389390
bool doMoreWorkOpaque(OtherObj*);
391+
SomeObj* provide();
390392

391393
@implementation OtherObj
392394
- (instancetype)init {
@@ -397,4 +399,13 @@ - (instancetype)init {
397399
- (void)doMoreWork:(OtherObj *)other {
398400
doMoreWorkOpaque(other);
399401
}
400-
@end
402+
403+
- (SomeObj*)getSomeObj {
404+
return RetainPtr<SomeObj *>(provide()).autorelease();
405+
}
406+
407+
- (void)storeSomeObj {
408+
auto *obj = [self getSomeObj];
409+
[obj doWork];
410+
}
411+
@end

0 commit comments

Comments
 (0)