[webkit.UncountedLambdaCapturesChecker] Add the support for WTF::ScopeExit and WTF::makeVisitor

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

@@ -50,7 +50,9 @@ class RawPtrRefLambdaCapturesChecker
       llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
       llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
       llvm::DenseSet<const ValueDecl *> ProtectedThisDecls;
+      llvm::DenseSet<const CallExpr *> CallToIgnore;
       llvm::DenseSet<const CXXConstructExpr *> ConstructToIgnore;
+      llvm::DenseMap<const VarDecl *, const LambdaExpr *> LambdaOwnerMap;
 
       QualType ClsType;
 
@@ -101,10 +103,60 @@ class RawPtrRefLambdaCapturesChecker
         auto *Init = VD->getInit();
         if (!Init)
           return true;
-        auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
-        if (!L)
+        if (auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts())) {
+          LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+          return true;
+        }
+        if (!VD->hasLocalStorage())
           return true;
-        LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+        if (auto *E = dyn_cast<ExprWithCleanups>(Init))
+          Init = E->getSubExpr();
+        if (auto *E = dyn_cast<CXXBindTemporaryExpr>(Init))
+          Init = E->getSubExpr();
+        if (auto *CE = dyn_cast<CallExpr>(Init)) {
+          if (auto *Callee = CE->getDirectCallee()) {
+            auto FnName = safeGetName(Callee);
+            unsigned ArgCnt = CE->getNumArgs();
+            if (FnName == "makeScopeExit" && ArgCnt == 1) {
+              auto *Arg = CE->getArg(0);
+              if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+                Arg = E->getSubExpr();
+              if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+                LambdaOwnerMap.insert(std::make_pair(VD, L));
+                CallToIgnore.insert(CE);
+                LambdasToIgnore.insert(L);
+              }
+            } else if (FnName == "makeVisitor") {
+              for (unsigned ArgIndex = 0; ArgIndex < ArgCnt; ++ArgIndex) {
+                auto *Arg = CE->getArg(ArgIndex);
+                if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+                  Arg = E->getSubExpr();
+                if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+                  LambdaOwnerMap.insert(std::make_pair(VD, L));
+                  CallToIgnore.insert(CE);
+                  LambdasToIgnore.insert(L);
+                }
+              }
+            }
+          }
+        } else if (auto *CE = dyn_cast<CXXConstructExpr>(Init)) {
+          if (auto *Ctor = CE->getConstructor()) {
+            if (auto *Cls = Ctor->getParent()) {
+              auto FnName = safeGetName(Cls);
+              unsigned ArgCnt = CE->getNumArgs();
+              if (FnName == "ScopeExit" && ArgCnt == 1) {
+                auto *Arg = CE->getArg(0);
+                if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+                  Arg = E->getSubExpr();
+                if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+                  LambdaOwnerMap.insert(std::make_pair(VD, L));
+                  ConstructToIgnore.insert(CE);
+                  LambdasToIgnore.insert(L);
+                }
+              }
+            }
+          }
+        }
         return true;
       }
 
@@ -114,6 +166,12 @@ class RawPtrRefLambdaCapturesChecker
         auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
         if (!VD)
           return true;
+        if (auto It = LambdaOwnerMap.find(VD); It != LambdaOwnerMap.end()) {
+          auto *L = It->second;
+          Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
+                                   ClsType);
+          return true;
+        }
         auto *Init = VD->getInit();
         if (!Init)
           return true;
@@ -167,10 +225,30 @@ class RawPtrRefLambdaCapturesChecker
       }
 
       bool VisitCallExpr(CallExpr *CE) override {
+        if (CallToIgnore.contains(CE))
+          return true;
         checkCalleeLambda(CE);
-        if (auto *Callee = CE->getDirectCallee())
+        if (auto *Callee = CE->getDirectCallee()) {
+          if (auto *Ns = Callee->getParent()) {
+            auto NsName = safeGetName(Ns);
+            bool IsVisitFn = safeGetName(Callee) == "visit";
+            bool ArgCnt = CE->getNumArgs();
+            if (IsVisitFn && ArgCnt && (NsName == "WTF" || NsName == "std")) {
+              if (auto *Arg = CE->getArg(0)) {
+                Arg = Arg->IgnoreParenCasts();
+                if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+                  if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+                    if (LambdaOwnerMap.contains(VD)) {
+                      DeclRefExprsToIgnore.insert(DRE);
+                      return true;
+                    }
+                  }
+                }
+              }
+            }
+          }
           checkParameters(CE, Callee);
-        else if (auto *CalleeE = CE->getCallee()) {
+        } else if (auto *CalleeE = CE->getCallee()) {
           if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
             if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
               checkParameters(CE, Callee);
@@ -280,7 +358,7 @@ class RawPtrRefLambdaCapturesChecker
         LambdasToIgnore.insert(L);
       }
 
-      bool hasProtectedThis(LambdaExpr *L) {
+      bool hasProtectedThis(const LambdaExpr *L) {
         for (const LambdaCapture &OtherCapture : L->captures()) {
           if (!OtherCapture.capturesVariable())
             continue;
@@ -378,7 +456,8 @@ class RawPtrRefLambdaCapturesChecker
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
+  void visitLambdaExpr(const LambdaExpr *L, bool shouldCheckThis,
+                       const QualType T,
                        bool ignoreParamVarDecl = false) const {
     if (TFA.isTrivial(L->getBody()))
       return;
@@ -410,7 +489,7 @@ class RawPtrRefLambdaCapturesChecker
   }
 
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
-                 const QualType T, LambdaExpr *L) const {
+                 const QualType T, const LambdaExpr *L) const {
     assert(CapturedVar);
 
     auto Location = Capture.getLocation();

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

@@ -107,6 +107,79 @@ class HashMap {
   ValueType* m_table { nullptr };
 };
 
+class ScopeExit final {
+public:
+  template<typename ExitFunctionParameter>
+  explicit ScopeExit(ExitFunctionParameter&& exitFunction)
+    : m_exitFunction(std::move(exitFunction)) {
+  }
+
+  ScopeExit(ScopeExit&& other)
+    : m_exitFunction(std::move(other.m_exitFunction))
+    , m_execute(std::move(other.m_execute)) {
+  }
+
+  ~ScopeExit() {
+    if (m_execute)
+      m_exitFunction();
+  }
+
+  WTF::Function<void()> take() {
+    m_execute = false;
+    return std::move(m_exitFunction);
+  }
+
+  void release() { m_execute = false; }
+
+  ScopeExit(const ScopeExit&) = delete;
+  ScopeExit& operator=(const ScopeExit&) = delete;
+  ScopeExit& operator=(ScopeExit&&) = delete;
+
+private:
+  WTF::Function<void()> m_exitFunction;
+  bool m_execute { true };
+};
+
+template<typename ExitFunction> ScopeExit makeScopeExit(ExitFunction&&);
+template<typename ExitFunction>
+ScopeExit makeScopeExit(ExitFunction&& exitFunction)
+{
+    return ScopeExit(std::move(exitFunction));
+}
+
+// Visitor adapted from http://stackoverflow.com/questions/25338795/is-there-a-name-for-this-tuple-creation-idiom
+
+template<class A, class... B> struct Visitor : Visitor<A>, Visitor<B...> {
+    Visitor(A a, B... b)
+        : Visitor<A>(a)
+        , Visitor<B...>(b...)
+    {
+    }
+
+    using Visitor<A>::operator ();
+    using Visitor<B...>::operator ();
+};
+
+template<class A> struct Visitor<A> : A {
+    Visitor(A a)
+        : A(a)
+    {
+    }
+
+    using A::operator();
+};
+
+template<class... F> Visitor<F...> makeVisitor(F... f)
+{
+    return Visitor<F...>(f...);
+}
+
+void opaqueFunction();
+template <typename Visitor, typename... Variants> void visit(Visitor&& v, Variants&&... values)
+{
+  opaqueFunction();
+}
+
 } // namespace WTF
 
 struct A {
@@ -501,3 +574,81 @@ void RefCountedObj::call() const
     };
     callLambda(lambda);
 }
+
+void scope_exit(RefCountable* obj) {
+  auto scope = WTF::makeScopeExit([&] {
+    obj->method();
+  });
+  someFunction();
+  WTF::ScopeExit scope2([&] {
+    obj->method();
+  });
+  someFunction();
+}
+
+void doWhateverWith(WTF::ScopeExit& obj);
+
+void scope_exit_with_side_effect(RefCountable* obj) {
+  auto scope = WTF::makeScopeExit([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  doWhateverWith(scope);
+}
+
+void scope_exit_static(RefCountable* obj) {
+  static auto scope = WTF::makeScopeExit([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+}
+
+WTF::Function<void()> scope_exit_take_lambda(RefCountable* obj) {
+  auto scope = WTF::makeScopeExit([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  return scope.take();
+}
+
+// FIXME: Ideally, we treat release() as a trivial function.
+void scope_exit_release(RefCountable* obj) {
+  auto scope = WTF::makeScopeExit([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  scope.release();
+}
+
+void make_visitor(RefCountable* obj) {
+  auto visitor = WTF::makeVisitor([&] {
+    obj->method();
+  });
+}
+
+void use_visitor(RefCountable* obj) {
+  auto visitor = WTF::makeVisitor([&] {
+    obj->method();
+  });
+  WTF::visit(visitor, obj);
+}
+
+template <typename Visitor, typename ObjectType>
+void bad_visit(Visitor&, ObjectType*) {
+  someFunction();
+}
+
+void static_visitor(RefCountable* obj) {
+  static auto visitor = WTF::makeVisitor([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+}
+
+void bad_use_visitor(RefCountable* obj) {
+  auto visitor = WTF::makeVisitor([&] {
+    obj->method();
+    // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  bad_visit(visitor, obj);
+}