balazske updated this revision to Diff 324629.
balazske marked 3 inline comments as done.
balazske added a comment.
- Fixed remaining formatting and rename problems.
- Removed tryExpandAsInteger, use matcher instead.
- Improved test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96719/new/
https://reviews.llvm.org/D96719
Files:
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/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp
clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
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/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-thread-canceltype-asynchronous.cpp
===================================================================
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -140,6 +140,7 @@
`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 @@
`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>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-thread-canceltype-asynchronous.rst
===================================================================
--- /dev/null
+++ 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>`_.
Index: clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
===================================================================
--- /dev/null
+++ 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.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
+ <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
-----------------------------
Index: clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.h
===================================================================
--- /dev/null
+++ 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
Index: clang-tools-extra/clang-tidy/concurrency/ThreadCanceltypeAsynchronousCheck.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
+++ 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::MtUnsafeCheck>(
"concurrency-mt-unsafe");
+ CheckFactories.registerCheck<ThreadCanceltypeAsynchronousCheck>(
+ "concurrency-thread-canceltype-asynchronous");
}
};
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
+++ 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
Index: clang-tools-extra/clang-tidy/cert/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -23,6 +23,7 @@
LINK_LIBS
clangTidy
clangTidyBugproneModule
+ clangTidyConcurrencyModule
clangTidyGoogleModule
clangTidyMiscModule
clangTidyPerformanceModule
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ 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<bugprone::BadSignalToKillThreadCheck>(
"cert-pos44-c");
+ CheckFactories
+ .registerCheck<concurrency::ThreadCanceltypeAsynchronousCheck>(
+ "cert-pos47-c");
// SIG
CheckFactories.registerCheck<bugprone::SignalHandlerCheck>("cert-sig30-c");
// STR
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
