[clang] [webkit.UncountedLambdaCapturesChecker] Add the support for WTF::ScopeExit and WTF::makeVisitor (PR #161926)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 3 17:07:19 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/161926.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp (+88-8)
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp (+151)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 033eb8cc299b0..460d9dbf1af5e 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,9 +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()) {
if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
@@ -280,7 +359,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,8 +457,9 @@ class RawPtrRefLambdaCapturesChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
- void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
- bool ignoreParamVarDecl = false) const {
+ void visitLambdaExpr(const LambdaExpr *L, bool shouldCheckThis,
+ const QualType T,
+ bool ignoreParamVarDecl = false) const {
if (TFA.isTrivial(L->getBody()))
return;
for (const LambdaCapture &C : L->captures()) {
@@ -410,7 +490,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);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/161926
More information about the cfe-commits
mailing list