https://github.com/rniwa updated
https://github.com/llvm/llvm-project/pull/108352
>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 1/5] [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",
>From 5abb2493cfc67f2bd1dcdd946ed9ecbef8929d2b Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 12 Sep 2024 02:32:28 -0700
Subject: [PATCH 2/5] Update warnings to explicitly state which types of
pointers should be used.
---
.../StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 455bb27e0207e9..9ea0abbee974bc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -172,7 +172,7 @@ class NoUncountedMemberChecker final : public
RawPtrRefMemberChecker {
}
const char* invariantName() const final {
- return "member variables must be ref-counted";
+ return "member variables must be Ref, RefPtr, WeakRef, or WeakPtr";
}
};
@@ -195,7 +195,7 @@ class NoUncheckedPtrMemberChecker final : public
RawPtrRefMemberChecker {
}
const char* invariantName() const final {
- return "member variables must be a CheckedPtr or CheckedRef";
+ return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or
WeakPtr";
}
};
>From b0f8992ef7f8642a24bfd4e8422705bf15cc3723 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 12 Sep 2024 02:50:32 -0700
Subject: [PATCH 3/5] Formatting.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 39 ++++++++-------
.../Checkers/WebKit/PtrTypesSemantics.h | 4 +-
.../WebKit/RawPtrRefMemberChecker.cpp | 47 +++++++++----------
3 files changed, 42 insertions(+), 48 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 09298102993f99..7084a09a03d686 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -55,8 +55,7 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch) {
std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
const char *IncMethodName,
- const char *DecMethodName)
-{
+ const char *DecMethodName) {
assert(R);
R = R->getDefinition();
@@ -72,15 +71,15 @@ std::optional<bool> isSmartPtrCompatible(const
CXXRecordDecl *R,
Paths.setOrigin(const_cast<CXXRecordDecl *>(R));
bool AnyInconclusiveBase = false;
- const auto hasPublicRefInBase =
- [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
- if (!hasRefInBase) {
- AnyInconclusiveBase = true;
- return false;
- }
- return (*hasRefInBase) != nullptr;
- };
+ const auto hasPublicRefInBase = [&](const CXXBaseSpecifier *Base,
+ CXXBasePath &) {
+ auto hasRefInBase = clang::hasPublicMethodInBase(Base, IncMethodName);
+ if (!hasRefInBase) {
+ AnyInconclusiveBase = true;
+ return false;
+ }
+ return (*hasRefInBase) != nullptr;
+ };
hasRef = hasRef || R->lookupInBases(hasPublicRefInBase, Paths,
/*LookupInDependent =*/true);
@@ -88,15 +87,15 @@ std::optional<bool> isSmartPtrCompatible(const
CXXRecordDecl *R,
return std::nullopt;
Paths.clear();
- const auto hasPublicDerefInBase =
- [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base,
DecMethodName);
- if (!hasDerefInBase) {
- AnyInconclusiveBase = true;
- return false;
- }
- return (*hasDerefInBase) != nullptr;
- };
+ const auto hasPublicDerefInBase = [&](const CXXBaseSpecifier *Base,
+ CXXBasePath &) {
+ auto hasDerefInBase = clang::hasPublicMethodInBase(Base, DecMethodName);
+ if (!hasDerefInBase) {
+ AnyInconclusiveBase = true;
+ return false;
+ }
+ return (*hasDerefInBase) != nullptr;
+ };
hasDeref = hasDeref || R->lookupInBases(hasPublicDerefInBase, Paths,
/*LookupInDependent =*/true);
if (AnyInconclusiveBase)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 08f9be49970394..25ec6f252b9702 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -38,11 +38,11 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch);
/// \returns true if \p Class is ref-countable, false if not, std::nullopt if
/// inconclusive.
-std::optional<bool> isRefCountable(const clang::CXXRecordDecl* Class);
+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);
+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);
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 9ea0abbee974bc..0a8dab737add04 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -35,10 +35,11 @@ class RawPtrRefMemberChecker
RawPtrRefMemberChecker(const char *description)
: Bug(this, description, "WebKit coding guidelines") {}
- virtual std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *)
const = 0;
+ 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;
+ virtual const char *typeName() const = 0;
+ virtual const char *invariant() const = 0;
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
BugReporter &BRArg) const {
@@ -79,9 +80,9 @@ class RawPtrRefMemberChecker
if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
// If we don't see the definition we just don't know.
if (MemberCXXRD->hasDefinition()) {
- std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
- if (isRCAble && *isRCAble)
- reportBug(Member, MemberType, MemberCXXRD, RD);
+ std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
+ if (isRCAble && *isRCAble)
+ reportBug(Member, MemberType, MemberCXXRD, RD);
}
}
}
@@ -136,14 +137,10 @@ class RawPtrRefMemberChecker
Os << " in ";
printQuotedQualifiedName(Os, ClassCXXRD);
Os << " is a "
- << (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
- << " to "
- << typeName()
- << " ";
+ << (isa<PointerType>(MemberType) ? "raw pointer" : "reference") << " to
"
+ << typeName() << " ";
printQuotedQualifiedName(Os, MemberCXXRD);
- Os << "; "
- << invariantName()
- << ".";
+ Os << "; " << invariant() << ".";
PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
BR->getSourceManager());
@@ -159,7 +156,8 @@ class NoUncountedMemberChecker final : public
RawPtrRefMemberChecker {
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
"reference-countable type") {}
- std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const
final {
+ std::optional<bool>
+ isPtrCompatible(const clang::CXXRecordDecl *R) const final {
return isRefCountable(R);
}
@@ -167,11 +165,9 @@ class NoUncountedMemberChecker final : public
RawPtrRefMemberChecker {
return isRefCounted(R);
}
- const char* typeName() const final {
- return "ref-countable type";
- }
+ const char *typeName() const final { return "ref-countable type"; }
- const char* invariantName() const final {
+ const char *invariant() const final {
return "member variables must be Ref, RefPtr, WeakRef, or WeakPtr";
}
};
@@ -182,7 +178,8 @@ class NoUncheckedPtrMemberChecker final : public
RawPtrRefMemberChecker {
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
"checked-pointer capable type") {}
- std::optional<bool> isPtrCompatible(const clang::CXXRecordDecl *R) const
final {
+ std::optional<bool>
+ isPtrCompatible(const clang::CXXRecordDecl *R) const final {
return isCheckedPtrCapable(R);
}
@@ -190,12 +187,11 @@ class NoUncheckedPtrMemberChecker final : public
RawPtrRefMemberChecker {
return isCheckedPtr(R);
}
- const char* typeName() const final {
- return "CheckedPtr capable type";
- }
+ const char *typeName() const final { return "CheckedPtr capable type"; }
- const char* invariantName() const final {
- return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or
WeakPtr";
+ const char *invariant() const final {
+ return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or "
+ "WeakPtr";
}
};
@@ -205,8 +201,7 @@ void ento::registerNoUncountedMemberChecker(CheckerManager
&Mgr) {
Mgr.registerChecker<NoUncountedMemberChecker>();
}
-bool ento::shouldRegisterNoUncountedMemberChecker(
- const CheckerManager &Mgr) {
+bool ento::shouldRegisterNoUncountedMemberChecker(const CheckerManager &Mgr) {
return true;
}
>From b96394d596d07a92fab03bcb083505d6ea0328ac Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 12 Sep 2024 02:54:49 -0700
Subject: [PATCH 4/5] Formatting 2.
---
.../StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 0a8dab737add04..2ce6bc330e0ca1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -82,7 +82,7 @@ class RawPtrRefMemberChecker
if (MemberCXXRD->hasDefinition()) {
std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
if (isRCAble && *isRCAble)
- reportBug(Member, MemberType, MemberCXXRD, RD);
+ reportBug(Member, MemberType, MemberCXXRD, RD);
}
}
}
>From a2cf3f8e24d7f86c4bc1f82a6bcf75884549dce6 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 26 Sep 2024 23:04:57 -0700
Subject: [PATCH 5/5] Change hasPublicMethodInBaseClass and
isSmartPtrCompatible to take StringRef. Also added simple documentation for
alpha.webkit.NoUncountedMemberChecker.
---
clang/docs/analyzer/checkers.rst | 21 +++++++++++++++++++
.../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +-
.../Checkers/WebKit/PtrTypesSemantics.cpp | 8 +++----
.../Checkers/WebKit/PtrTypesSemantics.h | 3 ++-
4 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 847bf4baf74887..b8c5bd86c146d6 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3440,6 +3440,27 @@ Check for non-determinism caused by sorting of pointers.
alpha.WebKit
^^^^^^^^^^^^
+.. _alpha-webkit-NoUncheckedPtrMemberChecker:
+
+alpha.webkit.NoUncheckedPtrMemberChecker
+"""""""""""""""""""""""""""""""""""""
+Raw pointers and references to an object which supports CheckedPtr or
CheckedRef can't be used as class members. Only CheckedPtr, CheckedRef, RefPtr,
or Ref are allowed.
+
+.. code-block:: cpp
+
+ struct CheckableObj {
+ void incrementPtrCount() {}
+ void decrementPtrCount() {}
+ };
+
+ struct Foo {
+ CheckableObj* ptr; // warn
+ CheckableObj& ptr; // warn
+ // ...
+ };
+
+See `WebKit Guidelines for Safer C++ Programming
<https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details.
+
.. _alpha-webkit-UncountedCallArgsChecker:
alpha.webkit.UncountedCallArgsChecker
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 4759f680fb4ff7..2575c511d45b73 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1773,7 +1773,7 @@ let ParentPackage = WebKitAlpha in {
def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
HelpText<"Check for no unchecked member variables.">,
- Documentation<NotDocumented>;
+ Documentation<HasDocumentation>;
def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">,
HelpText<"Check uncounted call arguments.">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7084a09a03d686..09deb987337db9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -20,7 +20,7 @@ using namespace clang;
namespace {
bool hasPublicMethodInBaseClass(const CXXRecordDecl *R,
- const char *NameToMatch) {
+ StringRef NameToMatch) {
assert(R);
assert(R->hasDefinition());
@@ -37,7 +37,7 @@ bool hasPublicMethodInBaseClass(const CXXRecordDecl *R,
namespace clang {
std::optional<const clang::CXXRecordDecl *>
-hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch) {
+hasPublicMethodInBase(const CXXBaseSpecifier *Base, StringRef NameToMatch) {
assert(Base);
const Type *T = Base->getType().getTypePtrOrNull();
@@ -54,8 +54,8 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch) {
}
std::optional<bool> isSmartPtrCompatible(const CXXRecordDecl *R,
- const char *IncMethodName,
- const char *DecMethodName) {
+ StringRef IncMethodName,
+ StringRef DecMethodName) {
assert(R);
R = R->getDefinition();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 25ec6f252b9702..40ad17f16fa09e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -34,7 +34,8 @@ class Type;
/// \returns CXXRecordDecl of the base if the type has ref as a public method,
/// nullptr if not, std::nullopt if inconclusive.
std::optional<const clang::CXXRecordDecl *>
-hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch);
+hasPublicMethodInBase(const CXXBaseSpecifier *Base,
+ llvm::StringRef NameToMatch);
/// \returns true if \p Class is ref-countable, false if not, std::nullopt if
/// inconclusive.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
