Skip to content

Commit

Permalink
clang-tidy: add check for non-trivial thread_local vars
Browse files Browse the repository at this point in the history
Do not allow thread_local vars with non-trivial destructors
  • Loading branch information
theuni committed May 22, 2024
1 parent a786fd2 commit 34c9cee
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 2 deletions.
4 changes: 2 additions & 2 deletions contrib/devtools/bitcoin-tidy/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy
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)
add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp nontrivial-threadlocal.cpp)
target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS})

# Disable RTTI and exceptions as necessary
Expand Down Expand Up @@ -58,7 +58,7 @@ else()
endif()

# 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_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp example_nontrivial-threadlocal.cpp)
add_dependencies(bitcoin-tidy-tests bitcoin-tidy)

set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
Expand Down
2 changes: 2 additions & 0 deletions contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include "logprintf.h"
#include "nontrivial-threadlocal.h"

#include <clang-tidy/ClangTidyModule.h>
#include <clang-tidy/ClangTidyModuleRegistry.h>
Expand All @@ -13,6 +14,7 @@ class BitcoinModule final : public clang::tidy::ClangTidyModule
void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override
{
CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf");
CheckFactories.registerCheck<bitcoin::NonTrivialThreadLocal>("bitcoin-nontrivial-threadlocal");
}
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#include <string>
thread_local std::string foo;
44 changes: 44 additions & 0 deletions contrib/devtools/bitcoin-tidy/nontrivial-threadlocal.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// 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 "nontrivial-threadlocal.h"

#include <clang/AST/ASTContext.h>
#include <clang/ASTMatchers/ASTMatchFinder.h>


// Copied from clang-tidy's UnusedRaiiCheck
namespace {
AST_MATCHER(clang::CXXRecordDecl, hasNonTrivialDestructor) {
// TODO: If the dtor is there but empty we don't want to warn either.
return Node.hasDefinition() && Node.hasNonTrivialDestructor();
}
} // namespace

namespace bitcoin {

void NonTrivialThreadLocal::registerMatchers(clang::ast_matchers::MatchFinder* finder)
{
using namespace clang::ast_matchers;

/*
thread_local std::string foo;
*/

finder->addMatcher(
varDecl(
hasThreadStorageDuration(),
hasType(hasCanonicalType(recordType(hasDeclaration(cxxRecordDecl(hasNonTrivialDestructor())))))
).bind("nontrivial_threadlocal"),
this);
}

void NonTrivialThreadLocal::check(const clang::ast_matchers::MatchFinder::MatchResult& Result)
{
if (const clang::VarDecl* var = Result.Nodes.getNodeAs<clang::VarDecl>("nontrivial_threadlocal")) {
const auto user_diag = diag(var->getBeginLoc(), "Variable with non-trivial destructor cannot be thread_local.");
}
}

} // namespace bitcoin
29 changes: 29 additions & 0 deletions contrib/devtools/bitcoin-tidy/nontrivial-threadlocal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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 NONTRIVIAL_THREADLOCAL_CHECK_H
#define NONTRIVIAL_THREADLOCAL_CHECK_H

#include <clang-tidy/ClangTidyCheck.h>

namespace bitcoin {

// Warn about any thread_local variable with a non-trivial destructor.
class NonTrivialThreadLocal final : public clang::tidy::ClangTidyCheck
{
public:
NonTrivialThreadLocal(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 // NONTRIVIAL_THREADLOCAL_CHECK_H

0 comments on commit 34c9cee

Please sign in to comment.