Index: clang/docs/analyzer/checkers.rst =================================================================== --- clang/docs/analyzer/checkers.rst +++ clang/docs/analyzer/checkers.rst @@ -1402,6 +1402,24 @@ 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 Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1632,4 +1632,8 @@ def RefCntblBaseVirtualDtorChecker : Checker<"RefCntblBaseVirtualDtor">, HelpText<"Check for any ref-countable base class having virtual destructor.">, Documentation; + +def WebKitNoUncountedMemberChecker : Checker<"WebKitNoUncountedMemberChecker">, + HelpText<"Check for no uncounted member variables.">, + Documentation; } // end webkit Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -121,6 +121,7 @@ VLASizeChecker.cpp ValistChecker.cpp VirtualCallChecker.cpp + WebKit/NoUncountedMembersChecker.cpp WebKit/PtrTypesSemantics.cpp WebKit/RefCntblBaseVirtualDtorChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h +++ clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h @@ -23,6 +23,14 @@ Os << "'"; } +template +void printQuotedName(llvm::raw_ostream &Os, const NamedDeclDerivedT &D) { + Os << "'"; + D->getNameForDiagnostic(Os, D->getASTContext().getPrintingPolicy(), + /*Qualified=*/false); + Os << "'"; +} + } // namespace clang #endif Index: clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp =================================================================== --- /dev/null +++ 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> { +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 { + 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(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(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(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(Bug, Os.str(), BSLoc); + Report->addRange(Member->getSourceRange()); + BR->emitReport(std::move(Report)); + } +}; +} // namespace + +void ento::registerWebKitNoUncountedMemberChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterWebKitNoUncountedMemberChecker( + const CheckerManager &Mgr) { + return true; +} Index: clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp =================================================================== --- /dev/null +++ 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@-1{{Member variable 'a' in 'members::Foo' is a raw pointer to ref-countable type 'RefCountable'}} + + protected: + RefPtr b; + + public: + RefCountable silenceWarningAboutInit; + RefCountable& c = silenceWarningAboutInit; +// expected-warning@-1{{Member variable 'c' in 'members::Foo' is a reference to ref-countable type 'RefCountable'}} + Ref d; + }; + + template + struct FooTmpl { + T* a; +// expected-warning@-1{{Member variable 'a' in 'members::FooTmpl' is a raw pointer to ref-countable type 'RefCountable'}} + }; + + void forceTmplToInstantiate(FooTmpl) {} +} + +namespace ignore_unions { + union Foo { + RefCountable* a; + RefPtr b; + Ref c; + }; + + template + union RefPtr { + T* a; + }; + + void forceTmplToInstantiate(RefPtr) {} +}