new loplugin:unnecessary locking

off by default, since each warning needs careful inspection

Change-Id: I805c1d1cdde531a1afdc76e87b22f879fc3c9753
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134641
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

30 files changed, 292 insertions, 83 deletions

diff --git a/accessibility/source/extended/AccessibleGridControlTableCell.cxx b/accessibility/source/extended/AccessibleGridControlTableCell.cxx
index a3a8afa4895a..20bf37698e9d 100644
--- a/accessibility/source/extended/AccessibleGridControlTableCell.cxx
+++ b/accessibility/source/extended/AccessibleGridControlTableCell.cxx
@@ -262,20 +262,14 @@ namespace accessibility
 
     OUString SAL_CALL AccessibleGridControlTableCell::getSelectedText(  )
     {
-        SolarMutexGuard aSolarGuard;
-
         return OUString();
     }
     sal_Int32 SAL_CALL AccessibleGridControlTableCell::getSelectionStart(  )
     {
-        SolarMutexGuard aSolarGuard;
-
         return 0;
     }
     sal_Int32 SAL_CALL AccessibleGridControlTableCell::getSelectionEnd(  )
     {
-        SolarMutexGuard aSolarGuard;
-
         return 0;
     }
     sal_Bool SAL_CALL AccessibleGridControlTableCell::setSelection( sal_Int32 nStartIndex, sal_Int32 nEndIndex )
diff --git a/compilerplugins/clang/test/unnecessarylocking.cxx b/compilerplugins/clang/test/unnecessarylocking.cxx
new file mode 100644
index 000000000000..6dda5d333191
--- /dev/null
+++ b/compilerplugins/clang/test/unnecessarylocking.cxx
@@ -0,0 +1,71 @@
+/* -*- 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 <mutex>
+
+static std::mutex gSolarMutex;
+
+class SolarMutexGuard : public std::unique_lock<std::mutex>
+{
+public:
+    SolarMutexGuard()
+        : std::unique_lock<std::mutex>(gSolarMutex)
+    {
+    }
+};
+
+namespace test1
+{
+struct Foo
+{
+    int m_foo;
+    // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
+    int bar1()
+    {
+        SolarMutexGuard guard;
+        return 1;
+    }
+    // no warning expected
+    int bar2()
+    {
+        SolarMutexGuard guard;
+        return m_foo;
+    }
+};
+}
+
+namespace test2
+{
+struct Foo
+{
+    std::mutex m_aMutex;
+    int m_foo;
+
+    // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
+    int bar1()
+    {
+        std::unique_lock guard(m_aMutex);
+        return 1;
+    }
+    // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
+    int bar2()
+    {
+        std::scoped_lock guard(m_aMutex);
+        return 1;
+    }
+    // no warning expected
+    int bar3()
+    {
+        std::scoped_lock guard(m_aMutex);
+        return m_foo;
+    }
+};
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unnecessarylocking.cxx b/compilerplugins/clang/unnecessarylocking.cxx
new file mode 100644
index 000000000000..c00758b810dc
--- /dev/null
+++ b/compilerplugins/clang/unnecessarylocking.cxx
@@ -0,0 +1,215 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * 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 <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+#include <unordered_set>
+
+#include <clang/AST/CXXInheritance.h>
+
+#include "config_clang.h"
+
+#include "plugin.hxx"
+#include "check.hxx"
+
+/**
+Look for methods that are taking a lock at the top of the method, but then not
+touching any object-local state. In which case the method might not need locking.
+
+TODO
+
+(*) check if the data being returned is never modified, in which case locking is not necessary
+
+*/
+
+namespace
+{
+class UnnecessaryLocking : public loplugin::FilteringPlugin<UnnecessaryLocking>
+{
+public:
+    explicit UnnecessaryLocking(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual bool preRun() override
+    {
+        //        StringRef fn(handler.getMainFileName());
+        //        if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/unoidl/source/sourceprovider-parser.cxx"))
+        //            return false;
+        return true;
+    }
+    virtual void run() override
+    {
+        if (preRun())
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool TraverseCXXMethodDecl(CXXMethodDecl*);
+    bool VisitCXXThisExpr(const CXXThisExpr*);
+
+private:
+    bool isSolarMutexLockGuardStmt(const Stmt*);
+    const Stmt* isOtherMutexLockGuardStmt(const Stmt*);
+    std::vector<bool> m_TouchesThis;
+    // so we ignore the CxxThisEpxr that references the maMutex in the guard expression
+    std::vector<const Stmt*> m_IgnoreThis;
+};
+
+bool UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
+{
+    if (ignoreLocation(cxxMethodDecl))
+        return true;
+
+    if (!cxxMethodDecl->isInstance())
+        return true;
+    if (!cxxMethodDecl->isThisDeclarationADefinition())
+        return true;
+
+    auto compoundStmt = dyn_cast_or_null<CompoundStmt>(cxxMethodDecl->getBody());
+    if (!compoundStmt || compoundStmt->size() < 1)
+        return true;
+
+    const Stmt* firstStmt = *compoundStmt->body_begin();
+    bool solarMutex = isSolarMutexLockGuardStmt(firstStmt);
+    const Stmt* ignoreThisStmt = nullptr;
+    if (!solarMutex)
+        ignoreThisStmt = isOtherMutexLockGuardStmt(firstStmt);
+    if (!solarMutex && ignoreThisStmt == nullptr)
+        return true;
+
+    m_TouchesThis.push_back(false);
+    m_IgnoreThis.push_back(ignoreThisStmt);
+
+    bool rv = FilteringPlugin::TraverseCXXMethodDecl(cxxMethodDecl);
+
+    if (!m_TouchesThis.back())
+    {
+        StringRef fn = getFilenameOfLocation(
+            compiler.getSourceManager().getSpellingLoc(cxxMethodDecl->getBeginLoc()));
+        if (
+            // template magic
+            !loplugin::isSamePathname(fn, SRCDIR "/include/comphelper/unique_disposing_ptr.hxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/inc/unobaseclass.hxx")
+            // toolkit needs to lock around access to static methods in vcl
+            && !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxtoolkit.cxx")
+            && !loplugin::isSamePathname(fn,
+                                         SRCDIR "/toolkit/source/controls/tree/treecontrolpeer.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxcontainer.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxdevice.cxx")
+            // touching shared global data
+            && !loplugin::isSamePathname(fn, SRCDIR
+                                         "/framework/source/fwi/classes/protocolhandlercache.cxx")
+            // lock around access to static methods in vcl
+            && !loplugin::isSamePathname(fn, SRCDIR "/framework/source/services/taskcreatorsrv.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/svx/source/dialog/SafeModeUI.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR
+                                         "/svx/source/accessibility/AccessibleFrameSelector.cxx")
+            // not sure
+            && !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/dialog/filedlghelper.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/appdispatchprovider.cxx")
+            // touching shared global data
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoftn.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unolinebreak.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoobj.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unorefmk.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unotbl.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/xml/xmltexti.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/dlelstnr.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/accdoc.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/acccontext.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unocontentcontrol.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unobkm.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/docuno.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/afmtuno.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/appluno.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/unoxml/source/dom/documentbuilder.cxx")
+            && !loplugin::isSamePathname(
+                   fn, SRCDIR "/sd/source/ui/accessibility/AccessibleDrawDocumentView.cxx")
+            && !loplugin::isSamePathname(fn, SRCDIR "/starmath/source/accessibility.cxx")
+            && !loplugin::isSamePathname(fn,
+                                         SRCDIR "/starmath/source/AccessibleSmElementsControl.cxx"))
+        {
+            report(DiagnosticsEngine::Warning, "unnecessary locking", cxxMethodDecl->getBeginLoc())
+                << cxxMethodDecl->getSourceRange();
+        }
+    }
+
+    m_TouchesThis.pop_back();
+    m_IgnoreThis.pop_back();
+
+    return rv;
+}
+
+bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const Stmt* stmt)
+{
+    auto declStmt = dyn_cast<DeclStmt>(stmt);
+    if (!declStmt)
+        return false;
+    if (!declStmt->isSingleDecl())
+        return false;
+    auto varDecl = dyn_cast<VarDecl>(declStmt->getSingleDecl());
+    if (!varDecl)
+        return false;
+    auto tc = loplugin::TypeCheck(varDecl->getType());
+    if (!tc.Class("SolarMutexGuard").GlobalNamespace()
+        && !tc.Class("SolarMutexClearableGuard").GlobalNamespace()
+        && !tc.Class("SolarMutexResettableGuard").GlobalNamespace()
+        && !tc.Class("SolarMutexTryAndBuyGuard").GlobalNamespace())
+        return false;
+    return true;
+}
+
+const Stmt* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt)
+{
+    auto declStmt = dyn_cast<DeclStmt>(stmt);
+    if (!declStmt)
+        return nullptr;
+    if (!declStmt->isSingleDecl())
+        return nullptr;
+    auto varDecl = dyn_cast<VarDecl>(declStmt->getSingleDecl());
+    if (!varDecl)
+        return nullptr;
+    auto tc = loplugin::TypeCheck(varDecl->getType());
+    if (!tc.Class("unique_lock").StdNamespace() && !tc.Class("scoped_lock").StdNamespace())
+        return nullptr;
+    auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit());
+    if (!cxxConstructExpr || cxxConstructExpr->getNumArgs() < 1)
+        return nullptr;
+    auto memberExpr = dyn_cast<MemberExpr>(cxxConstructExpr->getArg(0));
+    if (!memberExpr)
+        return nullptr;
+    auto thisStmt = memberExpr->getBase();
+    return thisStmt;
+}
+
+bool UnnecessaryLocking::VisitCXXThisExpr(const CXXThisExpr* cxxThisExpr)
+{
+    if (ignoreLocation(cxxThisExpr))
+        return true;
+    // just in case
+    if (m_TouchesThis.empty())
+        return true;
+    // already found something
+    if (m_TouchesThis.back())
+        return true;
+    if (m_IgnoreThis.back() && m_IgnoreThis.back() == cxxThisExpr)
+        return true;
+    m_TouchesThis.back() = true;
+    return true;
+}
+
+/** off by default because each warning needs to be carefully inspected */
+loplugin::Plugin::Registration<UnnecessaryLocking> unnecessarylocking("unnecessarylocking", false);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/editeng/source/accessibility/AccessibleImageBullet.cxx b/editeng/source/accessibility/AccessibleImageBullet.cxx
index 11324648722d..bc9e2172415a 100644
--- a/editeng/source/accessibility/AccessibleImageBullet.cxx
+++ b/editeng/source/accessibility/AccessibleImageBullet.cxx
@@ -130,18 +130,12 @@ namespace accessibility
 
     OUString SAL_CALL  AccessibleImageBullet::getAccessibleDescription()
     {
-
-        SolarMutexGuard aGuard;
-
         // Get the string from the resource for the specified id.
         return EditResId(RID_SVXSTR_A11Y_IMAGEBULLET_DESCRIPTION);
     }
 
     OUString SAL_CALL  AccessibleImageBullet::getAccessibleName()
     {
-
-        SolarMutexGuard aGuard;
-
         // Get the string from the resource for the specified id.
         return EditResId(RID_SVXSTR_A11Y_IMAGEBULLET_NAME);
     }
diff --git a/sc/source/core/data/dptabsrc.cxx b/sc/source/core/data/dptabsrc.cxx
index f41ed4700204..17a6cc17171c 100644
--- a/sc/source/core/data/dptabsrc.cxx
+++ b/sc/source/core/data/dptabsrc.cxx
@@ -1063,7 +1063,6 @@ const uno::Sequence<sheet::MemberResult>* ScDPSource::GetMemberResults( const Sc
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScDPSource::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     using beans::PropertyAttribute::READONLY;
 
     static const SfxItemPropertyMapEntry aDPSourceMap_Impl[] =
@@ -1391,8 +1390,6 @@ const ScDPItemData& ScDPDimension::GetSelectedData()
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScDPDimension::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
-
     static const SfxItemPropertyMapEntry aDPDimensionMap_Impl[] =
     {
         { SC_UNO_DP_FILTER,   0,  cppu::UnoType<uno::Sequence<sheet::TableFilterField>>::get(), 0, 0 },
@@ -2061,8 +2058,6 @@ uno::Sequence<sal_Int16> ScDPLevel::getSubTotals() const
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScDPLevel::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
-
     static const SfxItemPropertyMapEntry aDPLevelMap_Impl[] =
     {
         //TODO: change type of AutoShow/Layout/Sorting to API struct when available
@@ -2535,8 +2530,6 @@ void SAL_CALL ScDPMember::setName( const OUString& /* rNewName */ )
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScDPMember::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
-
     static const SfxItemPropertyMapEntry aDPMemberMap_Impl[] =
     {
         { SC_UNO_DP_ISVISIBLE, 0,  cppu::UnoType<bool>::get(),              0, 0 },
diff --git a/sc/source/ui/unoobj/cellsuno.cxx b/sc/source/ui/unoobj/cellsuno.cxx
index 38289f0108b2..763ee12f7b24 100644
--- a/sc/source/ui/unoobj/cellsuno.cxx
+++ b/sc/source/ui/unoobj/cellsuno.cxx
@@ -3753,7 +3753,6 @@ uno::Reference<sheet::XSheetCellRanges> SAL_CALL ScCellRangesBase::queryDependen
 
 uno::Reference<util::XSearchDescriptor> SAL_CALL ScCellRangesBase::createSearchDescriptor()
 {
-    SolarMutexGuard aGuard;
     return new ScCellSearchObj;
 }
 
@@ -3874,7 +3873,6 @@ uno::Reference<uno::XInterface> SAL_CALL ScCellRangesBase::findNext(
 
 uno::Reference<util::XReplaceDescriptor> SAL_CALL ScCellRangesBase::createReplaceDescriptor()
 {
-    SolarMutexGuard aGuard;
     return new ScCellSearchObj;
 }
 
@@ -8323,7 +8321,6 @@ OUString SAL_CALL ScTableColumnObj::getName()
 
 void SAL_CALL ScTableColumnObj::setName( const OUString& /* aNewName */ )
 {
-    SolarMutexGuard aGuard;
     throw uno::RuntimeException();      // read-only
 }
 
diff --git a/sc/source/ui/unoobj/chartuno.cxx b/sc/source/ui/unoobj/chartuno.cxx
index 5f47d853d79e..ddfee69fbd1f 100644
--- a/sc/source/ui/unoobj/chartuno.cxx
+++ b/sc/source/ui/unoobj/chartuno.cxx
@@ -723,7 +723,6 @@ OUString SAL_CALL ScChartObj::getName()
 
 void SAL_CALL ScChartObj::setName( const OUString& /* aName */ )
 {
-    SolarMutexGuard aGuard;
     throw uno::RuntimeException();      // name cannot be changed
 }
 
diff --git a/sc/source/ui/unoobj/docuno.cxx b/sc/source/ui/unoobj/docuno.cxx
index 548f8287f055..4645ec111d68 100644
--- a/sc/source/ui/unoobj/docuno.cxx
+++ b/sc/source/ui/unoobj/docuno.cxx
@@ -4135,7 +4135,6 @@ sal_Bool SAL_CALL ScTableColumnsObj::hasByName( const OUString& aName )
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScTableColumnsObj::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     static uno::Reference<beans::XPropertySetInfo> aRef(
         new SfxItemPropertySetInfo( lcl_GetColumnsPropertyMap() ));
     return aRef;
@@ -4344,7 +4343,6 @@ sal_Bool SAL_CALL ScTableRowsObj::hasElements()
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScTableRowsObj::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     static uno::Reference<beans::XPropertySetInfo> aRef(
         new SfxItemPropertySetInfo( lcl_GetRowsPropertyMap() ));
     return aRef;
diff --git a/sc/source/ui/unoobj/eventuno.cxx b/sc/source/ui/unoobj/eventuno.cxx
index a0acc6e610ec..88a38763783c 100644
--- a/sc/source/ui/unoobj/eventuno.cxx
+++ b/sc/source/ui/unoobj/eventuno.cxx
@@ -143,7 +143,6 @@ uno::Any SAL_CALL ScSheetEventsObj::getByName( const OUString& aName )
 
 uno::Sequence<OUString> SAL_CALL ScSheetEventsObj::getElementNames()
 {
-    SolarMutexGuard aGuard;
     auto aNames = uno::Sequence<OUString>(int(ScSheetEventId::COUNT));
     auto pNames = aNames.getArray();
     for (sal_Int32 nEvent=0; nEvent<int(ScSheetEventId::COUNT); ++nEvent)
@@ -153,7 +152,6 @@ uno::Sequence<OUString> SAL_CALL ScSheetEventsObj::getElementNames()
 
 sal_Bool SAL_CALL ScSheetEventsObj::hasByName( const OUString& aName )
 {
-    SolarMutexGuard aGuard;
     ScSheetEventId nEvent = lcl_GetEventFromName(aName);
     return (nEvent != ScSheetEventId::NOTFOUND);
 }
diff --git a/sc/source/ui/unoobj/nameuno.cxx b/sc/source/ui/unoobj/nameuno.cxx
index 54cadb9e8de7..ea54ee508831 100644
--- a/sc/source/ui/unoobj/nameuno.cxx
+++ b/sc/source/ui/unoobj/nameuno.cxx
@@ -361,7 +361,6 @@ uno::Reference<table::XCellRange> SAL_CALL ScNamedRangeObj::getReferredCells()
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScNamedRangeObj::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     static uno::Reference< beans::XPropertySetInfo >  aRef(new SfxItemPropertySetInfo( lcl_GetNamedRangeMap() ));
     return aRef;
 }
@@ -369,7 +368,6 @@ uno::Reference<beans::XPropertySetInfo> SAL_CALL ScNamedRangeObj::getPropertySet
 void SAL_CALL ScNamedRangeObj::setPropertyValue(
                         const OUString& rPropertyName, const uno::Any& /*aValue*/ )
 {
-    SolarMutexGuard aGuard;
     if ( rPropertyName == SC_UNONAME_ISSHAREDFMLA )
     {
         // Ignore this.
diff --git a/sc/source/ui/unoobj/shapeuno.cxx b/sc/source/ui/unoobj/shapeuno.cxx
index accef3fd990d..e8914d859b16 100644
--- a/sc/source/ui/unoobj/shapeuno.cxx
+++ b/sc/source/ui/unoobj/shapeuno.cxx
@@ -995,8 +995,6 @@ uno::Any SAL_CALL ScShapeObj::getPropertyDefault( const OUString& aPropertyName
 
 void SAL_CALL ScShapeObj::attach( const uno::Reference<text::XTextRange>& /* xTextRange */ )
 {
-    SolarMutexGuard aGuard;
-
     throw lang::IllegalArgumentException();     // anchor cannot be changed
 }
 
diff --git a/sc/source/ui/unoobj/targuno.cxx b/sc/source/ui/unoobj/targuno.cxx
index d14c1090f31e..8af19b58e4d2 100644
--- a/sc/source/ui/unoobj/targuno.cxx
+++ b/sc/source/ui/unoobj/targuno.cxx
@@ -189,7 +189,6 @@ uno::Reference< container::XNameAccess > SAL_CALL  ScLinkTargetTypeObj::getLinks
 
 uno::Reference< beans::XPropertySetInfo > SAL_CALL  ScLinkTargetTypeObj::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     static uno::Reference< beans::XPropertySetInfo >  aRef(new SfxItemPropertySetInfo( lcl_GetLinkTargetMap() ));
     return aRef;
 }
diff --git a/sc/source/ui/unoobj/tokenuno.cxx b/sc/source/ui/unoobj/tokenuno.cxx
index 0512bd005178..40cb3a613bc3 100644
--- a/sc/source/ui/unoobj/tokenuno.cxx
+++ b/sc/source/ui/unoobj/tokenuno.cxx
@@ -178,7 +178,6 @@ OUString SAL_CALL ScFormulaParserObj::printFormula(
 
 uno::Reference<beans::XPropertySetInfo> SAL_CALL ScFormulaParserObj::getPropertySetInfo()
 {
-    SolarMutexGuard aGuard;
     static uno::Reference< beans::XPropertySetInfo > aRef(new SfxItemPropertySetInfo( lcl_GetFormulaParserMap() ));
     return aRef;
 }
diff --git a/scripting/source/basprov/baslibnode.cxx b/scripting/source/basprov/baslibnode.cxx
index 015334f97ae0..33942c6b206c 100644
--- a/scripting/source/basprov/baslibnode.cxx
+++ b/scripting/source/basprov/baslibnode.cxx
@@ -120,8 +120,6 @@ namespace basprov
 
     sal_Int16 BasicLibraryNodeImpl::getType(  )
     {
-        SolarMutexGuard aGuard;
-
         return browse::BrowseNodeTypes::CONTAINER;
     }
 
diff --git a/scripting/source/basprov/basmodnode.cxx b/scripting/source/basprov/basmodnode.cxx
index 58d3ec65728d..986e3062d5cd 100644
--- a/scripting/source/basprov/basmodnode.cxx
+++ b/scripting/source/basprov/basmodnode.cxx
@@ -124,8 +124,6 @@ namespace basprov
 
     sal_Int16 BasicModuleNodeImpl::getType(  )
     {
-        SolarMutexGuard aGuard;
-
         return browse::BrowseNodeTypes::CONTAINER;
     }
 
diff --git a/sd/source/ui/accessibility/AccessibleOutlineView.cxx b/sd/source/ui/accessibility/AccessibleOutlineView.cxx
index 963a9ba3c5ac..4e020efeff9e 100644
--- a/sd/source/ui/accessibility/AccessibleOutlineView.cxx
+++ b/sd/source/ui/accessibility/AccessibleOutlineView.cxx
@@ -220,11 +220,8 @@ void SAL_CALL
 }
 
 /// Create a name for this view.
-OUString
-    AccessibleOutlineView::CreateAccessibleName()
+OUString AccessibleOutlineView::CreateAccessibleName()
 {
-    SolarMutexGuard aGuard;
-
     return SdResId(SID_SD_A11Y_I_OUTLINEVIEW_N);
 }
 
diff --git a/sfx2/source/sidebar/UnoDecks.cxx b/sfx2/source/sidebar/UnoDecks.cxx
index cbd9d1c214e7..2d07ea1a7a5b 100644
--- a/sfx2/source/sidebar/UnoDecks.cxx
+++ b/sfx2/source/sidebar/UnoDecks.cxx
@@ -129,8 +129,6 @@ uno::Any SAL_CALL SfxUnoDecks::getByIndex( sal_Int32 Index )
 // XElementAccess
 uno::Type SAL_CALL SfxUnoDecks::getElementType()
 {
-    SolarMutexGuard aGuard;
-
     return uno::Type();
 }
 
diff --git a/sfx2/source/sidebar/UnoPanels.cxx b/sfx2/source/sidebar/UnoPanels.cxx
index 40ebdbeb979f..4ef48eb5c587 100644
--- a/sfx2/source/sidebar/UnoPanels.cxx
+++ b/sfx2/source/sidebar/UnoPanels.cxx
@@ -141,8 +141,6 @@ uno::Any SAL_CALL SfxUnoPanels::getByIndex( sal_Int32 Index )
 // XElementAccess
 uno::Type SAL_CALL SfxUnoPanels::getElementType()
 {
-    SolarMutexGuard aGuard;
-
     return uno::Type();
 }
 
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index fc50ca7a672e..5115e03587ab 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -104,6 +104,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/unnecessaryoverride \
     compilerplugins/clang/test/unnecessaryoverride-dtor \
     compilerplugins/clang/test/unnecessaryparen \
+    compilerplugins/clang/test/unnecessarylocking \
     compilerplugins/clang/test/unoany \
     compilerplugins/clang/test/unoquery \
     compilerplugins/clang/test/unreffun \
diff --git a/starmath/source/accessibility.cxx b/starmath/source/accessibility.cxx
index c2ae7bb41e1a..25fd9532c758 100644
--- a/starmath/source/accessibility.cxx
+++ b/starmath/source/accessibility.cxx
@@ -321,9 +321,7 @@ OUString SAL_CALL SmGraphicAccessible::getAccessibleName()
 
 Reference< XAccessibleRelationSet > SAL_CALL SmGraphicAccessible::getAccessibleRelationSet()
 {
-    SolarMutexGuard aGuard;
-    Reference< XAccessibleRelationSet > xRelSet = new utl::AccessibleRelationSetHelper();
-    return xRelSet;   // empty relation set
+    return new utl::AccessibleRelationSetHelper(); // empty relation set
 }
 
 Reference< XAccessibleStateSet > SAL_CALL SmGraphicAccessible::getAccessibleStateSet()
diff --git a/sw/source/core/unocore/unoframe.cxx b/sw/source/core/unocore/unoframe.cxx
index e16b5540f2f7..d4140a8edf64 100644
--- a/sw/source/core/unocore/unoframe.cxx
+++ b/sw/source/core/unocore/unoframe.cxx
@@ -3167,7 +3167,6 @@ void SwXFrame::attach(const uno::Reference< text::XTextRange > & xTextRange)
 
 awt::Point SwXFrame::getPosition()
 {
-    SolarMutexGuard aGuard;
     uno::RuntimeException aRuntime;
     aRuntime.Message = "position cannot be determined with this method";
     throw aRuntime;
@@ -3175,7 +3174,6 @@ awt::Point SwXFrame::getPosition()
 
 void SwXFrame::setPosition(const awt::Point& /*aPosition*/)
 {
-    SolarMutexGuard aGuard;
     uno::RuntimeException aRuntime;
     aRuntime.Message = "position cannot be changed with this method";
     throw aRuntime;
diff --git a/sw/source/core/unocore/unolinebreak.cxx b/sw/source/core/unocore/unolinebreak.cxx
index 5c480b2619b7..b7f8a174ad17 100644
--- a/sw/source/core/unocore/unolinebreak.cxx
+++ b/sw/source/core/unocore/unolinebreak.cxx
@@ -184,8 +184,6 @@ uno::Reference<text::XTextRange> SAL_CALL SwXLineBreak::getAnchor()
 
 void SAL_CALL SwXLineBreak::dispose()
 {
-    SolarMutexGuard aGuard;
-
     SAL_WARN("sw.uno", "SwXLineBreak::dispose: not implemented");
 }
 
diff --git a/sw/source/core/unocore/unoparagraph.cxx b/sw/source/core/unocore/unoparagraph.cxx
index 7e6df7e7796b..d13bd6c65945 100644
--- a/sw/source/core/unocore/unoparagraph.cxx
+++ b/sw/source/core/unocore/unoparagraph.cxx
@@ -1203,7 +1203,6 @@ SwXParagraph::getPropertyDefault(const OUString& rPropertyName)
 void SAL_CALL
 SwXParagraph::attach(const uno::Reference< text::XTextRange > & /*xTextRange*/)
 {
-    SolarMutexGuard aGuard;
     // SwXParagraph will only created in order to be inserted by
     // 'insertTextContentBefore' or 'insertTextContentAfter' therefore
     // they cannot be attached
diff --git a/sw/source/core/unocore/unostyle.cxx b/sw/source/core/unocore/unostyle.cxx
index 5120081063cc..10142d3cde00 100644
--- a/sw/source/core/unocore/unostyle.cxx
+++ b/sw/source/core/unocore/unostyle.cxx
@@ -599,7 +599,6 @@ void SwXStyleFamilies::loadStylesFromURL(const OUString& rURL,
 
 uno::Sequence< beans::PropertyValue > SwXStyleFamilies::getStyleLoaderOptions()
 {
-    SolarMutexGuard aGuard;
     const uno::Any aVal(true);
     return comphelper::InitPropertySequence({
         { UNO_NAME_LOAD_TEXT_STYLES, aVal },
@@ -4588,13 +4587,11 @@ uno::Any SAL_CALL SwXTextTableStyle::getByName(const OUString& rName)
 
 css::uno::Sequence<OUString> SAL_CALL SwXTextTableStyle::getElementNames()
 {
-    SolarMutexGuard aGuard;
     return comphelper::mapKeysToSequence(GetCellStyleNameMap());
 }
 
 sal_Bool SAL_CALL SwXTextTableStyle::hasByName(const OUString& rName)
 {
-    SolarMutexGuard aGuard;
     const CellStyleNameMap& rMap = GetCellStyleNameMap();
     CellStyleNameMap::const_iterator iter = rMap.find(rName);
     return iter != rMap.end();
@@ -4840,7 +4837,6 @@ OUString SAL_CALL SwXTextCellStyle::getParentStyle()
 
 void SAL_CALL SwXTextCellStyle::setParentStyle(const OUString& /*sParentStyle*/)
 {
-    SolarMutexGuard aGuard;
     // Changing parent to one which is unaware of it will lead to a something unexpected. getName() rely on a parent.
     SAL_INFO("sw.uno", "Changing SwXTextCellStyle parent");
 }
diff --git a/sw/source/core/unocore/unotextmarkup.cxx b/sw/source/core/unocore/unotextmarkup.cxx
index 1bc8e7387f65..0ea41f5f132d 100644
--- a/sw/source/core/unocore/unotextmarkup.cxx
+++ b/sw/source/core/unocore/unotextmarkup.cxx
@@ -91,10 +91,7 @@ const ModelToViewHelper& SwXTextMarkup::GetConversionMap() const
 
 uno::Reference< container::XStringKeyMap > SAL_CALL SwXTextMarkup::getMarkupInfoContainer()
 {
-    SolarMutexGuard aGuard;
-
-    uno::Reference< container::XStringKeyMap > xProp = new SwXStringKeyMap;
-    return xProp;
+    return new SwXStringKeyMap;
 }
 
 void SAL_CALL SwXTextMarkup::commitTextRangeMarkup(::sal_Int32 nType, const OUString & aIdentifier, const uno::Reference< text::XTextRange> & xRange,
diff --git a/sw/source/uibase/uno/unotxdoc.cxx b/sw/source/uibase/uno/unotxdoc.cxx
index 41c1994f47d2..7f3079c5cf05 100644
--- a/sw/source/uibase/uno/unotxdoc.cxx
+++ b/sw/source/uibase/uno/unotxdoc.cxx
@@ -654,9 +654,7 @@ Reference< XPropertySet >  SwXTextDocument::getEndnoteSettings()
 
 Reference< util::XReplaceDescriptor >  SwXTextDocument::createReplaceDescriptor()
 {
-    SolarMutexGuard aGuard;
-    Reference< util::XReplaceDescriptor >  xRet = new SwXTextSearch;
-    return xRet;
+    return new SwXTextSearch;
 }
 
 SwUnoCursor* SwXTextDocument::CreateCursorForSearch(Reference< XTextCursor > & xCursor)
@@ -738,10 +736,7 @@ sal_Int32 SwXTextDocument::replaceAll(const Reference< util::XSearchDescriptor >
 
 Reference< util::XSearchDescriptor >  SwXTextDocument::createSearchDescriptor()
 {
-    SolarMutexGuard aGuard;
-    Reference< util::XSearchDescriptor >  xRet = new SwXTextSearch;
-    return xRet;
-
+    return new SwXTextSearch;
 }
 
 // Used for findAll/First/Next
@@ -3421,14 +3416,11 @@ int SwXTextDocument::getPart()
 
 OUString SwXTextDocument::getPartName(int nPart)
 {
-    SolarMutexGuard aGuard;
-
     return SwResId(STR_PAGE) + OUString::number(nPart + 1);
 }
 
 OUString SwXTextDocument::getPartHash(int nPart)
 {
-    SolarMutexGuard aGuard;
     OUString sPart(SwResId(STR_PAGE) + OUString::number(nPart + 1));
 
     return OUString::number(sPart.hashCode());
diff --git a/toolkit/source/awt/vclxwindows.cxx b/toolkit/source/awt/vclxwindows.cxx
index 03de455a3feb..e444d6d3ae9d 100644
--- a/toolkit/source/awt/vclxwindows.cxx
+++ b/toolkit/source/awt/vclxwindows.cxx
@@ -2260,7 +2260,6 @@ sal_Int16 VCLXMessageBox::execute()
 
 css::awt::Size SAL_CALL VCLXMessageBox::getMinimumSize()
 {
-    SolarMutexGuard aGuard;
     return css::awt::Size( 250, 100 );
 }
 
diff --git a/vcl/source/graphic/UnoGraphicProvider.cxx b/vcl/source/graphic/UnoGraphicProvider.cxx
index cab6659ce010..1f33a14ea1bc 100644
--- a/vcl/source/graphic/UnoGraphicProvider.cxx
+++ b/vcl/source/graphic/UnoGraphicProvider.cxx
@@ -438,8 +438,6 @@ uno::Reference< ::graphic::XGraphic > SAL_CALL GraphicProvider::queryGraphic( co
 
 uno::Sequence< uno::Reference<graphic::XGraphic> > SAL_CALL GraphicProvider::queryGraphics(const uno::Sequence< uno::Sequence<beans::PropertyValue> >& rMediaPropertiesSeq)
 {
-    SolarMutexGuard aGuard;
-
     // Turn properties into streams.
     std::vector< std::unique_ptr<SvStream> > aStreams;
     for (const auto& rMediaProperties : rMediaPropertiesSeq)
@@ -701,8 +699,6 @@ void ImplApplyFilterData( ::Graphic& rGraphic, const uno::Sequence< beans::Prope
 
 void SAL_CALL GraphicProvider::storeGraphic( const uno::Reference< ::graphic::XGraphic >& rxGraphic, const uno::Sequence< beans::PropertyValue >& rMediaProperties )
 {
-    SolarMutexGuard g;
-
     std::unique_ptr<SvStream> pOStm;
     OUString    aPath;
 
diff --git a/vcl/source/uitest/uno/uitest_uno.cxx b/vcl/source/uitest/uno/uitest_uno.cxx
index 4e5a6e0e0391..48fa2ef67967 100644
--- a/vcl/source/uitest/uno/uitest_uno.cxx
+++ b/vcl/source/uitest/uno/uitest_uno.cxx
@@ -66,33 +66,28 @@ UITestUnoObj::UITestUnoObj():
 
 sal_Bool SAL_CALL UITestUnoObj::executeCommand(const OUString& rCommand)
 {
-    SolarMutexGuard aGuard;
     return UITest::executeCommand(rCommand);
 }
 
 sal_Bool SAL_CALL UITestUnoObj::executeCommandWithParameters(const OUString& rCommand,
     const css::uno::Sequence< css::beans::PropertyValue >& rArgs)
 {
-    SolarMutexGuard aGuard;
     return UITest::executeCommandWithParameters(rCommand,rArgs);
 }
 
 sal_Bool SAL_CALL UITestUnoObj::executeDialog(const OUString& rCommand)
 {
-    SolarMutexGuard aGuard;
     return UITest::executeDialog(rCommand);
 }
 
 css::uno::Reference<css::ui::test::XUIObject> SAL_CALL UITestUnoObj::getTopFocusWindow()
 {
-    SolarMutexGuard aGuard;
     std::unique_ptr<UIObject> pObj = UITest::getFocusTopWindow();
     return new UIObjectUnoObj(std::move(pObj));
 }
 
 css::uno::Reference<css::ui::test::XUIObject> SAL_CALL UITestUnoObj::getFloatWindow()
 {
-    SolarMutexGuard aGuard;
     std::unique_ptr<UIObject> pObj = UITest::getFloatWindow();
     return new UIObjectUnoObj(std::move(pObj));
 }
diff --git a/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx b/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx
index 0bfc42b3b6e9..eefa92eaf502 100644
--- a/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx
+++ b/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx
@@ -306,8 +306,6 @@ sal_Bool XMLBasicExporterBase::filter( const Sequence< beans::PropertyValue >& /
 
     void XMLBasicExporterBase::cancel()
     {
-        std::scoped_lock aGuard( m_aMutex );
-
         // cancel export
     }