diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp index d8539eaaac49f..1d4e6dd572749 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -263,18 +263,43 @@ class ForwardDeclChecker : public Checker> { void visitCallArg(const Expr *Arg, const ParmVarDecl *Param, const Decl *DeclWithIssue) const { auto *ArgExpr = Arg->IgnoreParenCasts(); - if (auto *InnerCE = dyn_cast(Arg)) { - auto *InnerCallee = InnerCE->getDirectCallee(); - if (InnerCallee && InnerCallee->isInStdNamespace() && - safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { - ArgExpr = InnerCE->getArg(0); - if (ArgExpr) - ArgExpr = ArgExpr->IgnoreParenCasts(); + while (ArgExpr) { + ArgExpr = ArgExpr->IgnoreParenCasts(); + if (auto *InnerCE = dyn_cast(ArgExpr)) { + auto *InnerCallee = InnerCE->getDirectCallee(); + if (InnerCallee && InnerCallee->isInStdNamespace() && + safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { + ArgExpr = InnerCE->getArg(0); + continue; + } + } + if (auto *UO = dyn_cast(ArgExpr)) { + auto OpCode = UO->getOpcode(); + if (OpCode == UO_Deref || OpCode == UO_AddrOf) { + ArgExpr = UO->getSubExpr(); + continue; + } } + break; } + + if (auto *MemberCallExpr = dyn_cast(ArgExpr)) { + if (isOwnerPtrType(MemberCallExpr->getObjectType())) + return; + } + + if (auto *OpCE = dyn_cast(ArgExpr)) { + auto *Method = dyn_cast_or_null(OpCE->getDirectCallee()); + if (Method && isOwnerPtr(safeGetName(Method->getParent()))) { + if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1) + return; + } + } + if (isNullPtr(ArgExpr) || isa(ArgExpr) || isa(ArgExpr)) return; + if (auto *DRE = dyn_cast(ArgExpr)) { if (auto *ValDecl = DRE->getDecl()) { if (isa(ValDecl)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index e5c74bbaf3d6b..17aed16ee54cf 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -138,6 +138,11 @@ bool isCheckedPtr(const std::string &Name) { return Name == "CheckedPtr" || Name == "CheckedRef"; } +bool isOwnerPtr(const std::string &Name) { + return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || + Name == "UniqueRef" || Name == "LazyUniqueRef"; +} + bool isSmartPtrClass(const std::string &Name) { return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || Name == "WeakPtr" || Name == "WeakPtrFactory" || @@ -206,10 +211,7 @@ bool isRetainPtrOrOSPtrType(const clang::QualType T) { } bool isOwnerPtrType(const clang::QualType T) { - return isPtrOfType(T, [](auto Name) { - return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || - Name == "UniqueRef" || Name == "LazyUniqueRef"; - }); + return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); }); } std::optional isUncounted(const QualType T) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 8300a6c051f3e..12e2e2d75b75d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -143,6 +143,10 @@ 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, +/// and unique_ptr. +bool isOwnerPtr(const std::string &Name); + /// \returns true if \p Name is a smart pointer type name, false if not. bool isSmartPtrClass(const std::string &Name); diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm index 104b555c1c41d..8aad838b71b35 100644 --- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm +++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm @@ -11,6 +11,8 @@ Obj* provide_obj_ptr(); void receive_obj_ptr(Obj* p = nullptr); +void receive_obj_ref(Obj&); +void receive_obj_rref(Obj&&); sqlite3* open_db(); void close_db(sqlite3*); @@ -38,6 +40,16 @@ return obj; } +void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr& safeObj, WeakPtr weakObj, std::unique_ptr& uniqObj) { + receive_obj_ref(*obj); + receive_obj_ptr(&*obj); + receive_obj_rref(std::move(otherObj)); + receive_obj_ref(*safeObj.get()); + receive_obj_ptr(weakObj.get()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}} + receive_obj_ref(*uniqObj); +} + Obj&& provide_obj_rval(); void receive_obj_rval(Obj&& p); diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index a49faa1d25336..7055a94753a37 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -25,23 +25,23 @@ namespace std { template class unique_ptr { private: - T *t; + void *t; public: unique_ptr() : t(nullptr) { } unique_ptr(T *t) : t(t) { } ~unique_ptr() { if (t) - delete t; + delete static_cast(t); } template unique_ptr(unique_ptr&& u) : t(u.t) { u.t = nullptr; } - T *get() const { return t; } - T *operator->() const { return t; } - T &operator*() const { return *t; } + T *get() const { return static_cast(t); } + T *operator->() const { return get(); } + T &operator*() const { return *get(); } unique_ptr &operator=(T *) { return *this; } explicit operator bool() const { return !!t; } }; @@ -313,4 +313,90 @@ class UniqueRef { UniqueRef &operator=(T &) { return *this; } }; +class WeakPtrImpl { +private: + void* ptr { nullptr }; + mutable unsigned m_refCount { 0 }; + + template friend class CanMakeWeakPtr; + template friend class WeakPtr; + +public: + template + static Ref create(T& t) + { + return adoptRef(*new WeakPtrImpl(t)); + } + + void ref() const { m_refCount++; } + void deref() const { + m_refCount--; + if (!m_refCount) + delete const_cast(this); + } + + template + T* get() { return static_cast(ptr); } + operator bool() const { return !!ptr; } + void clear() { ptr = nullptr; } + +private: + template + WeakPtrImpl(T* t) + : ptr(static_cast(t)) + { } +}; + +template +class CanMakeWeakPtr { +private: + RefPtr impl; + + template friend class CanMakeWeakPtr; + template friend class WeakPtr; + + Ref createWeakPtrImpl() { + if (!impl) + impl = WeakPtrImpl::create(static_cast(*this)); + return *impl; + } + +public: + ~CanMakeWeakPtr() { + if (!impl) + return; + impl->clear(); + impl = nullptr; + } +}; + +template +class WeakPtr { +private: + RefPtr impl; + +public: + WeakPtr(T& t) { + *this = t; + } + WeakPtr(T* t) { + *this = t; + } + + template + WeakPtr operator=(U& obj) { + impl = obj.createWeakPtrImpl(); + } + + template + WeakPtr operator=(U* obj) { + impl = obj ? obj->createWeakPtrImpl() : nullptr; + } + + T* get() { + return impl ? impl->get() : nullptr; + } + +}; + #endif