Author: Balázs Kéri Date: 2021-02-22T12:42:20+01:00 New Revision: 7dc7f0c2ecc05b9a9c73e89facec24ad6d325cae
URL: https://github.com/llvm/llvm-project/commit/7dc7f0c2ecc05b9a9c73e89facec24ad6d325cae DIFF: https://github.com/llvm/llvm-project/commit/7dc7f0c2ecc05b9a9c73e89facec24ad6d325cae.diff LOG: [clang-tidy] Add new check 'concurrency-thread-canceltype-asynchronous' and alias 'cert-pos47-c'. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D96719 Added: clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst clang-tools-extra/docs/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.rst clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp Modified: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp clang-tools-extra/clang-tidy/cert/CMakeLists.txt clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index 3e0e3502f5e1..616cd3d7069f 100644 --- 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 @@ class CERTModule : public ClangTidyModule { // POS CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>( "cert-pos44-c"); + CheckFactories + .registerCheck<concurrency::ThreadCanceltypeAsynchronousCheck>( + "cert-pos47-c"); // SIG CheckFactories.registerCheck<bugprone::SignalHandlerCheck>("cert-sig30-c"); // STR diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt index e56de01b4780..6d40e9418fc6 100644 --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -23,6 +23,7 @@ add_clang_library(clangTidyCERTModule 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 index b757d6a60b78..65d2ace6645e 100644 --- a/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt @@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS 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 index ea4131e0ca51..7ae891d463f7 100644 --- 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 @@ class ConcurrencyModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<concurrency::MtUnsafeCheck>( "concurrency-mt-unsafe"); + CheckFactories.registerCheck<ThreadCanceltypeAsynchronousCheck>( + "concurrency-thread-canceltype-asynchronous"); } }; diff --git a/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp new file mode 100644 index 000000000000..736fff4c1ed6 --- /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<Expr>("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/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h b/clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h new file mode 100644 index 000000000000..773d33994118 --- /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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0f0b8cf20ce5..0481904398b8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,22 @@ The improvements are... Improvements to clang-tidy -------------------------- -The improvements are... +New checks +^^^^^^^^^^ + +- New :doc:`concurrency-thread-canceltype-asynchronous + <clang-tidy/checks/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 + <clang-tidy/checks/cert-pos47-c>` to + :doc:`concurrency-thread-canceltype-asynchronous + <clang-tidy/checks/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 index 000000000000..bdc7848d2434 --- /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 <concurrency-thread-canceltype-asynchronous.html>`_ 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 index 000000000000..7f058a89a2c3 --- /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 +<https://wiki.sei.cmu.edu/confluence/display/c/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 index 4d7c2b3107c5..3a82bc7fbe49 100644 --- 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-Tidy Checks `clang-analyzer-valist.Uninitialized <clang-analyzer-valist.Uninitialized.html>`_, `clang-analyzer-valist.Unterminated <clang-analyzer-valist.Unterminated.html>`_, `concurrency-mt-unsafe <concurrency-mt-unsafe.html>`_, + `concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_, `cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_, `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_, `cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes" @@ -334,6 +335,7 @@ Clang-Tidy Checks `cert-oop11-cpp <cert-oop11-cpp.html>`_, `performance-move-constructor-init <performance-move-constructor-init.html>`_, "Yes" `cert-oop54-cpp <cert-oop54-cpp.html>`_, `bugprone-unhandled-self-assignment <bugprone-unhandled-self-assignment.html>`_, `cert-pos44-c <cert-pos44-c.html>`_, `bugprone-bad-signal-to-kill-thread <bugprone-bad-signal-to-kill-thread.html>`_, + `cert-pos47-c <cert-pos47-c.html>`_, `concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_, `cert-str34-c <cert-str34-c.html>`_, `bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_, `clang-analyzer-core.CallAndMessage <clang-analyzer-core.CallAndMessage.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_, `clang-analyzer-core.DivideZero <clang-analyzer-core.DivideZero.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_, 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 index 000000000000..c69259f8b818 --- /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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
