[clang] 660cda5 - [Analyzer][WebKit] NoUncountedMembersChecker
Jan Korous via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 19:46:56 PDT 2020
Author: Jan Korous
Date: 2020-05-27T19:46:32-07:00
New Revision: 660cda572d6e05e55a9d959e61aba51790c0abbd
URL: https://github.com/llvm/llvm-project/commit/660cda572d6e05e55a9d959e61aba51790c0abbd
DIFF: https://github.com/llvm/llvm-project/commit/660cda572d6e05e55a9d959e61aba51790c0abbd.diff
LOG: [Analyzer][WebKit] NoUncountedMembersChecker
Differential Revision: https://reviews.llvm.org/D77178
Added:
clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
clang/test/Analysis/Checkers/WebKit/uncounted-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/DiagOutputUtils.h
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index dcf1f28994de..c977dde8c52f 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1403,6 +1403,24 @@ Ref-counted types hold their ref-countable data by a raw pointer and allow impli
struct Derived : RefCntblBase { }; // warn
+.. _webkit-WebKitNoUncountedMemberChecker:
+
+webkit.WebKitNoUncountedMemberChecker
+""""""""""""""""""""""""""""""""""""
+Raw pointers and references to uncounted types can't be used as class members. Only ref-counted types are allowed.
+
+.. code-block:: cpp
+ struct RefCntbl {
+ void ref() {}
+ void deref() {}
+ };
+
+ struct Foo {
+ RefCntbl * ptr; // warn
+ RefCntbl & ptr; // warn
+ // ...
+ };
+
.. _alpha-checkers:
Experimental Checkers
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 2ba3881c6135..2d69d8f34420 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1623,4 +1623,8 @@ let ParentPackage = WebKit in {
def RefCntblBaseVirtualDtorChecker : Checker<"RefCntblBaseVirtualDtor">,
HelpText<"Check for any ref-countable base class having virtual destructor.">,
Documentation<HasDocumentation>;
+
+def WebKitNoUncountedMemberChecker : Checker<"WebKitNoUncountedMemberChecker">,
+ HelpText<"Check for no uncounted member variables.">,
+ Documentation<HasDocumentation>;
} // end webkit
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4f885fadf415..b3dc7a9f6321 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -121,6 +121,7 @@ add_clang_library(clangStaticAnalyzerCheckers
VLASizeChecker.cpp
ValistChecker.cpp
VirtualCallChecker.cpp
+ WebKit/NoUncountedMembersChecker.cpp
WebKit/PtrTypesSemantics.cpp
WebKit/RefCntblBaseVirtualDtorChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
index 4979b8ffc2b2..781a8d746001 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
@@ -23,6 +23,14 @@ void printQuotedQualifiedName(llvm::raw_ostream &Os,
Os << "'";
}
+template <typename NamedDeclDerivedT>
+void printQuotedName(llvm::raw_ostream &Os, const NamedDeclDerivedT &D) {
+ Os << "'";
+ D->getNameForDiagnostic(Os, D->getASTContext().getPrintingPolicy(),
+ /*Qualified=*/false);
+ Os << "'";
+}
+
} // namespace clang
#endif
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
new file mode 100644
index 000000000000..89caf602a17e
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
@@ -0,0 +1,150 @@
+//=======- NoUncountedMembersChecker.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTUtils.h"
+#include "DiagOutputUtils.h"
+#include "PtrTypesSemantics.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/Casting.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NoUncountedMemberChecker
+ : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+ BugType Bug;
+ mutable BugReporter *BR;
+
+public:
+ NoUncountedMemberChecker()
+ : Bug(this,
+ "Member variable is a raw-poiner/reference to reference-countable "
+ "type",
+ "WebKit coding guidelines") {}
+
+ void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+ BugReporter &BRArg) const {
+ BR = &BRArg;
+
+ // The calls to checkAST* from AnalysisConsumer don't
+ // 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)
+ : Checker(Checker) {
+ assert(Checker);
+ }
+
+ bool shouldVisitTemplateInstantiations() const { return true; }
+ bool shouldVisitImplicitCode() const { return false; }
+
+ bool VisitRecordDecl(const RecordDecl *RD) {
+ Checker->visitRecordDecl(RD);
+ return true;
+ }
+ };
+
+ LocalVisitor visitor(this);
+ visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+ }
+
+ void visitRecordDecl(const RecordDecl *RD) const {
+ if (shouldSkipDecl(RD))
+ return;
+
+ for (auto Member : RD->fields()) {
+ const Type *MemberType = Member->getType().getTypePtrOrNull();
+ if (!MemberType)
+ continue;
+
+ if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
+ if (isRefCountable(MemberCXXRD))
+ reportBug(Member, MemberType, MemberCXXRD, RD);
+ }
+ }
+ }
+
+ bool shouldSkipDecl(const RecordDecl *RD) const {
+ if (!RD->isThisDeclarationADefinition())
+ return true;
+
+ if (RD->isImplicit())
+ return true;
+
+ if (RD->isLambda())
+ return true;
+
+ // If the construct doesn't have a source file, then it's not something
+ // we want to diagnose.
+ const auto RDLocation = RD->getLocation();
+ if (!RDLocation.isValid())
+ return true;
+
+ const auto Kind = RD->getTagKind();
+ // FIMXE: Should we check union members too?
+ if (Kind != TTK_Struct && Kind != TTK_Class)
+ return true;
+
+ // Ignore CXXRecords that come from system headers.
+ if (BR->getSourceManager().isInSystemHeader(RDLocation))
+ return true;
+
+ // Ref-counted smartpointers actually have raw-pointer to uncounted type as
+ // a member but we trust them to handle it correctly.
+ return isRefCounted(llvm::dyn_cast_or_null<CXXRecordDecl>(RD));
+ }
+
+ void reportBug(const FieldDecl *Member, const Type *MemberType,
+ const CXXRecordDecl *MemberCXXRD,
+ const RecordDecl *ClassCXXRD) const {
+ assert(Member);
+ assert(MemberType);
+ assert(MemberCXXRD);
+
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "Member variable ";
+ printQuotedName(Os, Member);
+ Os << " in ";
+ printQuotedQualifiedName(Os, ClassCXXRD);
+ Os << " is a "
+ << (isa<PointerType>(MemberType) ? "raw pointer" : "reference")
+ << " to ref-countable type ";
+ printQuotedQualifiedName(Os, MemberCXXRD);
+ Os << "; member variables must be ref-counted.";
+
+ PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(),
+ BR->getSourceManager());
+ auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+ Report->addRange(Member->getSourceRange());
+ BR->emitReport(std::move(Report));
+ }
+};
+} // namespace
+
+void ento::registerWebKitNoUncountedMemberChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<NoUncountedMemberChecker>();
+}
+
+bool ento::shouldRegisterWebKitNoUncountedMemberChecker(
+ const CheckerManager &Mgr) {
+ return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
new file mode 100644
index 000000000000..e88c0b3b0dd0
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.WebKitNoUncountedMemberChecker -verify %s
+
+#include "mock-types.h"
+
+namespace members {
+ struct Foo {
+ private:
+ RefCountable* a = nullptr;
+// expected-warning at -1{{Member variable 'a' in 'members::Foo' is a raw pointer to ref-countable type 'RefCountable'}}
+
+ protected:
+ RefPtr<RefCountable> b;
+
+ public:
+ RefCountable silenceWarningAboutInit;
+ RefCountable& c = silenceWarningAboutInit;
+// expected-warning at -1{{Member variable 'c' in 'members::Foo' is a reference to ref-countable type 'RefCountable'}}
+ Ref<RefCountable> d;
+ };
+
+ template<class T>
+ struct FooTmpl {
+ T* a;
+// expected-warning at -1{{Member variable 'a' in 'members::FooTmpl<RefCountable>' is a raw pointer to ref-countable type 'RefCountable'}}
+ };
+
+ void forceTmplToInstantiate(FooTmpl<RefCountable>) {}
+}
+
+namespace ignore_unions {
+ union Foo {
+ RefCountable* a;
+ RefPtr<RefCountable> b;
+ Ref<RefCountable> c;
+ };
+
+ template<class T>
+ union RefPtr {
+ T* a;
+ };
+
+ void forceTmplToInstantiate(RefPtr<RefCountable>) {}
+}
More information about the cfe-commits
mailing list