[webkit.UncountedLambdaCapturesChecker] Ignore trivial functions and [[clang::noescape]].

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

@@ -222,7 +222,13 @@ std::optional<bool> isUncheckedPtr(const QualType T) {
 std::optional<bool> isUnsafePtr(const QualType T) {
   if (T->isPointerType() || T->isReferenceType()) {
     if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
-      return isUncounted(CXXRD) || isUnchecked(CXXRD);
+      auto isUncountedPtr = isUncounted(CXXRD);
+      auto isUncheckedPtr = isUnchecked(CXXRD);
+      if (isUncountedPtr && isUncheckedPtr)
+        return *isUncountedPtr || *isUncheckedPtr;
+      if (isUncountedPtr)
+        return *isUncountedPtr;
+      return isUncheckedPtr;
     }
   }
   return false;

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

@@ -75,6 +75,10 @@ std::optional<bool> isUncountedPtr(const clang::QualType T);
 /// class, false if not, std::nullopt if inconclusive.
 std::optional<bool> isUncheckedPtr(const clang::QualType T);
 
+/// \returns true if \p T is either a raw pointer or reference to an uncounted
+/// or unchecked class, false if not, std::nullopt if inconclusive.
+std::optional<bool> isUnsafePtr(const QualType T);
+
 /// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
 /// variant, false if not.
 bool isSafePtrType(const clang::QualType T);

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

@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "ASTUtils.h"
 #include "DiagOutputUtils.h"
 #include "PtrTypesSemantics.h"
 #include "clang/AST/CXXInheritance.h"
@@ -26,6 +27,7 @@ class UncountedLambdaCapturesChecker
   BugType Bug{this, "Lambda capture of uncounted variable",
               "WebKit coding guidelines"};
   mutable BugReporter *BR = nullptr;
+  TrivialFunctionAnalysis TFA;
 
 public:
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -37,6 +39,11 @@ class UncountedLambdaCapturesChecker
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
       const UncountedLambdaCapturesChecker *Checker;
+      llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
+      QualType ClsType;
+
+      using Base = RecursiveASTVisitor<LocalVisitor>;
+
       explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
@@ -45,32 +52,118 @@ class UncountedLambdaCapturesChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
-      bool VisitLambdaExpr(LambdaExpr *L) {
-        Checker->visitLambdaExpr(L);
+      bool TraverseCXXMethodDecl(CXXMethodDecl *CXXMD) {
+        llvm::SaveAndRestore SavedDecl(ClsType);
+        ClsType = CXXMD->getThisType();
+        return Base::TraverseCXXMethodDecl(CXXMD);
+      }
+
+      bool shouldCheckThis() {
+        auto result = !ClsType.isNull() ? isUnsafePtr(ClsType) : std::nullopt;
+        return result && *result;
+      }
+
+      bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+        if (DeclRefExprsToIgnore.contains(DRE))
+          return true;
+        auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+        if (!VD)
+          return true;
+        auto *Init = VD->getInit();
+        if (!Init)
+          return true;
+        auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
+        if (!L)
+          return true;
+        Checker->visitLambdaExpr(L, shouldCheckThis());
         return true;
       }
+
+      // WTF::switchOn(T, F... f) is a variadic template function and couldn't
+      // be annotated with NOESCAPE. We hard code it here to workaround that.
+      bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
+        auto *NsDecl = Decl->getParent();
+        if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+          return false;
+        return safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn";
+      }
+
+      bool VisitCallExpr(CallExpr *CE) {
+        checkCalleeLambda(CE);
+        if (auto *Callee = CE->getDirectCallee()) {
+          bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
+          unsigned ArgIndex = 0;
+          for (auto *Param : Callee->parameters()) {
+            if (ArgIndex >= CE->getNumArgs())
+              return true;
+            auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
+            if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
+              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
+                Checker->visitLambdaExpr(L, shouldCheckThis());
+              }
+            }
+            ++ArgIndex;
+          }
+        }
+        return true;
+      }
+
+      void checkCalleeLambda(CallExpr *CE) {
+        auto *Callee = CE->getCallee();
+        if (!Callee)
+          return;
+        auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());
+        if (!DRE)
+          return;
+        auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl());
+        if (!MD || CE->getNumArgs() != 1)
+          return;
+        auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+        auto *ArgRef = dyn_cast<DeclRefExpr>(Arg);
+        if (!ArgRef)
+          return;
+        auto *VD = dyn_cast_or_null<VarDecl>(ArgRef->getDecl());
+        if (!VD)
+          return;
+        auto *Init = VD->getInit();
+        if (!Init)
+          return;
+        auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
+        if (!L)
+          return;
+        DeclRefExprsToIgnore.insert(ArgRef);
+        Checker->visitLambdaExpr(L, shouldCheckThis(),
+                                 /* ignoreParamVarDecl */ true);
+      }
     };
 
     LocalVisitor visitor(this);
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitLambdaExpr(LambdaExpr *L) const {
+  void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis,
+                       bool ignoreParamVarDecl = false) const {
+    if (TFA.isTrivial(L->getBody()))
+      return;
     for (const LambdaCapture &C : L->captures()) {
       if (C.capturesVariable()) {
         ValueDecl *CapturedVar = C.getCapturedVar();
+        if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
+          continue;
         QualType CapturedVarQualType = CapturedVar->getType();
-        if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
-          auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
-          if (IsUncountedPtr && *IsUncountedPtr)
-            reportBug(C, CapturedVar, CapturedVarType);
-        }
+        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);
       }
     }
   }
 
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
-                 const Type *T) const {
+                 const QualType T) const {
     assert(CapturedVar);
 
     SmallString<100> Buf;
@@ -89,7 +182,25 @@ class UncountedLambdaCapturesChecker
     }
 
     printQuotedQualifiedName(Os, Capture.getCapturedVar());
-    Os << " to uncounted type is unsafe.";
+    Os << " to ref-counted type or CheckedPtr-capable 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 {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    if (Capture.isExplicit()) {
+      Os << "Captured ";
+    } else {
+      Os << "Implicitly captured ";
+    }
+
+    Os << "raw-pointer 'this' to ref-counted type or CheckedPtr-capable type "
+          "is unsafe.";
 
     PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);

clang/test/Analysis/Checkers/WebKit/mock-types.h

@@ -135,7 +135,9 @@ struct RefCountable {
   void ref() {}
   void deref() {}
   void method();
+  void constMethod() const;
   int trivial() { return 123; }
+  RefCountable* next();
 };
 
 template <typename T> T *downcast(T *t) { return t; }