Reland "[Analyzer][WebKit] RefCntblBaseVirtualDtorChecker"

This reverts commit 1108f5c737.
This commit is contained in:
Jan Korous 2020-05-21 14:37:41 -07:00
parent e6b613254d
commit 54e91a3c70
11 changed files with 669 additions and 0 deletions

View File

@ -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:

View File

@ -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

View File

@ -121,6 +121,8 @@ add_clang_library(clangStaticAnalyzerCheckers
VLASizeChecker.cpp
ValistChecker.cpp
VirtualCallChecker.cpp
WebKit/PtrTypesSemantics.cpp
WebKit/RefCntblBaseVirtualDtorChecker.cpp
LINK_LIBS
clangAST

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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;
}

View File

@ -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

View File

@ -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@-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@-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@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl3<RefCntblBase>' but doesn't have virtual destructor}}
typedef DerivedClassTmpl3<RefCntblBase> Foo;
Foo c;

View File

@ -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@-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<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;
}