diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index d372c5d1ba626..17650a3e34dcf 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(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(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(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(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()) + 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(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(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 retainsRet; + if (auto *FnDecl = dyn_cast(DeclWithIssue)) + retainsRet = retainsReturnValue(FnDecl); + else if (auto *MethodDecl = dyn_cast(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(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 + std::optional 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(E); if (auto *POE = dyn_cast(E)) { if (unsigned ExprCount = POE->getNumSemanticExprs()) { auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); ObjCMsgExpr = dyn_cast(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(Receiver)) { - auto InnerSelector = InnerObjCMsgExpr->getSelector(); + if (auto *Inner = dyn_cast(Receiver)) { + if (InnerExpr) + *InnerExpr = Inner; + auto InnerSelector = Inner->getSelector(); return InnerSelector.getNameForSlot(0) == "alloc"; } else if (auto *CE = dyn_cast(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(CalleeExpr)) return IsOwnedResult::Skip; // Wait for instantiation. + if (isa(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 + 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(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 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 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 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 cf_out_argument() { auto surface = adoptCF(IOSurfaceCreate(nullptr)); CVPixelBufferRef rawBuffer = nullptr; @@ -60,7 +125,46 @@ void cast_retainptr() { RetainPtr v = static_cast(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 getObject() { + CFTypeRef obj = nullptr; + if (GetObj(&obj)) + return nullptr; + return adoptCF(obj); +} + +CFArrayRef CreateSingleArray(CFStringRef); +CFArrayRef CreateSingleArray(CFDictionaryRef); +CFArrayRef CreateSingleArray(CFArrayRef); +template +static RetainPtr 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 foo = adoptNS([[SomeObj alloc] init]); @@ -118,3 +222,90 @@ void mutable_copy_array() { void string_copy(NSString *str) { RetainPtr 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 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 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(CFCopyArray(cf1))); +} + +-(void)basic_wrong { + RetainPtr 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 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 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 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 cf_out_argument() { auto surface = adoptCF(IOSurfaceCreate(nullptr)); CVPixelBufferRef rawBuffer = nullptr; @@ -62,7 +130,46 @@ void cast_retainptr() { RetainPtr v = static_cast(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 getObject() { + CFTypeRef obj = nullptr; + if (GetObj(&obj)) + return nullptr; + return adoptCF(obj); +} + +CFArrayRef CreateSingleArray(CFStringRef); +CFArrayRef CreateSingleArray(CFDictionaryRef); +CFArrayRef CreateSingleArray(CFArrayRef); +template +static RetainPtr 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 foo = adoptNS([[SomeObj alloc] init]); @@ -120,3 +227,98 @@ void mutable_copy_array() { void string_copy(NSString *str) { RetainPtr 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 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 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(CFCopyArray(cf1))); +} + +-(void)basic_wrong { + RetainPtr 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 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 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 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