diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 9d34dfd3cea63..b3cd594a0f352 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -142,9 +142,31 @@ bool isASafeCallArg(const Expr *E) { return true; } } + if (isConstOwnerPtrMemberExpr(E)) + return true; // TODO: checker for method calls on non-refcounted objects return isa<CXXThisExpr>(E); } +bool isConstOwnerPtrMemberExpr(const clang::Expr *E) { + if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) { + if (auto *Callee = MCE->getDirectCallee()) { + auto Name = safeGetName(Callee); + if (Name == "get" || Name == "ptr") { + auto *ThisArg = MCE->getImplicitObjectArgument(); + E = ThisArg; + } + } + } + auto *ME = dyn_cast<MemberExpr>(E); + if (!ME) + return false; + auto *D = ME->getMemberDecl(); + if (!D) + return false; + auto T = D->getType(); + return isOwnerPtrType(T) && T.isConstQualified(); +} + } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index e972924e0c523..ddbef0fba0448 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -64,6 +64,9 @@ bool tryToFindPtrOrigin( /// \returns Whether \p E is a safe call arugment. bool isASafeCallArg(const clang::Expr *E); +/// \returns true if E is a MemberExpr accessing a const smart pointer type. +bool isConstOwnerPtrMemberExpr(const clang::Expr *E); + /// \returns name of AST node or empty string. template <typename T> std::string safeGetName(const T *ASTNode) { const auto *const ND = llvm::dyn_cast_or_null<clang::NamedDecl>(ASTNode); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 31bebdb07dbdc..76d3975536e26 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -145,25 +145,37 @@ bool isCtorOfSafePtr(const clang::FunctionDecl *F) { return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F); } -bool isSafePtrType(const clang::QualType T) { +template <typename Predicate> +static bool isPtrOfType(const clang::QualType T, Predicate Pred) { QualType type = T; while (!type.isNull()) { if (auto *elaboratedT = type->getAs<ElaboratedType>()) { type = elaboratedT->desugar(); continue; } - if (auto *specialT = type->getAs<TemplateSpecializationType>()) { - if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) { - auto name = decl->getNameAsString(); - return isRefType(name) || isCheckedPtr(name); - } + auto *SpecialT = type->getAs<TemplateSpecializationType>(); + if (!SpecialT) return false; - } - return false; + auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl(); + if (!Decl) + return false; + return Pred(Decl->getNameAsString()); } return false; } +bool isSafePtrType(const clang::QualType T) { + return isPtrOfType( + T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); }); +} + +bool isOwnerPtrType(const clang::QualType T) { + return isPtrOfType(T, [](auto Name) { + return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || + Name == "UniqueRef" || Name == "LazyUniqueRef"; + }); +} + std::optional<bool> isUncounted(const QualType T) { if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) { if (auto *Decl = Subst->getAssociatedDecl()) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 7c6c0a63f22ab..e9af4d8a0878e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -79,6 +79,10 @@ std::optional<bool> isUncheckedPtr(const clang::QualType T); /// variant, false if not. bool isSafePtrType(const clang::QualType T); +/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or +/// unique_ptr, false if not. +bool isOwnerPtrType(const clang::QualType T); + /// \returns true if \p F creates ref-countable object from uncounted parameter, /// false if not. bool isCtorOfRefCounted(const clang::FunctionDecl *F); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 06f8f43cee815..48c3dc4c94477 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -281,6 +281,9 @@ class RawPtrRefLocalVarsChecker if (isa<IntegerLiteral>(InitArgOrigin)) return true; + if (isConstOwnerPtrMemberExpr(InitArgOrigin)) + 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 0000000000000..76f80a12c1703 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp @@ -0,0 +1,63 @@ +// 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 + +namespace call_args_const_unique_ptr { + +class Foo { +public: + Foo(); + void bar(); + +private: + const std::unique_ptr<CheckedObj> m_obj1; + std::unique_ptr<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_unique_ptr 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 0000000000000..b3296507a5c92 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp @@ -0,0 +1,86 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +namespace std { +} + +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 + +namespace call_args_const_unique_ptr { + +class Foo { +public: + Foo(); + void bar(); + +private: + const std::unique_ptr<RefCountable> m_obj1; + std::unique_ptr<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_unique_ptr + +namespace call_args_const_unique_ref { + +class Foo { +public: + Foo(); + void bar(); + +private: + const UniqueRef<RefCountable> m_obj1; + UniqueRef<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_unique_ref 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 0000000000000..e52d1e735f637 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp @@ -0,0 +1,72 @@ +// 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 + +namespace call_args_const_unique_ptr { + +class Foo { +public: + Foo(); + void bar(); + +private: + const std::unique_ptr<CheckedObj> m_obj1; + std::unique_ptr<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 call_args_const_unique_ptr 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 0000000000000..03d16285f88b5 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp @@ -0,0 +1,72 @@ +// 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 + +namespace call_args_const_unique_ptr { + +class Foo { +public: + Foo(); + void bar(); + +private: + const std::unique_ptr<RefCountable> m_obj1; + std::unique_ptr<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 call_args_const_unique_ptr diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 9c9326f0f11cf..82dc4a6a5b3c8 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; } @@ -200,4 +197,52 @@ class RefCountableAndCheckable { int trivial() { return 0; } }; +template <typename T> +class UniqueRef { +private: + T *t; + +public: + UniqueRef(T &t) : t(&t) { } + ~UniqueRef() { + if (t) + delete t; + } + template <typename U> UniqueRef(UniqueRef<U>&& u) + : t(u.t) + { + u.t = nullptr; + } + T &get() const { return *t; } + T *operator->() const { return t; } + UniqueRef &operator=(T &) { return *this; } +}; + +namespace std { + +template <typename T> +class unique_ptr { +private: + T *t; + +public: + unique_ptr() : t(nullptr) { } + unique_ptr(T *t) : t(t) { } + ~unique_ptr() { + if (t) + delete t; + } + template <typename U> unique_ptr(unique_ptr<U>&& u) + : t(u.t) + { + u.t = nullptr; + } + T *get() const { return t; } + T *operator->() const { return t; } + T &operator*() { return *t; } + unique_ptr &operator=(T *) { return *this; } +}; + +}; + #endif