llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> This PR makes the checker not emit warning when a function is called with a return value of another function when the return value is of type Ref<T> or RefPtr<T>. --- Full diff: https://github.com/llvm/llvm-project/pull/81580.diff 7 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+6) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+23-2) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+3) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+6-5) - (added) clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp (+23) - (renamed) clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp (+21) - (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+1) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 4526fac64735bf..b76c0551c77bb0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -19,6 +19,10 @@ namespace clang { std::pair<const Expr *, bool> tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) { while (E) { + if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) { + E = tempExpr->getSubExpr(); + continue; + } if (auto *cast = dyn_cast<CastExpr>(E)) { if (StopAtFirstRefCountedObj) { if (auto *ConversionFunc = @@ -62,6 +66,8 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) { E = call->getArg(0); continue; } + if (isReturnValueRefCounted(callee)) + return {E, true}; if (isPtrConversion(callee)) { E = call->getArg(0); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index d2b66341058000..9e1035ac13ce31 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -118,6 +118,26 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) { || FunctionName == "Identifier"; } +bool isReturnValueRefCounted(const clang::FunctionDecl *F) { + assert(F); + auto *type = F->getReturnType().getTypePtrOrNull(); + while (type) { + if (auto *elaboratedT = dyn_cast<ElaboratedType>(type)) { + type = elaboratedT->desugar().getTypePtrOrNull(); + continue; + } + if (auto *specialT = dyn_cast<TemplateSpecializationType>(type)) { + if (auto* decl = specialT->getTemplateName().getAsTemplateDecl()) { + auto name = decl->getNameAsString(); + return name == "Ref" || name == "RefPtr"; + } + return false; + } + return false; + } + return false; +} + std::optional<bool> isUncounted(const CXXRecordDecl* Class) { // Keep isRefCounted first as it's cheaper. @@ -192,8 +212,9 @@ bool isPtrConversion(const FunctionDecl *F) { // FIXME: check # of params == 1 const auto FunctionName = safeGetName(F); if (FunctionName == "getPtr" || FunctionName == "WeakPtr" || - FunctionName == "dynamicDowncast" - || FunctionName == "downcast" || FunctionName == "bitwise_cast") + FunctionName == "dynamicDowncast" || FunctionName == "downcast" || + FunctionName == "checkedDowncast" || + FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast") return true; return false; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 45b21cc0918443..c2c5b74442ba43 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -50,6 +50,9 @@ std::optional<bool> isUncountedPtr(const clang::Type* T); /// false if not. bool isCtorOfRefCounted(const clang::FunctionDecl *F); +/// \returns true if \p F returns a ref-counted object, false if not. +bool isReturnValueRefCounted(const clang::FunctionDecl *F); + /// \returns true if \p M is getter of a ref-counted class, false if not. std::optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl* Method); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 31ccae8b097b89..cc4585a0b0eeff 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -148,13 +148,14 @@ class UncountedCallArgsChecker auto name = safeGetName(Callee); if (name == "adoptRef" || name == "getPtr" || name == "WeakPtr" || - name == "dynamicDowncast" || name == "downcast" || name == "bitwise_cast" || - name == "is" || name == "equal" || name == "hash" || - name == "isType" + name == "dynamicDowncast" || name == "downcast" || + name == "checkedDowncast" || name == "uncheckedDowncast" || + name == "bitwise_cast" || name == "is" || name == "equal" || + name == "hash" || name == "isType" || // FIXME: Most/all of these should be implemented via attributes. - || name == "equalIgnoringASCIICase" || + name == "equalIgnoringASCIICase" || name == "equalIgnoringASCIICaseCommon" || - name == "equalIgnoringNullity") + name == "equalIgnoringNullity" || name == "toString") return true; return false; diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp new file mode 100644 index 00000000000000..1c4b3df211b1e3 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s +// expected-no-diagnostics + +#include "mock-types.h" + +class RefCounted { +public: + void ref(); + void deref(); +}; + +class Object { +public: + void someFunction(RefCounted&); +}; + +RefPtr<Object> object(); +RefPtr<RefCounted> protectedTargetObject(); + +void testFunction() { + if (RefPtr obj = object()) + obj->someFunction(*protectedTargetObject()); +} diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp similarity index 55% rename from clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp rename to clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp index 28156623d9a0fd..a87446564870cd 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp @@ -23,13 +23,34 @@ class OtherObject { Derived* obj(); }; +class String { +}; + template<typename Target, typename Source> inline Target* dynamicDowncast(Source* source) { return static_cast<Target*>(source); } +template<typename Target, typename Source> +inline Target* checkedDowncast(Source* source) +{ + return static_cast<Target*>(source); +} + +template<typename Target, typename Source> +inline Target* uncheckedDowncast(Source* source) +{ + return static_cast<Target*>(source); +} + +template<typename... Types> +String toString(const Types&... values); + void foo(OtherObject* other) { dynamicDowncast<SubDerived>(other->obj()); + checkedDowncast<SubDerived>(other->obj()); + uncheckedDowncast<SubDerived>(other->obj()); + toString(other->obj()); } diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 5f570b8bee8cb8..814e0944145992 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -20,6 +20,7 @@ template <typename T> struct RefPtr { T *operator->() { return t; } T &operator*() { return *t; } RefPtr &operator=(T *) { return *this; } + operator bool() { return t; } }; template <typename T> bool operator==(const RefPtr<T> &, const RefPtr<T> &) { `````````` </details> https://github.com/llvm/llvm-project/pull/81580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
