Author: Ryosuke Niwa Date: 2025-04-27T14:24:32-07:00 New Revision: 16e5c3ddadaa8d25698a71c0308b1d0ca887d11f
URL: https://github.com/llvm/llvm-project/commit/16e5c3ddadaa8d25698a71c0308b1d0ca887d11f DIFF: https://github.com/llvm/llvm-project/commit/16e5c3ddadaa8d25698a71c0308b1d0ca887d11f.diff LOG: [alpha.webkit.RetainPtrCtorAdoptChecker] An assortment of small enhancements (#135329) 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. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp clang/test/Analysis/Checkers/WebKit/objc-mock-types.h clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index d3eee11311d91..bba1d48e6db11 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; + + 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); + } - bool hasOutArgument = false; + 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,46 @@ 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 +322,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; @@ -220,12 +342,94 @@ class RetainPtrCtorAdoptChecker reportLeak(Name, CE, DeclWithIssue); } - bool isAllocInit(const Expr *E) const { + 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); + } + } + + 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,20 +439,27 @@ 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"); + if (Callee->getDeclName().isIdentifier()) { + auto CalleeName = Callee->getName(); + return CalleeName.starts_with("alloc"); + } } } return false; @@ -354,6 +565,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 +588,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 +630,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 a4332df682060..93e7dfd77b9e9 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -61,7 +61,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); @@ -70,6 +70,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; @@ -130,11 +136,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@-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@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} + // expected-warning@-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@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} CFCopyArray(cf1); // expected-warning@-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]; +} + +@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; +} + +@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@-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()); +} + +@interface SomeObject : NSObject +-(void)basic_correct; +-(void)basic_wrong; +-(NSString *)leak_string; +-(NSString *)make_string NS_RETURNS_RETAINED; +@property (nonatomic, readonly) SomeObj *obj; +@end + +@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@-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@-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@-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@-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@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-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@-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]]; +} +@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@-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@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + [obj doWork]; +} + +@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@-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@-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; +} + +@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@-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()); +} + +@interface SomeObject : NSObject +-(void)basic_correct; +-(void)basic_wrong; +-(NSString *)leak_string; +-(NSString *)make_string NS_RETURNS_RETAINED; +@property (nonatomic, readonly) SomeObj *obj; +@end + +@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@-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@-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@-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@-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@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-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@-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@-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@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ptr = adoptNS([obj init]); + // expected-warning@-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@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} +@end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
