Add a compiler plugin to detect &*smart_pointer

Adds a compiler plugin to detect cases of dereferencing a smart
pointer (such as unique_ptr or UnoCursorPointer) and then immediately
taking the address of the reference. In that case it is better to just
call get() on the pointer. This is particularly important if the pointer
can be null because in that case the initial dereference results in
undefined behaviour.

Change-Id: Ib1cca00c9180572f446721b90f83aa7fddb88c7d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/211756
Reviewed-by: Neil Roberts <nroberts@libreoffice.org>
Tested-by: Jenkins

compilerplugins/clang/smartpointerderefaddress.cxx

@@ -0,0 +1,91 @@
+/* -*- 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/.
+ */
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+
+#include <clang/AST/CXXInheritance.h>
+
+#include "check.hxx"
+#include "plugin.hxx"
+
+/**
+ * Look for:
+ *     &(*x)
+ * when x is a smart pointer like unique_ptr. That can be transformed to:
+ *     x.get()
+ *
+ * This is important when x is null, because in that case *x is undefined behavior. This can lead to
+ * difficult bugs because the compiler will often let you get away with it, but some compilers might
+ * also take advantage of that to assume that x is never null and then optimise away chunks of code.
+ * See tdf#173585 for an example.
+ *
+ * Either way x.get() is easier to understand.
+ */
+
+namespace
+{
+class SmartPointerDerefAddress : public loplugin::FilteringPlugin<SmartPointerDerefAddress>
+{
+public:
+    explicit SmartPointerDerefAddress(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual void run() override
+    {
+        if (preRun())
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitUnaryOperator(UnaryOperator const*);
+};
+
+bool SmartPointerDerefAddress::VisitUnaryOperator(UnaryOperator const* unaryOperator)
+{
+    if (ignoreLocation(unaryOperator))
+        return true;
+    if (unaryOperator->getBeginLoc().isMacroID())
+        return true;
+    if (unaryOperator->getOpcode() != UO_AddrOf)
+        return true;
+    auto subExpr = unaryOperator->getSubExpr()->IgnoreParenImpCasts();
+    auto innerOp = dyn_cast<CXXOperatorCallExpr>(subExpr);
+    if (!innerOp || innerOp->getOperator() != OO_Star)
+        return true;
+
+    auto const tc = loplugin::TypeCheck(innerOp->getArg(0)->getType());
+
+    if (tc.ClassOrStruct("shared_ptr").StdNamespace()
+        || tc.ClassOrStruct("__shared_ptr_access").StdNamespace()
+        || tc.ClassOrStruct("unique_ptr").StdNamespace()
+        || tc.ClassOrStruct("UnoCursorPointer").Namespace("sw").GlobalNamespace())
+    {
+        report(DiagnosticsEngine::Warning,
+               "'*' followed by '&' operating on %0, rather use '.get()'",
+               unaryOperator->getBeginLoc())
+            << innerOp->getArg(0)->getType() << unaryOperator->getSourceRange();
+    }
+
+    return true;
+}
+
+loplugin::Plugin::Registration<SmartPointerDerefAddress>
+    smartpointerderefaddress("smartpointerderefaddress");
+
+} // namespace
+
+#endif // LO_CLANG_SHARED_PLUGINS
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

compilerplugins/clang/test/smartpointerderefaddress.cxx

@@ -0,0 +1,42 @@
+/* -*- 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 <sal/config.h>
+
+#include <memory>
+
+namespace sw
+{
+// Mockup of UnoCursorPointer because it’s probably difficult to include the header
+class UnoCursorPointer
+{
+public:
+    int& operator*() { return *m_pInteger; };
+
+private:
+    int* m_pInteger;
+};
+}
+
+int* function1(std::unique_ptr<int>& x)
+{
+    return &*x; // expected-error-re {{'*' followed by '&' operating on '{{.*}}unique_ptr{{.*}}', rather use '.get()' [loplugin:smartpointerderefaddress]}}
+}
+
+int* function2(std::shared_ptr<int>& x)
+{
+    return &*x; // expected-error-re {{'*' followed by '&' operating on '{{.*}}shared_ptr{{.*}}', rather use '.get()' [loplugin:smartpointerderefaddress]}}
+}
+
+int* function3(sw::UnoCursorPointer& x)
+{
+    return &*x; // expected-error-re {{'*' followed by '&' operating on '{{.*}}UnoCursorPointer{{.*}}', rather use '.get()' [loplugin:smartpointerderefaddress]}}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

solenv/CompilerTest_compilerplugins_clang.mk

@@ -84,6 +84,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/simplifydynamiccast \
     compilerplugins/clang/test/simplifypointertobool \
     compilerplugins/clang/test/singlevalfields \
+    compilerplugins/clang/test/smartpointerderefaddress \
     compilerplugins/clang/test/staticconstexpr \
     compilerplugins/clang/test/staticconstfield \
     compilerplugins/clang/test/staticdynamic \