Skip to content

[Analyzer] Support RefAllowingPartiallyDestroyed and RefPtrAllowingPartiallyDestroyed #82209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 19, 2024

This PR adds the support for WebKit's RefAllowingPartiallyDestroyed and RefPtrAllowingPartiallyDestroyed, which are smart pointer types which may be used after the destructor had started running.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 19, 2024

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the support for WebKit's RefAllowingPartiallyDestroyed and RefPtrAllowingPartiallyDestroyed, which are smart pointer types which may be used after the destructor had started running.


Full diff: https://github.com/llvm/llvm-project/pull/82209.diff

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+14-13)
  • (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+2-1)
  • (added) clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp (+46)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 6f236db0474079..1b349d48a9f881 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -103,15 +103,18 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
   return hasRef && hasDeref;
 }
 
+bool isRefType(const std::string& name) {
+  return name == "Ref" || name == "RefAllowingPartiallyDestroyed" ||
+         name == "RefPtr" || name == "RefPtrAllowingPartiallyDestroyed";
+}
+
 bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
   assert(F);
   const auto &FunctionName = safeGetName(F);
 
-  return FunctionName == "Ref" || FunctionName == "makeRef"
-
-         || FunctionName == "RefPtr" || FunctionName == "makeRefPtr"
-
-         || FunctionName == "UniqueRef" || FunctionName == "makeUniqueRef" ||
+  return isRefType(FunctionName) || FunctionName == "makeRef" ||
+         FunctionName == "makeRefPtr" || FunctionName == "UniqueRef" ||
+         FunctionName == "makeUniqueRef" ||
          FunctionName == "makeUniqueRefWithoutFastMallocCheck"
 
          || FunctionName == "String" || FunctionName == "AtomString" ||
@@ -131,7 +134,7 @@ bool isReturnValueRefCounted(const clang::FunctionDecl *F) {
     if (auto *specialT = type->getAs<TemplateSpecializationType>()) {
       if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) {
         auto name = decl->getNameAsString();
-        return name == "Ref" || name == "RefPtr";
+        return isRefType(name);
       }
       return false;
     }
@@ -172,20 +175,18 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
   if (isa<CXXMethodDecl>(M)) {
     const CXXRecordDecl *calleeMethodsClass = M->getParent();
     auto className = safeGetName(calleeMethodsClass);
-    auto methodName = safeGetName(M);
+    auto method = safeGetName(M);
 
-    if (((className == "Ref" || className == "RefPtr") &&
-         methodName == "get") ||
-        (className == "Ref" && methodName == "ptr") ||
+    if ((isRefType(className) && (method == "get" || method == "ptr")) ||
         ((className == "String" || className == "AtomString" ||
           className == "AtomStringImpl" || className == "UniqueString" ||
           className == "UniqueStringImpl" || className == "Identifier") &&
-         methodName == "impl"))
+         method == "impl"))
       return true;
 
     // Ref<T> -> T conversion
     // FIXME: Currently allowing any Ref<T> -> whatever cast.
-    if (className == "Ref" || className == "RefPtr") {
+    if (isRefType(className)) {
       if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
         if (auto *targetConversionType =
                 maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
@@ -202,7 +203,7 @@ bool isRefCounted(const CXXRecordDecl *R) {
   if (auto *TmplR = R->getTemplateInstantiationPattern()) {
     // FIXME: String/AtomString/UniqueString
     const auto &ClassName = safeGetName(TmplR);
-    return ClassName == "RefPtr" || ClassName == "Ref";
+    return isRefType(ClassName);
   }
   return false;
 }
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index d08a997aa8c043..e43641c0c61445 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -5,9 +5,10 @@ template <typename T> struct Ref {
   T *t;
 
   Ref() : t{} {};
-  Ref(T *) {}
+  Ref(T &) {}
   T *get() { return t; }
   T *ptr() { return t; }
+  T *operator->() { return t; }
   operator const T &() const { return *t; }
   operator T &() { return *t; }
 };
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp b/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp
new file mode 100644
index 00000000000000..f9c0f405968231
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+// expected-no-diagnostics
+
+#include "mock-types.h"
+
+template <typename T> struct RefAllowingPartiallyDestroyed {
+  T *t;
+
+  RefAllowingPartiallyDestroyed() : t{} {};
+  RefAllowingPartiallyDestroyed(T &) {}
+  T *get() { return t; }
+  T *ptr() { return t; }
+  T *operator->() { return t; }
+  operator const T &() const { return *t; }
+  operator T &() { return *t; }
+};
+
+template <typename T> struct RefPtrAllowingPartiallyDestroyed {
+  T *t;
+
+  RefPtrAllowingPartiallyDestroyed() : t(new T) {}
+  RefPtrAllowingPartiallyDestroyed(T *t) : t(t) {}
+  T *get() { return t; }
+  T *operator->() { return t; }
+  const T *operator->() const { return t; }
+  T &operator*() { return *t; }
+  RefPtrAllowingPartiallyDestroyed &operator=(T *) { return *this; }
+  operator bool() { return t; }
+};
+
+class RefCounted {
+public:
+  void ref() const;
+  void deref() const;
+  void someFunction();
+};
+
+RefAllowingPartiallyDestroyed<RefCounted> object1();
+RefPtrAllowingPartiallyDestroyed<RefCounted> object2();
+RefCounted* object3();
+
+void testFunction() {
+//  object1()->someFunction();
+//  object2()->someFunction();
+  RefPtr { object3() }->someFunction();
+}

Copy link

github-actions bot commented Feb 19, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@rniwa rniwa force-pushed the support-ref-allowing-partially-destroyed branch 3 times, most recently from d132b98 to 5e365d2 Compare February 19, 2024 09:42
@rniwa rniwa requested a review from haoNoQ February 20, 2024 00:19
…rtiallyDestroyed

This PR adds the support for WebKit's RefAllowingPartiallyDestroyed and
RefPtrAllowingPartiallyDestroyed, which are smart pointer types which may be used
after the destructor had started running.
@rniwa rniwa force-pushed the support-ref-allowing-partially-destroyed branch from 523724e to 1d53adb Compare February 20, 2024 04:48
Copy link
Collaborator

@haoNoQ haoNoQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I have one stylistic nitpick.

@rniwa rniwa merged commit 8b23d68 into llvm:main Feb 21, 2024
@rniwa rniwa deleted the support-ref-allowing-partially-destroyed branch February 21, 2024 07:02
rniwa added a commit to rniwa/llvm-project that referenced this pull request Mar 8, 2024
…rtiallyDestroyed (llvm#82209)

This PR adds the support for WebKit's RefAllowingPartiallyDestroyed and
RefPtrAllowingPartiallyDestroyed, which are smart pointer types which
may be used after the destructor had started running.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants