[Libreoffice-commits] core.git: compilerplugins/clang

[Michael Stahl]

 compilerplugins/clang/badstatics.cxx |   78 +++++++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 8 deletions(-)

New commits:
commit 2ee802245ed9614734506a9d46edcbcee53ea2cc
Author: Michael Stahl <mstahl at redhat.com>
Date:   Thu Nov 12 22:33:36 2015 +0100

    compilerplugins: enhance badstatics plugin to follow pointers
    
    .... and references. This gives numerous false positives as pointers may
    be re-set prior to shutdown, so needs a white-list.
    
    Change-Id: I19a011c6f19501cc31b3d9ae76b599296f132478
    Reviewed-on: https://gerrit.libreoffice.org/19949
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>

diff --git a/compilerplugins/clang/badstatics.cxx b/compilerplugins/clang/badstatics.cxx
index c41b12d..154098c 100644
--- a/compilerplugins/clang/badstatics.cxx
+++ b/compilerplugins/clang/badstatics.cxx
@@ -11,6 +11,11 @@
 
 namespace {
 
+static bool startsWith(const std::string& s, const char* other)
+{
+    return s.compare(0, strlen(other), other) == 0;
+}
+
 class BadStatics
     : public clang::RecursiveASTVisitor<BadStatics>
     , public loplugin::Plugin
@@ -25,10 +30,23 @@ public:
         }
     }
 
-    /*static*/ std::pair<bool, FieldDecl const*> isBadStaticType(
-            QualType const& rType, FieldDecl const*const pCurrentFieldDecl)
+    static std::pair<bool, FieldDecl const*> isBadStaticType(
+            QualType const& rpType, FieldDecl const*const pCurrentFieldDecl,
+            std::vector<QualType> const& rParents)
     {
-        QualType const pCanonical(rType.getUnqualifiedType().getCanonicalType());
+        QualType const pCanonical(rpType.getUnqualifiedType().getCanonicalType());
+        if (pCanonical->isPointerType() || pCanonical->isReferenceType()) {
+            QualType const pPointee(pCanonical->getPointeeType().getUnqualifiedType().getCanonicalType());
+            auto const iter(std::find(rParents.begin(), rParents.end(), pPointee));
+            if (iter == rParents.end())
+            {
+                std::vector<QualType> copy(rParents);
+                copy.push_back(pCanonical);
+                return isBadStaticType(pPointee, pCurrentFieldDecl, copy);
+            } else {
+                return std::make_pair(false, nullptr);
+            }
+        }
         RecordType const*const pRecordType(pCanonical->getAs<RecordType>());
         if (!pRecordType) {
             return std::make_pair(false, nullptr);
@@ -42,23 +60,36 @@ public:
             return std::make_pair(true, pCurrentFieldDecl);
         }
         RecordDecl const*const pDefinition(pRecordType->getDecl()->getDefinition());
-        assert(pDefinition);
+        if (!pDefinition) { // maybe no definition if it's a pointer/reference
+            return std::make_pair(false, nullptr);
+        }
+        if (   startsWith(type, "class vcl::DeleteOnDeinit")
+            || startsWith(type, "class std::weak_ptr") // not owning
+            || type == "class ImplWallpaper" // very odd static instance here
+            || type == "class Application" // numerous odd subclasses in vclmain::createApplication()
+            || type == "class DemoMtfApp" // one of these Application with own VclPtr
+           )
+        {
+            return std::make_pair(false, nullptr);
+        }
+        std::vector<QualType> copy(rParents);
+        copy.push_back(pCanonical);
         CXXRecordDecl const*const pDecl(dyn_cast<CXXRecordDecl>(pDefinition));
         assert(pDecl);
         for (auto it = pDecl->field_begin(); it != pDecl->field_end(); ++it) {
-            auto const ret(isBadStaticType((*it)->getType(), *it));
+            auto const ret(isBadStaticType((*it)->getType(), *it, copy));
             if (ret.first) {
                 return ret;
             }
         }
         for (auto it = pDecl->bases_begin(); it != pDecl->bases_end(); ++it) {
-            auto const ret(isBadStaticType((*it).getType(), pCurrentFieldDecl));
+            auto const ret(isBadStaticType((*it).getType(), pCurrentFieldDecl, copy));
             if (ret.first) {
                 return ret;
             }
         }
         for (auto it = pDecl->vbases_begin(); it != pDecl->vbases_end(); ++it) {
-            auto const ret(isBadStaticType((*it).getType(), pCurrentFieldDecl));
+            auto const ret(isBadStaticType((*it).getType(), pCurrentFieldDecl, copy));
             if (ret.first) {
                 return ret;
             }
@@ -75,7 +106,38 @@ public:
         if (pVarDecl->hasGlobalStorage()
             && pVarDecl->isThisDeclarationADefinition())
         {
-            auto const ret(isBadStaticType(pVarDecl->getType(), nullptr));
+            auto const name(pVarDecl->getName());
+            if (   name == "g_pI18NStatusInstance" // I18NStatus::free()
+                || name == "s_pPreviousView" // not a owning pointer
+                || name == "s_pDefCollapsed" // SvImpLBox::~SvImpLBox()
+                || name == "s_pDefExpanded"  // SvImpLBox::~SvImpLBox()
+                || name == "g_pDDSource" // SvTreeListBox::dispose()
+                || name == "g_pDDTarget" // SvTreeListBox::dispose()
+                || name == "g_pSfxApplication" // SfxApplication::~SfxApplication()
+                || name == "s_SidebarResourceManagerInstance" // ResourceManager::disposeDecks()
+                || name == "pStaticThesSubMenu" // wtf is this nonsense
+                || name == "s_pGallery" // this is not entirely clear but apparently the GalleryThemeCacheEntry are deleted by GalleryBrowser2::SelectTheme() or GalleryBrowser2::dispose()
+                || name == "s_ExtMgr" // TheExtensionManager::disposing()
+                || name == "s_pDocLockedInsertingLinks" // not owning
+                || name == "s_pVout" // _FrmFinit()
+                || name == "s_pPaintQueue" // SwPaintQueue::Remove()
+                || name == "gProp" // only owned (VclPtr) member cleared again
+                || name == "g_pColumnCacheLastTabFrm" // not owning
+                || name == "g_pColumnCacheLastCellFrm" // not owning
+                || name == "g_pRowCacheLastTabFrm" // not owning
+                || name == "g_pRowCacheLastCellFrm" // not owning
+                || name == "g_OszCtrl" // SwCrsrOszControl::Exit()
+                || name == "g_pSpellIter" // SwEditShell::SpellEnd()
+                || name == "g_pConvIter" // SwEditShell::SpellEnd()
+                || name == "g_pHyphIter" // SwEditShell::HyphEnd()
+                || name == "pFieldEditEngine" // ScGlobal::Clear()
+                || name == "xDrawClipDocShellRef" // ScGlobal::Clear()
+               ) // these variables appear unproblematic
+            {
+                return true;
+            }
+            auto const ret(isBadStaticType(pVarDecl->getType(), nullptr,
+                            std::vector<QualType>()));
             if (ret.first) {
                 report(DiagnosticsEngine::Warning,
                         "bad static variable causes crash on shutdown",