[WebKit checkers] Treat the return value of an instance method as an unsafe pointer origin

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") &&

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<bool> IsUnsafe = isUnsafePtr(E->getReceiverType());
       if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) {
-        if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(Receiver)) {
+        if (auto *InnerMsg =
+                dyn_cast<ObjCMessageExpr>(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<ObjCMessageExpr>(E);
-  }
-
   bool isSafeDecl(const Decl *D) const final {
     // Treat NS/CF globals in system header as immortal.
     return BR->getSourceManager().isInSystemHeader(D->getLocation());

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<ObjCMessageExpr>(E);
-  }
   bool isSafeDecl(const Decl *D) const final {
     // Treat NS/CF globals in system header as immortal.
     return BR->getSourceManager().isInSystemHeader(D->getLocation());

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;

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];

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<CFArrayRef>(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<SomeObj> 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<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());
@@ -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]}}

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<SomeObj *>(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 {

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