diff options
| -rw-r--r-- | compilerplugins/clang/test/weakobject.cxx | 31 | ||||
| -rw-r--r-- | compilerplugins/clang/weakobject.cxx | 189 | ||||
| -rw-r--r-- | solenv/CompilerTest_compilerplugins_clang.mk | 1 |
3 files changed, 85 insertions, 136 deletions
diff --git a/compilerplugins/clang/test/weakobject.cxx b/compilerplugins/clang/test/weakobject.cxx new file mode 100644 index 000000000000..7c7da55664d2 --- /dev/null +++ b/compilerplugins/clang/test/weakobject.cxx @@ -0,0 +1,31 @@ +/* -*- 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 "config_clang.h" + +namespace cppu +{ +class OWeakObject +{ +}; +} + +class Foo1 : public cppu::OWeakObject +{ +}; +class Foo2 : public cppu::OWeakObject +{ +}; + +// expected-error@+1 {{more than one copy of cppu::OWeakObject inherited [loplugin:weakobject]}} +class Foo3 : public Foo1, public Foo2 +{ +}; + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/weakobject.cxx b/compilerplugins/clang/weakobject.cxx index 055110ef9331..4801953cc44a 100644 --- a/compilerplugins/clang/weakobject.cxx +++ b/compilerplugins/clang/weakobject.cxx @@ -2,163 +2,80 @@ /* * 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/. + * Based on LLVM/Clang. + * + * This file is distributed under the University of Illinois Open Source + * License. See LICENSE.TXT for details. + * */ - #ifndef LO_CLANG_SHARED_PLUGINS -#include "check.hxx" +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include <set> +#include <unordered_set> #include "plugin.hxx" +#include "check.hxx" -/* OWeakObject::release() disposes weak references. If that isn't done - * because a sub-class improperly overrides release() then - * OWeakConnectionPoint::m_pObject continues to point to the deleted object - * and that could result in use-after-free. - */ - -namespace { +/* +Check for places where we end up with more than one copy of cppu::OweakObject in a class, which +really should not happen - we should be using one of the AggImplInheritanceHelper classes then +to inherit. +*/ -class WeakObject - : public loplugin::FilteringPlugin<WeakObject> +namespace +{ +class WeakObject : public loplugin::FilteringPlugin<WeakObject> { - public: - explicit WeakObject(loplugin::InstantiationData const& rData): FilteringPlugin(rData) - {} - - virtual bool preRun() override { - return compiler.getLangOpts().CPlusPlus; // no OWeakObject in C - } - void run() override { - if( preRun()) { - TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); - } + explicit WeakObject(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { } - bool isDerivedFromOWeakObject(CXXMethodDecl const*const pMethodDecl) + virtual bool preRun() override { - QualType const pClass(pMethodDecl->getParent()->getTypeForDecl(), 0); - if (loplugin::TypeCheck(pClass).Class("OWeakObject").Namespace("cppu")) - { - return true; - } - // hopefully it's faster to recurse overridden methods than the - // thicket of WeakImplHelper32 but that is purely speculation - for (auto it = pMethodDecl->begin_overridden_methods(); - it != pMethodDecl->end_overridden_methods(); ++it) - { - if (isDerivedFromOWeakObject(*it)) - { - return true; - } - } - return false; + return true; } - bool VisitCXXMethodDecl(CXXMethodDecl const*const pMethodDecl) + virtual void run() override { - if (ignoreLocation(pMethodDecl)) { - return true; - } - if (!pMethodDecl->isThisDeclarationADefinition() - || pMethodDecl->isLateTemplateParsed()) - { - return true; - } - if (!pMethodDecl->isInstance()) { - return true; - } -// this is too "simple", if a NamedDecl class has a getName() member expecting it to actually work would clearly be unreasonable if (pMethodDecl->getName() != "release") { - if (auto const i = pMethodDecl->getIdentifier()) { - if (i != nullptr && !i->isStr("release")) { - return true; - } - } - if (pMethodDecl->getNumParams() != 0) { - return true; - } - if (loplugin::TypeCheck(QualType(pMethodDecl->getParent()->getTypeForDecl(), 0)).Class("OWeakObject").Namespace("cppu")) - { - return true; - } + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } - CXXMethodDecl const* pOverridden(nullptr); - for (auto it = pMethodDecl->begin_overridden_methods(); - it != pMethodDecl->end_overridden_methods(); ++it) - { - if (auto const i = (*it)->getIdentifier()) { - if (i != nullptr && i->isStr("release")) { - pOverridden = *it; - break; - } - } - } - if (pOverridden == nullptr) - { - return true; - } - if (!isDerivedFromOWeakObject(pOverridden)) - { - return true; - } - CompoundStmt const*const pCompoundStatement( - dyn_cast<CompoundStmt>(pMethodDecl->getBody())); - for (auto i = pCompoundStatement->body_begin(); - i != pCompoundStatement->body_end(); ++i) - { - // note: this is not a CXXMemberCallExpr - CallExpr const*const pCallExpr(dyn_cast<CallExpr>(*i)); - if (pCallExpr) - { - // note: this is only sometimes a CXXMethodDecl - FunctionDecl const*const pCalled(pCallExpr->getDirectCallee()); - if (pCalled->getName() == "release") - { -//this never works && pCalled == pOverridden - if (pCalled->getParent() == pOverridden->getParent()) - { - return true; - } - // Allow this convenient shortcut: - auto td = dyn_cast<TypeDecl>(pCalled->getParent()); - if (td != nullptr - && (loplugin::TypeCheck(td).Class("OWeakObject").Namespace("cppu").GlobalNamespace() - || loplugin::TypeCheck(td).Class("OWeakAggObject").Namespace("cppu").GlobalNamespace())) - { - return true; - } - } - } - } + bool VisitCXXRecordDecl( const CXXRecordDecl* decl); - // allowlist - auto tc = loplugin::TypeCheck(pMethodDecl->getParent()); - if ( tc.Class("OWeakAggObject").Namespace("cppu").GlobalNamespace() // conditional call - || tc.Class("WeakComponentImplHelperBase").Namespace("cppu").GlobalNamespace() // extra magic - || tc.Class("WeakAggComponentImplHelperBase").Namespace("cppu").GlobalNamespace() // extra magic - || tc.Class("CDOMImplementation").Namespace("DOM").GlobalNamespace() // a static oddity - || tc.Class("SwXTextFrame").GlobalNamespace() // ambiguous, 3 parents - || tc.Class("SwXTextDocument").GlobalNamespace() // ambiguous, ~4 parents - || tc.Class("SdStyleSheet").GlobalNamespace() // same extra magic as WeakComponentImplHelperBase - || tc.Class("SdXImpressDocument").GlobalNamespace() // same extra magic as WeakComponentImplHelperBase - ) +}; + +bool WeakObject::VisitCXXRecordDecl(const CXXRecordDecl* decl) +{ + if (ignoreLocation(decl)) + return true; + if (!decl->hasDefinition()) + return true; + if (decl->hasAnyDependentBases()) + return true; + int cnt = 0; + decl->forallBases( + [&cnt] (const CXXRecordDecl *BaseDefinition) -> bool { + if (loplugin::DeclCheck(BaseDefinition).Class("OWeakObject").Namespace("cppu").GlobalNamespace()) + ++cnt; return true; - } - - report(DiagnosticsEngine::Warning, - "override of OWeakObject::release() does not call superclass release()", - pMethodDecl->getLocation()) - << pMethodDecl->getSourceRange(); - + }); + if (cnt < 2) return true; - } -}; + report(DiagnosticsEngine::Warning, "more than one copy of cppu::OWeakObject inherited", + compat::getBeginLoc(decl)) + << decl->getSourceRange(); + return true; +} -loplugin::Plugin::Registration<WeakObject> weakobject("weakobject"); +loplugin::Plugin::Registration<WeakObject> weakobject("weakobject", false); } // namespace diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index d3131efd0e7e..89bb3058f4a7 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -113,6 +113,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/useuniqueptr \ compilerplugins/clang/test/vclwidgets \ compilerplugins/clang/test/weakbase \ + compilerplugins/clang/test/weakobject \ compilerplugins/clang/test/writeonlyvars \ compilerplugins/clang/test/xmlimport \ )) |