[alpha.webkit.UnretainedLambdaCapturesChecker] Add the support for protectedSelf

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

@@ -147,7 +147,7 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
 bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
   const std::string &FunctionName = safeGetName(F);
   return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
-         FunctionName == "adoptCF";
+         FunctionName == "adoptCF" || FunctionName == "retainPtr";
 }
 
 bool isCtorOfSafePtr(const clang::FunctionDecl *F) {

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp

@@ -36,6 +36,7 @@ class RawPtrRefLambdaCapturesChecker
       : Bug(this, description, "WebKit coding guidelines") {}
 
   virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
+  virtual bool isPtrType(const std::string &) const = 0;
   virtual const char *ptrKind(QualType QT) const = 0;
 
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -68,6 +69,15 @@ class RawPtrRefLambdaCapturesChecker
         return DynamicRecursiveASTVisitor::TraverseCXXMethodDecl(CXXMD);
       }
 
+      bool TraverseObjCMethodDecl(ObjCMethodDecl *OCMD) override {
+        llvm::SaveAndRestore SavedDecl(ClsType);
+        if (OCMD && OCMD->isInstanceMethod()) {
+          if (auto *ImplParamDecl = OCMD->getSelfDecl())
+            ClsType = ImplParamDecl->getType();
+        }
+        return DynamicRecursiveASTVisitor::TraverseObjCMethodDecl(OCMD);
+      }
+
       bool VisitTypedefDecl(TypedefDecl *TD) override {
         if (Checker->RTC)
           Checker->RTC->visitTypedef(TD);
@@ -275,10 +285,10 @@ class RawPtrRefLambdaCapturesChecker
         auto *VD = dyn_cast<VarDecl>(ValueDecl);
         if (!VD)
           return false;
-        auto *Init = VD->getInit()->IgnoreParenCasts();
+        auto *Init = VD->getInit();
         if (!Init)
           return false;
-        const Expr *Arg = Init;
+        const Expr *Arg = Init->IgnoreParenCasts();
         do {
           if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Arg))
             Arg = BTE->getSubExpr()->IgnoreParenCasts();
@@ -287,7 +297,7 @@ class RawPtrRefLambdaCapturesChecker
             if (!Ctor)
               return false;
             auto clsName = safeGetName(Ctor->getParent());
-            if (isRefType(clsName) && CE->getNumArgs()) {
+            if (Checker->isPtrType(clsName) && CE->getNumArgs()) {
               Arg = CE->getArg(0)->IgnoreParenCasts();
               continue;
             }
@@ -307,6 +317,12 @@ class RawPtrRefLambdaCapturesChecker
               Arg = CE->getArg(0)->IgnoreParenCasts();
               continue;
             }
+            if (auto *Callee = CE->getDirectCallee()) {
+              if (isCtorOfSafePtr(Callee) && CE->getNumArgs() == 1) {
+                Arg = CE->getArg(0)->IgnoreParenCasts();
+                continue;
+              }
+            }
           }
           if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(Arg)) {
             auto OpCode = OpCE->getOperator();
@@ -315,7 +331,7 @@ class RawPtrRefLambdaCapturesChecker
               if (!Callee)
                 return false;
               auto clsName = safeGetName(Callee->getParent());
-              if (!isRefType(clsName) || !OpCE->getNumArgs())
+              if (!Checker->isPtrType(clsName) || !OpCE->getNumArgs())
                 return false;
               Arg = OpCE->getArg(0)->IgnoreParenCasts();
               continue;
@@ -330,8 +346,15 @@ class RawPtrRefLambdaCapturesChecker
           }
           break;
         } while (Arg);
-        if (auto *DRE = dyn_cast<DeclRefExpr>(Arg))
-          return ProtectedThisDecls.contains(DRE->getDecl());
+        if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+          auto *Decl = DRE->getDecl();
+          if (auto *ImplicitParam = dyn_cast<ImplicitParamDecl>(Decl)) {
+            auto kind = ImplicitParam->getParameterKind();
+            return kind == ImplicitParamKind::ObjCSelf ||
+                   kind == ImplicitParamKind::CXXThis;
+          }
+          return ProtectedThisDecls.contains(Decl);
+        }
         return isa<CXXThisExpr>(Arg);
       }
     };
@@ -351,10 +374,17 @@ class RawPtrRefLambdaCapturesChecker
         ValueDecl *CapturedVar = C.getCapturedVar();
         if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
           continue;
+        if (auto *ImplicitParam = dyn_cast<ImplicitParamDecl>(CapturedVar)) {
+          auto kind = ImplicitParam->getParameterKind();
+          if ((kind == ImplicitParamKind::ObjCSelf ||
+               kind == ImplicitParamKind::CXXThis) &&
+              !shouldCheckThis)
+            continue;
+        }
         QualType CapturedVarQualType = CapturedVar->getType();
         auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
         if (IsUncountedPtr && *IsUncountedPtr)
-          reportBug(C, CapturedVar, CapturedVarQualType);
+          reportBug(C, CapturedVar, CapturedVarQualType, L);
       } else if (C.capturesThis() && shouldCheckThis) {
         if (ignoreParamVarDecl) // this is always a parameter to this function.
           continue;
@@ -364,11 +394,12 @@ class RawPtrRefLambdaCapturesChecker
   }
 
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
-                 const QualType T) const {
+                 const QualType T, LambdaExpr *L) const {
     assert(CapturedVar);
 
-    if (isa<ImplicitParamDecl>(CapturedVar) && !Capture.getLocation().isValid())
-      return; // Ignore implicit captruing of self.
+    auto Location = Capture.getLocation();
+    if (isa<ImplicitParamDecl>(CapturedVar) && !Location.isValid())
+      Location = L->getBeginLoc();
 
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
@@ -387,7 +418,7 @@ class RawPtrRefLambdaCapturesChecker
     printQuotedQualifiedName(Os, CapturedVar);
     Os << " to " << ptrKind(T) << " type is unsafe.";
 
-    PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
+    PathDiagnosticLocation BSLoc(Location, BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
     BR->emitReport(std::move(Report));
   }
@@ -429,6 +460,10 @@ class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
     return result2;
   }
 
+  virtual bool isPtrType(const std::string &Name) const final {
+    return isRefType(Name) || isCheckedPtr(Name);
+  }
+
   const char *ptrKind(QualType QT) const final {
     if (isUncounted(QT))
       return "uncounted";
@@ -445,7 +480,11 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
   }
 
   std::optional<bool> isUnsafePtr(QualType QT) const final {
-    return RTC->isUnretained(QT);
+    return RTC->isUnretained(QT, /* ignoreARC */ true);
+  }
+
+  virtual bool isPtrType(const std::string &Name) const final {
+    return isRetainPtr(Name);
   }
 
   const char *ptrKind(QualType QT) const final { return "unretained"; }

clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm

@@ -123,19 +123,23 @@ explicit CallableWrapper(CallableType& callable)
 void raw_ptr() {
   SomeObj* obj = make_obj();
   auto foo1 = [obj](){
+    // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     [obj doWork];
   };
   call(foo1);
 
   auto foo2 = [&obj](){
+    // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     [obj doWork];
   };
   auto foo3 = [&](){
     [obj doWork];
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     obj = nullptr;
   };
   auto foo4 = [=](){
     [obj doWork];
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
   };
 
   auto cf = make_cf();
@@ -218,6 +222,7 @@ void noescape_lambda() {
     [otherObj doWork];
   }, [&](SomeObj *obj) {
     [otherObj doWork];
+    // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
   });
   ([&] {
     [someObj doWork];
@@ -242,11 +247,13 @@ void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf)
 {
   callFunction([&]() {
     [obj doWork];
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     CFArrayAppendValue(cf, nullptr);
     // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
   });
   callFunctionOpaque([&]() {
     [obj doWork];
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     CFArrayAppendValue(cf, nullptr);
     // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
   });
@@ -262,6 +269,7 @@ -(void)run;
 @implementation ObjWithSelf
 -(void)doWork {
   auto doWork = [&] {
+    // expected-warning@-1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
     someFunction();
     [delegate doWork];
   };

clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm

@@ -279,17 +279,33 @@ @interface ObjWithSelf : NSObject {
   RetainPtr<id> delegate;
 }
 -(void)doWork;
+-(void)doMoreWork;
 -(void)run;
 @end
 
 @implementation ObjWithSelf
 -(void)doWork {
   auto doWork = [&] {
+    // expected-warning@-1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
+    someFunction();
+    [delegate doWork];
+  };
+  auto doExtraWork = [&, protectedSelf = retainPtr(self)] {
     someFunction();
     [delegate doWork];
   };
   callFunctionOpaque(doWork);
+  callFunctionOpaque(doExtraWork);
 }
+
+-(void)doMoreWork {
+  auto doWork = [self, protectedSelf = retainPtr(self)] {
+    someFunction();
+    [self doWork];
+  };
+  callFunctionOpaque(doWork);
+}
+
 -(void)run {
   someFunction();
 }