https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/184563
>From 45db3884aa5a83eb63bf7d3a81e2a2c15e04dc9f Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Tue, 3 Mar 2026 23:55:08 -0800 Subject: [PATCH 1/2] [alpha.webkit.UncountedCallArgsChecker] Emit a warning for a WeakPtr argument. This PR fixes a bug in UncountedCallArgsChecker that it would not emit a warning when a function is called with a WeakPtr local variable as an argument. We normally don't generate a warning for a local variable passed to a function argument in UncountedCallArgsChecker as the variable may have a guardian in an outer scope but only UncountedLocalVarsChecker is capable of locating one. So rather than generating a warning in UncountedCallArgsChecker directly, we rely on UncountedLocalVarsChecker to generate a warning for the local variable. This all falls apart in the case of a WeakPtr local variable because a WeakPtr is explicitly allowed as a local variable by UncountedLocalVarsChecker. So, this PR fixes the bug by detecting this exact scenario (a WeakPtr local variable used as a function argument), and generate a warning directly in UncountedCallArgsChecker. Also added the support for recognizing more weak smart pointer types in WebKit. --- .../Checkers/WebKit/ASTUtils.cpp | 18 ++++++++++++++---- .../Checkers/WebKit/PtrTypesSemantics.cpp | 17 +++++++++++++++-- .../Checkers/WebKit/PtrTypesSemantics.h | 6 +++++- .../Analysis/Checkers/WebKit/call-args.cpp | 18 ++++++++++++++++++ .../test/Analysis/Checkers/WebKit/mock-types.h | 3 +++ 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 4a9a3be2b6614..f2ee9c742f4b8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -239,10 +239,19 @@ bool tryToFindPtrOrigin( bool isASafeCallArg(const Expr *E) { assert(E); + auto IsCheckedLocalVarOrParam = [](const VarDecl *Decl) { + if (auto *Type = Decl->getType().getTypePtrOrNull()) { + if (auto *CXXRD = Type->getAsCXXRecordDecl()) { + if (isWeakPtr(CXXRD)) + return false; + } + } + return Decl->isLocalVarDeclOrParm(); + }; if (auto *Ref = dyn_cast<DeclRefExpr>(E)) { auto *FoundDecl = Ref->getFoundDecl(); if (auto *D = dyn_cast_or_null<VarDecl>(FoundDecl)) { - if (isa<ParmVarDecl>(D) || D->isLocalVarDecl()) + if (IsCheckedLocalVarOrParam(D)) return true; if (auto *ImplicitP = dyn_cast<ImplicitParamDecl>(D)) { auto Kind = ImplicitP->getParameterKind(); @@ -253,9 +262,10 @@ bool isASafeCallArg(const Expr *E) { return true; } } else if (auto *BD = dyn_cast_or_null<BindingDecl>(FoundDecl)) { - VarDecl *VD = BD->getHoldingVar(); - if (VD && (isa<ParmVarDecl>(VD) || VD->isLocalVarDecl())) - return true; + if (VarDecl *VD = BD->getHoldingVar()) { + if (IsCheckedLocalVarOrParam(VD)) + return true; + } } } if (isa<CXXTemporaryObjectExpr>(E)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 7a7e22ead74a8..a95bf6faaab52 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -143,11 +143,17 @@ bool isOwnerPtr(const std::string &Name) { Name == "UniqueRef" || Name == "LazyUniqueRef"; } +static bool isWeakPtrClass(const std::string &Name) { + return Name == "WeakPtr" || Name == "SingleThreadPackedWeakPtr" || + Name == "SingleThreadWeakPtr" || Name == "ThreadSafeWeakPtr" || + Name == "ThreadSafeWeakOrStrongPtr" || Name == "InlineWeakPtr"; +} + bool isSmartPtrClass(const std::string &Name) { return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || - Name == "WeakPtr" || Name == "WeakPtrFactory" || + isWeakPtrClass(Name) || Name == "WeakPtrFactory" || Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" || - Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" || + Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakOrStrongPtr" || Name == "ThreadSafeWeakPtrControlBlock" || Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr"; @@ -399,6 +405,13 @@ bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) { return false; } +bool isWeakPtr(const CXXRecordDecl *R) { + assert(R); + if (auto *TmplR = R->getTemplateInstantiationPattern()) + return isWeakPtrClass(safeGetName(TmplR)); + return false; +} + bool isSmartPtr(const CXXRecordDecl *R) { assert(R); if (auto *TmplR = R->getTemplateInstantiationPattern()) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index bcb4eee16bd2c..d26a18c456c77 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -60,6 +60,10 @@ bool isCheckedPtr(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is a RetainPtr, false if not. bool isRetainPtrOrOSPtr(const clang::CXXRecordDecl *Class); +/// \returns true if \p Class is a weak smart pointer (WeakPtr, InlineWeakPtr, +/// etc...), false if not. +bool isWeakPtr(const clang::CXXRecordDecl *Class); + /// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...), /// false if not. bool isSmartPtr(const clang::CXXRecordDecl *Class); @@ -139,7 +143,7 @@ bool isCheckedPtr(const std::string &Name); /// \returns true if \p Name is RetainPtr or its variant, false if not. bool isRetainPtrOrOSPtr(const std::string &Name); -/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr, +/// \returns true if \p Name is an owning smart pointer such as Ref, CheckedPtr, /// and unique_ptr. bool isOwnerPtr(const std::string &Name); diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index 1a8bde29080ac..c74e7ab7b9d7e 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -440,3 +440,21 @@ namespace call_with_adopt_ref { adoptRef(new Obj)->method(); } } + +namespace call_with_weak_ptr { + + class RefCountableWithWeakPtr : public RefCountable, public CanMakeWeakPtr<RefCountableWithWeakPtr> { + }; + + RefCountableWithWeakPtr* provide(); + void consume(RefCountableWithWeakPtr*); + + void foo() { + WeakPtr weakPtr = provide(); + consume(weakPtr); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + weakPtr->method(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + } + +} diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index c139a5cb13de7..a5d9ed5ae174d 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -415,6 +415,9 @@ class WeakPtr { return *this; } + T* operator->() { return get(); } + operator T*() { return get(); } + T* get() { return impl ? impl->get<T>() : nullptr; } >From cb64aa72918dc2d7d79de6f1efc1d102c3f49df5 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Wed, 4 Mar 2026 00:07:40 -0800 Subject: [PATCH 2/2] Fix formatting. --- .../lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index f2ee9c742f4b8..ef2ae63ff95fa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -240,13 +240,13 @@ bool tryToFindPtrOrigin( bool isASafeCallArg(const Expr *E) { assert(E); auto IsCheckedLocalVarOrParam = [](const VarDecl *Decl) { - if (auto *Type = Decl->getType().getTypePtrOrNull()) { - if (auto *CXXRD = Type->getAsCXXRecordDecl()) { - if (isWeakPtr(CXXRD)) - return false; - } + if (auto *Type = Decl->getType().getTypePtrOrNull()) { + if (auto *CXXRD = Type->getAsCXXRecordDecl()) { + if (isWeakPtr(CXXRD)) + return false; } - return Decl->isLocalVarDeclOrParm(); + } + return Decl->isLocalVarDeclOrParm(); }; if (auto *Ref = dyn_cast<DeclRefExpr>(E)) { auto *FoundDecl = Ref->getFoundDecl(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
