Author: Ryosuke Niwa Date: 2025-10-23T23:19:28-07:00 New Revision: c07d305718744917ba5dc6693322e13a5c2314df
URL: https://github.com/llvm/llvm-project/commit/c07d305718744917ba5dc6693322e13a5c2314df DIFF: https://github.com/llvm/llvm-project/commit/c07d305718744917ba5dc6693322e13a5c2314df.diff LOG: [webkit.UncountedLambdaCapturesChecker] Add the support for WTF::ScopeExit and WTF::makeVisitor (#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 033eb8cc299b0..f60d1936b7584 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<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,14 @@ 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 (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 @@ class RawPtrRefLambdaCapturesChecker 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 @@ 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 +468,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 +501,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<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); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
