[clang] Detect a return value of Ref<T> & RefPtr<T> (PR #81580)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 13 00:39:34 PST 2024
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
More information about the cfe-commits
mailing list