From d044a85da745c49474308f60278159748f09f1f9 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 16 Oct 2025 15:15:39 -0700 Subject: [PATCH 1/3] [alpha.webkit.UnretainedCallArgsChecker] Treat getter on a dependent smart pointer type as safe (#161025) Add the support for recognizing smart pointer type appearing as the type of the object pointer in CXXDependentScopeMemberExpr. (cherry picked from commit 65c895dfe084860847e9e220ff9f1b283ebcb289) --- .../Checkers/WebKit/ASTUtils.cpp | 8 +++++ .../Checkers/WebKit/unretained-call-args.mm | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 419d2631fef81..84adbf318e9f8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -173,6 +173,14 @@ bool tryToFindPtrOrigin( if (isSingleton(E->getFoundDecl())) return callback(E, true); } + + if (auto *MemberExpr = dyn_cast(CalleeE)) { + auto *Base = MemberExpr->getBase(); + auto MemberName = MemberExpr->getMember().getAsString(); + bool IsGetter = MemberName == "get" || MemberName == "ptr"; + if (Base && isSafePtrType(Base->getType()) && IsGetter) + return callback(E, true); + } } // Sometimes, canonical type erroneously turns Ref into T. diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 4f231ee8b1c84..8bef24f93ceed 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -596,6 +596,35 @@ void foo() { } // namespace sel_string +namespace template_function { + +class Base { +public: + virtual ~Base() = default; + void send(dispatch_queue_t) const; + void ref() const; + void deref() const; +}; + +template +class Derived : public Base { +public: + virtual ~Derived() = default; + + void send(typename Traits::MessageType) const; + + virtual OSObjectPtr msg(typename Traits::MessageType) const = 0; +}; + +template +void Derived::send(typename Traits::MessageType messageType) const +{ + OSObjectPtr dictionary = msg(messageType); + Base::send(dictionary.get()); +} + +} // namespace template_function + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; From a9734be8b9d8f00d6682a30b743c05dff35a397a Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 23 Oct 2025 23:19:28 -0700 Subject: [PATCH 2/3] [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. (cherry picked from commit c07d305718744917ba5dc6693322e13a5c2314df) --- .../WebKit/RawPtrRefLambdaCapturesChecker.cpp | 107 ++++++++++++- .../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 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 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,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(CalleeE->IgnoreParenCasts())) { if (auto *Callee = dyn_cast_or_null(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(Arg->IgnoreParenCasts()); + if (!DRE) + return false; + auto *VD = dyn_cast(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(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(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 + 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); +} From b7ca0520b1ff072c623b48d456a846ff50a94f22 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Wed, 5 Nov 2025 14:59:50 -0800 Subject: [PATCH 3/3] [webkit.UncountedLambdaCapturesChecker] Assertion failure with coroutine body (#165650) Fix the assertion failure in TrivialFunctionAnalysis::isTrivialImpl with a coroutine body by caching the result with WithCachedResult. (cherry picked from commit 2d5170594147b42a37698760d6e0194eec4f1390) --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 4 ++++ ...ted-lambda-captures-co_await-assertion-failure.cpp | 11 +++++++++++ 2 files changed, 15 insertions(+) create mode 100644 clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-co_await-assertion-failure.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 1a7024d559b8d..a72ffa25e33a0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -591,6 +591,10 @@ class TrivialFunctionAnalysisVisitor return WithCachedResult(CS, [&]() { return VisitChildren(CS); }); } + bool VisitCoroutineBodyStmt(const CoroutineBodyStmt *CBS) { + return WithCachedResult(CBS, [&]() { return VisitChildren(CBS); }); + } + bool VisitReturnStmt(const ReturnStmt *RS) { // A return statement is allowed as long as the return value is trivial. if (auto *RV = RS->getRetValue()) diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-co_await-assertion-failure.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-co_await-assertion-failure.cpp new file mode 100644 index 0000000000000..a67f45700cd10 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-co_await-assertion-failure.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -std=c++20 -verify %s +// expected-no-diagnostics + +template +void foo(Arg&& arg) +{ + [&]{ + co_await [&](auto&&... args) { + }(arg); + }(); +}