[clang] [alpha.webkit.RetainPtrCtorAdoptChecker] An assortment of small enhancements (PR #135329)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 11 01:51:19 PDT 2025
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/135329
>From ffeb0f6f3252579fe77460ba05b57f7b68189bab Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 11 Apr 2025 01:15:55 -0700
Subject: [PATCH 1/2] [alpha.webkit.RetainPtrCtorAdoptChecker] An assortment of
small enhancements
This PR implements various small enhancements to alpha.webkit.RetainPtrCtorAdoptChecker:
- Detect leaks from [[X alloc] init] when ARC is disabled.
- Detect leaks from calling Create, Copy, and other +1 CF functions.
- Recognize [allocX() init] pattern where allocX is a C/C++ function.
- Recognize _init in addition to init as an init function.
- Recognize [[[X alloc] init] autorelease].
- Recognize CFBridgingRelease.
- Support CF_RETRUNS_RETAINED on out arguments of a C function.
- Support returning +1 object in Create, Copy, and other +1 functions or +1 selectors.
- Support variadic Create, Copy, and other +1 C/C++ functions.
To make these enhancements, this PR introduces new visit functions for ObjCMessageExpr,
ReturnStmt, VarDecl, and BinaryOperator. These functions look for a specific construct
mentioned above and adds an expression such as [[X alloc] init] or CreateX to a DenseSet
CreateOrCopyFnCall when the expression does not result in leaks. When the code to detect
leaks such as the one in visitObjCMessageExpr later encounters this expression, it can
bail out early if the expression is in the set.
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 275 ++++++++++++++++--
.../Checkers/WebKit/objc-mock-types.h | 10 +-
.../WebKit/retain-ptr-ctor-adopt-use-arc.mm | 195 ++++++++++++-
.../WebKit/retain-ptr-ctor-adopt-use.mm | 204 ++++++++++++-
4 files changed, 648 insertions(+), 36 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index d372c5d1ba626..348dc9752f8dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
#include "clang/Analysis/RetainSummaryManager.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
@@ -90,6 +91,26 @@ class RetainPtrCtorAdoptChecker
Checker->visitConstructExpr(CE, DeclWithIssue);
return true;
}
+
+ bool VisitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr) {
+ Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue);
+ return true;
+ }
+
+ bool VisitReturnStmt(const ReturnStmt *RS) {
+ Checker->visitReturnStmt(RS, DeclWithIssue);
+ return true;
+ }
+
+ bool VisitVarDecl(const VarDecl *VD) {
+ Checker->visitVarDecl(VD);
+ return true;
+ }
+
+ bool VisitBinaryOperator(const BinaryOperator *BO) {
+ Checker->visitBinaryOperator(BO);
+ return true;
+ }
};
LocalVisitor visitor(this);
@@ -101,13 +122,14 @@ class RetainPtrCtorAdoptChecker
}
bool isAdoptFn(const Decl *FnDecl) const {
- auto Name = safeGetName(FnDecl);
- return Name == "adoptNS" || Name == "adoptCF" || Name == "adoptNSArc" ||
- Name == "adoptCFArc";
+ return isAdoptFnName(safeGetName(FnDecl));
}
- bool isAdoptNS(const Decl *FnDecl) const {
- auto Name = safeGetName(FnDecl);
+ bool isAdoptFnName(const std::string& Name) const {
+ return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc";
+ }
+
+ bool isAdoptNS(const std::string& Name) const {
return Name == "adoptNS" || Name == "adoptNSArc";
}
@@ -116,44 +138,104 @@ class RetainPtrCtorAdoptChecker
if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
return;
- auto *F = CE->getDirectCallee();
- if (!F)
+ std::string FnName;
+ if (auto *F = CE->getDirectCallee()) {
+ FnName = safeGetName(F);
+ if (isAdoptFnName(FnName))
+ checkAdoptCall(CE, FnName, DeclWithIssue);
+ else {
+ checkCreateOrCopyFunction(CE, DeclWithIssue);
+ checkBridgingRelease(CE, F, DeclWithIssue);
+ }
return;
+ }
- if (!isAdoptFn(F) || !CE->getNumArgs()) {
- checkCreateOrCopyFunction(CE, F, DeclWithIssue);
+ auto *CalleeExpr = CE->getCallee();
+ if (!CalleeExpr)
return;
+ CalleeExpr = CalleeExpr->IgnoreParenCasts();
+ if (auto *UnresolvedExpr = dyn_cast<UnresolvedLookupExpr>(CalleeExpr)) {
+ auto Name = UnresolvedExpr->getName();
+ if (!Name.isIdentifier())
+ return;
+ FnName = Name.getAsString();
+ if (isAdoptFnName(FnName))
+ checkAdoptCall(CE, FnName, DeclWithIssue);
}
+ checkCreateOrCopyFunction(CE, DeclWithIssue);
+ }
+
+ void checkAdoptCall(const CallExpr *CE, const std::string& FnName,
+ const Decl *DeclWithIssue) const {
+ if (!CE->getNumArgs())
+ return;
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
auto Result = isOwned(Arg);
- auto Name = safeGetName(F);
if (Result == IsOwnedResult::Unknown)
Result = IsOwnedResult::NotOwned;
- if (isAllocInit(Arg) || isCreateOrCopy(Arg)) {
+
+ const Expr *Inner = nullptr;
+ if (isAllocInit(Arg, &Inner) || isCreateOrCopy(Arg)) {
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
return;
}
- if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip)
+ if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) {
+ CreateOrCopyFnCall.insert(Arg);
return;
+ }
if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
return;
}
- if (RTC.isARCEnabled() && isAdoptNS(F))
- reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+ if (RTC.isARCEnabled() && isAdoptFnName(FnName))
+ reportUseAfterFree(FnName, CE, DeclWithIssue, "when ARC is disabled");
else
- reportUseAfterFree(Name, CE, DeclWithIssue);
+ reportUseAfterFree(FnName, CE, DeclWithIssue);
}
- void checkCreateOrCopyFunction(const CallExpr *CE, const FunctionDecl *Callee,
- const Decl *DeclWithIssue) const {
- if (!isCreateOrCopyFunction(Callee))
+ void visitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr,
+ const Decl *DeclWithIssue) const {
+ if (BR->getSourceManager().isInSystemHeader(ObjCMsgExpr->getExprLoc()))
return;
- bool hasOutArgument = false;
+ auto Selector = ObjCMsgExpr->getSelector();
+ if (Selector.getAsString() == "autorelease") {
+ auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
+ if (!Receiver)
+ return;
+ ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver);
+ if (!ObjCMsgExpr)
+ return;
+ const Expr *Inner = nullptr;
+ if (!isAllocInit(ObjCMsgExpr, &Inner))
+ return;
+ CreateOrCopyFnCall.insert(ObjCMsgExpr);
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
+ return;
+ }
+
+ const Expr *Inner = nullptr;
+ if (!isAllocInit(ObjCMsgExpr, &Inner))
+ return;
+ if (RTC.isARCEnabled())
+ return; // ARC never leaks.
+ if (CreateOrCopyFnCall.contains(ObjCMsgExpr))
+ return;
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner); // Avoid double reporting.
+ reportLeak(ObjCMsgExpr, DeclWithIssue);
+ }
+
+ void checkCreateOrCopyFunction(const CallExpr *CE,
+ const Decl *DeclWithIssue) const {
unsigned ArgCount = CE->getNumArgs();
+ auto *CalleeDecl = CE->getCalleeDecl();
+ auto *FnDecl = CalleeDecl ? CalleeDecl->getAsFunction() : nullptr;
for (unsigned ArgIndex = 0; ArgIndex < ArgCount; ++ArgIndex) {
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
auto *Unary = dyn_cast<UnaryOperator>(Arg);
@@ -170,13 +252,45 @@ class RetainPtrCtorAdoptChecker
auto *Decl = DRE->getDecl();
if (!Decl)
continue;
- CreateOrCopyOutArguments.insert(Decl);
- hasOutArgument = true;
+ if (FnDecl && ArgIndex < FnDecl->getNumParams()) {
+ // Manually check attributes on argumenet since RetainSummaryManager
+ // basically ignores CF_RETRUNS_RETAINED on out arguments.
+ auto *ParamDecl = FnDecl->getParamDecl(ArgIndex);
+ if (ParamDecl->hasAttr<CFReturnsRetainedAttr>())
+ CreateOrCopyOutArguments.insert(Decl);
+ } else {
+ // No callee or a variadic argument. Conservatively assume it's an out argument.
+ if (RTC.isUnretained(Decl->getType()))
+ CreateOrCopyOutArguments.insert(Decl);
+ }
}
- if (!RTC.isUnretained(Callee->getReturnType()))
+ auto Summary = Summaries->getSummary(AnyCall(CE));
+ switch (Summary->getRetEffect().getKind()) {
+ case RetEffect::OwnedSymbol:
+ case RetEffect::OwnedWhenTrackedReceiver:
+ if (!CreateOrCopyFnCall.contains(CE))
+ reportLeak(CE, DeclWithIssue);
+ break;
+ default:
+ break;
+ }
+ }
+
+ void checkBridgingRelease(const CallExpr *CE, const FunctionDecl *Callee,
+ const Decl *DeclWithIssue) const {
+ if (safeGetName(Callee) != "CFBridgingRelease" || CE->getNumArgs() != 1)
+ return;
+
+ auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+ auto *InnerCE = dyn_cast<CallExpr>(Arg);
+ if (!InnerCE)
+ return;
+
+ auto *InnerF = InnerCE->getDirectCallee();
+ if (!InnerF || !isCreateOrCopyFunction(InnerF))
return;
- if (!hasOutArgument && !CreateOrCopyFnCall.contains(CE))
- reportLeak(CE, DeclWithIssue);
+
+ CreateOrCopyFnCall.insert(InnerCE);
}
void visitConstructExpr(const CXXConstructExpr *CE,
@@ -207,6 +321,13 @@ class RetainPtrCtorAdoptChecker
if (isCreateOrCopy(Arg))
CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
+ const Expr *Inner = nullptr;
+ if (isAllocInit(Arg, &Inner)) {
+ CreateOrCopyFnCall.insert(Arg);
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
+ }
+
if (Result == IsOwnedResult::Skip)
return;
@@ -219,13 +340,95 @@ class RetainPtrCtorAdoptChecker
else if (isCreateOrCopy(Arg))
reportLeak(Name, CE, DeclWithIssue);
}
+
+ void visitVarDecl(const VarDecl *VD) const {
+ auto *Init = VD->getInit();
+ if (!Init || !RTC.isARCEnabled())
+ return;
+ Init = Init->IgnoreParenCasts();
+ const Expr *Inner = nullptr;
+ if (isAllocInit(Init, &Inner)) {
+ CreateOrCopyFnCall.insert(Init);
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
+ }
+ }
- bool isAllocInit(const Expr *E) const {
+ void visitBinaryOperator(const BinaryOperator *BO) const {
+ if (!BO->isAssignmentOp())
+ return;
+ if (!isa<ObjCIvarRefExpr>(BO->getLHS()))
+ return;
+ auto *RHS = BO->getRHS()->IgnoreParenCasts();
+ const Expr *Inner = nullptr;
+ if (isAllocInit(RHS, &Inner)) {
+ CreateOrCopyFnCall.insert(RHS);
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
+ }
+ }
+
+ void visitReturnStmt(const ReturnStmt *RS, const Decl *DeclWithIssue) const {
+ if (!DeclWithIssue)
+ return;
+ auto *RetValue = RS->getRetValue();
+ if (!RetValue)
+ return;
+ RetValue = RetValue->IgnoreParenCasts();
+ std::optional<bool> retainsRet;
+ if (auto *FnDecl = dyn_cast<FunctionDecl>(DeclWithIssue))
+ retainsRet = retainsReturnValue(FnDecl);
+ else if (auto *MethodDecl = dyn_cast<ObjCMethodDecl>(DeclWithIssue))
+ retainsRet = retainsReturnValue(MethodDecl);
+ else
+ return;
+ if (!retainsRet || !*retainsRet) {
+ // Under ARC, returning [[X alloc] init] doesn't leak X.
+ if (RTC.isUnretained(RetValue->getType()))
+ return;
+ }
+ if (auto *CE = dyn_cast<CallExpr>(RetValue)) {
+ auto *Callee = CE->getDirectCallee();
+ if (!Callee || !isCreateOrCopyFunction(Callee))
+ return;
+ CreateOrCopyFnCall.insert(CE);
+ return;
+ }
+ const Expr *Inner = nullptr;
+ if (isAllocInit(RetValue, &Inner)) {
+ CreateOrCopyFnCall.insert(RetValue);
+ if (Inner)
+ CreateOrCopyFnCall.insert(Inner);
+ }
+ }
+
+ template <typename CallableType>
+ std::optional<bool> retainsReturnValue(const CallableType *FnDecl) const {
+ auto Summary = Summaries->getSummary(AnyCall(FnDecl));
+ auto RetEffect = Summary->getRetEffect();
+ switch (RetEffect.getKind()) {
+ case RetEffect::NoRet:
+ return std::nullopt;
+ case RetEffect::OwnedSymbol:
+ return true;
+ case RetEffect::NotOwnedSymbol:
+ return false;
+ case RetEffect::OwnedWhenTrackedReceiver:
+ return std::nullopt;
+ case RetEffect::NoRetHard:
+ return std::nullopt;
+ }
+ return std::nullopt;
+ }
+
+ bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const {
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
+ if (InnerExpr)
+ *InnerExpr = ObjCMsgExpr;
}
}
if (!ObjCMsgExpr)
@@ -235,17 +438,22 @@ class RetainPtrCtorAdoptChecker
if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
NameForFirstSlot.starts_with("mutableCopy"))
return true;
- if (!NameForFirstSlot.starts_with("init"))
+ if (!NameForFirstSlot.starts_with("init") &&
+ !NameForFirstSlot.starts_with("_init"))
return false;
if (!ObjCMsgExpr->isInstanceMessage())
return false;
auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
if (!Receiver)
return false;
- if (auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver)) {
- auto InnerSelector = InnerObjCMsgExpr->getSelector();
+ if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
+ if (InnerExpr)
+ *InnerExpr = Inner;
+ auto InnerSelector = Inner->getSelector();
return InnerSelector.getNameForSlot(0) == "alloc";
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
+ if (InnerExpr)
+ *InnerExpr = CE;
if (auto *Callee = CE->getDirectCallee()) {
auto CalleeName = Callee->getName();
return CalleeName.starts_with("alloc");
@@ -354,6 +562,8 @@ class RetainPtrCtorAdoptChecker
} else if (auto *CalleeExpr = CE->getCallee()) {
if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
return IsOwnedResult::Skip; // Wait for instantiation.
+ if (isa<UnresolvedLookupExpr>(CalleeExpr))
+ return IsOwnedResult::Skip; // Wait for instantiation.
}
auto Summary = Summaries->getSummary(AnyCall(CE));
auto RetEffect = Summary->getRetEffect();
@@ -375,7 +585,7 @@ class RetainPtrCtorAdoptChecker
return IsOwnedResult::Unknown;
}
- void reportUseAfterFree(std::string &Name, const CallExpr *CE,
+ void reportUseAfterFree(const std::string &Name, const CallExpr *CE,
const Decl *DeclWithIssue,
const char *condition = nullptr) const {
SmallString<100> Buf;
@@ -417,16 +627,17 @@ class RetainPtrCtorAdoptChecker
BR->emitReport(std::move(Report));
}
- void reportLeak(const CallExpr *CE, const Decl *DeclWithIssue) const {
+ template <typename ExprType>
+ void reportLeak(const ExprType *E, const Decl *DeclWithIssue) const {
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
Os << "The return value is +1 and results in a memory leak.";
- PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+ PathDiagnosticLocation BSLoc(E->getSourceRange().getBegin(),
BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
- Report->addRange(CE->getSourceRange());
+ Report->addRange(E->getSourceRange());
Report->setDeclWithIssue(DeclWithIssue);
BR->emitReport(std::move(Report));
}
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 3f075ca0a6e5b..7700400be72cc 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -60,7 +60,7 @@ typedef struct CF_BRIDGED_TYPE(id) __CVBuffer *CVBufferRef;
typedef CVBufferRef CVImageBufferRef;
typedef CVImageBufferRef CVPixelBufferRef;
typedef signed int CVReturn;
-CVReturn CVPixelBufferCreateWithIOSurface(CFAllocatorRef allocator, IOSurfaceRef surface, CFDictionaryRef pixelBufferAttributes, CVPixelBufferRef * pixelBufferOut);
+CVReturn CVPixelBufferCreateWithIOSurface(CFAllocatorRef allocator, IOSurfaceRef surface, CFDictionaryRef pixelBufferAttributes, CF_RETURNS_RETAINED CVPixelBufferRef * pixelBufferOut);
CFRunLoopRef CFRunLoopGetCurrent(void);
CFRunLoopRef CFRunLoopGetMain(void);
@@ -69,6 +69,12 @@ extern void CFRelease(CFTypeRef cf);
#define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr ""))
extern Class NSClassFromString(NSString *aClassName);
+#if __has_feature(objc_arc)
+id CFBridgingRelease(CFTypeRef X) {
+ return (__bridge_transfer id)X;
+}
+#endif
+
__attribute__((objc_root_class))
@interface NSObject
+ (instancetype) alloc;
@@ -129,11 +135,13 @@ __attribute__((objc_root_class))
@interface NSNumber : NSValue
- (char)charValue;
+- (int)intValue;
- (id)initWithInt:(int)value;
+ (NSNumber *)numberWithInt:(int)value;
@end
@interface SomeObj : NSObject
+- (instancetype)_init;
- (SomeObj *)mutableCopy;
- (SomeObj *)copyWithValue:(int)value;
- (void)doWork;
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
index 45db0506ae5f2..47203cbd27355 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
@@ -27,13 +27,78 @@ void basic_wrong() {
RetainPtr<CFMutableArrayRef> cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10);
// expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
- // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
// expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
CFCopyArray(cf1);
// expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
}
+void basic_correct_arc() {
+ auto *obj = [[SomeObj alloc] init];
+ [obj doWork];
+}
+
+ at implementation SomeObj {
+ NSNumber *_number;
+ SomeObj *_next;
+ SomeObj *_other;
+}
+
+- (instancetype)_init {
+ self = [super init];
+ _number = nil;
+ _next = nil;
+ _other = nil;
+ return self;
+}
+
+- (SomeObj *)mutableCopy {
+ auto *copy = [[SomeObj alloc] init];
+ [copy setValue:_number];
+ [copy setNext:_next];
+ [copy setOther:_other];
+ return copy;
+}
+
+- (SomeObj *)copyWithValue:(int)value {
+ auto *copy = [[SomeObj alloc] init];
+ [copy setValue:_number];
+ [copy setNext:_next];
+ [copy setOther:_other];
+ return copy;
+}
+
+- (void)doWork {
+ _number = [[NSNumber alloc] initWithInt:5];
+}
+
+- (SomeObj *)other {
+ return _other;
+}
+
+- (void)setOther:(SomeObj *)obj {
+ _other = obj;
+}
+
+- (SomeObj *)next {
+ return _next;
+}
+
+- (void)setNext:(SomeObj *)obj {
+ _next = obj;
+}
+
+- (int)value {
+ return [_number intValue];
+}
+
+- (void)setValue:(NSNumber *)value {
+ _number = value;
+}
+
+ at end;
+
RetainPtr<CVPixelBufferRef> cf_out_argument() {
auto surface = adoptCF(IOSurfaceCreate(nullptr));
CVPixelBufferRef rawBuffer = nullptr;
@@ -60,7 +125,46 @@ void cast_retainptr() {
RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
}
-SomeObj* allocSomeObj();
+CFTypeRef CopyWrapper() {
+ return CopyValueForSomething();
+}
+
+CFTypeRef LeakWrapper() {
+ return CopyValueForSomething();
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+NSArray *makeArray() NS_RETURNS_RETAINED {
+ return CFBridgingRelease(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+}
+
+extern Class (*getNSArrayClass)();
+NSArray *allocArrayInstance() NS_RETURNS_RETAINED {
+ return [[getNSArrayClass() alloc] init];
+}
+
+extern int (*GetObj)(CF_RETURNS_RETAINED CFTypeRef* objOut);
+RetainPtr<CFTypeRef> getObject() {
+ CFTypeRef obj = nullptr;
+ if (GetObj(&obj))
+ return nullptr;
+ return adoptCF(obj);
+}
+
+CFArrayRef CreateSingleArray(CFStringRef);
+CFArrayRef CreateSingleArray(CFDictionaryRef);
+CFArrayRef CreateSingleArray(CFArrayRef);
+template <typename ElementType>
+static RetainPtr<CFArrayRef> makeArrayWithSingleEntry(ElementType arg) {
+ return adoptCF(CreateSingleArray(arg));
+}
+
+void callMakeArayWithSingleEntry() {
+ auto dictionary = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, nullptr, nullptr, 0));
+ makeArrayWithSingleEntry(dictionary.get());
+}
+
+SomeObj* allocSomeObj() CF_RETURNS_RETAINED;
void adopt_retainptr() {
RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
@@ -118,3 +222,90 @@ void mutable_copy_array() {
void string_copy(NSString *str) {
RetainPtr<NSString> copy = adoptNS(str.copy);
}
+
+void alloc_init_spi() {
+ auto ptr = adoptNS([[SomeObj alloc] _init]);
+}
+
+void alloc_init_c_function() {
+ RetainPtr ptr = adoptNS([allocSomeObj() init]);
+}
+
+CFArrayRef make_array() CF_RETURNS_RETAINED;
+
+RetainPtr<CFArrayRef> adopt_make_array() {
+ return adoptCF(make_array());
+}
+
+ at interface SomeObject : NSObject
+-(void)basic_correct;
+-(void)basic_wrong;
+-(NSString *)leak_string;
+-(NSString *)make_string NS_RETURNS_RETAINED;
+ at property (nonatomic, readonly) SomeObj *obj;
+ at end
+
+ at implementation SomeObject
+-(void)basic_correct {
+ auto ns1 = adoptNS([SomeObj alloc]);
+ auto ns2 = adoptNS([[SomeObj alloc] init]);
+ RetainPtr<SomeObj> ns3 = [ns1.get() next];
+ auto ns4 = adoptNS([ns3 mutableCopy]);
+ auto ns5 = adoptNS([ns3 copyWithValue:3]);
+ auto ns6 = retainPtr([ns3 next]);
+ CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+ auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+ auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
+}
+
+-(void)basic_wrong {
+ RetainPtr<SomeObj> ns1 = [[SomeObj alloc] init];
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ auto ns2 = adoptNS([ns1.get() next]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
+ // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+-(NSString *)leak_string {
+ return [[NSString alloc] initWithUTF8String:"hello"];
+}
+
+-(NSString *)make_string {
+ return [[NSString alloc] initWithUTF8String:"hello"];
+}
+
+-(void)local_leak_string {
+ if ([[NSString alloc] initWithUTF8String:"hello"]) {
+ }
+}
+
+-(void)make_some_obj {
+ auto some_obj = adoptNS([allocSomeObj() init]);
+}
+
+-(void)alloc_init_bad_order {
+ auto *obj = [NSObject alloc];
+ auto ptr = adoptNS([obj init]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+-(void)alloc_init_good_order {
+ auto obj = adoptNS([NSObject alloc]);
+ (void)[obj init];
+}
+
+-(void)copy_assign_ivar {
+ _obj = [allocSomeObj() init];
+}
+
+-(void)do_more_work:(OtherObj *)otherObj {
+ [otherObj doMoreWork:[[OtherObj alloc] init]];
+}
+ at end
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
index c07f79c9f9650..83c87b14158b9 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -36,6 +36,74 @@ void basic_wrong() {
// expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
}
+void basic_correct_arc() {
+ auto *obj = [[SomeObj alloc] init];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ [obj doWork];
+}
+
+ at implementation SomeObj {
+ NSNumber *_number;
+ SomeObj *_next;
+ SomeObj *_other;
+}
+
+- (instancetype)_init {
+ self = [super init];
+ _number = nil;
+ _next = nil;
+ _other = nil;
+ return self;
+}
+
+- (SomeObj *)mutableCopy {
+ auto *copy = [[SomeObj alloc] init];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ [copy setValue:_number];
+ [copy setNext:_next];
+ [copy setOther:_other];
+ return copy;
+}
+
+- (SomeObj *)copyWithValue:(int)value {
+ auto *copy = [[SomeObj alloc] init];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ [copy setValue:_number];
+ [copy setNext:_next];
+ [copy setOther:_other];
+ return copy;
+}
+
+- (void)doWork {
+ _number = [[NSNumber alloc] initWithInt:5];
+}
+
+- (SomeObj *)other {
+ return _other;
+}
+
+- (void)setOther:(SomeObj *)obj {
+ _other = obj;
+}
+
+- (SomeObj *)next {
+ return _next;
+}
+
+- (void)setNext:(SomeObj *)obj {
+ _next = obj;
+}
+
+- (int)value {
+ return [_number intValue];
+}
+
+- (void)setValue:value {
+ _number = value;
+}
+
+ at end;
+
RetainPtr<CVPixelBufferRef> cf_out_argument() {
auto surface = adoptCF(IOSurfaceCreate(nullptr));
CVPixelBufferRef rawBuffer = nullptr;
@@ -62,7 +130,46 @@ void cast_retainptr() {
RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
}
-SomeObj* allocSomeObj();
+CFTypeRef CopyWrapper() {
+ return CopyValueForSomething();
+}
+
+CFTypeRef LeakWrapper() {
+ return CopyValueForSomething();
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+NSArray *makeArray() NS_RETURNS_RETAINED {
+ return (__bridge NSArray *)CFArrayCreateMutable(kCFAllocatorDefault, 10);
+}
+
+extern Class (*getNSArrayClass)();
+NSArray *allocArrayInstance() NS_RETURNS_RETAINED {
+ return [[getNSArrayClass() alloc] init];
+}
+
+extern int (*GetObj)(CF_RETURNS_RETAINED CFTypeRef* objOut);
+RetainPtr<CFTypeRef> getObject() {
+ CFTypeRef obj = nullptr;
+ if (GetObj(&obj))
+ return nullptr;
+ return adoptCF(obj);
+}
+
+CFArrayRef CreateSingleArray(CFStringRef);
+CFArrayRef CreateSingleArray(CFDictionaryRef);
+CFArrayRef CreateSingleArray(CFArrayRef);
+template <typename ElementType>
+static RetainPtr<CFArrayRef> makeArrayWithSingleEntry(ElementType arg) {
+ return adoptCF(CreateSingleArray(arg));
+}
+
+void callMakeArayWithSingleEntry() {
+ auto dictionary = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, nullptr, nullptr, 0));
+ makeArrayWithSingleEntry(dictionary.get());
+}
+
+SomeObj* allocSomeObj() CF_RETURNS_RETAINED;
void adopt_retainptr() {
RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
@@ -120,3 +227,98 @@ void mutable_copy_array() {
void string_copy(NSString *str) {
RetainPtr<NSString> copy = adoptNS(str.copy);
}
+
+void alloc_init_spi() {
+ auto ptr = adoptNS([[SomeObj alloc] _init]);
+}
+
+void alloc_init_c_function() {
+ RetainPtr ptr = adoptNS([allocSomeObj() init]);
+}
+
+void alloc_init_autorelease() {
+ [[[SomeObj alloc] init] autorelease];
+}
+
+CFArrayRef make_array() CF_RETURNS_RETAINED;
+
+RetainPtr<CFArrayRef> adopt_make_array() {
+ return adoptCF(make_array());
+}
+
+ at interface SomeObject : NSObject
+-(void)basic_correct;
+-(void)basic_wrong;
+-(NSString *)leak_string;
+-(NSString *)make_string NS_RETURNS_RETAINED;
+ at property (nonatomic, readonly) SomeObj *obj;
+ at end
+
+ at implementation SomeObject
+-(void)basic_correct {
+ auto ns1 = adoptNS([SomeObj alloc]);
+ auto ns2 = adoptNS([[SomeObj alloc] init]);
+ RetainPtr<SomeObj> ns3 = [ns1.get() next];
+ auto ns4 = adoptNS([ns3 mutableCopy]);
+ auto ns5 = adoptNS([ns3 copyWithValue:3]);
+ auto ns6 = retainPtr([ns3 next]);
+ CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+ auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+ auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
+}
+
+-(void)basic_wrong {
+ RetainPtr<SomeObj> ns1 = [[SomeObj alloc] init];
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ auto ns2 = adoptNS([ns1.get() next]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
+ // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+-(NSString *)leak_string {
+ return [[NSString alloc] initWithUTF8String:"hello"];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+-(NSString *)make_string {
+ return [[NSString alloc] initWithUTF8String:"hello"];
+}
+
+-(void)local_leak_string {
+ if ([[NSString alloc] initWithUTF8String:"hello"]) {
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ }
+}
+
+-(void)make_some_obj {
+ auto some_obj = adoptNS([allocSomeObj() init]);
+}
+
+-(void)alloc_init_bad_order {
+ auto *obj = [NSObject alloc];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ auto ptr = adoptNS([obj init]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+-(void)alloc_init_good_order {
+ auto obj = adoptNS([NSObject alloc]);
+ (void)[obj init];
+}
+
+-(void)copy_assign_ivar {
+ _obj = [allocSomeObj() init];
+}
+
+-(void)do_more_work:(OtherObj *)otherObj {
+ [otherObj doMoreWork:[[OtherObj alloc] init]];
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+ at end
>From 1a7cbf15ba01fd1a4f0bf12645ba07c1af7e735c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 11 Apr 2025 01:51:04 -0700
Subject: [PATCH 2/2] Fix formatting.
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 27 ++++++++++---------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 348dc9752f8dc..e2ba07606051e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -125,11 +125,11 @@ class RetainPtrCtorAdoptChecker
return isAdoptFnName(safeGetName(FnDecl));
}
- bool isAdoptFnName(const std::string& Name) const {
+ bool isAdoptFnName(const std::string &Name) const {
return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc";
}
- bool isAdoptNS(const std::string& Name) const {
+ bool isAdoptNS(const std::string &Name) const {
return Name == "adoptNS" || Name == "adoptNSArc";
}
@@ -165,7 +165,7 @@ class RetainPtrCtorAdoptChecker
checkCreateOrCopyFunction(CE, DeclWithIssue);
}
- void checkAdoptCall(const CallExpr *CE, const std::string& FnName,
+ void checkAdoptCall(const CallExpr *CE, const std::string &FnName,
const Decl *DeclWithIssue) const {
if (!CE->getNumArgs())
return;
@@ -259,20 +259,21 @@ class RetainPtrCtorAdoptChecker
if (ParamDecl->hasAttr<CFReturnsRetainedAttr>())
CreateOrCopyOutArguments.insert(Decl);
} else {
- // No callee or a variadic argument. Conservatively assume it's an out argument.
+ // No callee or a variadic argument.
+ // Conservatively assume it's an out argument.
if (RTC.isUnretained(Decl->getType()))
CreateOrCopyOutArguments.insert(Decl);
}
}
auto Summary = Summaries->getSummary(AnyCall(CE));
switch (Summary->getRetEffect().getKind()) {
- case RetEffect::OwnedSymbol:
- case RetEffect::OwnedWhenTrackedReceiver:
- if (!CreateOrCopyFnCall.contains(CE))
- reportLeak(CE, DeclWithIssue);
- break;
- default:
- break;
+ case RetEffect::OwnedSymbol:
+ case RetEffect::OwnedWhenTrackedReceiver:
+ if (!CreateOrCopyFnCall.contains(CE))
+ reportLeak(CE, DeclWithIssue);
+ break;
+ default:
+ break;
}
}
@@ -340,7 +341,7 @@ class RetainPtrCtorAdoptChecker
else if (isCreateOrCopy(Arg))
reportLeak(Name, CE, DeclWithIssue);
}
-
+
void visitVarDecl(const VarDecl *VD) const {
auto *Init = VD->getInit();
if (!Init || !RTC.isARCEnabled())
@@ -446,7 +447,7 @@ class RetainPtrCtorAdoptChecker
auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
if (!Receiver)
return false;
- if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
+ if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
if (InnerExpr)
*InnerExpr = Inner;
auto InnerSelector = Inner->getSelector();
More information about the cfe-commits
mailing list