[clang-tools-extra] 8d288a0 - [clang-tidy] Add bugprone-bad-signal-to-kill-thread check and its alias cert-pos44-c
Abel Kocsis via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 11 08:47:49 PST 2019
Author: Abel Kocsis
Date: 2019-11-11T17:47:14+01:00
New Revision: 8d288a0668a574863d52784084ff565c89f7366e
URL: https://github.com/llvm/llvm-project/commit/8d288a0668a574863d52784084ff565c89f7366e
DIFF: https://github.com/llvm/llvm-project/commit/8d288a0668a574863d52784084ff565c89f7366e.diff
LOG: [clang-tidy] Add bugprone-bad-signal-to-kill-thread check and its alias cert-pos44-c
Added:
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/CERTTidyModule.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/bugprone/BadSignalToKillThreadCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
new file mode 100644
index 000000000000..1f3dec497c07
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
new file mode 100644
index 000000000000..d39fdec2e7de
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
@@ -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
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 074aa7a8ba6b..99eff40ebf42 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -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>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index ab3363f43733..4b499e68069c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -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
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 341968b6fa6b..23820446cd17 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -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 {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 40b7aed41784..a94ee3defd1e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
new file mode 100644
index 000000000000..05f5c9363973
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.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>`_.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
new file mode 100644
index 000000000000..5bc48a685afd
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index b499a84fdcfe..9e6e3472d064 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.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
diff --git a/clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp b/clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
new file mode 100644
index 000000000000..5de2001e26b9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
@@ -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;
+}
More information about the cfe-commits
mailing list