[llvm] 3c09843 - [alpha.webkit.NoUncheckedPtrMemberChecker] Introduce member variable checker for CheckedPtr/CheckedRef (#108352)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 27 00:42:21 PDT 2024
Author: Ryosuke Niwa
Date: 2024-09-27T00:42:18-07:00
New Revision: 3c0984309ed338560f902a918d6f99959b4c7c33
URL: https://github.com/llvm/llvm-project/commit/3c0984309ed338560f902a918d6f99959b4c7c33
DIFF: https://github.com/llvm/llvm-project/commit/3c0984309ed338560f902a918d6f99959b4c7c33.diff
LOG: [alpha.webkit.NoUncheckedPtrMemberChecker] Introduce member variable checker for CheckedPtr/CheckedRef (#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.
Added:
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/test/Analysis/Checkers/WebKit/mock-types.h
llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
Removed:
clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 47c6fc680deb1b..9847d449d76d04 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3442,6 +3442,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 7da0d0a87e8c0c..747ebd8c2e4dee 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1764,6 +1764,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
let ParentPackage = WebKitAlpha in {
+def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
+ HelpText<"Check for no unchecked member variables.">,
+ Documentation<HasDocumentation>;
+
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 54c99c3c1b37f9..4d145be808f6d8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -19,8 +19,7 @@ using namespace clang;
namespace {
-bool hasPublicMethodInBaseClass(const CXXRecordDecl *R,
- const char *NameToMatch) {
+bool hasPublicMethodInBaseClass(const CXXRecordDecl *R, StringRef NameToMatch) {
assert(R);
assert(R->hasDefinition());
@@ -37,7 +36,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();
@@ -53,16 +52,17 @@ 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,
+ StringRef IncMethodName,
+ StringRef DecMethodName) {
assert(R);
R = R->getDefinition();
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;
@@ -70,15 +70,15 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
Paths.setOrigin(const_cast<CXXRecordDecl *>(R));
bool AnyInconclusiveBase = false;
- const auto hasPublicRefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
- 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);
@@ -86,15 +86,15 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
return std::nullopt;
Paths.clear();
- const auto hasPublicDerefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
- 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)
@@ -103,11 +103,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);
@@ -217,6 +229,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 e2d0342bebd52c..3528c52a7d659d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -34,15 +34,23 @@ 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.
-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);
/// \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 63%
rename from clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
rename to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 69a0eb3086ab72..2ce6bc330e0ca1 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,21 @@ 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 *invariant() const = 0;
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
BugReporter &BRArg) const {
@@ -46,8 +49,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,9 +80,9 @@ 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);
- if (isRCAble && *isRCAble)
- reportBug(Member, MemberType, MemberCXXRD, RD);
+ std::optional<bool> isRCAble = isPtrCompatible(MemberCXXRD);
+ if (isRCAble && *isRCAble)
+ reportBug(Member, MemberType, MemberCXXRD, RD);
}
}
}
@@ -114,7 +117,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;
}
@@ -134,10 +137,10 @@ class NoUncountedMemberChecker
Os << " in ";
printQuotedQualifiedName(Os, ClassCXXRD);
Os << " is a "
- << (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
- << " to ref-countable type ";
+ << (isa<PointerType>(MemberType) ? "raw pointer" : "reference") << " to "
+ << typeName() << " ";
printQuotedQualifiedName(Os, MemberCXXRD);
- Os << "; member variables must be ref-counted.";
+ Os << "; " << invariant() << ".";
PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
BR->getSourceManager());
@@ -146,13 +149,67 @@ 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 *invariant() const final {
+ return "member variables must be Ref, RefPtr, WeakRef, or WeakPtr";
+ }
+};
+
+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 *invariant() const final {
+ return "member variables must be a CheckedPtr, CheckedRef, WeakRef, or "
+ "WeakPtr";
+ }
+};
+
} // namespace
void ento::registerNoUncountedMemberChecker(CheckerManager &Mgr) {
Mgr.registerChecker<NoUncountedMemberChecker>();
}
-bool ento::shouldRegisterNoUncountedMemberChecker(
+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..0189b0cd50fcc9
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+
+namespace members {
+
+ struct Foo {
+ private:
+ CheckedObj* a = nullptr;
+// expected-warning at -1{{Member variable 'a' in 'members::Foo' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+ CheckedObj& b;
+// expected-warning at -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 at -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",
More information about the llvm-commits
mailing list