[clang] c07d305 - [webkit.UncountedLambdaCapturesChecker] Add the support for WTF::ScopeExit and WTF::makeVisitor (#161926)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 23 23:19:32 PDT 2025
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 at -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 at -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 at -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 at -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 at -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 at -1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ bad_visit(visitor, obj);
+}
More information about the cfe-commits
mailing list