[clang] Detect a return value of Ref<T> & RefPtr<T> (PR #81580)

[Ryosuke Niwa via cfe-commits]

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81580

>From 24fc75756094d36e72c69805c9d476d8144ed869 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 13 Feb 2024 00:35:36 -0800
Subject: [PATCH 1/2] [alpha.webkit.UncountedCallArgsChecker] Detect a return
 value of `Ref<T>` & `RefPtr<T>`

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>`.
---
 .../Checkers/WebKit/ASTUtils.cpp              |  6 +++++
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 20 ++++++++++++++++
 .../Checkers/WebKit/PtrTypesSemantics.h       |  3 +++
 .../call-args-protected-return-value.cpp      | 23 +++++++++++++++++++
 .../Analysis/Checkers/WebKit/mock-types.h     |  1 +
 5 files changed, 53 insertions(+)
 create mode 100644 clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp

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..70c94dd0e960ab 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.
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/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/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> &) {

>From b32d7ea03607d724b302db666a03247f5c100b8c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 13 Feb 2024 19:27:57 -0800
Subject: [PATCH 2/2] Address review comment by eliminating calls to
 getTypePtrOrNull in favor of getAs.

---
 .../Checkers/WebKit/PtrTypesSemantics.cpp              | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 70c94dd0e960ab..843c4d5350709e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -120,13 +120,13 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
 
 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();
+  QualType type = F->getReturnType();
+  while (!type.isNull()) {
+    if (auto *elaboratedT = type->getAs<ElaboratedType>()) {
+      type = elaboratedT->desugar();
       continue;
     }
-    if (auto *specialT = dyn_cast<TemplateSpecializationType>(type)) {
+    if (auto *specialT = type->getAs<TemplateSpecializationType>()) {
       if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) {
         auto name = decl->getNameAsString();
         return name == "Ref" || name == "RefPtr";