[clang] [WebKit checkers] Treat passing of a member variable which is capable of CheckedPtr as safe. (PR #142485)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 6 13:06:03 PDT 2025
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/142485
>From e0d5e1092893d6e93ead44763d5789cba84c34f9 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Mon, 2 Jun 2025 14:52:38 -0600
Subject: [PATCH 1/2] [WebKit checkers] Treat passing of a member variable
which is capable of CheckedPtr as safe.
It's safe for a member function of a class or struct to call a function or allocate a local variable
with a pointer or a reference to a member variable since "this" pointer, and therefore all its members,
will be kept alive by its caller so recognize as such.
---
.../StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 15 +++++++++++++++
.../StaticAnalyzer/Checkers/WebKit/ASTUtils.h | 4 ++++
.../Checkers/WebKit/RawPtrRefCallArgsChecker.cpp | 4 ++++
.../WebKit/RawPtrRefLocalVarsChecker.cpp | 3 +++
.../Checkers/WebKit/call-args-checked.cpp | 16 ++++++++++++++++
.../Checkers/WebKit/unchecked-local-vars.cpp | 16 ++++++++++++++++
6 files changed, 58 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index f087fc8fa19fd..b6f452e814830 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -231,6 +231,21 @@ bool isConstOwnerPtrMemberExpr(const clang::Expr *E) {
return isOwnerPtrType(T) && T.isConstQualified();
}
+bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
+ auto *ME = dyn_cast<MemberExpr>(E);
+ if (!ME)
+ return false;
+ auto *D = ME->getMemberDecl();
+ if (!D)
+ return false;
+ auto T = D->getType();
+ auto *CXXRD = T->getAsCXXRecordDecl();
+ if (!CXXRD)
+ return false;
+ auto result = isCheckedPtrCapable(CXXRD);
+ return result && *result;
+}
+
class EnsureFunctionVisitor
: public ConstStmtVisitor<EnsureFunctionVisitor, bool> {
public:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
index e2cc7b976adfc..8302bbe3084c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -69,6 +69,10 @@ bool isASafeCallArg(const clang::Expr *E);
/// \returns true if E is a MemberExpr accessing a const smart pointer type.
bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
+/// \returns true if E is a MemberExpr accessing a member variable which
+/// supports CheckedPtr.
+bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E);
+
/// \returns true if E is a CXXMemberCallExpr which returns a const smart
/// pointer type.
class EnsureFunctionAnalysis {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index 78675e55cf0ba..6bc39ab565041 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -443,6 +443,10 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
return isRefOrCheckedPtrType(type);
}
+ bool isSafeExpr(const Expr *E) const final {
+ return isExprToGetCheckedPtrCapableMember(E);
+ }
+
const char *ptrKind() const final { return "unchecked"; }
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 4fb47703e3984..7cd86a682c0a9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -417,6 +417,9 @@ class UncheckedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
bool isSafePtrType(const QualType type) const final {
return isRefOrCheckedPtrType(type);
}
+ bool isSafeExpr(const Expr *E) const final {
+ return isExprToGetCheckedPtrCapableMember(E);
+ }
const char *ptrKind() const final { return "unchecked"; }
};
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
index e24b04dcd3cf9..4ad365f0f937e 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
@@ -32,6 +32,22 @@ static void baz() {
} // namespace call_args_checked
+namespace call_args_member {
+
+void consume(CheckedObj&);
+
+struct WrapperObj {
+ CheckedObj checked;
+ CheckedObj& checkedRef;
+ void foo() {
+ consume(checked);
+ consume(checkedRef);
+ // expected-warning at -1{{Call argument is unchecked and unsafe [alpha.webkit.UncheckedCallArgsChecker]}}
+ }
+};
+
+} // namespace call_args_checked
+
namespace call_args_default {
void someFunction(RefCountableAndCheckable* = makeObj());
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
index 3bc75230fcf82..64ab22f74c558 100644
--- a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
@@ -290,6 +290,22 @@ void foo() {
} // namespace local_assignment_to_global
+namespace member_var {
+
+ struct WrapperObj {
+ CheckedObj checked;
+ CheckedObj& checkedRef;
+ void foo() {
+ auto *a = &checked;
+ a->method();
+ auto *b = &checkedRef;
+ // expected-warning at -1{{Local variable 'b' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ b->method();
+ }
+ };
+
+}
+
namespace local_refcountable_checkable_object {
RefCountableAndCheckable* provide_obj();
>From d5d7a3b3b854f3e89443b307373a2c874939ae53 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 6 Jun 2025 14:05:14 -0600
Subject: [PATCH 2/2] Restrict the safe member access to `this` pointer.
---
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 5 +++++
clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp | 4 ++++
.../test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp | 6 ++++++
3 files changed, 15 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index b6f452e814830..30d4dee13b797 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -235,6 +235,11 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
auto *ME = dyn_cast<MemberExpr>(E);
if (!ME)
return false;
+ auto *Base = ME->getBase();
+ if (!Base)
+ return false;
+ if (!isa<CXXThisExpr>(Base->IgnoreParenCasts()))
+ return false;
auto *D = ME->getMemberDecl();
if (!D)
return false;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
index 4ad365f0f937e..b6cb70ec0257e 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
@@ -44,6 +44,10 @@ struct WrapperObj {
consume(checkedRef);
// expected-warning at -1{{Call argument is unchecked and unsafe [alpha.webkit.UncheckedCallArgsChecker]}}
}
+ void bar(WrapperObj& other) {
+ consume(other.checked);
+ // expected-warning at -1{{Call argument is unchecked and unsafe [alpha.webkit.UncheckedCallArgsChecker]}}
+ }
};
} // namespace call_args_checked
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
index 64ab22f74c558..2984f8ba4eefa 100644
--- a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
@@ -302,6 +302,12 @@ namespace member_var {
// expected-warning at -1{{Local variable 'b' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
b->method();
}
+
+ void bar(WrapperObj& wrapper) {
+ CheckedObj* ptr = &wrapper.checked;
+ // expected-warning at -1{{Local variable 'ptr' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ ptr->method();
+ }
};
}
More information about the cfe-commits
mailing list