[clang] bf005a1 - [alpha.webkit.UnretainedCallArgsChecker] Emit a warning for a non-const RetainPtr member (#184243)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 13:52:50 PDT 2026
Author: Ryosuke Niwa
Date: 2026-05-27T13:52:44-07:00
New Revision: bf005a1227a4822c7c2535dd5f5f3626fbe441b2
URL: https://github.com/llvm/llvm-project/commit/bf005a1227a4822c7c2535dd5f5f3626fbe441b2
DIFF: https://github.com/llvm/llvm-project/commit/bf005a1227a4822c7c2535dd5f5f3626fbe441b2.diff
LOG: [alpha.webkit.UnretainedCallArgsChecker] Emit a warning for a non-const RetainPtr member (#184243)
This PR fixes a bug in UnretainedCallArgsChecker that it wouldn't emit a
warning when calling a function with the return value of a getter of a
RetainPtr non-const member variable even if such a member variable could
be mutated during such a function call.
The bug caused by tryToFindPtrOrigin treating any call of a getter on a
smart pointer member variable as safe. Fixed the bug by limiting this to
only when the variable is a local variable or a function parameter.
In addition, this PR fixes a bug in WebKit checkers that it would
erroneously emit a warning when calling a getter on a const RetainPtr
member variable beacuse isOwnerPtr was returning false for RetainPtr.
This false negative was previously masked / hidden by the false positive
fixed in this PR.
---------
Co-authored-by: Balázs Benics <benicsbalazs at gmail.com>
Added:
clang/test/Analysis/Checkers/WebKit/unretained-call-args-member.mm
Modified:
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/test/Analysis/Checkers/WebKit/call-args.cpp
clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 8f5f2abf17f02..7b4d231620314 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -108,9 +108,14 @@ bool tryToFindPtrOrigin(
if (auto *decl = memberCall->getMethodDecl()) {
std::optional<bool> IsGetterOfRefCt = isGetterOfSafePtr(decl);
if (IsGetterOfRefCt && *IsGetterOfRefCt) {
- E = memberCall->getImplicitObjectArgument();
- if (StopAtFirstRefCountedObj) {
- return callback(E, true);
+ E = memberCall->getImplicitObjectArgument()->IgnoreParenCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *Decl = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
+ if (Decl->isLocalVarDeclOrParm()) {
+ if (StopAtFirstRefCountedObj)
+ return callback(E, true);
+ }
+ }
}
continue;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index ab431af8ff391..f1515701cc6f3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -139,8 +139,8 @@ bool isCheckedPtr(const std::string &Name) {
}
bool isOwnerPtr(const std::string &Name) {
- return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
- Name == "UniqueRef" || Name == "LazyUniqueRef";
+ return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
+ Name == "unique_ptr" || Name == "UniqueRef" || Name == "LazyUniqueRef";
}
static bool isWeakPtrClass(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 fc829c45da9c5..f15991134c58a 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -500,6 +500,48 @@ namespace call_with_adopt_ref {
}
}
+namespace call_on_member {
+
+ class SomeObj {
+ public:
+ static Ref<SomeObj> create() { return adoptRef(*new SomeObj); }
+
+ void ref() const;
+ void deref() const;
+
+ void doWork() {
+ m_obj->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ m_obj.get()->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ m_constObj->method();
+ }
+
+ void localWork() {
+ RefPtr obj = provide();
+ obj->method();
+ obj.get()->method();
+ }
+
+ void argWork(RefPtr<RefCountable> arg) {
+ arg->method();
+ arg.get()->method();
+ }
+
+ void temporaryWork() {
+ RefPtr { provide() }->method();
+ RefPtr { provide() }.get()->method();
+ }
+
+ void work();
+
+ private:
+ RefPtr<RefCountable> m_obj;
+ const RefPtr<RefCountable> m_constObj;
+ };
+
+}
+
namespace call_with_weak_ptr {
class RefCountableWithWeakPtr : public RefCountable, public CanMakeWeakPtr<RefCountableWithWeakPtr> {
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 503d11bea3089..ff6c133940ca9 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -447,6 +447,9 @@ template<typename T> static inline void releaseOSObject(T ptr)
template<typename T> class OSObjectPtr {
public:
+ using ValueType = typename RemovePointer<T>::Type;
+ using PtrType = ValueType*;
+
OSObjectPtr()
: m_ptr(nullptr)
{
@@ -460,6 +463,7 @@ template<typename T> class OSObjectPtr {
T get() const { return m_ptr; }
+ operator PtrType() const { return m_ptr; }
explicit operator bool() const { return m_ptr; }
bool operator!() const { return !m_ptr; }
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-member.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-member.mm
new file mode 100644
index 0000000000000..8f48927a0c62a
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-member.mm
@@ -0,0 +1,184 @@
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedCallArgsChecker -verify %s
+
+#include "objc-mock-types.h"
+
+void consume_cf(CFMutableArrayRef);
+void consume_obj(SomeObj *);
+
+namespace call_args_const_retainptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RetainPtr<SomeObj> m_constObj;
+ RetainPtr<SomeObj> m_obj;
+};
+
+void Foo::bar() {
+ [m_constObj doWork]; // no-warning
+ [m_obj doWork]; // expected-warning{{Receiver is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_member
+
+namespace call_args_const_retainptr_cf_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RetainPtr<CFMutableArrayRef> m_cf1;
+ RetainPtr<CFMutableArrayRef> m_cf2;
+};
+
+void Foo::bar() {
+ consume_cf(m_cf1.get()); // no-warning
+ consume_cf(m_cf2.get()); // expected-warning{{Call argument is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_cf_member
+
+namespace call_args_const_retainptr_struct_member {
+
+struct Bar {
+ Bar();
+ void baz();
+
+ const RetainPtr<SomeObj> m_constObj;
+ RetainPtr<SomeObj> m_obj;
+};
+
+void Bar::baz() {
+ [m_constObj doWork]; // no-warning
+ [m_obj doWork]; // expected-warning{{Receiver is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_struct_member
+
+namespace call_args_const_retainptr_cf_struct_member {
+
+struct Bar {
+ Bar();
+ void baz();
+
+ const RetainPtr<CFMutableArrayRef> m_cf1;
+ RetainPtr<CFMutableArrayRef> m_cf2;
+};
+
+void Bar::baz() {
+ consume_cf(m_cf1.get()); // no-warning
+ consume_cf(m_cf2.get()); // expected-warning{{Call argument is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_cf_struct_member
+
+namespace call_args_const_retainptr_get_as_objc_arg {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RetainPtr<SomeObj> m_constObj;
+ RetainPtr<SomeObj> m_obj;
+};
+
+void Foo::bar() {
+ consume_obj(m_constObj.get()); // no-warning
+ consume_obj(m_obj.get()); // expected-warning{{Call argument is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_get_as_objc_arg
+
+namespace call_args_const_retainptr_implicit_conv_arg {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RetainPtr<SomeObj> m_constObj;
+ RetainPtr<SomeObj> m_obj;
+};
+
+void Foo::bar() {
+ consume_obj(m_constObj); // no-warning
+ consume_obj(m_obj); // expected-warning{{Call argument is unretained and unsafe}}
+}
+
+} // namespace call_args_const_retainptr_implicit_conv_arg
+
+namespace call_args_const_osobjectptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const OSObjectPtr<SomeObj *> m_constObj;
+ OSObjectPtr<SomeObj *> m_obj;
+};
+
+void Foo::bar() {
+ consume_obj(m_constObj.get()); // no-warning
+ consume_obj(m_obj.get()); // expected-warning{{Call argument is unretained and unsafe}}
+}
+
+} // namespace call_args_const_osobjectptr_member
+
+namespace call_args_const_osobjectptr_receiver {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const OSObjectPtr<SomeObj *> m_constObj;
+ OSObjectPtr<SomeObj *> m_obj;
+};
+
+void Foo::bar() {
+ [m_constObj doWork]; // no-warning
+ [m_obj doWork]; // expected-warning{{Receiver is unretained and unsafe}}
+}
+
+} // namespace call_args_const_osobjectptr_receiver
+
+namespace call_args_retainptr_local {
+
+void testLocal(SomeObj *input) {
+ RetainPtr<SomeObj> localObj = input;
+ [localObj doWork]; // no-warning
+ consume_obj(localObj.get()); // no-warning
+ consume_cf(RetainPtr<CFMutableArrayRef>().get()); // no-warning
+}
+
+} // namespace call_args_retainptr_local
+
+namespace call_args_retainptr_protected_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ RetainPtr<SomeObj> m_obj;
+};
+
+void Foo::bar() {
+ auto protectedObj = m_obj;
+ [protectedObj doWork]; // no-warning (local copy is safe)
+}
+
+} // namespace call_args_retainptr_protected_member
More information about the cfe-commits
mailing list