[WebKit checkers] Treat Objective-C message send return value as safe #133605
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Objective-C selectors are supposed to return autoreleased object. Treat these return values as safe.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesObjective-C selectors are supposed to return autoreleased object. Treat these return values as safe. Full diff: https://github.com/llvm/llvm-project/pull/133605.diff 6 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index ce8f0df697b06..13088920cfa19 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -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,
@@ -233,6 +234,8 @@ class RawPtrRefCallArgsChecker
return true;
if (EFA.isACallToEnsureFn(ArgOrigin))
return true;
+ if (isSafeExpr(ArgOrigin))
+ return true;
return false;
});
}
@@ -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"; }
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index d413e33a490c5..9975d1a91b681 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -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,
@@ -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())) {
@@ -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"; }
};
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index ef46a7c0a2925..059a203e3b0d1 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -71,6 +71,7 @@ __attribute__((objc_root_class))
+ (Class) superclass;
- (instancetype) init;
- (instancetype)retain;
+- (instancetype)autorelease;
- (void)release;
- (BOOL)isKindOfClass:(Class)aClass;
@end
@@ -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; }
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
index eb4735da60a05..f1f4d912663aa 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
@@ -18,6 +18,7 @@ void foo() {
@interface AnotherObj : NSObject
- (void)foo:(SomeObj *)obj;
+- (SomeObj *)getSomeObj;
@end
@implementation AnotherObj
@@ -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
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index 55e795ee9a598..dd21864300387 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -405,6 +405,7 @@ void idcf(CFTypeRef obj) {
@interface TestObject : NSObject
- (void)doWork:(NSString *)msg, ...;
- (void)doWorkOnSelf;
+- (SomeObj *)getSomeObj;
@end
@implementation TestObject
@@ -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
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
index 0a3d9e54fa024..a71a80ea3d647 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
@@ -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];
}
@@ -387,6 +388,7 @@ unsigned ccf(CFTypeRef obj) {
} // ptr_conversion
bool doMoreWorkOpaque(OtherObj*);
+SomeObj* provide();
@implementation OtherObj
- (instancetype)init {
@@ -397,4 +399,13 @@ - (instancetype)init {
- (void)doMoreWork:(OtherObj *)other {
doMoreWorkOpaque(other);
}
-@end
\ No newline at end of file
+
+- (SomeObj*)getSomeObj {
+ return RetainPtr<SomeObj *>(provide()).autorelease();
+}
+
+- (void)storeSomeObj {
+ auto *obj = [self getSomeObj];
+ [obj doWork];
+}
+@end
|
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesObjective-C selectors are supposed to return autoreleased object. Treat these return values as safe. Full diff: https://github.com/llvm/llvm-project/pull/133605.diff 6 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index ce8f0df697b06..13088920cfa19 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -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,
@@ -233,6 +234,8 @@ class RawPtrRefCallArgsChecker
return true;
if (EFA.isACallToEnsureFn(ArgOrigin))
return true;
+ if (isSafeExpr(ArgOrigin))
+ return true;
return false;
});
}
@@ -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"; }
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index d413e33a490c5..9975d1a91b681 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -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,
@@ -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())) {
@@ -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"; }
};
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index ef46a7c0a2925..059a203e3b0d1 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -71,6 +71,7 @@ __attribute__((objc_root_class))
+ (Class) superclass;
- (instancetype) init;
- (instancetype)retain;
+- (instancetype)autorelease;
- (void)release;
- (BOOL)isKindOfClass:(Class)aClass;
@end
@@ -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; }
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
index eb4735da60a05..f1f4d912663aa 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
@@ -18,6 +18,7 @@ void foo() {
@interface AnotherObj : NSObject
- (void)foo:(SomeObj *)obj;
+- (SomeObj *)getSomeObj;
@end
@implementation AnotherObj
@@ -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
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index 55e795ee9a598..dd21864300387 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -405,6 +405,7 @@ void idcf(CFTypeRef obj) {
@interface TestObject : NSObject
- (void)doWork:(NSString *)msg, ...;
- (void)doWorkOnSelf;
+- (SomeObj *)getSomeObj;
@end
@implementation TestObject
@@ -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
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
index 0a3d9e54fa024..a71a80ea3d647 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
@@ -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];
}
@@ -387,6 +388,7 @@ unsigned ccf(CFTypeRef obj) {
} // ptr_conversion
bool doMoreWorkOpaque(OtherObj*);
+SomeObj* provide();
@implementation OtherObj
- (instancetype)init {
@@ -397,4 +399,13 @@ - (instancetype)init {
- (void)doMoreWork:(OtherObj *)other {
doMoreWorkOpaque(other);
}
-@end
\ No newline at end of file
+
+- (SomeObj*)getSomeObj {
+ return RetainPtr<SomeObj *>(provide()).autorelease();
+}
+
+- (void)storeSomeObj {
+ auto *obj = [self getSomeObj];
+ [obj doWork];
+}
+@end
|
|
Thanks for the review! |
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Apr 5, 2025
…llvm#133605) Objective-C selectors are supposed to return autoreleased object. Treat these return values as safe.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective-C selectors are supposed to return autoreleased object. Treat these return values as safe.