compilerplugins/clang/classmemaccess.cxx | 133 ++++++++++++++++++++++++++ compilerplugins/clang/test/classmemaccess.cxx | 36 +++++++ include/basic/sbxvar.hxx | 7 - 3 files changed, 173 insertions(+), 3 deletions(-)
New commits: commit 8f4fdb405a8470b9d06ab14ced120da89b213e82 Author: Stephan Bergmann <sberg...@redhat.com> AuthorDate: Mon Oct 7 17:31:02 2019 +0200 Commit: Stephan Bergmann <sberg...@redhat.com> CommitDate: Tue Oct 8 11:30:17 2019 +0200 New loplugin:classmemaccess ...to complement and improve upon GCC's -Wclass-memaccess. See the comment at the head of compilerplugins/clang/classmemaccess.cxx for details. (It is not yet clear to me whether we would want to get this upstreamed into Clang.) 35d21e4bf6f66b3bbc7a44fcf184cb721b524a94 "Remove redundant memsets" was a case that benefited from looking through toplevel casts to void*. (Though the code in include/basic/sbxvar.hxx needs a slightly more verbose way to deliberately silence the warning now.) d03041e19215592f21ba1222d3cfa29e1f94260a "Drop bogus memsets" is one example of various cases that GCC -Wclass-memaccess failed to catch due to the use of array instead of pointer types. Change-Id: I6a9bfc34e3536834af35fdf4fb7ceeb31f31f8c0 Reviewed-on: https://gerrit.libreoffice.org/80421 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sberg...@redhat.com> diff --git a/compilerplugins/clang/classmemaccess.cxx b/compilerplugins/clang/classmemaccess.cxx new file mode 100644 index 000000000000..56fe3fe39ab0 --- /dev/null +++ b/compilerplugins/clang/classmemaccess.cxx @@ -0,0 +1,133 @@ +/* -*- 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 "check.hxx" +#include "plugin.hxx" + +// Similar to GCC "warning: ‘void* memset(void*, int, size_t)’ writing to an object of non-trivial +// type ‘...’; use assignment instead [-Wclass-memaccess]", but looking through toplevel cast to +// void* and taking arrays into account in addition to pointers. (Clang has +// -Wdynamic-class-memaccess, but that only warns about memset overwriting a vtable pointer. GCC +// deliberately does not warn when there is a toplevel cast to void*, see +// <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81327> "[8 Regression] cast to void* does not +// suppress -Wclass-memaccess".) + +namespace +{ +class ClassMemAccess final : public loplugin::FilteringPlugin<ClassMemAccess> +{ +public: + explicit ClassMemAccess(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + bool VisitCallExpr(CallExpr* expr) + { + if (ignoreLocation(expr)) + { + return true; + } + auto const fdecl = expr->getDirectCallee(); + if (fdecl == nullptr) + { + return true; + } + auto dc = loplugin::DeclCheck(fdecl).Function("memset"); + if (!(dc.GlobalNamespace() || dc.StdNamespace())) + { + return true; + } + if (expr->getNumArgs() != 3) + { + if (isDebugMode()) + { + report(DiagnosticsEngine::Fatal, + "unexpected call to %0 with %1 instead of 3 arguments", expr->getExprLoc()) + << fdecl << expr->getNumArgs() << expr->getSourceRange(); + report(DiagnosticsEngine::Note, "%0 declared here", fdecl->getLocation()) + << fdecl << fdecl->getSourceRange(); + } + return true; + } + auto e = expr->getArg(0)->IgnoreParenImpCasts(); + while (auto const cast = dyn_cast<ExplicitCastExpr>(e)) + { + if (!loplugin::TypeCheck(cast->getTypeAsWritten()).Pointer().Void()) + { + break; + } + e = cast->getSubExprAsWritten()->IgnoreParenImpCasts(); + } + QualType t; + if (auto const t1 = e->getType()->getAs<clang::PointerType>()) + { + t = t1->getPointeeType(); + } + else if (e->getType()->isArrayType()) + { + t = e->getType(); + while (auto const t2 = t->getAsArrayTypeUnsafe()) + { + t = t2->getElementType(); + } + } + else + { + if (isDebugMode()) + { + report(DiagnosticsEngine::Fatal, + "unexpected call to %0 with first argument of non-pointer type %1", + expr->getExprLoc()) + << fdecl << e->getType() << expr->getSourceRange(); + report(DiagnosticsEngine::Note, "%0 declared here", fdecl->getLocation()) + << fdecl << fdecl->getSourceRange(); + } + return true; + } + auto const decl = t->getAsCXXRecordDecl(); + if (decl == nullptr) + { + return true; + } + if (!decl->isCompleteDefinition()) + { + return true; // conservatively assume it may be trivial + } + if (decl->isTrivial()) + { + return true; + } + report(DiagnosticsEngine::Warning, + "%0 writing to an object of non-trivial type %1; use assignment instead", + expr->getExprLoc()) + << fdecl << decl << expr->getSourceRange(); + return true; + } + + bool preRun() override { return compiler.getLangOpts().CPlusPlus; } + +private: + void run() override + { + if (preRun()) + { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + } +}; + +loplugin::Plugin::Registration<ClassMemAccess> classmemaccess("classmemaccess"); +} + +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/test/classmemaccess.cxx b/compilerplugins/clang/test/classmemaccess.cxx new file mode 100644 index 000000000000..05bb457f87e3 --- /dev/null +++ b/compilerplugins/clang/test/classmemaccess.cxx @@ -0,0 +1,36 @@ +/* -*- 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 <cstring> + +void f(struct Incomplete* p1, struct S* p2); + +struct S +{ + S(); +}; + +void f(struct Incomplete* p1, struct S* p2) +{ + S s; + // expected-error@+1 {{writing to an object of non-trivial type 'S'; use assignment instead [loplugin:classmemaccess]}} + std::memset(&s, 0, sizeof s); + // expected-error@+1 {{writing to an object of non-trivial type 'S'; use assignment instead [loplugin:classmemaccess]}} + std::memset(static_cast<void*>(&s), 0, sizeof s); + auto const disableWarning = static_cast<void*>(&s); + std::memset(disableWarning, 0, sizeof s); + S a[1][1]; + // expected-error@+1 {{writing to an object of non-trivial type 'S'; use assignment instead [loplugin:classmemaccess]}} + std::memset(a, 0, sizeof a); + std::memset(p1, 0, 10); // conservatively assume Incomplete may be trivial + // expected-error@+1 {{writing to an object of non-trivial type 'S'; use assignment instead [loplugin:classmemaccess]}} + std::memset(p2, 0, 10); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/include/basic/sbxvar.hxx b/include/basic/sbxvar.hxx index 446f1fe4c1ec..270db0d137af 100644 --- a/include/basic/sbxvar.hxx +++ b/include/basic/sbxvar.hxx @@ -81,9 +81,10 @@ struct SbxValues void clear(SbxDataType type) { // A hacky way of zeroing the union value corresponding to the given type (even though the // relevant zero value need not be represented by all-zero bits, in general) without evoking - // GCC 8 -Wclass-memaccess, and without having to turn the anonymous union into a non- - // anonymous one: - std::memset(static_cast<void *>(this), 0, offsetof(SbxValues, eType)); + // GCC 8 -Wclass-memaccess or loplugin:classmemaccess, and without having to turn the + // anonymous union into a non-anonymous one: + auto const p = static_cast<void *>(this); + std::memset(p, 0, offsetof(SbxValues, eType)); eType = type; } }; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits