diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -56,6 +56,7 @@ #include "SuspiciousStringCompareCheck.h" #include "SwappedArgumentsCheck.h" #include "TerminatingContinueCheck.h" +#include "ThreadCanceltypeAsynchronousCheck.h" #include "ThrowKeywordMissingCheck.h" #include "TooSmallLoopVariableCheck.h" #include "UndefinedMemoryManipulationCheck.h" @@ -165,6 +166,8 @@ "bugprone-swapped-arguments"); CheckFactories.registerCheck( "bugprone-terminating-continue"); + CheckFactories.registerCheck( + "bugprone-thread-canceltype-asynchronous"); CheckFactories.registerCheck( "bugprone-throw-keyword-missing"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -51,6 +51,7 @@ SuspiciousStringCompareCheck.cpp SwappedArgumentsCheck.cpp TerminatingContinueCheck.cpp + ThreadCanceltypeAsynchronousCheck.cpp ThrowKeywordMissingCheck.cpp TooSmallLoopVariableCheck.cpp UndefinedMemoryManipulationCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.h b/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.h @@ -0,0 +1,37 @@ +//===--- 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_BUGPRONE_THREADCANCELTYPEASYNCHRONOUSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_THREADCANCELTYPEASYNCHRONOUSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// 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/bugprone-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; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + Optional> PthreadCancelAsynchronousValue; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_THREADCANCELTYPEASYNCHRONOUSCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp @@ -0,0 +1,79 @@ +//===--- 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 bugprone { + +void ThreadCanceltypeAsynchronousCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(allOf(callee(functionDecl(hasName("::pthread_setcanceltype"))), + argumentCountIs(2)), + hasArgument(0, integerLiteral().bind("integer-literal"))) + .bind("setcanceltype"), + this); +} + +static Preprocessor *PP; + +void ThreadCanceltypeAsynchronousCheck::check( + const MatchFinder::MatchResult &Result) { + if (!PthreadCancelAsynchronousValue) { + const auto IsAsynchronous = [](const auto &KeyValue) -> bool { + return KeyValue.first->getName() == "PTHREAD_CANCEL_ASYNCHRONOUS" && + KeyValue.first->hasMacroDefinition(); + }; + const auto TryExpandAsInteger = + [](Preprocessor::macro_iterator It) -> Optional { + if (It == PP->macro_end()) + return llvm::None; + const MacroInfo *MI = PP->getMacroInfo(It->first); + const Token &T = MI->tokens().back(); + if (!T.isLiteral() || !T.getLiteralData()) + return llvm::None; + 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 AsynchronousMacro = llvm::find_if(PP->macros(), IsAsynchronous); + + PthreadCancelAsynchronousValue = TryExpandAsInteger(AsynchronousMacro); + } + + if (!*PthreadCancelAsynchronousValue) + return; + + const auto *MatchedExpr = Result.Nodes.getNodeAs("setcanceltype"); + const auto *MatchedIntLiteral = + Result.Nodes.getNodeAs("integer-literal"); + if (MatchedIntLiteral->getValue() == **PthreadCancelAsynchronousValue) { + diag(MatchedExpr->getBeginLoc(), + "asynchronous cancelability type should not be used"); + } +} + +void ThreadCanceltypeAsynchronousCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *Pp, Preprocessor *ModuleExpanderPP) { + PP = Pp; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -14,6 +14,7 @@ #include "../bugprone/SignalHandlerCheck.h" #include "../bugprone/SignedCharMisuseCheck.h" #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" +#include "../bugprone/ThreadCanceltypeAsynchronousCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" @@ -110,6 +111,8 @@ // POS CheckFactories.registerCheck( "cert-pos44-c"); + CheckFactories.registerCheck( + "cert-pos47-c"); // SIG CheckFactories.registerCheck("cert-sig30-c"); // STR diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,22 @@ Improvements to clang-tidy -------------------------- -The improvements are... +New checks +^^^^^^^^^^ + +- New :doc:`bugprone-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 + ` to + :doc:`bugprone-thread-canceltype-asynchronous + ` was added. Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - bugprone-thread-canceltype-asynchronous + +bugprone-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 +`_. 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 --- /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=bugprone-thread-canceltype-asynchronous.html + +cert-pos47-c +============ + +The cert-pos47-c check is an alias, please see +`bugprone-thread-canceltype-asynchronous `_ 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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -95,6 +95,7 @@ `bugprone-suspicious-string-compare `_, "Yes" `bugprone-swapped-arguments `_, "Yes" `bugprone-terminating-continue `_, "Yes" + `bugprone-thread-canceltype-asynchronous `_, "No" `bugprone-throw-keyword-missing `_, `bugprone-too-small-loop-variable `_, `bugprone-undefined-memory-manipulation `_, @@ -334,6 +335,7 @@ `cert-oop11-cpp `_, `performance-move-constructor-init `_, "Yes" `cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_, `cert-pos44-c `_, `bugprone-bad-signal-to-kill-thread `_, + `cert-pos47-c `_, `bugprone-thread-canceltype-asynchronous `_, `cert-str34-c `_, `bugprone-signed-char-misuse `_, `clang-analyzer-core.CallAndMessage `_, `Clang Static Analyzer `_, `clang-analyzer-core.DivideZero `_, `Clang Static Analyzer `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp @@ -0,0 +1,32 @@ +// RUN: %check_clang_tidy %s bugprone-thread-canceltype-asynchronous %t + +#define PTHREAD_CANCEL_DEFERRED 0 +#define PTHREAD_CANCEL_ASYNCHRONOUS 1 + +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: asynchronous cancelability type should not be used [bugprone-thread-canceltype-asynchronous] + return 1; + } + + if ((result = pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype)) != 0) { + return 1; + } + + return 0; +} + +int f() { + int result, oldtype; + + if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype)) != 0) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: asynchronous cancelability type should not be used [bugprone-thread-canceltype-asynchronous] + return 1; + } + + return 0; +}