Index: clang/include/clang/AST/ASTTypeTraits.h =================================================================== --- clang/include/clang/AST/ASTTypeTraits.h +++ clang/include/clang/AST/ASTTypeTraits.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_AST_ASTTYPETRAITS_H #include "clang/AST/ASTFwd.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/TypeLoc.h" @@ -136,6 +137,7 @@ NKI_QualType, NKI_TypeLoc, NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc, + NKI_CXXBaseSpecifier, NKI_CXXCtorInitializer, NKI_NestedNameSpecifier, NKI_Decl, @@ -198,6 +200,7 @@ KIND_TO_KIND_ID(Stmt) KIND_TO_KIND_ID(Type) KIND_TO_KIND_ID(OMPClause) +KIND_TO_KIND_ID(CXXBaseSpecifier) #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl) #include "clang/AST/DeclNodes.inc" #define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED) @@ -510,6 +513,10 @@ struct DynTypedNode::BaseConverter< TypeLoc, void> : public ValueConverter {}; +template <> +struct DynTypedNode::BaseConverter< + CXXBaseSpecifier, void> : public PtrConverter {}; + // The only operation we allow on unsupported types is \c get. // This allows to conveniently use \c DynTypedNode when having an arbitrary // AST node that is not supported, but prevents misuse - a user cannot create Index: clang/include/clang/ASTMatchers/ASTMatchers.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchers.h +++ clang/include/clang/ASTMatchers/ASTMatchers.h @@ -548,6 +548,11 @@ extern const internal::VariadicDynCastAllOfMatcher templateTypeParmDecl; +/// Matches C++ base specifier. +//TODO +extern const internal::VariadicAllOfMatcher + cxxBaseSpecifier; + /// Matches public C++ declarations. /// /// Given @@ -2835,6 +2840,79 @@ return Matcher(M).matches(*InterfaceDecl, Finder, Builder); } +// TODO later implement isDerivedFrom by this +AST_MATCHER_P( + CXXRecordDecl, + hasABaseSpec, + internal::Matcher, InnerMatcher) { + if (const auto *RD = dyn_cast(&Node)) + return Finder->hasABaseSpec(RD, InnerMatcher, Builder); + return false; +} + +// TODO doc +AST_MATCHER( + CXXBaseSpecifier, + isPublicBase +) { + if (const auto *BS = dyn_cast(&Node)) + return BS->getAccessSpecifier() == AS_public; + return false; +} + +// TODO doc +AST_MATCHER( + CXXBaseSpecifier, + isPrivateBase +) { + if (const auto *BS = dyn_cast(&Node)) + return BS->getAccessSpecifier() == AS_private; + return false; +} + +// TODO doc +AST_MATCHER( + CXXBaseSpecifier, + isProtectedBase +) { + if (const auto *BS = dyn_cast(&Node)) + return BS->getAccessSpecifier() == AS_protected; + return false; +} + +// TODO doc +AST_MATCHER( + CXXBaseSpecifier, + isVirtualBase +) { + if (const auto *BS = dyn_cast(&Node)) + return BS->isVirtual(); + return false; +} + +// TODO doc +AST_MATCHER_P(Type, typeOfRecord, internal::Matcher, InnerMatcher) { + if (!Node.isRecordType()) + return false; + + if (const auto* RT = dyn_cast(&Node)) { + if (const auto *RD = RT->getDecl()) + return InnerMatcher.matches(*RD, Finder, Builder); + } else if (const auto* STTPT = dyn_cast(&Node)) { + if (auto* SubstituttedType = STTPT->getReplacementType().getTypePtrOrNull()) { + // FIXME: refactor - this is copy-pasted branch from above + if (!SubstituttedType->isRecordType()) + return false; + + if (const auto* RT = dyn_cast(SubstituttedType)) { + if (const auto *RD = RT->getDecl()) + return InnerMatcher.matches(*RD, Finder, Builder); + } + } + } + return false; +} + /// Similar to \c isDerivedFrom(), but also matches classes that directly /// match \c Base. AST_POLYMORPHIC_MATCHER_P_OVERLOAD( @@ -3467,7 +3545,7 @@ /// /// Usable as: Matcher, Matcher AST_POLYMORPHIC_MATCHER_P_OVERLOAD( - hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl), + hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl, CXXBaseSpecifier), internal::Matcher, InnerMatcher, 1) { QualType QT = internal::getUnderlyingType(Node); if (!QT.isNull()) Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchersInternal.h +++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -130,6 +130,9 @@ return TSI->getType(); return QualType(); } +inline QualType getUnderlyingType(const CXXBaseSpecifier &Node) { + return Node.getType(); +} /// Unifies obtaining the FunctionProtoType pointer from both /// FunctionProtoType and FunctionDecl nodes.. @@ -982,6 +985,13 @@ BoundNodesTreeBuilder *Builder, bool Directly) = 0; + /// Returns true if \p Declaration has a base specifier matching \p BaseSpec. + /// + /// A class is not considered to be derived from itself. + virtual bool hasABaseSpec(const CXXRecordDecl *ClassDecl, + const Matcher &BaseSpecMatcher, + BoundNodesTreeBuilder *Builder) = 0; + /// Returns true if the given Objective-C class is directly or indirectly /// derived from a base class matching \c base. /// Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -114,6 +114,9 @@ def Fuchsia : Package<"fuchsia">; def FuchsiaAlpha : Package<"fuchsia">, ParentPackage; +def WebKit : Package<"webkit">; +def WebKitAlpha : Package<"webkit">, ParentPackage; + //===----------------------------------------------------------------------===// // Core Checkers. //===----------------------------------------------------------------------===// @@ -1501,3 +1504,23 @@ } // end fuchsia +//===----------------------------------------------------------------------===// +// WebKit checkers. +//===----------------------------------------------------------------------===// + +let ParentPackage = WebKitAlpha in { + +// TODO documentation? + +def WebKitRefCntblBaseVirtualDtorChecker : Checker<"WebKitRefCntblBaseVirtualDtor">, + HelpText<"Check for any ref-countable base class having virtual destructor.">, + Documentation; + +def WebKitNoUncountedMemberChecker : Checker<"WebKitNoUncountedMemberChecker">, + HelpText<"Check for no uncounted member variables.">, + Documentation; + +def WebKitUncountedCallArgsChecker : Checker<"WebKitUncountedCallArgsChecker">, + HelpText<"Check uncounted call arguments.">, + Documentation; +} // end webkit Index: clang/lib/AST/ASTTypeTraits.cpp =================================================================== --- clang/lib/AST/ASTTypeTraits.cpp +++ clang/lib/AST/ASTTypeTraits.cpp @@ -27,6 +27,7 @@ { NKI_None, "NestedNameSpecifierLoc" }, { NKI_None, "QualType" }, { NKI_None, "TypeLoc" }, + { NKI_None, "CXXBaseSpecifier" }, { NKI_None, "CXXCtorInitializer" }, { NKI_None, "NestedNameSpecifier" }, { NKI_None, "Decl" }, Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp =================================================================== --- clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -18,6 +18,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/CXXInheritance.h" #include "clang/AST/RecursiveASTVisitor.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringMap.h" @@ -495,6 +496,10 @@ BoundNodesTreeBuilder *Builder, bool Directly) override; + bool hasABaseSpec(const CXXRecordDecl *ClassDecl, + const Matcher &BaseSpecMatcher, + BoundNodesTreeBuilder *Builder) override; + // Implements ASTMatchFinder::matchesChildOf. bool matchesChildOf(const DynTypedNode &Node, ASTContext &Ctx, const DynTypedMatcher &Matcher, @@ -931,6 +936,29 @@ return false; } +bool MatchASTVisitor::hasABaseSpec(const CXXRecordDecl *ClassDecl, + const Matcher &BaseSpecMatcher, + BoundNodesTreeBuilder *Builder) { + if (!ClassDecl->hasDefinition()) + return false; + + CXXBasePaths Paths; + // FIXME: Every time someone casts away const specifier a kitten dies. + Paths.setOrigin(const_cast(ClassDecl)); + + const auto basePredicate = [this, Builder, &BaseSpecMatcher](const CXXBaseSpecifier *BaseSpec, CXXBasePath &IgnoredParam) { + BoundNodesTreeBuilder Result(*Builder); + if (BaseSpecMatcher.matches(*BaseSpec, this, &Result)) { + *Builder = std::move(Result); + return true; + } + return false; + }; + + return ClassDecl->lookupInBases(basePredicate, Paths, + /*LookupInDependent =*/true); +} + // Returns true if the given Objective-C class is directly or indirectly // derived from a matching base class. A class is not considered to be derived // from itself. Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp =================================================================== --- clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -667,6 +667,8 @@ nonTypeTemplateParmDecl; const internal::VariadicDynCastAllOfMatcher templateTypeParmDecl; +const internal::VariadicAllOfMatcher + cxxBaseSpecifier; const internal::VariadicAllOfMatcher qualType; const internal::VariadicAllOfMatcher type; const internal::VariadicAllOfMatcher typeLoc; Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -120,6 +120,10 @@ VLASizeChecker.cpp ValistChecker.cpp VirtualCallChecker.cpp + WebKit/ASTUtils.cpp + WebKit/NoUncountedMembersChecker.cpp + WebKit/PtrTypesSemantics.cpp + WebKit/RefCntblBaseVirtualDtorChecker.cpp LINK_LIBS clangAST Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -0,0 +1,70 @@ +//=======- ASTUtis.h ---------------------------------------------*- 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYZER_WEBKIT_ASTUTILS_H +#define LLVM_CLANG_ANALYZER_WEBKIT_ASTUTILS_H + +#include "clang/AST/Decl.h" +#include "llvm/ADT/APInt.h" +#include "llvm/Support/Casting.h" + +#include +#include + +namespace clang { +class CXXRecordDecl; +class CXXBaseSpecifier; +class FunctionDecl; +class CXXMethodDecl; +class Expr; + +/// If passed expression is of type uncounted pointer/reference we try to find +/// the origin of this pointer. Example: Origin can be a local variable, nullptr +/// constant or this-pointer. +/// +/// Certain subexpression nodes represent transformations that don't affect +/// where the memory address originates from. We try to traverse such +/// subexpressions to get to the relevant child nodes. Whenever we encounter a +/// subexpression that either can't be ignored, we don't model its semantics or +/// that has multiple children we stop. +/// +/// \p E is an expression of uncounted pointer/reference type. +/// If \p StopAtFirstRefCountedObj is true and we encounter a subexpression that +/// represents ref-counted object during the traversal we return relevant +/// sub-expression and true. +/// +/// \returns subexpression that we traversed to and if \p +/// StopAtFirstRefCountedObj is true we also return whether we stopped early. +std::pair +tryToFindPtrOrigin(const clang::Expr *E, bool StopAtFirstRefCountedObj); + +/// For \p E referring to a ref-countable/-counted pointer/reference we return +/// whether it's a safe call argument. Examples: function parameter or +/// this-pointer. The logic relies on the set of recursive rules we enforce for +/// WebKit codebase. +/// +/// \returns Whether \p E is a safe call arugment. +bool isASafeCallArg(const clang::Expr *E); + +/// \returns name of AST node or empty string. +template std::string safeGetName(const T *ASTNode) { + const auto *const ND = llvm::dyn_cast_or_null(ASTNode); + if (!ND) + return ""; + + // In case F is for example "operator|" the getName() method below would + // assert. + if (!ND->getDeclName().isIdentifier()) + return ""; + + return ND->getName().str(); +} + +} // namespace clang + +#endif Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -0,0 +1,91 @@ +//=======- ASTUtils.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 "PtrTypesSemantics.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" + +using llvm::Optional; +namespace clang { + +std::pair +tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) { + while (E) { + if (auto *cast = dyn_cast(E)) { + if (StopAtFirstRefCountedObj) { + if (auto *ConversionFunc = + dyn_cast_or_null(cast->getConversionFunction())) { + if (isCtorOfRefCounted(ConversionFunc)) + return {E, true}; + } + } + E = cast->getSubExpr(); + continue; + } + if (auto *call = dyn_cast(E)) { + if (auto *memberCall = dyn_cast(call)) { + if (isGetterOfRefCounted(memberCall->getMethodDecl())) { + E = memberCall->getImplicitObjectArgument(); + if (StopAtFirstRefCountedObj) { + return {E, true}; + } + continue; + } + } + + if (auto *operatorCall = dyn_cast(E)) { + if (operatorCall->getNumArgs() == 1) { + E = operatorCall->getArg(0); + continue; + } + } + + if (auto *callee = call->getDirectCallee()) { + if (isCtorOfRefCounted(callee)) { + if (StopAtFirstRefCountedObj) + return {E, true}; + + E = call->getArg(0); + continue; + } + + if (isPtrConversion(callee)) { + E = call->getArg(0); + continue; + } + } + } + if (auto *unaryOp = dyn_cast(E)) { + // FIXME: Currently accepts ANY unary operator. Is it OK? + E = unaryOp->getSubExpr(); + continue; + } + + break; + } + // Some other expression. + return {E, false}; +} + +bool isASafeCallArg(const Expr *E) { + assert(E); + if (auto *Ref = dyn_cast(E)) { + if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) { + if (isa(D) || D->isLocalVarDecl()) + return true; + } + } + + // TODO: checker for method calls on non-refcounted objects + return isa(E); +} + +} // namespace clang Index: clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h @@ -0,0 +1,28 @@ +//=======- DiagOutputUtils.h -------------------------------------*- 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYZER_WEBKIT_DIAGPRINTUTILS_H +#define LLVM_CLANG_ANALYZER_WEBKIT_DIAGPRINTUTILS_H + +#include "clang/AST/Decl.h" +#include "llvm/Support/raw_ostream.h" + +namespace clang { + +template +void printQuotedQualifiedName(llvm::raw_ostream &Os, + const NamedDeclDerivedT &D) { + Os << "'"; + D->getNameForDiagnostic(Os, D->getASTContext().getPrintingPolicy(), + /*Qualified=*/true); + Os << "'"; +} + +} // namespace clang + +#endif Index: clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp @@ -0,0 +1,173 @@ +//=======- 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/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchFinder.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" + +using namespace clang; +using namespace ento; +using namespace ast_matchers; + +namespace { + +class NoUncountedMemberChecker + : public Checker> { +private: + // lazily created bug instance + mutable std::unique_ptr Bug; + mutable BugReporter *BR; + +public: + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, + BugReporter &BRArg) const { + BR = &BRArg; + + DeclarationMatcher RefCountableClassMatcher = + cxxRecordDecl( + anyOf( isClass(), isStruct() ), + anyOf( + allOf( + hasMethod(allOf(hasName("ref"), isPublic())), + hasMethod(allOf(hasName("deref"), isPublic())) + ), + hasABaseSpec( + cxxBaseSpecifier( + allOf( + isPublicBase(), + hasType( + cxxRecordDecl( + hasMethod(allOf(hasName("ref"), isPublic())), + hasMethod(allOf(hasName("deref"), isPublic())) + ) + ) + ) + ) + ) + ) + ); + + DeclarationMatcher RefCountedClassMatcher = + cxxRecordDecl( + anyOf( isClass(), isStruct() ), + anyOf( + hasName("RefPtr"), + hasName("Ref") + ) + ); + + DeclarationMatcher UncountedClassMatcher = + cxxRecordDecl( + RefCountableClassMatcher, + unless(RefCountedClassMatcher) + ); + + TypeMatcher RawPtrToUncounted = + type( + anyOf( + pointerType(), + referenceType() + ), + pointee( + typeOfRecord( + cxxRecordDecl( + UncountedClassMatcher + ).bind("UncountedCXXRecord") + ) + ) + ); + + DeclarationMatcher OffendingFieldMatcher = + fieldDecl( + hasType(RawPtrToUncounted), + hasParent( + cxxRecordDecl( + anyOf( isClass(), isStruct() ), + isDefinition(), + unless(anyOf( + RefCountedClassMatcher, + isImplicit(), + isLambda(), + isExpansionInSystemHeader() + )) + ).bind("ProblematicClass") + ) + ).bind("ProblematicField"); + + MatchFinder F; + + class Callback : public MatchFinder::MatchCallback { + public: + const NoUncountedMemberChecker &Checker; + Callback(const NoUncountedMemberChecker &Checker) + : Checker(Checker) {} + virtual void run(const MatchFinder::MatchResult &Result) { + auto Field = Result.Nodes.getNodeAs("ProblematicField"); + const Type* FieldT = Field->getType().getTypePtr(); + Checker.reportBug( + Field, + FieldT, + Result.Nodes.getNodeAs("UncountedCXXRecord"), + Result.Nodes.getNodeAs("ProblematicClass") + ); + } + }; + Callback CB(*this); + + F.addMatcher(OffendingFieldMatcher, &CB); + F.matchAST(TUD->getASTContext()); + } + + void reportBug(const FieldDecl *Member, const Type *MemberType, + const CXXRecordDecl *MemberCXXRD, + const RecordDecl *ClassCXXRD) const { + assert(Member); + assert(MemberType); + assert(MemberCXXRD); + + if (!Bug) + Bug = std::make_unique(this, "Uncounted member variable", + "WebKit"); + + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + Os << "Member variable "; + printQuotedQualifiedName(Os, Member); + + Os << " is a " + << (isa(MemberType) ? "raw pointer" : "reference") + << " to ref-countable type "; + printQuotedQualifiedName(Os, MemberCXXRD); + + 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 LangOptions &LO) { + return true; +} Index: clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -0,0 +1,59 @@ +//=======- PtrTypesSemantics.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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H +#define LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H + +namespace clang { +class CXXBaseSpecifier; +class CXXMethodDecl; +class CXXRecordDecl; +class Expr; +class FunctionDecl; +class Type; + +// Ref-countability of a type is implicitly defined by Ref and RefPtr +// implementation. It can be modeled as: type T having public methods ref() and +// deref() + +// In WebKit there are two ref-counted templated smart pointers: RefPtr and +// Ref. + +/// \returns CXXRecordDecl of the base if the type is ref-countable, nullptr if +/// not. +const clang::CXXRecordDecl *isRefCountable(const clang::CXXBaseSpecifier *Base); + +/// \returns true if \p Class is ref-countable, false if not. +/// Asserts that \p Class IS a definition. +bool isRefCountable(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 ref-countable AND not ref-counted, false if +/// not. Asserts that \p Class IS a definition. +bool isUncounted(const clang::CXXRecordDecl *Class); + +/// \returns true if \p T is either a raw pointer or reference to an uncounted +/// class, false if not. +bool isUncountedPtr(const clang::Type *T); + +/// \returns true if \p F creates ref-countable object from uncounted parameter, +/// false if not. +bool isCtorOfRefCounted(const clang::FunctionDecl *F); + +/// \returns true if \p M is getter of a ref-counted class, false if not. +bool isGetterOfRefCounted(const clang::CXXMethodDecl *Method); + +/// \returns true if \p F is a conversion between ref-countable or ref-counted +/// pointer types. +bool isPtrConversion(const FunctionDecl *F); + +} // namespace clang + +#endif Index: clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -0,0 +1,172 @@ +//=======- PtrTypesSemantics.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 "PtrTypesSemantics.h" +#include "ASTUtils.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" + +using llvm::Optional; +using namespace clang; + +namespace { + +bool hasPublicRefAndDeref(const CXXRecordDecl *R) { + assert(R); + + bool hasRef = false; + bool hasDeref = false; + for (const CXXMethodDecl *MD : R->methods()) { + const auto MethodName = safeGetName(MD); + + if (MethodName == "ref" && MD->getAccess() == AS_public) { + if (hasDeref) + return true; + hasRef = true; + } else if (MethodName == "deref" && MD->getAccess() == AS_public) { + if (hasRef) + return true; + hasDeref = true; + } + } + return false; +} + +} // namespace + +namespace clang { + +const CXXRecordDecl *isRefCountable(const CXXBaseSpecifier *Base) { + assert(Base); + + const Type *T = Base->getType().getTypePtrOrNull(); + if (!T) + return nullptr; + + const CXXRecordDecl *R = T->getAsCXXRecordDecl(); + if (!R) + return nullptr; + + return hasPublicRefAndDeref(R) ? R : nullptr; +}; + +bool isRefCountable(const CXXRecordDecl *R) { + assert(R); + + R = R->getDefinition(); + assert(R); + + if (hasPublicRefAndDeref(R)) + return true; + + CXXBasePaths Paths; + Paths.setOrigin(const_cast(R)); + + const auto isRefCountableBase = [](const CXXBaseSpecifier *Base, + CXXBasePath &) { + return clang::isRefCountable(Base); + }; + + return R->lookupInBases(isRefCountableBase, Paths, + /*LookupInDependent =*/true); +} + +bool isCtorOfRefCounted(const clang::FunctionDecl *F) { + assert(F); + const auto &FunctionName = safeGetName(F); + + return FunctionName == "Ref" || FunctionName == "makeRef" + + || FunctionName == "RefPtr" || FunctionName == "makeRefPtr" + + || FunctionName == "UniqueRef" || FunctionName == "makeUniqueRef" || + FunctionName == "makeUniqueRefWithoutFastMallocCheck" + + || FunctionName == "String" || FunctionName == "AtomString" || + FunctionName == "UniqueString" + // FIXME: Implement as attribute. + || FunctionName == "Identifier"; +} + +bool isUncounted(const CXXRecordDecl *Class) { + // Keep isRefCounted first as it's cheaper. + return !isRefCounted(Class) && isRefCountable(Class); +} + +bool isUncountedPtr(const Type *T) { + assert(T); + + if (T->isPointerType() || T->isReferenceType()) { + if (auto *CXXRD = T->getPointeeCXXRecordDecl()) { + return isUncounted(CXXRD); + } + } + return false; +} + +bool isGetterOfRefCounted(const CXXMethodDecl *M) { + assert(M); + + if (auto *calleeMethodDecl = dyn_cast(M)) { + const CXXRecordDecl *calleeMethodsClass = M->getParent(); + auto className = safeGetName(calleeMethodsClass); + auto methodName = safeGetName(M); + + if (((className == "Ref" || className == "RefPtr") && + methodName == "get") || + ((className == "String" || className == "AtomString" || + className == "AtomStringImpl" || className == "UniqueString" || + className == "UniqueStringImpl" || className == "Identifier") && + methodName == "impl")) + return true; + + // Ref -> T conversion + // FIXME: Currently allowing any Ref -> whatever cast. + if (className == "Ref" || className == "RefPtr") { + if (auto *maybeRefToRawOperator = dyn_cast(M)) { + if (auto *targetConversionType = + maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) { + if (isUncountedPtr(targetConversionType)) { + return true; + } + } + } + } + } + return false; +} + +bool isRefCounted(const CXXRecordDecl *R) { + assert(R); + if (auto *TmplR = R->getTemplateInstantiationPattern()) { + // FIXME: String/AtomString/UniqueString + const auto &ClassName = safeGetName(TmplR); + return ClassName == "RefPtr" || ClassName == "Ref"; + } + return false; +} + +bool isPtrConversion(const FunctionDecl *F) { + assert(F); + if (isCtorOfRefCounted(F)) + return true; + + // FIXME: check # of params == 1 + const auto FunctionName = safeGetName(F); + if (FunctionName == "getPtr" || FunctionName == "WeakPtr" || + FunctionName == "makeWeakPtr" + + || FunctionName == "downcast" || FunctionName == "bitwise_cast") + return true; + + return false; +} + +} // namespace clang Index: clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -0,0 +1,129 @@ +//=======- RefCntblBaseVirtualDtor.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 "DiagOutputUtils.h" +#include "PtrTypesSemantics.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/DeclCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchFinder.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" + +using namespace clang; +using namespace ento; +using namespace ast_matchers; + +namespace { + +class RefCntblBaseVirtualDtorChecker + : public Checker> { +private: + BugType Bug; + mutable BugReporter *BR; + +public: + + RefCntblBaseVirtualDtorChecker() + : Bug(this, "Ref-countable base class doesn't have virtual d-tor", + "WebKit") + { } + + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, + BugReporter &BRArg) const { + BR = &BRArg; + + DeclarationMatcher OffendingClassDefMatcher = + cxxRecordDecl( + anyOf( isClass(), isStruct() ), + hasABaseSpec( + cxxBaseSpecifier( + allOf( + isPublicBase(), + hasType( + cxxRecordDecl( + hasMethod(allOf(hasName("ref"), isPublic())), + hasMethod(allOf(hasName("deref"), isPublic())), + hasMethod( + allOf( + cxxDestructorDecl(), + unless(isVirtual()) + ) + ) + ).bind("ProblematicBaseClass") + ) + ) + ).bind("ProblematicBaseSpecifier") + ), + isDefinition(), + unless(anyOf( + isImplicit(), + isLambda(), + isExpansionInSystemHeader() + )) + ).bind("ProblematicDerivedClass"); + + MatchFinder F; + + class Callback : public MatchFinder::MatchCallback { + public: + const RefCntblBaseVirtualDtorChecker &Checker; + Callback(const RefCntblBaseVirtualDtorChecker &Checker) + : Checker(Checker) {} + virtual void run(const MatchFinder::MatchResult &Result) { + Checker.reportBug( + Result.Nodes.getNodeAs("ProblematicDerivedClass"), + Result.Nodes.getNodeAs("ProblematicBaseSpecifier"), + Result.Nodes.getNodeAs("ProblematicBaseClass") + ); + } + }; + Callback CB(*this); + + F.addMatcher(OffendingClassDefMatcher, &CB); + F.matchAST(TUD->getASTContext()); + } + + void reportBug(const CXXRecordDecl *DerivedClass, + const CXXBaseSpecifier *BaseSpec, + const CXXRecordDecl *ProblematicBaseClass) const { + assert(DerivedClass); + assert(BaseSpec); + assert(ProblematicBaseClass); + + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + Os << (ProblematicBaseClass->isClass() ? "Class" : "Struct") << " "; + printQuotedQualifiedName(Os, ProblematicBaseClass); + + Os << " is used as a base of " + << (DerivedClass->isClass() ? "class" : "struct") << " "; + printQuotedQualifiedName(Os, DerivedClass); + + Os << " but doesn't have virtual destructor"; + + PathDiagnosticLocation BSLoc(BaseSpec->getSourceRange().getBegin(), + BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(BaseSpec->getSourceRange()); + BR->emitReport(std::move(Report)); + } +}; +} // namespace + +void ento::registerWebKitRefCntblBaseVirtualDtorChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterWebKitRefCntblBaseVirtualDtorChecker( + const LangOptions &LO) { + return true; +} Index: clang/test/Analysis/Checkers/WebKit/mock-types.h =================================================================== --- /dev/null +++ clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -0,0 +1,48 @@ +#ifndef mock_types_1103988513531 +#define mock_types_1103988513531 + +template struct Ref { + T t; + + Ref() : t{} {}; + Ref(T *) {} + T *get() { return nullptr; } + operator const T &() const { return t; } + operator T &() { return t; } +}; + +template struct RefPtr { + T *t; + + RefPtr() : t(new T) {} + RefPtr(T *t) : t(t) {} + T *get() { return t; } + T *operator->() { return t; } + T &operator*() { return *t; } + RefPtr &operator=(T *) { return *this; } +}; + +template bool operator==(const RefPtr &, const RefPtr &) { + return false; +} + +template bool operator==(const RefPtr &, T *) { return false; } + +template bool operator==(const RefPtr &, T &) { return false; } + +template bool operator!=(const RefPtr &, const RefPtr &) { + return false; +} + +template bool operator!=(const RefPtr &, T *) { return false; } + +template bool operator!=(const RefPtr &, T &) { return false; } + +struct RefCountable { + void ref() {} + void deref() {} +}; + +template T *downcast(T *t) { return t; } + +#endif Index: clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.WebKitRefCntblBaseVirtualDtor -verify %s + +struct RefCntblBase { + void ref() {} + void deref() {} +}; + +template +struct DerivedClassTmpl1 : T { }; +// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl1' but doesn't have virtual destructor}} + +DerivedClassTmpl1 a; + + + +template +struct DerivedClassTmpl2 : T { }; +// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl2' but doesn't have virtual destructor}} + +template int foo(T) { DerivedClassTmpl2 f; return 42; } +int b = foo(RefCntblBase{}); + + + +template +struct DerivedClassTmpl3 : T { }; +// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl3' but doesn't have virtual destructor}} + +typedef DerivedClassTmpl3 Foo; +Foo c; Index: clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.WebKitRefCntblBaseVirtualDtor -verify %s + +struct RefCntblBase { + void ref() {} + void deref() {} +}; + +struct Derived : RefCntblBase { }; +// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'Derived' but doesn't have virtual destructor}} + +struct DerivedWithVirtualDtor : RefCntblBase { +// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedWithVirtualDtor' but doesn't have virtual destructor}} + virtual ~DerivedWithVirtualDtor() {} +}; + + + +template +struct DerivedClassTmpl : T { }; +typedef DerivedClassTmpl Foo; + + + +struct RandomBase {}; +struct RandomDerivedClass : RandomBase { }; + + + +struct FakeRefCntblBase1 { + private: + void ref() {} + void deref() {} +}; +struct Quiet1 : FakeRefCntblBase1 {}; + +struct FakeRefCntblBase2 { + protected: + void ref() {} + void deref() {} +}; +struct Quiet2 : FakeRefCntblBase2 {}; + +class FakeRefCntblBase3 { + void ref() {} + void deref() {} +}; +struct Quiet3 : FakeRefCntblBase3 {}; +struct Quiet4 : private RefCntblBase {}; +class Quiet5 : RefCntblBase {}; + +void foo () { + Derived d; +} 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=alpha.webkit.WebKitNoUncountedMemberChecker -verify %s + +#include "mock-types.h" + +namespace members { + struct Foo { + private: + RefCountable* a = nullptr; +// expected-warning@-1{{Member variable 'members::Foo::a' is a raw pointer to ref-countable type 'RefCountable'}} + + protected: + RefPtr b; + + public: + RefCountable silenceWarningAboutInit; + RefCountable& c = silenceWarningAboutInit; +// expected-warning@-1{{Member variable 'members::Foo::c' is a reference to ref-countable type 'RefCountable'}} + Ref d; + }; + + template + struct FooTmpl { + T* a; +// expected-warning@-1{{Member variable 'members::FooTmpl::a' 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) {} +} Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp =================================================================== --- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp +++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp @@ -1324,7 +1324,7 @@ } TEST(TypeMatching, PointerTypes) { - // FIXME: Reactive when these tests can be more specific (not matching + // FIXME: Reactivate when these tests can be more specific (not matching // implicit code on certain platforms), likely when we have hasDescendant for // Types/TypeLocs. //EXPECT_TRUE(matchAndVerifyResultTrue( @@ -1396,6 +1396,12 @@ hasType(lValueReferenceType())))); EXPECT_TRUE(matches(Fragment, varDecl(hasName("ref"), hasType(rValueReferenceType())))); + EXPECT_TRUE(matches( + "struct a { int* b; };", + fieldDecl(hasType(pointerType())))); + EXPECT_TRUE(matches( + "struct a { int& b; };", + fieldDecl(hasType(referenceType())))); } TEST(TypeMatching, AutoRefTypes) {