[clang] [RawPtrRefMemberChecker] Member varible checker should allow T* in smart pointer classes (PR #136503)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

This PR fixes member variable checker to allow the usage of T* in smart pointer classes. e.g. alpha.webkit.NoUncheckedPtrMemberChecker should allow T* to appear within RefPtr.

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


5 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+7) 
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+4) 
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+2-15) 
- (modified) clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp (+9) 
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp (+12-3) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 811888e119449..ba0c7fd77b410 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -436,6 +436,13 @@ bool isRetainPtr(const CXXRecordDecl *R) {
   return false;
 }
 
+bool isSmartPtr(const CXXRecordDecl *R) {
+  assert(R);
+  if (auto *TmplR = R->getTemplateInstantiationPattern())
+    return isSmartPtrClass(safeGetName(TmplR));
+  return false;
+}
+
 bool isPtrConversion(const FunctionDecl *F) {
   assert(F);
   if (isCtorOfRefCounted(F))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 97c9d0510e67d..f9fcfe9878d54 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -58,6 +58,10 @@ bool isCheckedPtr(const clang::CXXRecordDecl *Class);
 /// \returns true if \p Class is a RetainPtr, false if not.
 bool isRetainPtr(const clang::CXXRecordDecl *Class);
 
+/// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...),
+/// false if not.
+bool isSmartPtr(const clang::CXXRecordDecl *Class);
+
 /// \returns true if \p Class is ref-countable AND not ref-counted, false if
 /// not, std::nullopt if inconclusive.
 std::optional<bool> isUncounted(const clang::QualType T);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index a003fc200727c..10b9749319a57 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -41,7 +41,6 @@ class RawPtrRefMemberChecker
   virtual std::optional<bool>
   isPtrCompatible(const clang::QualType,
                   const clang::CXXRecordDecl *R) const = 0;
-  virtual bool isPtrCls(const clang::CXXRecordDecl *) const = 0;
   virtual const char *typeName() const = 0;
   virtual const char *invariant() const = 0;
 
@@ -205,8 +204,8 @@ class RawPtrRefMemberChecker
     // Ref-counted smartpointers actually have raw-pointer to uncounted type as
     // a member but we trust them to handle it correctly.
     auto CXXRD = llvm::dyn_cast_or_null<CXXRecordDecl>(RD);
-    if (CXXRD)
-      return isPtrCls(CXXRD);
+    if (CXXRD && isSmartPtr(CXXRD))
+      return true;
 
     return false;
   }
@@ -270,10 +269,6 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
     return R ? isRefCountable(R) : std::nullopt;
   }
 
-  bool isPtrCls(const clang::CXXRecordDecl *R) const final {
-    return isRefCounted(R);
-  }
-
   const char *typeName() const final { return "ref-countable type"; }
 
   const char *invariant() const final {
@@ -293,10 +288,6 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
     return R ? isCheckedPtrCapable(R) : std::nullopt;
   }
 
-  bool isPtrCls(const clang::CXXRecordDecl *R) const final {
-    return isCheckedPtr(R);
-  }
-
   const char *typeName() const final { return "CheckedPtr capable type"; }
 
   const char *invariant() const final {
@@ -319,10 +310,6 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
     return RTC->isUnretained(QT);
   }
 
-  bool isPtrCls(const clang::CXXRecordDecl *R) const final {
-    return isRetainPtr(R);
-  }
-
   const char *typeName() const final { return "retainable type"; }
 
   const char *invariant() const final {
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
index 0189b0cd50fcc..048ffbffcdefb 100644
--- a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
@@ -50,3 +50,12 @@ namespace ignore_unions {
   void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
 
 } // namespace ignore_unions
+
+namespace checked_ptr_ref_ptr_capable {
+
+  RefCountableAndCheckable* provide();
+  void foo() {
+    RefPtr<RefCountableAndCheckable> foo = provide();
+  }
+
+} // checked_ptr_ref_ptr_capable
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
index 1bdbaedefbfeb..130777a9a5fee 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
@@ -34,7 +34,7 @@ namespace members {
   private:
     RefCountable* a = nullptr;
   };
-}
+} // members
 
 namespace ignore_unions {
   union Foo {
@@ -49,7 +49,7 @@ namespace ignore_unions {
   };
 
   void forceTmplToInstantiate(RefPtr<RefCountable>) {}
-}
+} // ignore_unions
 
 namespace ignore_system_header {
 
@@ -67,4 +67,13 @@ namespace ignore_non_ref_countable {
   struct Bar {
     Foo* foo;
   };
-}
\ No newline at end of file
+} // ignore_non_ref_countable
+
+namespace checked_ptr_ref_ptr_capable {
+
+  RefCountableAndCheckable* provide();
+  void foo() {
+    CheckedPtr<RefCountableAndCheckable> foo = provide();
+  }
+
+} // checked_ptr_ref_ptr_capable

``````````

</details>


https://github.com/llvm/llvm-project/pull/136503