Skip to content

Commit bf1d278

Browse files
authored
[WebKit checkers] Treat Objective-C message send return value as safe (#133605)
Objective-C selectors are supposed to return autoreleased object. Treat these return values as safe.
1 parent 19aec00 commit bf1d278

File tree

6 files changed

+52
-2
lines changed

6 files changed

+52
-2
lines changed

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp

+8
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,
@@ -233,6 +234,8 @@ class RawPtrRefCallArgsChecker
233234
return true;
234235
if (EFA.isACallToEnsureFn(ArgOrigin))
235236
return true;
237+
if (isSafeExpr(ArgOrigin))
238+
return true;
236239
return false;
237240
});
238241
}
@@ -469,6 +472,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
469472
return isRetainPtrType(type);
470473
}
471474

475+
bool isSafeExpr(const Expr *E) const final {
476+
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
477+
isa<ObjCMessageExpr>(E);
478+
}
479+
472480
const char *ptrKind() const final { return "unretained"; }
473481
};
474482

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ class RawPtrRefLocalVarsChecker
179179
virtual std::optional<bool> isUnsafePtr(const QualType T) const = 0;
180180
virtual bool isSafePtr(const CXXRecordDecl *) const = 0;
181181
virtual bool isSafePtrType(const QualType) const = 0;
182+
virtual bool isSafeExpr(const Expr *) const { return false; }
182183
virtual const char *ptrKind() const = 0;
183184

184185
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -300,6 +301,9 @@ class RawPtrRefLocalVarsChecker
300301
if (EFA.isACallToEnsureFn(InitArgOrigin))
301302
return true;
302303

304+
if (isSafeExpr(InitArgOrigin))
305+
return true;
306+
303307
if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
304308
if (auto *MaybeGuardian =
305309
dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
@@ -426,6 +430,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
426430
bool isSafePtrType(const QualType type) const final {
427431
return isRetainPtrType(type);
428432
}
433+
bool isSafeExpr(const Expr *E) const final {
434+
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
435+
isa<ObjCMessageExpr>(E);
436+
}
429437
const char *ptrKind() const final { return "unretained"; }
430438
};
431439

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

+5
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; }

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

+9
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

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

+9
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ void idcf(CFTypeRef obj) {
405405
@interface TestObject : NSObject
406406
- (void)doWork:(NSString *)msg, ...;
407407
- (void)doWorkOnSelf;
408+
- (SomeObj *)getSomeObj;
408409
@end
409410

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

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

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

+13-2
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)