diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -15,6 +15,7 @@ #include "../bugprone/SignedCharMisuseCheck.h" #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" +#include "../concurrency/ThreadCanceltypeAsynchronousCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" #include "../misc/NonCopyableObjects.h" @@ -110,6 +111,9 @@ // POS CheckFactories.registerCheck( "cert-pos44-c"); + CheckFactories + .registerCheck( + "cert-pos47-c"); // SIG CheckFactories.registerCheck("cert-sig30-c"); // STR diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -23,6 +23,7 @@ LINK_LIBS clangTidy clangTidyBugproneModule + clangTidyConcurrencyModule clangTidyGoogleModule clangTidyMiscModule clangTidyPerformanceModule diff --git a/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt b/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangTidyConcurrencyModule ConcurrencyTidyModule.cpp MtUnsafeCheck.cpp + ThreadCanceltypeAsynchronousCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp b/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp --- a/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "MtUnsafeCheck.h" +#include "ThreadCanceltypeAsynchronousCheck.h" namespace clang { namespace tidy { @@ -20,6 +21,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "concurrency-mt-unsafe"); + CheckFactories.registerCheck( + "concurrency-thread-canceltype-asynchronous"); } }; diff --git a/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h @@ -0,0 +1,34 @@ +//===--- ThreadCanceltypeAsynchronousCheck.h - clang-tidy -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_THREADCANCELTYPEASYNCHRONOUSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_THREADCANCELTYPEASYNCHRONOUSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace concurrency { + +/// Finds ``pthread_setcanceltype`` function calls where a thread's +/// cancellation type is set to asynchronous. +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.html +class ThreadCanceltypeAsynchronousCheck : public ClangTidyCheck { +public: + ThreadCanceltypeAsynchronousCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace concurrency +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_THREADCANCELTYPEASYNCHRONOUSCHECK_H diff --git a/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp @@ -0,0 +1,39 @@ +//===--- ThreadCanceltypeAsynchronousCheck.cpp - clang-tidy ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ThreadCanceltypeAsynchronousCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace concurrency { + +void ThreadCanceltypeAsynchronousCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr( + allOf(callee(functionDecl(hasName("::pthread_setcanceltype"))), + argumentCountIs(2)), + hasArgument(0, isExpandedFromMacro("PTHREAD_CANCEL_ASYNCHRONOUS"))) + .bind("setcanceltype"), + this); +} + +void ThreadCanceltypeAsynchronousCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedExpr = Result.Nodes.getNodeAs("setcanceltype"); + diag(MatchedExpr->getBeginLoc(), "the cancel type for a pthread should not " + "be 'PTHREAD_CANCEL_ASYNCHRONOUS'"); +} + +} // namespace concurrency +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,22 @@ Improvements to clang-tidy -------------------------- -The improvements are... +New checks +^^^^^^^^^^ + +- New :doc:`concurrency-thread-canceltype-asynchronous + ` check. + + Finds ``pthread_setcanceltype`` function calls where a thread's cancellation + type is set to asynchronous. + +New check aliases +^^^^^^^^^^^^^^^^^ + +- New alias :doc:`cert-pos47-c + ` to + :doc:`concurrency-thread-canceltype-asynchronous + ` was added. Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst @@ -0,0 +1,9 @@ +.. title:: clang-tidy - cert-pos47-c +.. meta:: + :http-equiv=refresh: 5;URL=concurrency-thread-canceltype-asynchronous.html + +cert-pos47-c +============ + +The cert-pos47-c check is an alias, please see +`concurrency-thread-canceltype-asynchronous `_ for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.rst b/clang-tools-extra/docs/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - concurrency-thread-canceltype-asynchronous + +concurrency-thread-canceltype-asynchronous +========================================== + +Finds ``pthread_setcanceltype`` function calls where a thread's cancellation +type is set to asynchronous. Asynchronous cancellation type +(``PTHREAD_CANCEL_ASYNCHRONOUS``) is generally unsafe, use type +``PTHREAD_CANCEL_DEFERRED`` instead which is the default. Even with deferred +cancellation, a cancellation point in an asynchronous signal handler may still +be acted upon and the effect is as if it was an asynchronous cancellation. + +.. code-block: c++ + + pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype); + +This check corresponds to the CERT C Coding Standard rule +`POS47-C. Do not use threads that can be canceled asynchronously +`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -140,6 +140,7 @@ `clang-analyzer-valist.Uninitialized `_, `clang-analyzer-valist.Unterminated `_, `concurrency-mt-unsafe `_, + `concurrency-thread-canceltype-asynchronous `_, `cppcoreguidelines-avoid-goto `_, `cppcoreguidelines-avoid-non-const-global-variables `_, `cppcoreguidelines-init-variables `_, "Yes" @@ -334,6 +335,7 @@ `cert-oop11-cpp `_, `performance-move-constructor-init `_, "Yes" `cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_, `cert-pos44-c `_, `bugprone-bad-signal-to-kill-thread `_, + `cert-pos47-c `_, `concurrency-thread-canceltype-asynchronous `_, `cert-str34-c `_, `bugprone-signed-char-misuse `_, `clang-analyzer-core.CallAndMessage `_, `Clang Static Analyzer `_, `clang-analyzer-core.DivideZero `_, `Clang Static Analyzer `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp b/clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp @@ -0,0 +1,52 @@ +// RUN: %check_clang_tidy %s concurrency-thread-canceltype-asynchronous %t + +#define ONE (1 << 0) + +#define PTHREAD_CANCEL_DEFERRED 0 +// define the macro intentionally complex +#define PTHREAD_CANCEL_ASYNCHRONOUS ONE + +#define ASYNCHR PTHREAD_CANCEL_ASYNCHRONOUS + +int pthread_setcanceltype(int type, int *oldtype); + +int main() { + int result, oldtype; + + if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype)) != 0) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: the cancel type for a pthread should not be 'PTHREAD_CANCEL_ASYNCHRONOUS' [concurrency-thread-canceltype-asynchronous] + return 1; + } + + if ((result = pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype)) != 0) { + return 1; + } + + return 0; +} + +int f1() { + int result, oldtype; + + if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype)) != 0) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: the cancel type for a pthread should not be 'PTHREAD_CANCEL_ASYNCHRONOUS' [concurrency-thread-canceltype-asynchronous] + return 1; + } + + if ((result = pthread_setcanceltype(ASYNCHR, &oldtype)) != 0) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: the cancel type for a pthread should not be 'PTHREAD_CANCEL_ASYNCHRONOUS' [concurrency-thread-canceltype-asynchronous] + return 1; + } + + return 0; +} + +int f2(int type) { + int result, oldtype; + + if ((result = pthread_setcanceltype(type, &oldtype)) != 0) { + return 1; + } + + return 0; +}