[clang-tidy] Add bugprone-bad-signal-to-kill-thread checker and alias cert-pos44-c

Author: Abel Kocsis

bad-signal-to-kill-thread.patch

@@ -0,0 +1,70 @@
+//===--- BadSignalToKillThreadCheck.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 "BadSignalToKillThreadCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void BadSignalToKillThreadCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(allOf(callee(functionDecl(hasName("::pthread_kill"))),
+                     argumentCountIs(2)),
+               hasArgument(1, integerLiteral().bind("integer-literal")))
+          .bind("thread-kill"),
+      this);
+}
+
+static Preprocessor *PP;
+
+void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto IsSigterm = [](const auto &KeyValue) -> bool {
+    return KeyValue.first->getName() == "SIGTERM";
+  };
+  const auto TryExpandAsInteger =
+      [](Preprocessor::macro_iterator It) -> Optional<unsigned> {
+    if (It == PP->macro_end())
+      return llvm::None;
+    const MacroInfo *MI = PP->getMacroInfo(It->first);
+    const Token &T = MI->tokens().back();
+    StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+
+    llvm::APInt IntValue;
+    constexpr unsigned AutoSenseRadix = 0;
+    if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+      return llvm::None;
+    return IntValue.getZExtValue();
+  };
+
+  const auto SigtermMacro = llvm::find_if(PP->macros(), IsSigterm);
+
+  if (!SigtermValue && !(SigtermValue = TryExpandAsInteger(SigtermMacro)))
+    return;
+
+  const auto *MatchedExpr = Result.Nodes.getNodeAs<Expr>("thread-kill");
+  const auto *MatchedIntLiteral =
+      Result.Nodes.getNodeAs<IntegerLiteral>("integer-literal");
+  if (MatchedIntLiteral->getValue() == *SigtermValue) {
+    diag(MatchedExpr->getBeginLoc(),
+         "thread should not be terminated by raising the 'SIGTERM' signal");
+  }
+}
+
+void BadSignalToKillThreadCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *pp, Preprocessor *ModuleExpanderPP) {
+  PP = pp;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp

@@ -0,0 +1,37 @@
+//===--- BadSignalToKillThreadCheck.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_BUGPRONE_BADSIGNALTOKILLTHREADCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BADSIGNALTOKILLTHREADCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds ``pthread_kill`` function calls when thread is terminated by
+/// ``SIGTERM`` signal.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.html
+class BadSignalToKillThreadCheck : public ClangTidyCheck {
+public:
+  BadSignalToKillThreadCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  Optional<unsigned> SigtermValue;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BADSIGNALTOKILLTHREADCHECK_H

clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h

@@ -12,6 +12,7 @@
 #include "../cppcoreguidelines/NarrowingConversionsCheck.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
+#include "BadSignalToKillThreadCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "BranchCloneCheck.h"
 #include "CopyConstructorInitCheck.h"
@@ -68,6 +69,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-argument-comment");
     CheckFactories.registerCheck<AssertSideEffectCheck>(
         "bugprone-assert-side-effect");
+    CheckFactories.registerCheck<BadSignalToKillThreadCheck>(
+        "bugprone-bad-signal-to-kill-thread");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "bugprone-bool-pointer-implicit-conversion");
     CheckFactories.registerCheck<BranchCloneCheck>(

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyBugproneModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
+  BadSignalToKillThreadCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
   BranchCloneCheck.cpp
   BugproneTidyModule.cpp

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../bugprone/UnhandledSelfAssignmentCheck.h"
+#include "../bugprone/BadSignalToKillThreadCheck.h"
 #include "../google/UnnamedNamespaceInHeaderCheck.h"
 #include "../misc/NewDeleteOverloadsCheck.h"
 #include "../misc/NonCopyableObjects.h"
@@ -82,6 +83,9 @@ class CERTModule : public ClangTidyModule {
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
     CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
         "cert-msc32-c");
+    // POS
+    CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>(
+        "cert-pos44-c");
   }
 
   ClangTidyOptions getModuleOptions() override {

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -67,6 +67,12 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New :doc:`bugprone-bad-signal-to-kill-thread
+  <clang-tidy/checks/bugprone-bad-signal-to-kill-thread>` check.
+
+  Finds ``pthread_kill`` function calls when a thread is terminated by 
+  raising ``SIGTERM`` signal.
+
 - New :doc:`bugprone-dynamic-static-initializers
   <clang-tidy/checks/bugprone-dynamic-static-initializers>` check.
 
@@ -88,6 +94,11 @@ Improvements to clang-tidy
   Without the null terminator it can result in undefined behaviour when the
   string is read.
 
+- New alias :doc:`cert-pos44-cpp
+  <clang-tidy/checks/cert-pos44-cpp>` to
+  :doc:`bugprone-bad-signal-to-kill-thread
+  <clang-tidy/checks/bugprone-bad-signal-to-kill-thread>` was added.
+
 - New :doc:`cppcoreguidelines-init-variables
   <clang-tidy/checks/cppcoreguidelines-init-variables>` check.
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -0,0 +1,16 @@
+.. title:: clang-tidy - bugprone-bad-signal-to-kill-thread
+
+bugprone-bad-signal-to-kill-thread
+==================================
+
+Finds ``pthread_kill`` function calls when a thread is terminated by 
+raising ``SIGTERM`` signal and the signal kills the entire process, not 
+just the individual thread. Use any signal except ``SIGTERM``.
+
+.. code-block: c++
+
+    pthread_kill(thread, SIGTERM);
+
+This check corresponds to the CERT C Coding Standard rule
+`POS44-C. Do not use signals to terminate threads
+<https://wiki.sei.cmu.edu/confluence/display/c/POS44-C.+Do+not+use+signals+to+terminate+threads>`_.

clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst

@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-pos44-c
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-bad-signal-to-kill-thread.html
+
+cert-pos44-c
+============
+
+The cert-pos44-c check is an alias, please see
+`bugprone-bad-signal-to-kill-thread <bugprone-bad-signal-to-kill-thread.html>`_ for more information.

clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst

@@ -39,6 +39,7 @@ Clang-Tidy Checks
    boost-use-to-string
    bugprone-argument-comment
    bugprone-assert-side-effect
+   bugprone-bad-signal-to-kill-thread
    bugprone-bool-pointer-implicit-conversion
    bugprone-branch-clone
    bugprone-copy-constructor-init
@@ -105,6 +106,7 @@ Clang-Tidy Checks
    cert-msc51-cpp
    cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp>
    cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) <cert-oop54-cpp>
+   cert-pos44-c (redirects to bugprone-bad-signal-to-kill-thread) <cert-pos44-c>
    clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) <clang-analyzer-core.CallAndMessage>
    clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) <clang-analyzer-core.DivideZero>
    clang-analyzer-core.DynamicTypePropagation

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-bad-signal-to-kill-thread %t
+
+#define SIGTERM 15
+#define SIGINT 2
+using pthread_t = int;
+using pthread_attr_t = int;
+
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+                   void *(*start_routine)(void *), void *arg);
+
+int pthread_kill(pthread_t thread, int sig);
+
+int pthread_cancel(pthread_t thread);
+
+void *test_func_return_a_pointer(void *foo);
+
+int main() {
+  int result;
+  pthread_t thread;
+
+  if ((result = pthread_create(&thread, nullptr, test_func_return_a_pointer, 0)) != 0) {
+  }
+  if ((result = pthread_kill(thread, SIGTERM)) != 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: thread should not be terminated by raising the 'SIGTERM' signal [bugprone-bad-signal-to-kill-thread]
+  }
+
+  //compliant solution
+  if ((result = pthread_cancel(thread)) != 0) {
+  }
+
+  if ((result = pthread_kill(thread, SIGINT)) != 0) {
+  }
+  if ((result = pthread_kill(thread, 0xF)) != 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: thread should not be terminated by raising the 'SIGTERM' signal [bugprone-bad-signal-to-kill-thread]
+  }
+
+  return 0;
+}