New loplugin:dyncastvisibility

...to find uses of dynamic_cast where the static (base) type has hidden
visibility while the dynamic (derived) one has default visibility, and which may
thus fail at least on macOS like happened in
d5ed3cd6db "Make WinMtfFontStyle's base class
EMFIO_DLLPUBLIC, too".

libcxxabi's __dynamic_cast takes static_type and dst_type arguments.  Now, if
dst_type (the derived type, with default visibility) is taken from .so A (and
thus references the version of the base type info hidden in .so A) but the
__dynamic_cast call is made from .so B, it passes for static_type the base type
information hidden in .so B, and __dynamic_cast will consider the cast to fail.
I'm not sure whether hidden intermediary types (in the hierarchy between the
dynamic_cast's base and derived types) acutally cause a problem too, but lets
flag them with the plugin anyway.

The fixes use SAL_DLLPUBLIC_RTTI.  For one, there appear to be no other reasons
than type visibility to make those classes SAL_DLLPUBLIC.  For another, this
nicely avoids any actual changes on Windows (where SAL_DLLPUBLIC expands to
nothing, and many of the affected classes were explicityl introduced into class
hierarchies as "MSVC hacks").

Change-Id: Ia85a9635cebffb1009a9efc1484b8bd4025585d4
Reviewed-on: https://gerrit.libreoffice.org/41802
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This commit is contained in:
Stephan Bergmann
2017-09-01 20:14:25 +02:00
parent 326295bf10
commit 595371e520
15 changed files with 178 additions and 17 deletions

View File

@@ -0,0 +1,161 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
/*
* This file is part of the LibreOffice project.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
#include <cassert>
#include <set>
#include "plugin.hxx"
namespace {
using Bases = std::set<CXXRecordDecl const *>;
Visibility getTypeVisibility(CXXRecordDecl const * decl) {
assert(decl->isThisDeclarationADefinition());
if (auto const opt = decl->getExplicitVisibility(
NamedDecl::VisibilityForType))
{
return *opt;
}
if (auto const opt = decl->getExplicitVisibility(
NamedDecl::VisibilityForValue))
{
return *opt;
}
auto const vis = decl->getVisibility();
return vis == DefaultVisibility && decl->isInAnonymousNamespace()
? HiddenVisibility : vis;
}
// Check whether 'decl' is derived from 'base', gathering any 'bases' between
// 'decl' and 'base', and whether any of those 'bases' or 'base' are 'hidden'
// (i.e., have non-default visibility):
bool isDerivedFrom(
CXXRecordDecl const * decl, CXXRecordDecl const * base, Bases * bases,
bool * hidden)
{
bool derived = false;
for (auto const i: decl->bases()) {
auto const bd
= (cast<CXXRecordDecl>(i.getType()->getAs<RecordType>()->getDecl())
->getDefinition());
assert(bd != nullptr);
if (bd == base) {
*hidden |= getTypeVisibility(base) != DefaultVisibility;
derived = true;
}
else if (bd->isDerivedFrom(base)) {
if (bases->insert(bd).second) {
auto const d = isDerivedFrom(bd, base, bases, hidden);
assert(d);
*hidden |= getTypeVisibility(bd) != DefaultVisibility;
}
derived = true;
}
}
return derived;
}
StringRef vis(Visibility v) {
switch (v) {
case HiddenVisibility:
return "hidden";
case ProtectedVisibility:
return "protected";
case DefaultVisibility:
return "default";
default:
llvm_unreachable("unknown visibility");
}
}
class Visitor final:
public RecursiveASTVisitor<Visitor>, public loplugin::Plugin
{
public:
explicit Visitor(InstantiationData const & data): Plugin(data) {}
bool shouldVisitTemplateInstantiations() const { return true; }
bool VisitCXXDynamicCastExpr(CXXDynamicCastExpr const * expr) {
if (ignoreLocation(expr)) {
return true;
}
auto td = expr->getTypeAsWritten();
if (auto const t = td->getAs<ReferenceType>()) {
td = t->getPointeeType();
}
while (auto const t = td->getAs<PointerType>()) {
td = t->getPointeeType();
}
auto const rtd = td->getAs<RecordType>();
if (rtd == nullptr) {
return true;
}
auto const rdd = cast<CXXRecordDecl>(rtd->getDecl())->getDefinition();
assert(rdd != nullptr);
if (getTypeVisibility(rdd) != DefaultVisibility) {
return true;
}
auto ts = expr->getSubExpr()->getType();
while (auto const t = ts->getAs<PointerType>()) {
ts = t->getPointeeType();
}
auto const rts = ts->getAs<RecordType>();
assert(rts != nullptr);
auto const rds = cast<CXXRecordDecl>(rts->getDecl())->getDefinition();
assert(rds != nullptr);
Bases bs;
bool hidden = false;
if (!(isDerivedFrom(rdd, rds, &bs, &hidden) && hidden)) {
return true;
}
report(
DiagnosticsEngine::Warning,
("dynamic_cast from %0 with %1 type visibility to %2 with %3 type"
" visibility"),
expr->getExprLoc())
<< ts << vis(getTypeVisibility(rds)) << td
<< vis(getTypeVisibility(rdd)) << expr->getSourceRange();
report(
DiagnosticsEngine::Note,
"base class %0 with %1 type visibility defined here",
rds->getLocation())
<< ts << vis(getTypeVisibility(rds)) << rds->getSourceRange();
for (auto const i: bs) {
if (getTypeVisibility(i) != DefaultVisibility) {
report(
DiagnosticsEngine::Note,
("intermediary class %0 with %1 type visibility defined"
" here"),
i->getLocation())
<< i << vis(getTypeVisibility(i)) << i->getSourceRange();
}
}
report(
DiagnosticsEngine::Note,
"derived class %0 with %1 type visibility defined here",
rdd->getLocation())
<< td << vis(getTypeVisibility(rdd)) << rdd->getSourceRange();
return true;
}
private:
void run() override {
if (compiler.getLangOpts().CPlusPlus) {
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
}
};
static loplugin::Plugin::Registration<Visitor> reg("dyncastvisibility");
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

View File

@@ -33,7 +33,7 @@ class SvXMLUnitConverter;
*/
// MSVC hack:
class SvxLanguageItem_Base: public SfxEnumItem<LanguageType> {
class SAL_DLLPUBLIC_RTTI SvxLanguageItem_Base: public SfxEnumItem<LanguageType> {
protected:
explicit SvxLanguageItem_Base(sal_uInt16 nWhich, LanguageType nValue):
SfxEnumItem(nWhich, nValue)

View File

@@ -81,7 +81,7 @@ public:
enum class SotClipboardFormatId : sal_uLong;
class BaseStorage : public StorageBase
class SAL_DLLPUBLIC_RTTI BaseStorage : public StorageBase
{
public:
virtual const OUString& GetName() const = 0;

View File

@@ -30,7 +30,7 @@
class SfxAllEnumValueArr;
// MSVC hack:
class SfxAllEnumItem_Base: public SfxEnumItem<sal_uInt16> {
class SAL_DLLPUBLIC_RTTI SfxAllEnumItem_Base: public SfxEnumItem<sal_uInt16> {
protected:
explicit SfxAllEnumItem_Base(sal_uInt16 nWhich, sal_uInt16 nValue):
SfxEnumItem(nWhich, nValue)

View File

@@ -26,7 +26,7 @@
template<typename EnumT>
class SfxEnumItem : public SfxEnumItemInterface
class SAL_DLLPUBLIC_RTTI SfxEnumItem : public SfxEnumItemInterface
{
EnumT m_nValue;

View File

@@ -294,7 +294,7 @@ class SdrObject;
class SdrPathObj;
class SdrModel;
class SAL_WARN_UNUSED SvxPreviewBase : public Control
class SAL_WARN_UNUSED SAL_DLLPUBLIC_RTTI SvxPreviewBase : public Control
{
private:
SdrModel* mpModel;

View File

@@ -26,7 +26,7 @@
#include <svx/svxdllapi.h>
// MSVC hack:
class SdrGrafModeItem_Base: public SfxEnumItem<GraphicDrawMode> {
class SAL_DLLPUBLIC_RTTI SdrGrafModeItem_Base: public SfxEnumItem<GraphicDrawMode> {
protected:
SdrGrafModeItem_Base(GraphicDrawMode eMode):
SfxEnumItem(SDRATTR_GRAFMODE, eMode) {}

View File

@@ -233,7 +233,7 @@ public:
// including a HelperInterface implementation
template< typename... Ifc >
class ScVbaCollectionBase : public InheritedHelperInterfaceImpl< Ifc... >
class SAL_DLLPUBLIC_RTTI ScVbaCollectionBase : public InheritedHelperInterfaceImpl< Ifc... >
{
typedef InheritedHelperInterfaceImpl< Ifc... > BaseColBase;
protected:
@@ -341,7 +341,7 @@ public:
};
template < typename... Ifc > // where Ifc must implement XCollectionTest
class CollTestImplHelper : public ScVbaCollectionBase< ::cppu::WeakImplHelper< Ifc... > >
class SAL_DLLPUBLIC_RTTI CollTestImplHelper : public ScVbaCollectionBase< ::cppu::WeakImplHelper< Ifc... > >
{
typedef ScVbaCollectionBase< ::cppu::WeakImplHelper< Ifc... > > ImplBase;

View File

@@ -73,7 +73,7 @@ public:
void End();
};
class View : public FmFormView
class SAL_DLLPUBLIC_RTTI View : public FmFormView
{
public:

View File

@@ -37,7 +37,7 @@ class DrawDocShell;
class View;
}
class SdTransferable : public TransferableHelper, public SfxListener
class SAL_DLLPUBLIC_RTTI SdTransferable : public TransferableHelper, public SfxListener
{
public:

View File

@@ -30,7 +30,7 @@
class SwDoc;
namespace sw{ class DocumentStylePoolManager; }
class SwFormatColl : public SwFormat
class SAL_DLLPUBLIC_RTTI SwFormatColl : public SwFormat
{
protected:
SwFormatColl( SwAttrPool& rPool, const sal_Char* pFormatName,

View File

@@ -292,7 +292,7 @@ private:
};
/// Starts a section of nodes in the document model.
class SwStartNode: public SwNode
class SAL_DLLPUBLIC_RTTI SwStartNode: public SwNode
{
friend class SwNode;
friend class SwNodes;

View File

@@ -37,7 +37,7 @@
class SfxItemPool;
class SvXMLAttrContainerItem;
class SwTextAttr
class SAL_DLLPUBLIC_RTTI SwTextAttr
{
private:
SfxPoolItem * const m_pAttr;
@@ -123,7 +123,7 @@ public:
void dumpAsXml(struct _xmlTextWriter* pWriter) const;
};
class SwTextAttrEnd : public virtual SwTextAttr
class SAL_DLLPUBLIC_RTTI SwTextAttrEnd : public virtual SwTextAttr
{
protected:
sal_Int32 m_nEnd;
@@ -135,7 +135,7 @@ public:
};
// attribute that must not overlap others
class SwTextAttrNesting : public SwTextAttrEnd
class SAL_DLLPUBLIC_RTTI SwTextAttrNesting : public SwTextAttrEnd
{
protected:
SwTextAttrNesting( SfxPoolItem & i_rAttr,

View File

@@ -34,7 +34,7 @@ class SwTextFrame;
// implemented in cntfrm.cxx, used in cntfrm.cxx and crsrsh.cxx
extern bool GetFrameInPage( const SwContentFrame*, SwWhichPage, SwPosPage, SwPaM* );
class SwContentFrame: public SwFrame, public SwFlowFrame
class SAL_DLLPUBLIC_RTTI SwContentFrame: public SwFrame, public SwFlowFrame
{
friend void MakeNxt( SwFrame *pFrame, SwFrame *pNxt ); // calls MakePrtArea

View File

@@ -36,7 +36,7 @@ class SfxItemSet;
class SwCursorShell;
struct DBTextStruct_Impl;
class SwBaseShell: public SfxShell
class SAL_DLLPUBLIC_RTTI SwBaseShell: public SfxShell
{
SwView &rView;