diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 033eb8cc299b0..20fef32a6be66 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -50,7 +50,9 @@ class RawPtrRefLambdaCapturesChecker llvm::DenseSet DeclRefExprsToIgnore; llvm::DenseSet LambdasToIgnore; llvm::DenseSet ProtectedThisDecls; + llvm::DenseSet CallToIgnore; llvm::DenseSet ConstructToIgnore; + llvm::DenseMap LambdaOwnerMap; QualType ClsType; @@ -101,10 +103,60 @@ class RawPtrRefLambdaCapturesChecker auto *Init = VD->getInit(); if (!Init) return true; - auto *L = dyn_cast_or_null(Init->IgnoreParenCasts()); - if (!L) + if (auto *L = dyn_cast_or_null(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(Init)) + Init = E->getSubExpr(); + if (auto *E = dyn_cast(Init)) + Init = E->getSubExpr(); + if (auto *CE = dyn_cast(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(Arg)) + Arg = E->getSubExpr(); + if (auto *L = dyn_cast(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(Arg)) + Arg = E->getSubExpr(); + if (auto *L = dyn_cast(Arg)) { + LambdaOwnerMap.insert(std::make_pair(VD, L)); + CallToIgnore.insert(CE); + LambdasToIgnore.insert(L); + } + } + } + } + } else if (auto *CE = dyn_cast(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(Arg)) + Arg = E->getSubExpr(); + if (auto *L = dyn_cast(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(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(Arg)) { + if (auto *VD = dyn_cast(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(CalleeE->IgnoreParenCasts())) { if (auto *Callee = dyn_cast_or_null(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(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(); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index a4ad741182f56..fd1eecdda64fd 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -107,6 +107,79 @@ class HashMap { ValueType* m_table { nullptr }; }; +class ScopeExit final { +public: + template + 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 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 m_exitFunction; + bool m_execute { true }; +}; + +template ScopeExit makeScopeExit(ExitFunction&&); +template +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 struct Visitor : Visitor, Visitor { + Visitor(A a, B... b) + : Visitor(a) + , Visitor(b...) + { + } + + using Visitor::operator (); + using Visitor::operator (); +}; + +template struct Visitor : A { + Visitor(A a) + : A(a) + { + } + + using A::operator(); +}; + +template Visitor makeVisitor(F... f) +{ + return Visitor(f...); +} + +void opaqueFunction(); +template 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 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 +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); +}