From: Timothy Day Date: Thu, 13 Jul 2023 04:19:41 +0000 (+0000) Subject: LU-16961 clang: plugins and build system integration X-Git-Tag: 2.15.58~24 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=d684885098c40fee2951feb410bec739717ac9bc LU-16961 clang: plugins and build system integration Clang has a plugin system. Compiler extensions can be created by making a shared library and loading it via the "-fplugin" options. This makes it simple to implement custom warnings and static analyzers. This patch adds a plugin to detect functions that should have been made static. This plugin has been run over the majority of the Lustre tree and patches have been submitted for all warnings. The plugin did not return any false positives in my testing. It also add the "--enable-compiler-plugins" configure option, which automatically builds and sets up the in-tree C compiler plugins. The option force-enables the plugin regardless of which compiler is in use. This behavior could be changed if there is ever a need to support GCC specific plugins. Also, add the configure checks needed to support building C++ in the Lustre tree. Clang and GCC plugins (and the compilers themselves) are written in C++. The license for the plugin mirrors that of the LLVM project itself. This leaves the door open for contributing this plugin upstream in the future. This isn't being upstreamed now because it lacks any significant user community. Hence, the plugin does not appear to meet the requirements for upstreaming based on https://clang.llvm.org/get_involved.html. Test-Parameters: trivial Signed-off-by: Timothy Day Change-Id: I747ed91b53e765cc58e91a3eb9ec6c12b9908a96 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51659 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Shaun Tancheff Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- diff --git a/autoMakefile.am b/autoMakefile.am index a1d3b88..85c32fc 100644 --- a/autoMakefile.am +++ b/autoMakefile.am @@ -93,7 +93,15 @@ endif # LINUX endif # MODULES -all: undef.h +all: plugins + +# compiler plugins should be built as soon as possible, +# so they can be available to the rest of the build +# system +plugins: undef.h +if CC_PLUGINS + $(MAKE) all -C cc-plugins +endif # CC_PLUGINS undef.h: config.h.in grep -v config.h.in config.h.in > $@ diff --git a/cc-plugins/.gitignore b/cc-plugins/.gitignore new file mode 100644 index 0000000..10a7e8d --- /dev/null +++ b/cc-plugins/.gitignore @@ -0,0 +1 @@ +/Makefile.in diff --git a/cc-plugins/FindStatic.cpp b/cc-plugins/FindStatic.cpp new file mode 100644 index 0000000..46b0a5d --- /dev/null +++ b/cc-plugins/FindStatic.cpp @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// +// This file is part of Lustre, http://www.lustre.org/ +// +// cc-plugins/FindStatic.cpp +// +// Clang plugin to find functions which could be made +// static. +// +// Author: Timothy Day +// + +#include +#include +#include +#include +#include +#include +#include + +using namespace clang; + +namespace { + +static std::unordered_map functionMap; + +bool isWarnable(FunctionDecl *MethodDecl) { + auto isValidDef = + MethodDecl->isThisDeclarationADefinition() && MethodDecl->hasBody(); + auto isNotDeclared = + (functionMap.count(MethodDecl->getNameAsString()) == 0); + auto isNotStatic = !MethodDecl->isStatic(); + auto couldBeStatic = + MethodDecl->getASTContext().getSourceManager().isInMainFile( + MethodDecl->getLocation()) && + !MethodDecl->isMain(); + + return isValidDef && isNotDeclared && isNotStatic && couldBeStatic; +} + +class FindStaticDeclVisitor : public RecursiveASTVisitor { + DiagnosticsEngine &Diags; + unsigned WarningFoundStatic; + +public: + FindStaticDeclVisitor(DiagnosticsEngine &Diags) : Diags(Diags) { + WarningFoundStatic = Diags.getCustomDiagID( + DiagnosticsEngine::Warning, "Should this function be static?"); + } + + bool VisitFunctionDecl(FunctionDecl *MethodDecl) { + if (isWarnable(MethodDecl)) { + Diags.Report(MethodDecl->getLocation(), WarningFoundStatic); + } + + return true; + } +}; + +class ScanDeclVisitor : public RecursiveASTVisitor { +public: + bool VisitFunctionDecl(FunctionDecl *MethodDecl) { + if (!isWarnable(MethodDecl)) { + functionMap.insert({MethodDecl->getNameAsString(), 1}); + } + + return true; + } +}; + +class PrintFunctionsConsumer : public ASTConsumer { +public: + void HandleTranslationUnit(ASTContext &Context) override { + ScanDeclVisitor Scanner; + Scanner.TraverseDecl(Context.getTranslationUnitDecl()); + + FindStaticDeclVisitor Visitor(Context.getDiagnostics()); + Visitor.TraverseDecl(Context.getTranslationUnitDecl()); + } +}; + +class FindStaticAction : public PluginASTAction { + std::set ParsedTemplates; + +protected: + std::unique_ptr CreateASTConsumer(CompilerInstance &CI, + llvm::StringRef) override { + return std::make_unique(); + } + + /// Consume plugin arguments; the plugin has no args, so always + /// succeed. + /// + /// \returns true when the parsing succeeds + bool ParseArgs(const CompilerInstance &CI, + const std::vector &args) override { + return true; + } + + /// Determines when to run action, in this case, automatically + /// after the main AST action. + /// + /// \returns when the action should run + PluginASTAction::ActionType getActionType() override { + return AddAfterMainAction; + } +}; + +} // namespace + +static FrontendPluginRegistry::Add + X("find-static", "find potential static functions"); diff --git a/cc-plugins/Makefile.am b/cc-plugins/Makefile.am new file mode 100644 index 0000000..dd9f405 --- /dev/null +++ b/cc-plugins/Makefile.am @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0-only + +# +# This file is part of Lustre, http://www.lustre.org/ +# +# cc-plugins/Makefile.am +# +# Automake file for compiler plugins. +# + +if CC_PLUGINS +lib_LTLIBRARIES = libfindstatic.la +libfindstatic_la_SOURCES = FindStatic.cpp +endif # CC_PLUGINS diff --git a/config/lustre-build.m4 b/config/lustre-build.m4 index 3e8d613..c853504 100644 --- a/config/lustre-build.m4 +++ b/config/lustre-build.m4 @@ -629,10 +629,12 @@ AS_IF([test -n "$SNMP_DIST_SUBDIR"], [LS_CONFIGURE]) LB_CONDITIONALS LB_CONFIG_HEADERS +LPLUG_CONFIGURE LIBCFS_CONFIG_FILES LB_CONFIG_FILES LN_CONFIG_FILES LC_CONFIG_FILES +LPLUG_CONFIG_FILES AS_IF([test -n "$SNMP_DIST_SUBDIR"], [LS_CONFIG_FILES]) AC_SUBST(ac_configure_args) diff --git a/config/lustre-compiler-plugins.m4 b/config/lustre-compiler-plugins.m4 new file mode 100644 index 0000000..6f58dbd --- /dev/null +++ b/config/lustre-compiler-plugins.m4 @@ -0,0 +1,46 @@ +# SPDX-License-Identifier: GPL-2.0 + +# +# This file is part of Lustre, http://www.lustre.org/ +# +# config/lustre-compiler-plugins.m4 +# +# Configure compliler plugin settings +# + +# +# LPLUG_ENABLE +# +# Simple flag to enable compiler plugins. +# +AC_DEFUN([LPLUG_ENABLE], [ +AC_ARG_ENABLE([compiler-plugins], + AS_HELP_STRING([--enable-compiler-plugins], [Enable compiler plugins])) + +AS_IF([test "x$enable_compiler_plugins" == "xyes"], [ +CFLAGS="$CFLAGS -fplugin=$(pwd)/cc-plugins/.libs/libfindstatic.so" +], []) +AM_CONDITIONAL([CC_PLUGINS], [test x$enable_compiler_plugins = xyes]) +]) # LPLUG_ENABLE + +# +# LPLUG_CONFIGURE +# +# main configure steps +# +AC_DEFUN([LPLUG_CONFIGURE], [ +LPLUG_ENABLE +]) # LPLUG_CONFIGURE + +# +# LPLUG_CONFIG_FILES +# +# files that should be generated with AC_OUTPUT +# +AC_DEFUN([LPLUG_CONFIG_FILES], [ +AS_IF([test "x$enable_compiler_plugins" == "xyes"], [ +AC_CONFIG_FILES([ +cc-plugins/Makefile +]) +], []) +]) # LPLUG_CONFIG_FILES diff --git a/config/lustre-toolchain.m4 b/config/lustre-toolchain.m4 index d8c85d8..571df1d 100644 --- a/config/lustre-toolchain.m4 +++ b/config/lustre-toolchain.m4 @@ -31,6 +31,7 @@ fi HOSTCC="$LLVM_PREFIX"clang"$LLVM_SUFFIX" HOSTCXX="$LLVM_PREFIX"clang++"$LLVM_SUFFIX" CC="$LLVM_PREFIX"clang"$LLVM_SUFFIX" +CXX="$LLVM_PREFIX"clang++"$LLVM_SUFFIX" LD="$LLVM_PREFIX"ld.lld"$LLVM_SUFFIX" AR="$LLVM_PREFIX"llvm-ar"$LLVM_SUFFIX" NM="$LLVM_PREFIX"llvm-nm"$LLVM_SUFFIX" @@ -179,6 +180,7 @@ AC_DEFUN([LTC_CC_NO_STRINGOP_OVERFLOW], [ AC_DEFUN([LTC_TOOLCHAIN_CONFIGURE], [ AC_REQUIRE([LTC_LLVM_TOOLCHAIN]) AC_REQUIRE([AC_PROG_CC]) +AC_REQUIRE([AC_PROG_CXX]) AM_PROG_AS AC_CHECK_TOOLS(AR, ar) @@ -213,6 +215,7 @@ EXTRA_KCFLAGS: $EXTRA_KCFLAGS LD: $LD +CXX: $CXX CPPFLAGS: $CPPFLAGS Type 'make' to build Lustre.