https://github.com/rniwa created
https://github.com/llvm/llvm-project/pull/108352
This PR introduces new WebKit checker to warn a member variable that is a raw
reference or a raw pointer to an object, which is capable of creating a
CheckedRef/CheckedPtr.
>From ac0447762c98da3cfb41a6b462034e3ab410bc33 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 12 Sep 2024 02:13:12 -0700
Subject: [PATCH] [alpha.webkit.NoUncheckedPtrMemberChecker] Introduce member
variable checker for CheckedPtr/CheckedRef
This PR introduces new WebKit checker to warn a member variable that is a raw
reference or
a raw pointer to an object, which is capable of creating a
CheckedRef/CheckedPtr.
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 4 +
.../StaticAnalyzer/Checkers/CMakeLists.txt | 2 +-
.../Checkers/WebKit/PtrTypesSemantics.cpp | 37 ++++++--
.../Checkers/WebKit/PtrTypesSemantics.h | 7 ++
...Checker.cpp => RawPtrRefMemberChecker.cpp} | 88 ++++++++++++++++---
.../Analysis/Checkers/WebKit/mock-types.h | 48 ++++++++++
.../Checkers/WebKit/unchecked-members.cpp | 53 +++++++++++
.../lib/StaticAnalyzer/Checkers/BUILD.gn | 2 +-
8 files changed, 219 insertions(+), 22 deletions(-)
rename clang/lib/StaticAnalyzer/Checkers/WebKit/{NoUncountedMembersChecker.cpp
=> RawPtrRefMemberChecker.cpp} (66%)
create mode 100644 clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 585246547b3dce..4759f680fb4ff7 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1771,6 +1771,10 @@ def UncountedLambdaCapturesChecker :
Checker<"UncountedLambdaCapturesChecker">,
let ParentPackage = WebKitAlpha in {
+def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
+ HelpText<"Check for no unchecked member variables.">,
+ Documentation<NotDocumented>;
+
def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">,
HelpText<"Check uncounted call arguments.">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 414282d58f779f..6da3665ab9a4df 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -132,7 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers
VLASizeChecker.cpp
ValistChecker.cpp
VirtualCallChecker.cpp
- WebKit/NoUncountedMembersChecker.cpp
+ WebKit/RawPtrRefMemberChecker.cpp
WebKit/ASTUtils.cpp
WebKit/PtrTypesSemantics.cpp
WebKit/RefCntblBaseVirtualDtorChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index f48b2fd9dca71b..09298102993f99 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -53,7 +53,9 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch) {
return hasPublicMethodInBaseClass(R, NameToMatch) ? R : nullptr;
}
-std::optional<bool> isRefCountable(const CXXRecordDecl* R)
+std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
+ const char *IncMethodName,
+ const char *DecMethodName)
{
assert(R);
@@ -61,8 +63,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
if (!R)
return std::nullopt;
- bool hasRef = hasPublicMethodInBaseClass(R, "ref");
- bool hasDeref = hasPublicMethodInBaseClass(R, "deref");
+ bool hasRef = hasPublicMethodInBaseClass(R, IncMethodName);
+ bool hasDeref = hasPublicMethodInBaseClass(R, DecMethodName);
if (hasRef && hasDeref)
return true;
@@ -71,8 +73,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
bool AnyInconclusiveBase = false;
const auto hasPublicRefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
+ [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
+ auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
if (!hasRefInBase) {
AnyInconclusiveBase = true;
return false;
@@ -87,8 +89,8 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
Paths.clear();
const auto hasPublicDerefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
+ [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
+ auto hasDerefInBase = clang::hasPublicMethodInBase(Base,
DecMethodName);
if (!hasDerefInBase) {
AnyInconclusiveBase = true;
return false;
@@ -103,11 +105,23 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
return hasRef && hasDeref;
}
+std::optional<bool> isRefCountable(const clang::CXXRecordDecl *R) {
+ return isSmartPtrCompatible(R, "ref", "deref");
+}
+
+std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl *R) {
+ return isSmartPtrCompatible(R, "incrementPtrCount", "decrementPtrCount");
+}
+
bool isRefType(const std::string &Name) {
return Name == "Ref" || Name == "RefAllowingPartiallyDestroyed" ||
Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
}
+bool isCheckedPtr(const std::string &Name) {
+ return Name == "CheckedPtr" || Name == "CheckedRef";
+}
+
bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
assert(F);
const std::string &FunctionName = safeGetName(F);
@@ -218,6 +232,15 @@ bool isRefCounted(const CXXRecordDecl *R) {
return false;
}
+bool isCheckedPtr(const CXXRecordDecl *R) {
+ assert(R);
+ if (auto *TmplR = R->getTemplateInstantiationPattern()) {
+ const auto &ClassName = safeGetName(TmplR);
+ return isCheckedPtr(ClassName);
+ }
+ 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 2932e62ad06e4b..08f9be49970394 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -40,9 +40,16 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch);
/// inconclusive.
std::optional<bool> isRefCountable(const clang::CXXRecordDecl* Class);
+/// \returns true if \p Class is checked-pointer compatible, false if not,
+/// std::nullopt if inconclusive.
+std::optional<bool> isCheckedPtrCapable(const clang::CXXRecordDecl* Class);
+
/// \returns true if \p Class is ref-counted, false if not.
bool isRefCounted(const clang::CXXRecordDecl *Class);
+/// \returns true if \p Class is a CheckedPtr / CheckedRef, false if not.
+bool isCheckedPtr(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/NoUncountedMembersChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
similarity index 66%
rename from
clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
rename to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 69a0eb3086ab72..455bb27e0207e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -1,4 +1,4 @@
-//=======- NoUncountedMembersChecker.cpp -------------------------*- C++
-*-==//
+//=======- RawPtrRefMemberChecker.cpp ----------------------------*- C++
-*-==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -25,18 +25,20 @@ using namespace ento;
namespace {
-class NoUncountedMemberChecker
+class RawPtrRefMemberChecker
: public Checker<check::ASTDecl<TranslationUnitDecl>> {
private:
BugType Bug;
mutable BugReporter *BR;
public:
- NoUncountedMemberChecker()
- : Bug(this,
- "Member variable is a raw-pointer/reference to reference-countable
"
- "type",
- "WebKit coding guidelines") {}
+ RawPtrRefMemberChecker(const char *description)
+ : Bug(this, description, "WebKit coding guidelines") {}
+
+ virtual std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *)
const = 0;
+ virtual bool isPtrCls(const clang::CXXRecordDecl *) const = 0;
+ virtual const char* typeName() const = 0;
+ virtual const char* invariantName() const = 0;
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
BugReporter &BRArg) const {
@@ -46,8 +48,8 @@ class NoUncountedMemberChecker
// visit template instantiations or lambda classes. We
// want to visit those, so we make our own RecursiveASTVisitor.
struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
- const NoUncountedMemberChecker *Checker;
- explicit LocalVisitor(const NoUncountedMemberChecker *Checker)
+ const RawPtrRefMemberChecker *Checker;
+ explicit LocalVisitor(const RawPtrRefMemberChecker *Checker)
: Checker(Checker) {
assert(Checker);
}
@@ -77,7 +79,7 @@ class NoUncountedMemberChecker
if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
// If we don't see the definition we just don't know.
if (MemberCXXRD->hasDefinition()) {
- std::optional<bool> isRCAble = isRefCountable(MemberCXXRD);
+ std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
if (isRCAble && *isRCAble)
reportBug(Member, MemberType, MemberCXXRD, RD);
}
@@ -114,7 +116,7 @@ class NoUncountedMemberChecker
// a member but we trust them to handle it correctly.
auto CXXRD = llvm::dyn_cast_or_null<CXXRecordDecl>(RD);
if (CXXRD)
- return isRefCounted(CXXRD);
+ return isPtrCls(CXXRD);
return false;
}
@@ -135,9 +137,13 @@ class NoUncountedMemberChecker
printQuotedQualifiedName(Os, ClassCXXRD);
Os << " is a "
<< (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
- << " to ref-countable type ";
+ << " to "
+ << typeName()
+ << " ";
printQuotedQualifiedName(Os, MemberCXXRD);
- Os << "; member variables must be ref-counted.";
+ Os << "; "
+ << invariantName()
+ << ".";
PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
BR->getSourceManager());
@@ -146,6 +152,53 @@ class NoUncountedMemberChecker
BR->emitReport(std::move(Report));
}
};
+
+class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
+public:
+ NoUncountedMemberChecker()
+ : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
+ "reference-countable type") {}
+
+ std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const
final {
+ return isRefCountable(R);
+ }
+
+ bool isPtrCls(const clang::CXXRecordDecl *R) const final {
+ return isRefCounted(R);
+ }
+
+ const char* typeName() const final {
+ return "ref-countable type";
+ }
+
+ const char* invariantName() const final {
+ return "member variables must be ref-counted";
+ }
+};
+
+class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
+public:
+ NoUncheckedPtrMemberChecker()
+ : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
+ "checked-pointer capable type") {}
+
+ std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const
final {
+ return isCheckedPtrCapable(R);
+ }
+
+ bool isPtrCls(const clang::CXXRecordDecl *R) const final {
+ return isCheckedPtr(R);
+ }
+
+ const char* typeName() const final {
+ return "CheckedPtr capable type";
+ }
+
+ const char* invariantName() const final {
+ return "member variables must be a CheckedPtr or CheckedRef";
+ }
+};
+
} // namespace
void ento::registerNoUncountedMemberChecker(CheckerManager &Mgr) {
@@ -156,3 +209,12 @@ bool ento::shouldRegisterNoUncountedMemberChecker(
const CheckerManager &Mgr) {
return true;
}
+
+void ento::registerNoUncheckedPtrMemberChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<NoUncheckedPtrMemberChecker>();
+}
+
+bool ento::shouldRegisterNoUncheckedPtrMemberChecker(
+ const CheckerManager &Mgr) {
+ return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h
b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index c427b22fd683e5..933b4c5e62a79c 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -108,4 +108,52 @@ struct RefCountable {
template <typename T> T *downcast(T *t) { return t; }
+template <typename T> struct CheckedRef {
+private:
+ T *t;
+
+public:
+ CheckedRef() : t{} {};
+ CheckedRef(T &t) : t(t) { t->incrementPtrCount(); }
+ CheckedRef(const CheckedRef& o) : t(o.t) { if (t) t->incrementPtrCount(); }
+ ~CheckedRef() { if (t) t->decrementPtrCount(); }
+ 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 CheckedPtr {
+private:
+ T *t;
+
+public:
+ CheckedPtr() : t(nullptr) {}
+ CheckedPtr(T *t)
+ : t(t) {
+ if (t)
+ t->incrementPtrCount();
+ }
+ CheckedPtr(Ref<T>&& o)
+ : t(o.leakRef())
+ { }
+ ~CheckedPtr() {
+ if (t)
+ t->decrementPtrCount();
+ }
+ T *get() { return t; }
+ T *operator->() { return t; }
+ const T *operator->() const { return t; }
+ T &operator*() { return *t; }
+ CheckedPtr &operator=(T *) { return *this; }
+ operator bool() const { return t; }
+};
+
+class CheckedObj {
+public:
+ void incrementPtrCount();
+ void decrementPtrCount();
+};
+
#endif
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
new file mode 100644
index 00000000000000..4450d5cab60f0a
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_analyze_cc1
-analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+namespace members {
+
+ struct Foo {
+ private:
+ CheckedObj* a = nullptr;
+// expected-warning@-1{{Member variable 'a' in 'members::Foo' is a raw pointer
to CheckedPtr capable type 'CheckedObj'}}
+ CheckedObj& b;
+// expected-warning@-1{{Member variable 'b' in 'members::Foo' is a reference
to CheckedPtr capable type 'CheckedObj'}}
+
+ [[clang::suppress]]
+ CheckedObj* a_suppressed = nullptr;
+
+ [[clang::suppress]]
+ CheckedObj& b_suppressed;
+
+ CheckedPtr<CheckedObj> c;
+ CheckedRef<CheckedObj> d;
+
+ public:
+ Foo();
+ };
+
+ template <typename S>
+ struct FooTmpl {
+ S* e;
+// expected-warning@-1{{Member variable 'e' in 'members::FooTmpl<CheckedObj>'
is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+ };
+
+ void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
+
+} // namespace members
+
+namespace ignore_unions {
+
+ union Foo {
+ CheckedObj* a;
+ CheckedPtr<CheckedObj> c;
+ CheckedRef<CheckedObj> d;
+ };
+
+ template<class T>
+ union FooTmpl {
+ T* a;
+ };
+
+ void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
+
+} // namespace ignore_unions
diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
index 3b640ae41b9f62..7a6c360e88c14e 100644
--- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
@@ -141,7 +141,7 @@ static_library("Checkers") {
"VforkChecker.cpp",
"VirtualCallChecker.cpp",
"WebKit/ASTUtils.cpp",
- "WebKit/NoUncountedMembersChecker.cpp",
+ "WebKit/RawPtrRefMemberChecker.cpp",
"WebKit/PtrTypesSemantics.cpp",
"WebKit/RefCntblBaseVirtualDtorChecker.cpp",
"WebKit/UncountedCallArgsChecker.cpp",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
