diff options
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp | 107 | ||||
| -rw-r--r-- | clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp | 151 |
2 files changed, 250 insertions, 8 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 033eb8c..f60d193 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -50,7 +50,9 @@ public: 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 @@ public: 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 @@ public: 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,14 @@ public: } bool VisitCallExpr(CallExpr *CE) override { + if (CallToIgnore.contains(CE)) + return true; checkCalleeLambda(CE); - if (auto *Callee = CE->getDirectCallee()) + if (auto *Callee = CE->getDirectCallee()) { + if (isVisitFunction(CE, Callee)) + 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); @@ -179,6 +241,34 @@ public: return true; } + bool isVisitFunction(CallExpr *CallExpr, FunctionDecl *FnDecl) { + bool IsVisitFn = safeGetName(FnDecl) == "visit"; + if (!IsVisitFn) + return false; + bool ArgCnt = CallExpr->getNumArgs(); + if (!ArgCnt) + return false; + auto *Ns = FnDecl->getParent(); + if (!Ns) + return false; + auto NsName = safeGetName(Ns); + if (NsName != "WTF" && NsName != "std") + return false; + auto *Arg = CallExpr->getArg(0); + if (!Arg) + return false; + auto *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreParenCasts()); + if (!DRE) + return false; + auto *VD = dyn_cast<VarDecl>(DRE->getDecl()); + if (!VD) + return false; + if (!LambdaOwnerMap.contains(VD)) + return false; + DeclRefExprsToIgnore.insert(DRE); + return true; + } + void checkParameters(CallExpr *CE, FunctionDecl *Callee) { unsigned ArgIndex = isa<CXXOperatorCallExpr>(CE); bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee); @@ -280,7 +370,7 @@ public: LambdasToIgnore.insert(L); } - bool hasProtectedThis(LambdaExpr *L) { + bool hasProtectedThis(const LambdaExpr *L) { for (const LambdaCapture &OtherCapture : L->captures()) { if (!OtherCapture.capturesVariable()) continue; @@ -378,7 +468,8 @@ public: 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 +501,7 @@ public: } 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 a4ad741..fd1eecd 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 @@ private: 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); +} |