diff options
author | fanquake <fanquake@gmail.com> | 2022-07-19 15:51:47 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-08-03 17:52:24 +0100 |
commit | 1c976c691cc4b20f43071aabf36c7afed1571057 (patch) | |
tree | d81d4a689e0e8966879c7123eb1e6a154a7c26df /contrib/devtools | |
parent | 7de23cceb8ac13fcc709453ef0fa14fb93c460b0 (diff) | |
download | bitcoin-1c976c691cc4b20f43071aabf36c7afed1571057.tar.xz |
tidy: Integrate bicoin-tidy clang-tidy plugin
Enable `bitcoin-unterminated-logprintf`.
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Diffstat (limited to 'contrib/devtools')
-rw-r--r-- | contrib/devtools/bitcoin-tidy/CMakeLists.txt | 45 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/README | 8 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp | 22 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/example_logprintf.cpp | 91 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/logprintf.cpp | 62 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/logprintf.h | 28 |
6 files changed, 256 insertions, 0 deletions
diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt new file mode 100644 index 0000000000..9ed18696d4 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -0,0 +1,45 @@ +cmake_minimum_required(VERSION 3.9) + +project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core") + +include(GNUInstallDirs) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED True) +set(CMAKE_CXX_EXTENSIONS False) + +# TODO: Figure out how to avoid the terminfo check +find_package(LLVM REQUIRED CONFIG) +find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy" HINTS ${LLVM_TOOLS_BINARY_DIR}) +message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") +message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}") + +add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp) +target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) + +# Disable RTTI and exceptions as necessary +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /GR-) +else() + target_compile_options(bitcoin-tidy PRIVATE -fno-rtti) + target_compile_options(bitcoin-tidy PRIVATE -fno-exceptions) +endif() + +# Add warnings +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /W4) +else() + target_compile_options(bitcoin-tidy PRIVATE -Wall) + target_compile_options(bitcoin-tidy PRIVATE -Wextra) +endif() + +set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=${CMAKE_BINARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}bitcoin-tidy${CMAKE_SHARED_LIBRARY_SUFFIX}" "-checks=-*,bitcoin-*") + +# Create a dummy library that runs clang-tidy tests as a side-effect of building +add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp) +add_dependencies(bitcoin-tidy-tests bitcoin-tidy) + +set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") + + +install(TARGETS bitcoin-tidy LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/contrib/devtools/bitcoin-tidy/README b/contrib/devtools/bitcoin-tidy/README new file mode 100644 index 0000000000..1669fe98f5 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/README @@ -0,0 +1,8 @@ +# Bitcoin Tidy + +Example Usage: + +```bash +cmake -S . -B build -DLLVM_DIR=/path/to/lib/cmake/llvm -DCMAKE_BUILD_TYPE=Release +make -C build -j$(nproc) +``` diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp new file mode 100644 index 0000000000..0f34d37793 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -0,0 +1,22 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include <clang-tidy/ClangTidyModule.h> +#include <clang-tidy/ClangTidyModuleRegistry.h> + +class BitcoinModule final : public clang::tidy::ClangTidyModule +{ +public: + void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override + { + CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf"); + } +}; + +static clang::tidy::ClangTidyModuleRegistry::Add<BitcoinModule> + X("bitcoin-module", "Adds bitcoin checks."); + +volatile int BitcoinModuleAnchorSource = 0; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp new file mode 100644 index 0000000000..a3d2768964 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp @@ -0,0 +1,91 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +// Warn about any use of LogPrintf that does not end with a newline. +#include <string> + +enum LogFlags { + NONE +}; + +enum Level { + None +}; + +template <typename... Args> +static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args) +{ +} + +#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) +#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) + +// Use a macro instead of a function for conditional logging to prevent +// evaluating arguments when logging for the category is not enabled. +#define LogPrint(category, ...) \ + do { \ + LogPrintf(__VA_ARGS__); \ + } while (0) + + +class CWallet +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + +public: + template <typename... Params> + void WalletLogPrintf(std::string fmt, Params... parameters) const + { + LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + }; +}; + +void good_func() +{ + LogPrintf("hello world!\n"); +} +void good_func2() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi\n"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi\n"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi\n"); + delete walletptr; +} +void bad_func() +{ + LogPrintf("hello world!"); +} +void bad_func2() +{ + LogPrintf(""); +} +void bad_func3() +{ + // Ending in "..." has no special meaning. + LogPrintf("hello world!..."); +} +void bad_func4_ignored() +{ + LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf) +} +void bad_func5() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi"); + delete walletptr; +} diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp new file mode 100644 index 0000000000..1690c8fde0 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp @@ -0,0 +1,62 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include <clang/AST/ASTContext.h> +#include <clang/ASTMatchers/ASTMatchFinder.h> + + +namespace { +AST_MATCHER(clang::StringLiteral, unterminated) +{ + size_t len = Node.getLength(); + if (len > 0 && Node.getCodeUnit(len - 1) == '\n') { + return false; + } + return true; +} +} // namespace + +namespace bitcoin { + +void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) +{ + using namespace clang::ast_matchers; + + /* + Logprintf(..., ..., ..., ..., ..., "foo", ...) + */ + + finder->addMatcher( + callExpr( + callee(functionDecl(hasName("LogPrintf_"))), + hasArgument(5, stringLiteral(unterminated()).bind("logstring"))), + this); + + /* + CWallet wallet; + auto walletptr = &wallet; + wallet.WalletLogPrintf("foo"); + wallet->WalletLogPrintf("foo"); + */ + finder->addMatcher( + cxxMemberCallExpr( + thisPointerType(qualType(hasDeclaration(cxxRecordDecl(hasName("CWallet"))))), + callee(cxxMethodDecl(hasName("WalletLogPrintf"))), + hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), + this); +} + +void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) +{ + if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs<clang::StringLiteral>("logstring")) { + const clang::ASTContext& ctx = *Result.Context; + const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf"); + const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo()); + user_diag << clang::FixItHint::CreateInsertion(loc, "\\n"); + } +} + +} // namespace bitcoin diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h new file mode 100644 index 0000000000..466849ef01 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.h @@ -0,0 +1,28 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef LOGPRINTF_CHECK_H +#define LOGPRINTF_CHECK_H + +#include <clang-tidy/ClangTidyCheck.h> + +namespace bitcoin { + +class LogPrintfCheck final : public clang::tidy::ClangTidyCheck +{ +public: + LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) + : clang::tidy::ClangTidyCheck(Name, Context) {} + + bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override + { + return LangOpts.CPlusPlus; + } + void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; + void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; +}; + +} // namespace bitcoin + +#endif // LOGPRINTF_CHECK_H |