[Libreoffice-commits] core.git: compilerplugins/clang include/editeng include/sot include/svl include/svx include/vbahelper sd/source sw/inc sw/source

Stephan Bergmann sbergman at redhat.com
Mon Sep 4 07:08:39 UTC 2017


 compilerplugins/clang/dyncastvisibility.cxx |  161 ++++++++++++++++++++++++++++
 include/editeng/langitem.hxx                |    2 
 include/sot/stg.hxx                         |    2 
 include/svl/aeitem.hxx                      |    2 
 include/svl/eitem.hxx                       |    2 
 include/svx/dlgctrl.hxx                     |    2 
 include/svx/sdgmoitm.hxx                    |    2 
 include/vbahelper/vbacollectionimpl.hxx     |    4 
 sd/source/ui/inc/View.hxx                   |    2 
 sd/source/ui/inc/sdxfer.hxx                 |    2 
 sw/inc/fmtcol.hxx                           |    2 
 sw/inc/node.hxx                             |    2 
 sw/inc/txatbase.hxx                         |    6 -
 sw/source/core/inc/cntfrm.hxx               |    2 
 sw/source/uibase/inc/basesh.hxx             |    2 
 15 files changed, 178 insertions(+), 17 deletions(-)

New commits:
commit 595371e520ce4f64ad9d99a7866bdb8404271b6e
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Fri Sep 1 20:14:25 2017 +0200

    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
    d5ed3cd6dbd22bb18542778f1c48f4d5b3ae0f95 "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 at libreoffice.org>
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/dyncastvisibility.cxx b/compilerplugins/clang/dyncastvisibility.cxx
new file mode 100644
index 000000000000..e212d7f2254d
--- /dev/null
+++ b/compilerplugins/clang/dyncastvisibility.cxx
@@ -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: */
diff --git a/include/editeng/langitem.hxx b/include/editeng/langitem.hxx
index d901d510f92d..e90b6cbee1f2 100644
--- a/include/editeng/langitem.hxx
+++ b/include/editeng/langitem.hxx
@@ -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)
diff --git a/include/sot/stg.hxx b/include/sot/stg.hxx
index e565c3e7b266..f0d8ee70ff1b 100644
--- a/include/sot/stg.hxx
+++ b/include/sot/stg.hxx
@@ -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;
diff --git a/include/svl/aeitem.hxx b/include/svl/aeitem.hxx
index 9f99ea85a934..73cb3b345a86 100644
--- a/include/svl/aeitem.hxx
+++ b/include/svl/aeitem.hxx
@@ -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)
diff --git a/include/svl/eitem.hxx b/include/svl/eitem.hxx
index e8933f0bcdaf..cbfc1a4930a0 100644
--- a/include/svl/eitem.hxx
+++ b/include/svl/eitem.hxx
@@ -26,7 +26,7 @@
 
 
 template<typename EnumT>
-class SfxEnumItem : public SfxEnumItemInterface
+class SAL_DLLPUBLIC_RTTI SfxEnumItem : public SfxEnumItemInterface
 {
     EnumT m_nValue;
 
diff --git a/include/svx/dlgctrl.hxx b/include/svx/dlgctrl.hxx
index 4fbbacde7361..612548934266 100644
--- a/include/svx/dlgctrl.hxx
+++ b/include/svx/dlgctrl.hxx
@@ -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;
diff --git a/include/svx/sdgmoitm.hxx b/include/svx/sdgmoitm.hxx
index 9b0eeb431464..7e508e16cabd 100644
--- a/include/svx/sdgmoitm.hxx
+++ b/include/svx/sdgmoitm.hxx
@@ -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) {}
diff --git a/include/vbahelper/vbacollectionimpl.hxx b/include/vbahelper/vbacollectionimpl.hxx
index db65f542dcb2..94afa6daee1a 100644
--- a/include/vbahelper/vbacollectionimpl.hxx
+++ b/include/vbahelper/vbacollectionimpl.hxx
@@ -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;
 
diff --git a/sd/source/ui/inc/View.hxx b/sd/source/ui/inc/View.hxx
index 98c1f0100170..51951f54d8e1 100644
--- a/sd/source/ui/inc/View.hxx
+++ b/sd/source/ui/inc/View.hxx
@@ -73,7 +73,7 @@ public:
     void End();
 };
 
-class View : public FmFormView
+class SAL_DLLPUBLIC_RTTI View : public FmFormView
 {
 public:
 
diff --git a/sd/source/ui/inc/sdxfer.hxx b/sd/source/ui/inc/sdxfer.hxx
index 6eb32921dce3..c062f768f5a5 100644
--- a/sd/source/ui/inc/sdxfer.hxx
+++ b/sd/source/ui/inc/sdxfer.hxx
@@ -37,7 +37,7 @@ class DrawDocShell;
 class View;
 }
 
-class SdTransferable : public TransferableHelper, public SfxListener
+class SAL_DLLPUBLIC_RTTI SdTransferable : public TransferableHelper, public SfxListener
 {
 public:
 
diff --git a/sw/inc/fmtcol.hxx b/sw/inc/fmtcol.hxx
index 1efc50c130ae..5511ccd7d022 100644
--- a/sw/inc/fmtcol.hxx
+++ b/sw/inc/fmtcol.hxx
@@ -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,
diff --git a/sw/inc/node.hxx b/sw/inc/node.hxx
index 48411c5facc6..632ae629336e 100644
--- a/sw/inc/node.hxx
+++ b/sw/inc/node.hxx
@@ -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;
diff --git a/sw/inc/txatbase.hxx b/sw/inc/txatbase.hxx
index 5415c9904586..758a72736ae2 100644
--- a/sw/inc/txatbase.hxx
+++ b/sw/inc/txatbase.hxx
@@ -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,
diff --git a/sw/source/core/inc/cntfrm.hxx b/sw/source/core/inc/cntfrm.hxx
index 5826ca4ff9b9..72d544d13606 100644
--- a/sw/source/core/inc/cntfrm.hxx
+++ b/sw/source/core/inc/cntfrm.hxx
@@ -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
 
diff --git a/sw/source/uibase/inc/basesh.hxx b/sw/source/uibase/inc/basesh.hxx
index 04c2db8e9034..17d0a8336601 100644
--- a/sw/source/uibase/inc/basesh.hxx
+++ b/sw/source/uibase/inc/basesh.hxx
@@ -36,7 +36,7 @@ class SfxItemSet;
 class SwCursorShell;
 
 struct DBTextStruct_Impl;
-class SwBaseShell: public SfxShell
+class SAL_DLLPUBLIC_RTTI SwBaseShell: public SfxShell
 {
     SwView      &rView;
 


More information about the Libreoffice-commits mailing list