[alpha.webkit.UnretainedLambdaCapturesChecker] Add a WebKit checker for lambda capturing NS or CF types.

clang/docs/analyzer/checkers.rst

@@ -3522,6 +3522,18 @@ Raw pointers and references to an object which supports CheckedPtr or CheckedRef
 
 See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details.
 
+alpha.webkit.UnretainedLambdaCapturesChecker
+""""""""""""""""""""""""""""""""""""""""""""
+Raw pointers and references to NS or CF types can't be captured in lambdas. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled, and only RetainPtr is allowed for NS types when ARC is disabled.
+
+.. code-block:: cpp
+
+ void foo(NSObject *a, NSObject *b) {
+   [&, a](){ // warn about 'a'
+     do_something(b); // warn about 'b'
+   };
+ };
+
 .. _alpha-webkit-UncountedCallArgsChecker:
 
 alpha.webkit.UncountedCallArgsChecker

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -1765,6 +1765,10 @@ def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
   HelpText<"Check for no unchecked member variables.">,
   Documentation<HasDocumentation>;
 
+def UnretainedLambdaCapturesChecker : Checker<"UnretainedLambdaCapturesChecker">,
+  HelpText<"Check unretained lambda captures.">,
+  Documentation<HasDocumentation>;
+
 def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">,
   HelpText<"Check uncounted call arguments.">,
   Documentation<HasDocumentation>;

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -128,14 +128,14 @@ add_clang_library(clangStaticAnalyzerCheckers
   VLASizeChecker.cpp
   ValistChecker.cpp
   VirtualCallChecker.cpp
-  WebKit/RawPtrRefMemberChecker.cpp
   WebKit/ASTUtils.cpp
   WebKit/MemoryUnsafeCastChecker.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
   WebKit/RawPtrRefCallArgsChecker.cpp
-  WebKit/UncountedLambdaCapturesChecker.cpp
+  WebKit/RawPtrRefLambdaCapturesChecker.cpp
   WebKit/RawPtrRefLocalVarsChecker.cpp
+  WebKit/RawPtrRefMemberChecker.cpp
 
   LINK_LIBS
   clangAST

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

@@ -21,15 +21,23 @@ using namespace clang;
 using namespace ento;
 
 namespace {
-class UncountedLambdaCapturesChecker
+class RawPtrRefLambdaCapturesChecker
     : public Checker<check::ASTDecl<TranslationUnitDecl>> {
 private:
-  BugType Bug{this, "Lambda capture of uncounted variable",
-              "WebKit coding guidelines"};
+  BugType Bug;
   mutable BugReporter *BR = nullptr;
   TrivialFunctionAnalysis TFA;
 
+protected:
+  mutable std::optional<RetainTypeChecker> RTC;
+
 public:
+  RawPtrRefLambdaCapturesChecker(const char *description)
+      : Bug(this, description, "WebKit coding guidelines") {}
+
+  virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
+  virtual const char *ptrKind(QualType QT) const = 0;
+
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
                     BugReporter &BRArg) const {
     BR = &BRArg;
@@ -38,15 +46,15 @@ class UncountedLambdaCapturesChecker
     // visit template instantiations or lambda classes. We
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : DynamicRecursiveASTVisitor {
-      const UncountedLambdaCapturesChecker *Checker;
+      const RawPtrRefLambdaCapturesChecker *Checker;
       llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
       llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
       llvm::DenseSet<const ValueDecl *> ProtectedThisDecls;
       llvm::DenseSet<const CXXConstructExpr *> ConstructToIgnore;
 
       QualType ClsType;
 
-      explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
+      explicit LocalVisitor(const RawPtrRefLambdaCapturesChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
         ShouldVisitTemplateInstantiations = true;
@@ -60,16 +68,23 @@ class UncountedLambdaCapturesChecker
         return DynamicRecursiveASTVisitor::TraverseCXXMethodDecl(CXXMD);
       }
 
+      bool VisitTypedefDecl(TypedefDecl *TD) override {
+        if (Checker->RTC)
+          Checker->RTC->visitTypedef(TD);
+        return true;
+      }
+
       bool shouldCheckThis() {
         auto result =
-            !ClsType.isNull() ? isUnsafePtr(ClsType, false) : std::nullopt;
+            !ClsType.isNull() ? Checker->isUnsafePtr(ClsType) : std::nullopt;
         return result && *result;
       }
 
       bool VisitLambdaExpr(LambdaExpr *L) override {
         if (LambdasToIgnore.contains(L))
           return true;
-        Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L));
+        Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
+                                 ClsType);
         return true;
       }
 
@@ -97,7 +112,8 @@ class UncountedLambdaCapturesChecker
         if (!L)
           return true;
         LambdasToIgnore.insert(L);
-        Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L));
+        Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
+                                 ClsType);
         return true;
       }
 
@@ -122,8 +138,8 @@ class UncountedLambdaCapturesChecker
             if (auto *L = findLambdaInArg(Arg)) {
               LambdasToIgnore.insert(L);
               if (!Param->hasAttr<NoEscapeAttr>())
-                Checker->visitLambdaExpr(L, shouldCheckThis() &&
-                                                !hasProtectedThis(L));
+                Checker->visitLambdaExpr(
+                    L, shouldCheckThis() && !hasProtectedThis(L), ClsType);
             }
             ++ArgIndex;
           }
@@ -143,8 +159,8 @@ class UncountedLambdaCapturesChecker
             if (auto *L = findLambdaInArg(Arg)) {
               LambdasToIgnore.insert(L);
               if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
-                Checker->visitLambdaExpr(L, shouldCheckThis() &&
-                                                !hasProtectedThis(L));
+                Checker->visitLambdaExpr(
+                    L, shouldCheckThis() && !hasProtectedThis(L), ClsType);
             }
             ++ArgIndex;
           }
@@ -169,14 +185,22 @@ class UncountedLambdaCapturesChecker
         auto *CtorArg = CE->getArg(0)->IgnoreParenCasts();
         if (!CtorArg)
           return nullptr;
-        if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg)) {
+        auto *InnerCE = dyn_cast_or_null<CXXConstructExpr>(CtorArg);
+        if (InnerCE && InnerCE->getNumArgs())
+          CtorArg = InnerCE->getArg(0)->IgnoreParenCasts();
+        auto updateIgnoreList = [&] {
           ConstructToIgnore.insert(CE);
+          if (InnerCE)
+            ConstructToIgnore.insert(InnerCE);
+        };
+        if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg)) {
+          updateIgnoreList();
           return Lambda;
         }
         if (auto *TempExpr = dyn_cast<CXXBindTemporaryExpr>(CtorArg)) {
           E = TempExpr->getSubExpr()->IgnoreParenCasts();
           if (auto *Lambda = dyn_cast<LambdaExpr>(E)) {
-            ConstructToIgnore.insert(CE);
+            updateIgnoreList();
             return Lambda;
           }
         }
@@ -189,10 +213,14 @@ class UncountedLambdaCapturesChecker
         auto *Init = VD->getInit();
         if (!Init)
           return nullptr;
+        if (auto *Lambda = dyn_cast<LambdaExpr>(Init)) {
+          updateIgnoreList();
+          return Lambda;
+        }
         TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
         if (!TempExpr)
           return nullptr;
-        ConstructToIgnore.insert(CE);
+        updateIgnoreList();
         return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
       }
 
@@ -226,7 +254,7 @@ class UncountedLambdaCapturesChecker
         DeclRefExprsToIgnore.insert(ArgRef);
         LambdasToIgnore.insert(L);
         Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
-                                 /* ignoreParamVarDecl */ true);
+                                 ClsType, /* ignoreParamVarDecl */ true);
       }
 
       bool hasProtectedThis(LambdaExpr *L) {
@@ -293,10 +321,12 @@ class UncountedLambdaCapturesChecker
     };
 
     LocalVisitor visitor(this);
+    if (RTC)
+      RTC->visitTranslationUnitDecl(TUD);
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis,
+  void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
                        bool ignoreParamVarDecl = false) const {
     if (TFA.isTrivial(L->getBody()))
       return;
@@ -306,13 +336,13 @@ class UncountedLambdaCapturesChecker
         if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
           continue;
         QualType CapturedVarQualType = CapturedVar->getType();
-        auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType(), false);
+        auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
         if (IsUncountedPtr && *IsUncountedPtr)
           reportBug(C, CapturedVar, CapturedVarQualType);
       } else if (C.capturesThis() && shouldCheckThis) {
         if (ignoreParamVarDecl) // this is always a parameter to this function.
           continue;
-        reportBugOnThisPtr(C);
+        reportBugOnThisPtr(C, T);
       }
     }
   }
@@ -321,6 +351,9 @@ class UncountedLambdaCapturesChecker
                  const QualType T) const {
     assert(CapturedVar);
 
+    if (isa<ImplicitParamDecl>(CapturedVar) && !Capture.getLocation().isValid())
+      return; // Ignore implicit captruing of self.
+
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
 
@@ -329,22 +362,22 @@ class UncountedLambdaCapturesChecker
     } else {
       Os << "Implicitly captured ";
     }
-    if (T->isPointerType()) {
+    if (isa<PointerType>(T) || isa<ObjCObjectPointerType>(T)) {
       Os << "raw-pointer ";
     } else {
-      assert(T->isReferenceType());
       Os << "reference ";
     }
 
-    printQuotedQualifiedName(Os, Capture.getCapturedVar());
-    Os << " to ref-counted type or CheckedPtr-capable type is unsafe.";
+    printQuotedQualifiedName(Os, CapturedVar);
+    Os << " to " << ptrKind(T) << " type is unsafe.";
 
     PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
     BR->emitReport(std::move(Report));
   }
 
-  void reportBugOnThisPtr(const LambdaCapture &Capture) const {
+  void reportBugOnThisPtr(const LambdaCapture &Capture,
+                          const QualType T) const {
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
 
@@ -354,14 +387,54 @@ class UncountedLambdaCapturesChecker
       Os << "Implicitly captured ";
     }
 
-    Os << "raw-pointer 'this' to ref-counted type or CheckedPtr-capable type "
-          "is unsafe.";
+    Os << "raw-pointer 'this' to " << ptrKind(T) << " type is unsafe.";
 
     PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
     BR->emitReport(std::move(Report));
   }
 };
+
+class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
+public:
+  UncountedLambdaCapturesChecker()
+      : RawPtrRefLambdaCapturesChecker("Lambda capture of uncounted or "
+                                       "unchecked variable") {}
+
+  std::optional<bool> isUnsafePtr(QualType QT) const final {
+    auto result1 = isUncountedPtr(QT);
+    auto result2 = isUncheckedPtr(QT);
+    if (result1 && *result1)
+      return true;
+    if (result2 && *result2)
+      return true;
+    if (result1)
+      return *result1;
+    return result2;
+  }
+
+  const char *ptrKind(QualType QT) const final {
+    if (isUncounted(QT))
+      return "uncounted";
+    return "unchecked";
+  }
+};
+
+class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
+public:
+  UnretainedLambdaCapturesChecker()
+      : RawPtrRefLambdaCapturesChecker("Lambda capture of unretained "
+                                       "variables") {
+    RTC = RetainTypeChecker();
+  }
+
+  std::optional<bool> isUnsafePtr(QualType QT) const final {
+    return RTC->isUnretained(QT);
+  }
+
+  const char *ptrKind(QualType QT) const final { return "unretained"; }
+};
+
 } // namespace
 
 void ento::registerUncountedLambdaCapturesChecker(CheckerManager &Mgr) {
@@ -372,3 +445,12 @@ bool ento::shouldRegisterUncountedLambdaCapturesChecker(
     const CheckerManager &mgr) {
   return true;
 }
+
+void ento::registerUnretainedLambdaCapturesChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<UnretainedLambdaCapturesChecker>();
+}
+
+bool ento::shouldRegisterUnretainedLambdaCapturesChecker(
+    const CheckerManager &mgr) {
+  return true;
+}