[clang] 54e91a3 - Reland "[Analyzer][WebKit] RefCntblBaseVirtualDtorChecker"

Jan Korous via cfe-commits cfe-commits at lists.llvm.org
Thu May 21 16:43:14 PDT 2020


Author: Jan Korous
Date: 2020-05-21T16:41:00-07:00
New Revision: 54e91a3c701040d9d2e467bd483c197073b2c5e4

URL: https://github.com/llvm/llvm-project/commit/54e91a3c701040d9d2e467bd483c197073b2c5e4
DIFF: https://github.com/llvm/llvm-project/commit/54e91a3c701040d9d2e467bd483c197073b2c5e4.diff

LOG: Reland "[Analyzer][WebKit] RefCntblBaseVirtualDtorChecker"

This reverts commit 1108f5c737dbdab0277874a7e5b237491839c43a.

Added: 
    clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
    clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
    clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
    clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
    clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
    clang/test/Analysis/Checkers/WebKit/mock-types.h
    clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
    clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp

Modified: 
    clang/docs/analyzer/checkers.rst
    clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 0bfb6456dc82..ea3f0e838b6a 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1374,6 +1374,33 @@ double freed, or use after freed. This check attempts to find such problems.
    zx_handle_close(sb);
  }
 
+WebKit
+^^^^^^
+
+WebKit is an open-source web browser engine available for macOS, iOS and Linux.
+This section describes checkers that can find issues in WebKit codebase.
+
+Most of the checkers focus on memory management for which WebKit uses custom implementation of reference counted smartpointers.
+Checker are formulated in terms related to ref-counting:
+* *Ref-counted type* is either ``Ref<T>`` or ``RefPtr<T>``.
+* *Ref-countable type* is any type that implements ``ref()`` and ``deref()`` methods as ``RefPtr<>`` is a template (i. e. relies on duck typing).
+* *Uncounted type* is ref-countable but not ref-counted type.
+
+.. _webkit-RefCntblBaseVirtualDtor:
+
+webkit.RefCntblBaseVirtualDtor
+""""""""""""""""""""""""""""""""""""
+All uncounted types used as base classes must have a virtual destructor.
+
+Ref-counted types hold their ref-countable data by a raw pointer and allow implicit upcasting from ref-counted pointer to derived type to ref-counted pointer to base type. This might lead to an object of (dynamic) derived type being deleted via pointer to the base class type which C++ standard defines as UB in case the base class doesn't have virtual destructor ``[expr.delete]``.
+
+.. code-block:: cpp
+ struct RefCntblBase {
+   void ref() {}
+   void deref() {}
+ };
+
+ struct Derived : RefCntblBase { }; // warn
 
 .. _alpha-checkers:
 

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 93c4d964d772..f0ad8326929e 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -116,6 +116,9 @@ def NonDeterminismAlpha : Package<"nondeterminism">, ParentPackage<Alpha>;
 def Fuchsia : Package<"fuchsia">;
 def FuchsiaAlpha : Package<"fuchsia">, ParentPackage<Alpha>;
 
+def WebKit : Package<"webkit">;
+def WebKitAlpha : Package<"webkit">, ParentPackage<Alpha>;
+
 //===----------------------------------------------------------------------===//
 // Core Checkers.
 //===----------------------------------------------------------------------===//
@@ -1620,3 +1623,13 @@ def FuchsiaLockChecker : Checker<"Lock">,
 
 } // end fuchsia
 
+//===----------------------------------------------------------------------===//
+// WebKit checkers.
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = WebKit in {
+
+def RefCntblBaseVirtualDtorChecker : Checker<"RefCntblBaseVirtualDtor">,
+  HelpText<"Check for any ref-countable base class having virtual destructor.">,
+  Documentation<HasDocumentation>;
+} // end webkit

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index bcf2dfdb8326..4f885fadf415 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -121,6 +121,8 @@ add_clang_library(clangStaticAnalyzerCheckers
   VLASizeChecker.cpp
   ValistChecker.cpp
   VirtualCallChecker.cpp
+  WebKit/PtrTypesSemantics.cpp
+  WebKit/RefCntblBaseVirtualDtorChecker.cpp
 
   LINK_LIBS
   clangAST

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
new file mode 100644
index 000000000000..26d79cfcd9b5
--- /dev/null
+++ b/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 <string>
+#include <utility>
+
+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<const clang::Expr *, bool>
+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 <typename T> std::string safeGetName(const T *ASTNode) {
+  const auto *const ND = llvm::dyn_cast_or_null<clang::NamedDecl>(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

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
new file mode 100644
index 000000000000..4979b8ffc2b2
--- /dev/null
+++ b/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 <typename NamedDeclDerivedT>
+void printQuotedQualifiedName(llvm::raw_ostream &Os,
+                              const NamedDeclDerivedT &D) {
+  Os << "'";
+  D->getNameForDiagnostic(Os, D->getASTContext().getPrintingPolicy(),
+                          /*Qualified=*/true);
+  Os << "'";
+}
+
+} // namespace clang
+
+#endif

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
new file mode 100644
index 000000000000..adb6253df965
--- /dev/null
+++ b/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<CXXRecordDecl *>(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<CXXMethodDecl>(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> -> T conversion
+    // FIXME: Currently allowing any Ref<T> -> whatever cast.
+    if (className == "Ref" || className == "RefPtr") {
+      if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(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

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
new file mode 100644
index 000000000000..83d9c0bcc13b
--- /dev/null
+++ b/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<T> and RefPtr<T>
+// 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<T> and
+// Ref<T>.
+
+/// \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

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
new file mode 100644
index 000000000000..81ce284c2dc7
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -0,0 +1,167 @@
+//=======- 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/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"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class RefCntblBaseVirtualDtorChecker
+    : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+  BugType Bug;
+  mutable BugReporter *BR;
+
+public:
+  RefCntblBaseVirtualDtorChecker()
+      : Bug(this,
+            "Reference-countable base class doesn't have virtual destructor",
+            "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 RefCntblBaseVirtualDtorChecker *Checker;
+      explicit LocalVisitor(const RefCntblBaseVirtualDtorChecker *Checker)
+          : Checker(Checker) {
+        assert(Checker);
+      }
+
+      bool shouldVisitTemplateInstantiations() const { return true; }
+      bool shouldVisitImplicitCode() const { return false; }
+
+      bool VisitCXXRecordDecl(const CXXRecordDecl *RD) {
+        Checker->visitCXXRecordDecl(RD);
+        return true;
+      }
+    };
+
+    LocalVisitor visitor(this);
+    visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+  }
+
+  void visitCXXRecordDecl(const CXXRecordDecl *RD) const {
+    if (shouldSkipDecl(RD))
+      return;
+
+    CXXBasePaths Paths;
+    Paths.setOrigin(RD);
+
+    const CXXBaseSpecifier *ProblematicBaseSpecifier = nullptr;
+    const CXXRecordDecl *ProblematicBaseClass = nullptr;
+
+    const auto IsPublicBaseRefCntblWOVirtualDtor =
+        [RD, &ProblematicBaseSpecifier,
+         &ProblematicBaseClass](const CXXBaseSpecifier *Base, CXXBasePath &) {
+          const auto AccSpec = Base->getAccessSpecifier();
+          if (AccSpec == AS_protected || AccSpec == AS_private ||
+              (AccSpec == AS_none && RD->isClass()))
+            return false;
+
+          llvm::Optional<const clang::CXXRecordDecl *> MaybeRefCntblBaseRD =
+              isRefCountable(Base);
+          if (!MaybeRefCntblBaseRD.hasValue())
+            return false;
+
+          const CXXRecordDecl *RefCntblBaseRD = MaybeRefCntblBaseRD.getValue();
+          if (!RefCntblBaseRD)
+            return false;
+
+          const auto *Dtor = RefCntblBaseRD->getDestructor();
+          if (!Dtor || !Dtor->isVirtual()) {
+            ProblematicBaseSpecifier = Base;
+            ProblematicBaseClass = RefCntblBaseRD;
+            return true;
+          }
+
+          return false;
+        };
+
+    if (RD->lookupInBases(IsPublicBaseRefCntblWOVirtualDtor, Paths,
+                          /*LookupInDependent =*/true)) {
+      reportBug(RD, ProblematicBaseSpecifier, ProblematicBaseClass);
+    }
+  }
+
+  bool shouldSkipDecl(const CXXRecordDecl *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();
+    if (Kind != TTK_Struct && Kind != TTK_Class)
+      return true;
+
+    // Ignore CXXRecords that come from system headers.
+    if (BR->getSourceManager().getFileCharacteristic(RDLocation) !=
+        SrcMgr::C_User)
+      return true;
+
+    return false;
+  }
+
+  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<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(BaseSpec->getSourceRange());
+    BR->emitReport(std::move(Report));
+  }
+};
+} // namespace
+
+void ento::registerRefCntblBaseVirtualDtorChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<RefCntblBaseVirtualDtorChecker>();
+}
+
+bool ento::shouldRegisterRefCntblBaseVirtualDtorChecker(
+    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
new file mode 100644
index 000000000000..5f570b8bee8c
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -0,0 +1,48 @@
+#ifndef mock_types_1103988513531
+#define mock_types_1103988513531
+
+template <typename T> struct Ref {
+  T t;
+
+  Ref() : t{} {};
+  Ref(T *) {}
+  T *get() { return nullptr; }
+  operator const T &() const { return t; }
+  operator T &() { return t; }
+};
+
+template <typename T> 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 <typename T> bool operator==(const RefPtr<T> &, const RefPtr<T> &) {
+  return false;
+}
+
+template <typename T> bool operator==(const RefPtr<T> &, T *) { return false; }
+
+template <typename T> bool operator==(const RefPtr<T> &, T &) { return false; }
+
+template <typename T> bool operator!=(const RefPtr<T> &, const RefPtr<T> &) {
+  return false;
+}
+
+template <typename T> bool operator!=(const RefPtr<T> &, T *) { return false; }
+
+template <typename T> bool operator!=(const RefPtr<T> &, T &) { return false; }
+
+struct RefCountable {
+  void ref() {}
+  void deref() {}
+};
+
+template <typename T> T *downcast(T *t) { return t; }
+
+#endif

diff  --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
new file mode 100644
index 000000000000..3338fa9368e4
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
+
+struct RefCntblBase {
+  void ref() {}
+  void deref() {}
+};
+
+template<class T>
+struct DerivedClassTmpl1 : T { };
+// expected-warning at -1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl1<RefCntblBase>' but doesn't have virtual destructor}}
+
+DerivedClassTmpl1<RefCntblBase> a;
+
+
+
+template<class T>
+struct DerivedClassTmpl2 : T { };
+// expected-warning at -1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl2<RefCntblBase>' but doesn't have virtual destructor}}
+
+template<class T> int foo(T) { DerivedClassTmpl2<T> f; return 42; }
+int b = foo(RefCntblBase{});
+
+
+
+template<class T>
+struct DerivedClassTmpl3 : T { };
+// expected-warning at -1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl3<RefCntblBase>' but doesn't have virtual destructor}}
+
+typedef DerivedClassTmpl3<RefCntblBase> Foo;
+Foo c;

diff  --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
new file mode 100644
index 000000000000..1fc59c108b0e
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
+
+struct RefCntblBase {
+  void ref() {}
+  void deref() {}
+};
+
+struct Derived : RefCntblBase { };
+// expected-warning at -1{{Struct 'RefCntblBase' is used as a base of struct 'Derived' but doesn't have virtual destructor}}
+
+struct DerivedWithVirtualDtor : RefCntblBase {
+// expected-warning at -1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedWithVirtualDtor' but doesn't have virtual destructor}}
+  virtual ~DerivedWithVirtualDtor() {}
+};
+
+
+
+template<class T>
+struct DerivedClassTmpl : T { };
+typedef DerivedClassTmpl<RefCntblBase> 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;
+}


        


More information about the cfe-commits mailing list