Skip to content

[WebKit checkers] Treat Objective-C message send return value as safe #133605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class RawPtrRefCallArgsChecker
virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
virtual bool isSafePtr(const CXXRecordDecl *Record) const = 0;
virtual bool isSafePtrType(const QualType type) const = 0;
virtual bool isSafeExpr(const Expr *) const { return false; }
virtual const char *ptrKind() const = 0;

void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
Expand Down Expand Up @@ -233,6 +234,8 @@ class RawPtrRefCallArgsChecker
return true;
if (EFA.isACallToEnsureFn(ArgOrigin))
return true;
if (isSafeExpr(ArgOrigin))
return true;
return false;
});
}
Expand Down Expand Up @@ -469,6 +472,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
return isRetainPtrType(type);
}

bool isSafeExpr(const Expr *E) const final {
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
isa<ObjCMessageExpr>(E);
}

const char *ptrKind() const final { return "unretained"; }
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ class RawPtrRefLocalVarsChecker
virtual std::optional<bool> isUnsafePtr(const QualType T) const = 0;
virtual bool isSafePtr(const CXXRecordDecl *) const = 0;
virtual bool isSafePtrType(const QualType) const = 0;
virtual bool isSafeExpr(const Expr *) const { return false; }
virtual const char *ptrKind() const = 0;

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

if (isSafeExpr(InitArgOrigin))
return true;

if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
if (auto *MaybeGuardian =
dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
Expand Down Expand Up @@ -426,6 +430,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
bool isSafePtrType(const QualType type) const final {
return isRetainPtrType(type);
}
bool isSafeExpr(const Expr *E) const final {
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
isa<ObjCMessageExpr>(E);
}
const char *ptrKind() const final { return "unretained"; }
};

Expand Down
5 changes: 5 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ __attribute__((objc_root_class))
+ (Class) superclass;
- (instancetype) init;
- (instancetype)retain;
- (instancetype)autorelease;
- (void)release;
- (BOOL)isKindOfClass:(Class)aClass;
@end
Expand Down Expand Up @@ -221,6 +222,10 @@ template <typename T> struct RetainPtr {
operator PtrType() const { return t; }
operator bool() const { return t; }

#if !__has_feature(objc_arc)
PtrType autorelease() { [[clang::suppress]] return [t autorelease]; }
#endif

private:
CFTypeRef toCFTypeRef(id ptr) { return (__bridge CFTypeRef)ptr; }
CFTypeRef toCFTypeRef(const void* ptr) { return (CFTypeRef)ptr; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ void foo() {

@interface AnotherObj : NSObject
- (void)foo:(SomeObj *)obj;
- (SomeObj *)getSomeObj;
@end

@implementation AnotherObj
Expand All @@ -27,4 +28,12 @@ - (void)foo:(SomeObj*)obj {
CFArrayAppendValue(provide_cf(), nullptr);
// expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe [alpha.webkit.UnretainedCallArgsChecker]}}
}

- (SomeObj *)getSomeObj {
return provide();
}

- (void)doWorkOnSomeObj {
[[self getSomeObj] doWork];
}
@end
9 changes: 9 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ void idcf(CFTypeRef obj) {
@interface TestObject : NSObject
- (void)doWork:(NSString *)msg, ...;
- (void)doWorkOnSelf;
- (SomeObj *)getSomeObj;
@end

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

- (SomeObj *)getSomeObj {
return RetainPtr<SomeObj *>(provide()).autorelease();
}

- (void)doWorkOnSomeObj {
[[self getSomeObj] doWork];
}

@end
15 changes: 13 additions & 2 deletions clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ void bar(SomeObj *) {}
} // namespace raw_ptr

namespace pointer {
SomeObj *provide();
void foo_ref() {
SomeObj *bar = [[SomeObj alloc] init];
SomeObj *bar = provide();
// expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
[bar doWork];
}
Expand Down Expand Up @@ -387,6 +388,7 @@ unsigned ccf(CFTypeRef obj) {
} // ptr_conversion

bool doMoreWorkOpaque(OtherObj*);
SomeObj* provide();

@implementation OtherObj
- (instancetype)init {
Expand All @@ -397,4 +399,13 @@ - (instancetype)init {
- (void)doMoreWork:(OtherObj *)other {
doMoreWorkOpaque(other);
}
@end

- (SomeObj*)getSomeObj {
return RetainPtr<SomeObj *>(provide()).autorelease();
}

- (void)storeSomeObj {
auto *obj = [self getSomeObj];
[obj doWork];
}
@end