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

In bf1d278, we started treating the return value of any selector invocation as safe.
This isn't quite right since not every return value is autorelease'd. So start treating
these as unsafe again for messages sent to an instance but keep treating safe for messages
sent to a class since those should always be autorelease'd.

This PR also moves the check from isSafeExpr in local variable and call arguments checkers
to tryToFindPtrOrigin so that this semantic change could be more easily implemented. This
PR also fixes RawPtrRefCallArgsChecker so that it recognizes alloc-init pattern even if
allocWithZone or _init instead of alloc and init was used.

Author: rniwa

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,21 @@ 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"))
+          auto SelName = Selector.getNameForSlot(0);
+          if (InnerSelector.getNameForSlot(0).starts_with("alloc") &&
+              (SelName.starts_with("init") || SelName.starts_with("_init")))
             return;
         }
         reportBugOnReceiver(Receiver, D);
@@ -476,11 +484,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

@@ -195,6 +195,7 @@ extern NSApplication * NSApp;
 @end
 
 @interface SomeObj : NSObject
++ (SomeObj *)sharedInstance;
 - (instancetype)_init;
 - (SomeObj *)mutableCopy;
 - (SomeObj *)copyWithValue:(int)value;

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;

clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm

@@ -445,6 +445,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 +591,7 @@ @interface TestObject : NSObject
 - (void)doWork:(NSString *)msg, ...;
 - (void)doWorkOnSelf;
 - (SomeObj *)getSomeObj;
++ (SomeObj *)sharedObj;
 @end
 
 @implementation TestObject
@@ -606,8 +616,15 @@ - (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