[clang] [Webkit Checkers] Treat const member variables as a safe origin (PR #115594)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 9 00:17:58 PST 2024


https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/115594

Treat const Ref, RefPtr, CheckedRef, CheckedPtr member variables as safe pointer origin in WebKit's local variable and call arguments checkers.

>From 28ee8321eb6e405fd1ebae9043c3ffafe20a4b35 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 9 Nov 2024 00:14:36 -0800
Subject: [PATCH] [Webkit Checkers] Treat const member variables as a safe
 origin

Treat const Ref, RefPtr, CheckedRef, CheckedPtr member variables as safe pointer origin
in WebKit's local variable and call arguments checkers.
---
 .../Checkers/WebKit/ASTUtils.cpp              |  6 +++
 .../WebKit/RawPtrRefLocalVarsChecker.cpp      |  8 +++
 .../WebKit/call-args-checked-const-member.cpp | 43 ++++++++++++++++
 .../WebKit/call-args-counted-const-member.cpp | 43 ++++++++++++++++
 .../local-vars-checked-const-member.cpp       | 50 +++++++++++++++++++
 .../local-vars-counted-const-member.cpp       | 50 +++++++++++++++++++
 .../Analysis/Checkers/WebKit/mock-types.h     | 25 ++++------
 7 files changed, 211 insertions(+), 14 deletions(-)
 create mode 100644 clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
 create mode 100644 clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
 create mode 100644 clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
 create mode 100644 clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 9d34dfd3cea636..ad2be6f6793cea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -142,6 +142,12 @@ bool isASafeCallArg(const Expr *E) {
         return true;
     }
   }
+  if (auto *ME = dyn_cast<MemberExpr>(E)) {
+    if (auto *D = ME->getMemberDecl()) {
+      auto T = D->getType();
+      return isSafePtrType(T) && T.isConstQualified();
+    }
+  }
 
   // TODO: checker for method calls on non-refcounted objects
   return isa<CXXThisExpr>(E);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 06f8f43cee8151..be954ad3026c22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -281,6 +281,14 @@ class RawPtrRefLocalVarsChecker
                 if (isa<IntegerLiteral>(InitArgOrigin))
                   return true;
 
+                if (auto *ME = dyn_cast<MemberExpr>(InitArgOrigin)) {
+                  if (auto *D = ME->getMemberDecl()) {
+                    auto T = D->getType();
+                    if (isSafePtrType(T) && T.isConstQualified())
+                      return true;
+                  }
+                }
+
                 if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
                   if (auto *MaybeGuardian =
                           dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
new file mode 100644
index 00000000000000..6a54ac7b2ca791
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace call_args_const_checkedptr_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const CheckedPtr<CheckedObj> m_obj1;
+  CheckedPtr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+  m_obj1->method();
+  m_obj2->method();
+  // expected-warning at -1{{Call argument for 'this' parameter is unchecked and unsafe}}
+}
+
+} // namespace call_args_const_checkedptr_member
+
+namespace call_args_const_checkedref_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const CheckedRef<CheckedObj> m_obj1;
+  CheckedRef<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+  m_obj1->method();
+  m_obj2->method();
+  // expected-warning at -1{{Call argument for 'this' parameter is unchecked and unsafe}}
+}
+
+} // namespace call_args_const_checkedref_member
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
new file mode 100644
index 00000000000000..33af5ad9014de4
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace call_args_const_refptr_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const RefPtr<RefCountable> m_obj1;
+  RefPtr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+  m_obj1->method();
+  m_obj2->method();
+  // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_refptr_member
+
+namespace call_args_const_ref_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const Ref<RefCountable> m_obj1;
+  Ref<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+  m_obj1->method();
+  m_obj2->method();
+  // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_ref_member
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
new file mode 100644
index 00000000000000..3deef66c271096
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedLocalVarsChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+void someFunction();
+
+namespace local_vars_const_checkedptr_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const CheckedPtr<CheckedObj> m_obj1;
+  CheckedPtr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+  auto* obj1 = m_obj1.get();
+  obj1->method();
+  auto* obj2 = m_obj2.get();
+  // expected-warning at -1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+  obj2->method();
+}
+
+} // namespace local_vars_const_checkedptr_member
+
+namespace local_vars_const_checkedref_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const CheckedRef<CheckedObj> m_obj1;
+  CheckedRef<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+  auto& obj1 = m_obj1.get();
+  obj1.method();
+  auto& obj2 = m_obj2.get();
+  // expected-warning at -1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+  obj2.method();
+}
+
+} // namespace local_vars_const_ref_member
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
new file mode 100644
index 00000000000000..6d6a7718244002
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedLocalVarsChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+void someFunction();
+
+namespace local_vars_const_refptr_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const RefPtr<RefCountable> m_obj1;
+  RefPtr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+  auto* obj1 = m_obj1.get();
+  obj1->method();
+  auto* obj2 = m_obj2.get();
+  // expected-warning at -1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  obj2->method();
+}
+
+} // namespace local_vars_const_refptr_member
+
+namespace local_vars_const_ref_member {
+
+class Foo {
+public:
+  Foo();
+  void bar();
+
+private:
+  const Ref<RefCountable> m_obj1;
+  Ref<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+  auto& obj1 = m_obj1.get();
+  obj1.method();
+  auto& obj2 = m_obj2.get();
+  // expected-warning at -1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  obj2.method();
+}
+
+} // namespace local_vars_const_ref_member
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 9c9326f0f11cfb..8f84589477bff1 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -66,11 +66,10 @@ template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTra
     t = o.t;
     o.t = tmp;
   }
-  T &get() { return *PtrTraits::unwrap(t); }
-  T *ptr() { return PtrTraits::unwrap(t); }
-  T *operator->() { return PtrTraits::unwrap(t); }
-  operator const T &() const { return *PtrTraits::unwrap(t); }
-  operator T &() { return *PtrTraits::unwrap(t); }
+  T &get() const { return *PtrTraits::unwrap(t); }
+  T *ptr() const { return PtrTraits::unwrap(t); }
+  T *operator->() const { return PtrTraits::unwrap(t); }
+  operator T &() const { return *PtrTraits::unwrap(t); }
   T* leakRef() { return PtrTraits::exchange(t, nullptr); }
 };
 
@@ -102,9 +101,8 @@ template <typename T> struct RefPtr {
     t = o.t;
     o.t = tmp;
   }
-  T *get() { return t; }
-  T *operator->() { return t; }
-  const T *operator->() const { return t; }
+  T *get() const { return t; }
+  T *operator->() const { return t; }
   T &operator*() { return *t; }
   RefPtr &operator=(T *t) {
     RefPtr o(t);
@@ -149,9 +147,9 @@ template <typename T> struct CheckedRef {
   CheckedRef(T &t) : t(&t) { t.incrementCheckedPtrCount(); }
   CheckedRef(const CheckedRef &o) : t(o.t) { if (t) t->incrementCheckedPtrCount(); }
   ~CheckedRef() { if (t) t->decrementCheckedPtrCount(); }
-  T &get() { return *t; }
-  T *ptr() { return t; }
-  T *operator->() { return t; }
+  T &get() const { return *t; }
+  T *ptr() const { return t; }
+  T *operator->() const { return t; }
   operator const T &() const { return *t; }
   operator T &() { return *t; }
 };
@@ -174,9 +172,8 @@ template <typename T> struct CheckedPtr {
     if (t)
       t->decrementCheckedPtrCount();
   }
-  T *get() { return t; }
-  T *operator->() { return t; }
-  const T *operator->() const { return t; }
+  T *get() const { return t; }
+  T *operator->() const { return t; }
   T &operator*() { return *t; }
   CheckedPtr &operator=(T *) { return *this; }
   operator bool() const { return t; }



More information about the cfe-commits mailing list