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

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

@@ -63,10 +63,6 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
 /// class, false if not, std::nullopt if inconclusive.
 std::optional<bool> isUncountedPtr(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,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ASTUtils.h"
 #include "DiagOutputUtils.h"
 #include "PtrTypesSemantics.h"
 #include "clang/AST/CXXInheritance.h"
@@ -27,7 +26,6 @@ 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,
@@ -39,8 +37,6 @@ 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;
-
       explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
@@ -49,100 +45,32 @@ class UncountedLambdaCapturesChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
-      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()->IgnoreParenCasts();
-        auto *L = dyn_cast_or_null<LambdaExpr>(Init);
-        if (!L)
-          return true;
+      bool VisitLambdaExpr(LambdaExpr *L) {
         Checker->visitLambdaExpr(L);
         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())
-              break;
-            auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
-            if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
-              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg))
-                Checker->visitLambdaExpr(L);
-            }
-            ++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()->IgnoreParenCasts();
-        auto *L = dyn_cast_or_null<LambdaExpr>(Init);
-        if (!L)
-          return;
-        DeclRefExprsToIgnore.insert(ArgRef);
-        Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
-      }
     };
 
     LocalVisitor visitor(this);
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitLambdaExpr(LambdaExpr *L, bool ignoreParamVarDecl = false) const {
-    if (TFA.isTrivial(L->getBody()))
-      return;
+  void visitLambdaExpr(LambdaExpr *L) const {
     for (const LambdaCapture &C : L->captures()) {
       if (C.capturesVariable()) {
         ValueDecl *CapturedVar = C.getCapturedVar();
-        if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
-          continue;
         QualType CapturedVarQualType = CapturedVar->getType();
-        auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
-        if (IsUncountedPtr && *IsUncountedPtr)
-          reportBug(C, CapturedVar, CapturedVarQualType);
-      } else if (C.capturesThis()) {
-        if (ignoreParamVarDecl) // this is always a parameter to this function.
-          continue;
-        reportBugOnThisPtr(C);
+        if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
+          auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
+          if (IsUncountedPtr && *IsUncountedPtr)
+            reportBug(C, CapturedVar, CapturedVarType);
+        }
       }
     }
   }
 
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
-                 const QualType T) const {
+                 const Type *T) const {
     assert(CapturedVar);
 
     SmallString<100> Buf;
@@ -161,25 +89,7 @@ class UncountedLambdaCapturesChecker
     }
 
     printQuotedQualifiedName(Os, Capture.getCapturedVar());
-    Os << " to ref-counted / 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 / CheckedPtr capable type is "
-          "unsafe.";
+    Os << " to uncounted 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,9 +135,7 @@ 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; }

clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp

@@ -1,73 +1,39 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
-//#include "mock-types.h"
-
-struct RefCountable {
-//  static Ref<RefCountable> create();
-  void ref() {}
-  void deref() {}
-  void method();
-  void constMethod() const;
-  int trivial() { return 123; }
-  RefCountable* next();
-};
-
-RefCountable* make_obj();
-
-void someFunction();
-template <typename Callback> void call(Callback callback) {
-  someFunction();
-  callback();
-}
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker %s 2>&1 | FileCheck %s --strict-whitespace
+#include "mock-types.h"
 
 void raw_ptr() {
-  RefCountable* ref_countable = make_obj();
-  auto foo1 = [ref_countable](){
-    // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-    ref_countable->method();
-  };
-  auto foo2 = [&ref_countable](){
-    // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-    ref_countable->method();
-  };
-  auto foo3 = [&](){
-    ref_countable->method();
-    // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-    ref_countable = nullptr;
-  };
-
-  auto foo4 = [=](){
-    ref_countable->method();
-    // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-  };
-
-  call(foo1);
-  call(foo2);
-  call(foo3);
-  call(foo4);
+  RefCountable* ref_countable = nullptr;
+  auto foo1 = [ref_countable](){};
+  // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  // CHECK-NEXT:{{^   6 | }}  auto foo1 = [ref_countable](){};
+  // CHECK-NEXT:{{^     | }}               ^
+  auto foo2 = [&ref_countable](){};
+  // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  auto foo3 = [&](){ ref_countable = nullptr; };
+  // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  // CHECK-NEXT:{{^  12 | }}  auto foo3 = [&](){ ref_countable = nullptr; };
+  // CHECK-NEXT:{{^     | }}                     ^
+  auto foo4 = [=](){ (void) ref_countable; };
+  // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
 
   // Confirm that the checker respects [[clang::suppress]].
   RefCountable* suppressed_ref_countable = nullptr;
   [[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
-  // no warning.
-  call(foo5);
+  // CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
 }
 
 void references() {
   RefCountable automatic;
   RefCountable& ref_countable_ref = automatic;
-  auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); };
-  // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-  auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); };
-  // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-  auto foo3 = [&](){ ref_countable_ref.method(); };
-  // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-  auto foo4 = [=](){ ref_countable_ref.constMethod(); };
-  // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
 
-  call(foo1);
-  call(foo2);
-  call(foo3);
-  call(foo4);
+  auto foo1 = [ref_countable_ref](){};
+  // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  auto foo2 = [&ref_countable_ref](){};
+  // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  auto foo3 = [&](){ (void) ref_countable_ref; };
+  // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  auto foo4 = [=](){ (void) ref_countable_ref; };
+  // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
 }
 
 void quiet() {
@@ -79,82 +45,5 @@ void quiet() {
 
   auto foo3 = [&]() {};
   auto foo4 = [=]() {};
-
-  call(foo3);
-  call(foo4);
-
   RefCountable *ref_countable = nullptr;
 }
-
-template <typename Callback>
-void map(RefCountable* start, [[clang::noescape]] Callback&& callback)
-{
-  while (start) {
-    callback(*start);
-    start = start->next();
-  }
-}
-
-template <typename Callback1, typename Callback2>
-void doubleMap(RefCountable* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2)
-{
-  while (start) {
-    callback1(*start);
-    callback2(*start);
-    start = start->next();
-  }
-}
-
-void noescape_lambda() {
-  RefCountable* someObj = make_obj();
-  RefCountable* otherObj = make_obj();
-  map(make_obj(), [&](RefCountable& obj) {
-    otherObj->method();
-  });
-  doubleMap(make_obj(), [&](RefCountable& obj) {
-    otherObj->method();
-  }, [&](RefCountable& obj) {
-    otherObj->method();
-    // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-  });
-  ([&] {
-    someObj->method();
-  })();
-}
-
-void lambda_capture_param(RefCountable* obj) {
-  auto someLambda = [&] {
-    obj->method();
-  };
-  someLambda();
-  someLambda();
-}
-
-struct RefCountableWithLambdaCapturingThis {
-  void ref() const;
-  void deref() const;
-  void nonTrivial();
-
-  void method_captures_this_safe() {
-    auto lambda = [&]() {
-      nonTrivial();
-    };
-    lambda();
-  }
-
-  void method_captures_this_unsafe() {
-    auto lambda = [&]() {
-      nonTrivial();
-      // expected-warning@-1{{Implicitly captured raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
-    };
-    call(lambda);
-  }
-};
-
-void trivial_lambda() {
-  RefCountable* ref_countable = make_obj();
-  auto trivial_lambda = [&]() {
-    return ref_countable->trivial();
-  };
-  trivial_lambda();
-}