llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> Treat const Ref, RefPtr, CheckedRef, CheckedPtr member variables as safe pointer origin in WebKit's local variable and call arguments checkers. --- Full diff: https://github.com/llvm/llvm-project/pull/115594.diff 7 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+6) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp (+8) - (added) clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp (+43) - (added) clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp (+43) - (added) clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp (+50) - (added) clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp (+50) - (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+11-14) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 9d34dfd3cea636..ad2be6f6793cea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -142,6 +142,12 @@ bool isASafeCallArg(const Expr *E) { return true; } } + if (auto *ME = dyn_cast<MemberExpr>(E)) { + if (auto *D = ME->getMemberDecl()) { + auto T = D->getType(); + return isSafePtrType(T) && T.isConstQualified(); + } + } // TODO: checker for method calls on non-refcounted objects return isa<CXXThisExpr>(E); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 06f8f43cee8151..be954ad3026c22 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -281,6 +281,14 @@ class RawPtrRefLocalVarsChecker if (isa<IntegerLiteral>(InitArgOrigin)) return true; + if (auto *ME = dyn_cast<MemberExpr>(InitArgOrigin)) { + if (auto *D = ME->getMemberDecl()) { + auto T = D->getType(); + if (isSafePtrType(T) && T.isConstQualified()) + return true; + } + } + if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) { if (auto *MaybeGuardian = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) { diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp new file mode 100644 index 00000000000000..6a54ac7b2ca791 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp @@ -0,0 +1,43 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s + +#include "mock-types.h" + +namespace call_args_const_checkedptr_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const CheckedPtr<CheckedObj> m_obj1; + CheckedPtr<CheckedObj> m_obj2; +}; + +void Foo::bar() { + m_obj1->method(); + m_obj2->method(); + // expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}} +} + +} // namespace call_args_const_checkedptr_member + +namespace call_args_const_checkedref_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const CheckedRef<CheckedObj> m_obj1; + CheckedRef<CheckedObj> m_obj2; +}; + +void Foo::bar() { + m_obj1->method(); + m_obj2->method(); + // expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}} +} + +} // namespace call_args_const_checkedref_member diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp new file mode 100644 index 00000000000000..33af5ad9014de4 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp @@ -0,0 +1,43 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +namespace call_args_const_refptr_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const RefPtr<RefCountable> m_obj1; + RefPtr<RefCountable> m_obj2; +}; + +void Foo::bar() { + m_obj1->method(); + m_obj2->method(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} +} + +} // namespace call_args_const_refptr_member + +namespace call_args_const_ref_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const Ref<RefCountable> m_obj1; + Ref<RefCountable> m_obj2; +}; + +void Foo::bar() { + m_obj1->method(); + m_obj2->method(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} +} + +} // namespace call_args_const_ref_member diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp new file mode 100644 index 00000000000000..3deef66c271096 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp @@ -0,0 +1,50 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedLocalVarsChecker -verify %s + +#include "mock-types.h" +#include "mock-system-header.h" + +void someFunction(); + +namespace local_vars_const_checkedptr_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const CheckedPtr<CheckedObj> m_obj1; + CheckedPtr<CheckedObj> m_obj2; +}; + +void Foo::bar() { + auto* obj1 = m_obj1.get(); + obj1->method(); + auto* obj2 = m_obj2.get(); + // expected-warning@-1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}} + obj2->method(); +} + +} // namespace local_vars_const_checkedptr_member + +namespace local_vars_const_checkedref_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const CheckedRef<CheckedObj> m_obj1; + CheckedRef<CheckedObj> m_obj2; +}; + +void Foo::bar() { + auto& obj1 = m_obj1.get(); + obj1.method(); + auto& obj2 = m_obj2.get(); + // expected-warning@-1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}} + obj2.method(); +} + +} // namespace local_vars_const_ref_member diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp new file mode 100644 index 00000000000000..6d6a7718244002 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp @@ -0,0 +1,50 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedLocalVarsChecker -verify %s + +#include "mock-types.h" +#include "mock-system-header.h" + +void someFunction(); + +namespace local_vars_const_refptr_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const RefPtr<RefCountable> m_obj1; + RefPtr<RefCountable> m_obj2; +}; + +void Foo::bar() { + auto* obj1 = m_obj1.get(); + obj1->method(); + auto* obj2 = m_obj2.get(); + // expected-warning@-1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + obj2->method(); +} + +} // namespace local_vars_const_refptr_member + +namespace local_vars_const_ref_member { + +class Foo { +public: + Foo(); + void bar(); + +private: + const Ref<RefCountable> m_obj1; + Ref<RefCountable> m_obj2; +}; + +void Foo::bar() { + auto& obj1 = m_obj1.get(); + obj1.method(); + auto& obj2 = m_obj2.get(); + // expected-warning@-1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + obj2.method(); +} + +} // namespace local_vars_const_ref_member diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 9c9326f0f11cfb..8f84589477bff1 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -66,11 +66,10 @@ template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTra t = o.t; o.t = tmp; } - T &get() { return *PtrTraits::unwrap(t); } - T *ptr() { return PtrTraits::unwrap(t); } - T *operator->() { return PtrTraits::unwrap(t); } - operator const T &() const { return *PtrTraits::unwrap(t); } - operator T &() { return *PtrTraits::unwrap(t); } + T &get() const { return *PtrTraits::unwrap(t); } + T *ptr() const { return PtrTraits::unwrap(t); } + T *operator->() const { return PtrTraits::unwrap(t); } + operator T &() const { return *PtrTraits::unwrap(t); } T* leakRef() { return PtrTraits::exchange(t, nullptr); } }; @@ -102,9 +101,8 @@ template <typename T> struct RefPtr { t = o.t; o.t = tmp; } - T *get() { return t; } - T *operator->() { return t; } - const T *operator->() const { return t; } + T *get() const { return t; } + T *operator->() const { return t; } T &operator*() { return *t; } RefPtr &operator=(T *t) { RefPtr o(t); @@ -149,9 +147,9 @@ template <typename T> struct CheckedRef { CheckedRef(T &t) : t(&t) { t.incrementCheckedPtrCount(); } CheckedRef(const CheckedRef &o) : t(o.t) { if (t) t->incrementCheckedPtrCount(); } ~CheckedRef() { if (t) t->decrementCheckedPtrCount(); } - T &get() { return *t; } - T *ptr() { return t; } - T *operator->() { return t; } + T &get() const { return *t; } + T *ptr() const { return t; } + T *operator->() const { return t; } operator const T &() const { return *t; } operator T &() { return *t; } }; @@ -174,9 +172,8 @@ template <typename T> struct CheckedPtr { if (t) t->decrementCheckedPtrCount(); } - T *get() { return t; } - T *operator->() { return t; } - const T *operator->() const { return t; } + T *get() const { return t; } + T *operator->() const { return t; } T &operator*() { return *t; } CheckedPtr &operator=(T *) { return *this; } operator bool() const { return t; } `````````` </details> https://github.com/llvm/llvm-project/pull/115594 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
