[clang-tools-extra] 7dc7f0c - [clang-tidy] Add new check 'concurrency-thread-canceltype-asynchronous' and alias 'cert-pos47-c'.

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 22 03:32:51 PST 2021


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;
+}


        


More information about the cfe-commits mailing list