diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index c1a5000f8b647..73956711c4068 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -196,6 +196,8 @@ bool tryToFindPtrOrigin( if (isSafePtrType(Method->getReturnType())) return callback(E, true); } + if (ObjCMsgExpr->isClassMessage()) + return callback(E, true); auto Selector = ObjCMsgExpr->getSelector(); auto NameForFirstSlot = Selector.getNameForSlot(0); if ((NameForFirstSlot == "class" || NameForFirstSlot == "superclass") && diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 791e70998477f..5be1742913776 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -178,13 +178,20 @@ class RawPtrRefCallArgsChecker return; auto Selector = E->getSelector(); + auto SelName = Selector.getNameForSlot(0); + bool IsSafeSel = SelName.starts_with("copy") || SelName.contains("Copy") || + SelName == "isEqual" || SelName == "isEqualToString"; + if (Selector.getNumArgs() <= 1 && IsSafeSel) + return; // These selectors are assumed to be readonly. + if (auto *Receiver = E->getInstanceReceiver()) { std::optional IsUnsafe = isUnsafePtr(E->getReceiverType()); if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) { - if (auto *InnerMsg = dyn_cast(Receiver)) { + if (auto *InnerMsg = + dyn_cast(Receiver->IgnoreParenCasts())) { auto InnerSelector = InnerMsg->getSelector(); - if (InnerSelector.getNameForSlot(0) == "alloc" && - Selector.getNameForSlot(0).starts_with("init")) + if (InnerSelector.getNameForSlot(0).starts_with("alloc") && + (SelName.starts_with("init") || SelName.starts_with("_init"))) return; } reportBugOnReceiver(Receiver, D); @@ -476,11 +483,6 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker { return isRetainPtrOrOSPtrType(type); } - bool isSafeExpr(const Expr *E) const final { - return ento::cocoa::isCocoaObjectRef(E->getType()) && - isa(E); - } - bool isSafeDecl(const Decl *D) const final { // Treat NS/CF globals in system header as immortal. return BR->getSourceManager().isInSystemHeader(D->getLocation()); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index c13df47920f72..c950dc91088b9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -441,10 +441,6 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { bool isSafePtrType(const QualType type) const final { return isRetainPtrOrOSPtrType(type); } - bool isSafeExpr(const Expr *E) const final { - return ento::cocoa::isCocoaObjectRef(E->getType()) && - isa(E); - } bool isSafeDecl(const Decl *D) const final { // Treat NS/CF globals in system header as immortal. return BR->getSourceManager().isInSystemHeader(D->getLocation()); diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index a5fc3d7f9a932..1fcfe6e06f721 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -160,6 +160,8 @@ __attribute__((objc_root_class)) - ( const char *)UTF8String; - (id)initWithUTF8String:(const char *)nullTerminatedCString; - (NSString *)copy; +- (NSString *)mutableCopy; +- (BOOL)isEqualToString:(NSString *)aString; + (id)stringWithUTF8String:(const char *)nullTerminatedCString; @end @@ -195,8 +197,10 @@ extern NSApplication * NSApp; @end @interface SomeObj : NSObject ++ (SomeObj *)sharedInstance; - (instancetype)_init; - (SomeObj *)mutableCopy; +- (BOOL)isEqual:(SomeObj *)other; - (SomeObj *)copyWithValue:(int)value; - (void)doWork; - (SomeObj *)other; 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 47203cbd27355..18100168fd6eb 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 @@ -45,6 +45,10 @@ @implementation SomeObj { SomeObj *_other; } ++ (SomeObj *)sharedInstance { + return nil; +} + - (instancetype)_init { self = [super init]; _number = nil; @@ -61,6 +65,10 @@ - (SomeObj *)mutableCopy { return copy; } +- (BOOL)isEqual:(SomeObj *)other { + return self.value == other.value && self.next == other.next && _other == other.other; +} + - (SomeObj *)copyWithValue:(int)value { auto *copy = [[SomeObj alloc] init]; [copy setValue:_number]; 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 45705615f3196..06a308f4e4cbc 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 @@ -15,6 +15,7 @@ void basic_correct() { auto ns6 = retainPtr([ns3 next]); auto ns7 = retainPtr((SomeObj *)0); auto ns8 = adoptNS(nil); + auto ns9 = adoptNS([[SomeObj allocWithZone:nullptr] _init]); CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault)); auto cf3 = adoptCF(checked_cf_cast(CFCopyArray(cf1))); @@ -28,6 +29,8 @@ void basic_wrong() { // 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 ns3 = [[SomeObj allocWithZone:nullptr] init]; + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [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()); @@ -50,6 +53,10 @@ @implementation SomeObj { SomeObj *_other; } ++ (SomeObj *)sharedInstance { + return nil; +} + - (instancetype)_init { self = [super init]; _number = nil; @@ -67,6 +74,10 @@ - (SomeObj *)mutableCopy { return copy; } +- (BOOL)isEqual:(SomeObj *)other { + return self.value == other.value && self.next == other.next && _other == other.other; +} + - (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]}} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 5dc3b38ccb61c..d6c21e3c0c9b8 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -6,6 +6,8 @@ SomeObj *provide(); void consume_obj(SomeObj*); +NSString *provide_str(); + CFMutableArrayRef provide_cf(); void consume_cf(CFMutableArrayRef); @@ -445,6 +447,15 @@ void foo() { void foo() { auto obj = adoptNS([[SomeObj alloc] init]); [obj doWork]; + auto obj2 = adoptNS([[SomeObj alloc] _init]); + [obj2 doWork]; + } + + void bar(NSZone *zone) { + auto obj = adoptNS([[SomeObj allocWithZone:zone] init]); + [obj doWork]; + auto obj2 = adoptNS([(SomeObj *)[SomeObj allocWithZone:zone] _init]); + [obj2 doWork]; } } @@ -582,6 +593,7 @@ @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; - (SomeObj *)getSomeObj; ++ (SomeObj *)sharedObj; @end @implementation TestObject @@ -600,14 +612,25 @@ - (void)doWorkOnSelf { [self doWork:__null]; [self doWork:nil]; [NSApp run]; + [provide() isEqual:provide()]; + [provide_str() isEqualToString:@"foo"]; + [provide_str() copyWithZone:nullptr]; + [provide_str() mutableCopy]; } - (SomeObj *)getSomeObj { return RetainPtr(provide()).autorelease(); } ++ (SomeObj *)sharedObj +{ + return adoptNS([[SomeObj alloc] init]).autorelease(); +} + - (void)doWorkOnSomeObj { [[self getSomeObj] doWork]; + // expected-warning@-1{{Receiver is unretained and unsafe}} + [[TestObject sharedObj] doWork]; } - (CGImageRef)createImage { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index f49e7bdb3e79c..2b4a388f344a2 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -592,6 +592,9 @@ - (SomeObj*)getSomeObj { - (void)storeSomeObj { auto *obj = [self getSomeObj]; + // expected-warning@-1{{Local variable 'obj' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} [obj doWork]; + auto *obj2 = [SomeObj sharedInstance]; + [obj2 doWork]; } @end