[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon created this revision.
segoon added a reviewer: alexfh.
Herald added subscribers: cfe-commits, lxfind, modocache, xazax.hun, mgorny.
Herald added a project: clang.
segoon requested review of this revision.

Checks for some thread-unsafe functions against a black list of 
known-to-be-unsafe functions. Usually they access static variables without 
synchronization (e.g. gmtime(3)) or utilize signals in a racy way (e.g. 
sleep(3)).

The patch adds a check instead of auto-fix as thread-safe alternatives usually 
have API with an additional argument (e.g. gmtime(3) v.s. gmtime_r(3)) or have 
a different semantics (e.g. exit(3) v.s. __exit(3)), so it is a rather tricky 
or non-expected fix.

The check is used in Yandex Taxi backend and has caught many unpleasant bugs. A 
similar patch for coroutine-unsafe API is coming next.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t
+
+#include 
+#include 
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  time_t tt{};
+  auto tm = gmtime(&tt);
+  // CHECK-MESSAGES: :[[@LINE-3]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(&tt);
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - misc-mt-unsafe
+
+misc-mt-unsafe
+==
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (e.g. gmtime(3)) or utilize signals in a racy way.
+
+Examples:
+
+.. code-block:: c++
+
+tm = gmtime(timep); // uses a global buffer
+
+sleep(1); // implementation may use SIGALRM
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
@@ -30,7 +30,7 @@
`abseil-time-comparison `_, "Yes"
`abseil-time-subtraction `_, "Yes"
`abseil-upgrade-duration-conversions `_, "Yes"
-   `altera-struct-pack-align `_,
+   `altera-struct-pack-align `_, "Yes"
`android-cloexec-accept `_, "Yes"
`android-cloexec-accept4 `_,
`android-cloexec-creat `_, "Yes"
@@ -143,6 +143,7 @@
`cppcoreguidelines-narrowing-conversions `_,
`cppcoreguidelines-no-malloc `_,
`cppcoreguidelines-owning-memory `_,
+   `cppcoreguidelines-prefer-member-initializer `_, "Yes"
`cppcoreguidelines-pro-bounds-array-to-pointer-decay `_,
`cppcoreguidelines-pro-bounds-constant-array-index `_, "Yes"
`cppcoreguidelines-pro-bounds-pointer-arithmetic `_,
@@ -179,7 +180,6 @@
`google-readability-todo `_,
`google-runtime-int `_,
`google-runtime-operator `_,
-   `google-runtime-references `_,
`google-upgrade-googletest-case `_, "Yes"
`hicpp-avoid-goto `_,
`hicpp-exception-baseclass `_,
@@ -198,6 +198,7 @@
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
+   `misc-mt-unsafe `_, "Yes"
`misc-new-delete-overloads `_,
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,10 @@
   Finds condition variables in nested ``if`` statements that were also checked
   in the outer ``if`` statement and were not changed.
 
+- New :doc:`misc-mt-unsafe ` check.
+
+  Finds thread-unsafe functions usage.
+
 - New :doc:`readability-function-cognitive-complexity
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
@@ -0,0 +1,34 @@
+//=

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 303499.
segoon added a comment.

don't include any system headers in tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - misc-mt-unsafe
+
+misc-mt-unsafe
+==
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (e.g. gmtime(3)) or utilize signals in a racy way.
+
+Examples:
+
+.. code-block:: c++
+
+tm = gmtime(timep); // uses a global buffer
+
+sleep(1); // implementation may use SIGALRM
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
@@ -30,7 +30,7 @@
`abseil-time-comparison `_, "Yes"
`abseil-time-subtraction `_, "Yes"
`abseil-upgrade-duration-conversions `_, "Yes"
-   `altera-struct-pack-align `_,
+   `altera-struct-pack-align `_, "Yes"
`android-cloexec-accept `_, "Yes"
`android-cloexec-accept4 `_,
`android-cloexec-creat `_, "Yes"
@@ -143,6 +143,7 @@
`cppcoreguidelines-narrowing-conversions `_,
`cppcoreguidelines-no-malloc `_,
`cppcoreguidelines-owning-memory `_,
+   `cppcoreguidelines-prefer-member-initializer `_, "Yes"
`cppcoreguidelines-pro-bounds-array-to-pointer-decay `_,
`cppcoreguidelines-pro-bounds-constant-array-index `_, "Yes"
`cppcoreguidelines-pro-bounds-pointer-arithmetic `_,
@@ -179,7 +180,6 @@
`google-readability-todo `_,
`google-runtime-int `_,
`google-runtime-operator `_,
-   `google-runtime-references `_,
`google-upgrade-googletest-case `_, "Yes"
`hicpp-avoid-goto `_,
`hicpp-exception-baseclass `_,
@@ -198,6 +198,7 @@
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
+   `misc-mt-unsafe `_, "Yes"
`misc-new-delete-overloads `_,
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,10 @@
   Finds condition variables in nested ``if`` statements that were also checked
   in the outer ``if`` statement and were not changed.
 
+- New :doc:`misc-mt-unsafe ` check.
+
+  Finds thread-unsafe functions usage.
+
 - New :doc:`readability-function-cognitive-complexity
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
@@ -0,0 +1,34 @@
+//===--- MtUnsafeCheck.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_MISC_MTUNSAFECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MTUNSAFECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Checks that non-thread-safe funct

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon marked an inline comment as done.
segoon added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp:3-4
+
+#include 
+#include 
+

lebedev.ri wrote:
> Tests should be hermetic, they can not use headers from system
fixed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon marked an inline comment as done.
segoon added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:33
`abseil-upgrade-duration-conversions 
`_, "Yes"
-   `altera-struct-pack-align `_,
+   `altera-struct-pack-align `_, "Yes"
`android-cloexec-accept `_, "Yes"

njames93 wrote:
> Can you undo this change and all other unrelated changes to this file.
These changes are added by ./add_new_check.py. I guess the file was altered by 
hands w/o using the script the last time(s).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D90944#2379688 , @njames93 wrote:

> It appears to not check for signs that the code is running in a multi 
> threaded manner, This will result in many false positives in code that is 
> known to be single threaded.

I'm not sure there is a trustworthy check whether a source is going to be used 
in MT environment. A program can be linked with threads libraries but still use 
a single thread and use MT-unsafe API with no problem. I'd rather disable the 
check by default and let the user explicitly enable the check. I'm not sure 
what's clang-tidy policy for such case - should I create a new module instead 
of "misc"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 303532.
segoon added a comment.

- use static instead of namespace {}
- don't use SourceRange()
- revert unrelated changes to .rst


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - misc-mt-unsafe
+
+misc-mt-unsafe
+==
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (e.g. gmtime(3)) or utilize signals in a racy way.
+
+Examples:
+
+.. code-block:: c++
+
+tm = gmtime(timep); // uses a global buffer
+
+sleep(1); // implementation may use SIGALRM
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
@@ -198,6 +198,7 @@
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
+   `misc-mt-unsafe `_,
`misc-new-delete-overloads `_,
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,10 @@
   Finds condition variables in nested ``if`` statements that were also checked
   in the outer ``if`` statement and were not changed.
 
+- New :doc:`misc-mt-unsafe ` check.
+
+  Finds thread-unsafe functions usage.
+
 - New :doc:`readability-function-cognitive-complexity
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
@@ -0,0 +1,34 @@
+//===--- MtUnsafeCheck.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_MISC_MTUNSAFECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MTUNSAFECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Checks that non-thread-safe functions are not used.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-mt-unsafe.html
+class MtUnsafeCheck : public ClangTidyCheck {
+public:
+  MtUnsafeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MTUNSAFECHECK_H
Index: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
@@ -0,0 +1,199 @@
+//===--- MtUnsafeCheck.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache Lice

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 304189.
segoon edited the summary of this revision.
segoon added a comment.

- add `Libc` option
- improve docs


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t -- -config='{CheckOptions: [{key: "misc-mt-unsafe.Libc", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t -- -config='{CheckOptions: [{key: "misc-mt-unsafe.Libc", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - misc-mt-unsafe
+
+misc-mt-unsafe
+==
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (e.g. gmtime(3)) or utilize signals in a racy way.
+The set of functions to check is specified with the `Libc` option.
+
+Examples:
+
+.. code-block:: c++
+
+tm = gmtime(timep); // uses a global buffer
+
+sleep(1); // implementation may use SIGALRM
+
+.. option:: Libc
+
+  Specifies which functions in libc should be considered thread-safe,
+  possible values are `posix`, `glibc`, or `any`.
+  POSIX.1-2001 in "2.9.1 Thread-Safety" defines that all functions
+  specified in the standard are thread-safe except a predefined list of
+  thread-unsafe functions.
+  Glibc defines some of them as thread-safe (e.g. dirname(3)), but adds
+  non-POSIX thread-unsafe ones (e.g. getopt_long(3)).
+  If you want to identify thread-unsafe API for at least one 

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 305460.
segoon added a comment.

- minor changes to docs


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t -- -config='{CheckOptions: [{key: "misc-mt-unsafe.Libc", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t -- -config='{CheckOptions: [{key: "misc-mt-unsafe.Libc", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s misc-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [misc-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [misc-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [misc-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-mt-unsafe.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - misc-mt-unsafe
+
+misc-mt-unsafe
+==
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (e.g. gmtime(3)) or utilize signals in a racy way.
+The set of functions to check is specified with the `Libc` option.
+
+Examples:
+
+.. code-block:: c++
+
+tm = gmtime(timep); // uses a global buffer
+
+sleep(1); // implementation may use SIGALRM
+
+.. option:: Libc
+
+  Specifies which functions in libc should be considered thread-safe,
+  possible values are `posix`, `glibc`, or `any`.
+
+  `posix` means POSIX defined thread-unsafe functions. POSIX.1-2001
+  in "2.9.1 Thread-Safety" defines that all functions specified in the
+  standard are thread-safe except a predefined list of thread-unsafe
+  functions.
+
+  Glibc defines some of them as thread-safe (e.g. dirname(3)), but adds
+  non-POSIX thread-unsafe ones (e.g. getopt_long(3)). Glibc's list is
+  compiled from GNU web documentation wi

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D90944#2380910 , @lebedev.ri wrote:

> What i would however like to be improved, is better docs.

I hope I'll addressed your questions in documentation. Please tell me whether 
you still have any unanswered questions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:295
+  case MtUnsafeCheck::LibcType::Any:
+return hasAnyName(anyFunctions);
+  }

lebedev.ri wrote:
> return anyOf(hasAnyName(posixFunctions), hasAnyName(glibcFunctions));
It would be a bit slower than now - the current code removes duplicates. 
If/when other FunctionSets are added, duplicate list will be much bigger.



Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:309-310
+void MtUnsafeCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(callExpr(callee(functionDecl(hasAnyMtUnsafeNames(Libc
+ .bind("mt-unsafe"),
+ this);

lebedev.ri wrote:
> Is there any way to invert the direction of this matcher,
> instead of checking each call that it's callee isn't one of the bad ones,
> look through all function decls, and for all the bad ones, diagnose all calls 
> to them?
I'm not sure it is possible without additional costs - it would require somehow 
marking bad FunctionDecl, it requires an addition of state.

I'm not an expert in LLVM AST, maybe it is very cheap and easy to do, but I 
don't see how, do you?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement concurrent-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 305764.
segoon retitled this revision from "[clang-tidy] implement misc-mt-unsafe" to 
"[clang-tidy] implement concurrent-mt-unsafe".
segoon added a comment.
Herald added subscribers: sstefan1, arphaman.
Herald added a reviewer: jdoerfert.

- move plugin to `concurrent` group
- naming fixes
- use anyOf
- Libc -> FunctionSet


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/concurrent/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrent/ConcurrentTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrent/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrent/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrent-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrent-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrent-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrent-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrent-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrent-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrent-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrent-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrent-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrent-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrent-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrent-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrent-``Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to 

[PATCH] D91656: [clang-tidy] add concurrent module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon created this revision.
segoon added a reviewer: njames93.
Herald added subscribers: cfe-commits, lxfind, arphaman, modocache, xazax.hun, 
mgorny.
Herald added a project: clang.
segoon requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

The module will contain checks related to concurrent programming (including 
threads, fibers, coroutines, etc.).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/concurrent/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrent/ConcurrentTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrent-``Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,10 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrent`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers, coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrent/ConcurrentTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrent/ConcurrentTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrentTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrent {
+
+class ConcurrentModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrent
+
+// Register the ConcurrentTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrent-module", "Adds concurrent checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrentModule.
+volatile int ConcurrentModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrent/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrent/CMakeLists.txt
@@ -0,0 +1,23 @@
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_library(clangTidyConcurrentModule
+  ConcurrentTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrentModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrentModule.
+extern volatile int ConcurrentModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrentModuleAnchorDestination =
+ConcurrentModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMake

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 306020.
segoon retitled this revision from "[clang-tidy] implement 
concurrent-mt-unsafe" to "[clang-tidy] implement concurrency-mt-unsafe".
segoon added a comment.

- concurrent -> concurrency
- split the patch apart


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
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
@@ -138,6 +138,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - concurrency-mt-unsafe
+
+concurrency-mt-unsafe
+

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 306022.
segoon retitled this revision from "[clang-tidy] add concurrent module" to 
"[clang-tidy] add concurrency module".
segoon added a comment.

- concurrent -> concurrency


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extr

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 306044.
segoon added a comment.

- remove garbage files


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  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/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrency`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers,
+  coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrencyTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrency {
+
+class ConcurrencyModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrency
+
+// Register the ConcurrencyTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrency-module", "Adds concurrency checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrencyModule.
+volatile int ConcurrencyModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
@@ -0,0 +1,23 @@
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_library(clangTidyConcurrencyModule
+  ConcurrencyTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrencyModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrencyModule.
+extern volatile int ConcurrencyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrencyModuleAnchorDestination =
+ConcurrencyModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -73,6 +73,7 @@
 add_subdirectory(performance)
 add_subdirectory(portability)
 add_subdirectory(readability)
+add_subdirectory(concurrency)
 add_subdirectory(zircon)
 set(ALL_CLANG_TIDY_CHECKS
   clangTidy

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D91656#2402245 , @njames93 wrote:

> Can you remove the code related to adding the mt-unsafe-posix check.

Sorry, I've added unnecessary files. Removed now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

jdoerfert, alexfh, hokein, aaron.ballman,

Any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307039.
segoon added a comment.

- sort order
- do not link with FrontendOpenMP


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  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/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrency`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers,
+  coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrencyTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrency {
+
+class ConcurrencyModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrency
+
+// Register the ConcurrencyTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrency-module", "Adds concurrency checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrencyModule.
+volatile int ConcurrencyModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_library(clangTidyConcurrencyModule
+  ConcurrencyTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrencyModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrencyModule.
+extern volatile int ConcurrencyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrencyModuleAnchorDestination =
+ConcurrencyModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -53,6 +53,7 @@
 add_subdirectory(abseil)
 add_subdirectory(altera)
 add_subdirectory(boost)
+add_subdirectory(concurrency)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppco

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D91656#2411072 , @lebedev.ri wrote:

> LGTM

thanks for comments, fixed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

lebedev.ri, any comments?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307884.
segoon added a comment.

- move static vectors out of namespace
- mark mt-unsafe decls and check for marks in exprs


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
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
@@ -138,6 +138,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - concurrency-mt-unsafe
+
+concurrency-mt-unsafe
+=
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe f

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307883.
segoon added a comment.

- fix sort order


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  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/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrency`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers,
+  coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrencyTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrency {
+
+class ConcurrencyModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrency
+
+// Register the ConcurrencyTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrency-module", "Adds concurrency checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrencyModule.
+volatile int ConcurrencyModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_library(clangTidyConcurrencyModule
+  ConcurrencyTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrencyModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrencyModule.
+extern volatile int ConcurrencyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrencyModuleAnchorDestination =
+ConcurrencyModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -55,6 +55,7 @@
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
+add_subdirectory(concurrency)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(darwin)
 add_subdirectory(fuchsia)
@@ -81,6 +82,7 

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D90944#2418845 , @lebedev.ri wrote:

> In D90944#2418792 , @segoon wrote:
>
>> - mark mt-unsafe decls and check for marks in exprs
>
> Eeeh.
> I was thinking of either some smart matcher "match any function declaration
> with name from these lists, and then match every call to said decl".

I tried to utilize bind()+equalsBoundNode(), but it seems it's impossible to 
mark and use the mark in a single matcher.

> But the current implementation, i'm not sure this approach is even legal for 
> checks.

The trick is stolen from abseil/UpgradeDurationConversionsCheck.h. If it's 
invalid here, then abseil should be fixed too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307903.
segoon added a comment.

- revert decls marking


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
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
@@ -138,6 +138,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - concurrency-mt-unsafe
+
+concurrency-mt-unsafe
+=
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronizat

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307979.
segoon added a comment.

- git dif -U


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91656/new/

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  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/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrency`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers,
+  coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrencyTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrency {
+
+class ConcurrencyModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrency
+
+// Register the ConcurrencyTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrency-module", "Adds concurrency checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrencyModule.
+volatile int ConcurrencyModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_library(clangTidyConcurrencyModule
+  ConcurrencyTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrencyModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrencyModule.
+extern volatile int ConcurrencyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrencyModuleAnchorDestination =
+ConcurrencyModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -55,6 +55,7 @@
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
+add_subdirectory(concurrency)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(darwin)
 add_subdirectory(fuchsia)
@@ -81,6 +82,7 

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307981.
segoon added a comment.

- git diff -U


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

Files:
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
  clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "posix"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  ::sleep(2);
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t -- -config='{CheckOptions: [{key: "concurrency-mt-unsafe.FunctionSet", value: "glibc"}]}'
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  dirname(nullptr);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s concurrency-mt-unsafe %t
+
+extern unsigned int sleep (unsigned int __seconds);
+extern int *gmtime (const int *__timer);
+extern int *gmtime_r (const int *__timer, char*);
+extern char *dirname (char *__path);
+
+void foo() {
+  sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+  ::sleep(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  auto tm = gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function is not thread safe [concurrency-mt-unsafe]
+  tm = ::gmtime(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: function is not thread safe [concurrency-mt-unsafe]
+
+  tm = gmtime_r(nullptr, nullptr);
+  tm = ::gmtime_r(nullptr, nullptr);
+
+  dirname(nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function is not thread safe [concurrency-mt-unsafe]
+}
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
@@ -138,6 +138,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - concurrency-mt-unsafe
+
+concurrency-mt-unsafe
+=
+
+Checks for some thread-unsafe functions against a black list of
+known-to-be-unsafe functions. Usually they access static variables without
+synchronization (

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D90944#2419420 , @lebedev.ri wrote:

> Please upload all patches with full context (`-U`)

Done.

> I'm guessing you'll need help committing this, in which case please specify 
> `Author ` to be used for `git commit --author="<>"`

Vasily Kulikov 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90944/new/

https://reviews.llvm.org/D90944

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

alexfh, hi! any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 322685.
segoon added a comment.
Herald added a subscriber: nullptr.cpp.

- explicitly state in docs that the check must be used only for async code


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.h
  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/concurrency-async-blocking.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.c
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
@@ -0,0 +1,417 @@
+// RUN: %check_clang_tidy %s concurrency-async-blocking %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.WaitableExtra", value: "my::Future;my::cv"}, {key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.FunctionsExtra", value: "my_sleep;my::sleep"}, {key: "concurrency-async-blocking.TypesExtra", value: "my::big_lock;my::other_lock"}]}'
+
+/* Poor man's declaration of std::mutex and friends */
+namespace std {
+namespace chrono {
+class seconds {
+public:
+  seconds(int);
+};
+} // namespace chrono
+
+class mutex {
+public:
+  void lock();
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+
+  template 
+  void try_lock_for(Duration);
+};
+class recursive_mutex {};
+class recursive_timed_mutex {};
+class shared_mutex {};
+class shared_timed_mutex {};
+class mutex_suffix {};
+class prefix_mutex {};
+
+template 
+class unique_lock {
+public:
+  unique_lock(Lock &);
+
+  void lock();
+  template 
+  void try_lock_for(Duration);
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+};
+
+} // namespace std
+
+namespace ns {
+class mutex {};
+} // namespace ns
+
+class mutex {};
+
+template 
+class nonlock {};
+
+namespace my {
+class mutex {
+public:
+  void lock();
+};
+class shared_mutex {};
+class non_mutex {
+public:
+  void lock();
+};
+} // namespace my
+
+void test_lockable() {
+  std::mutex m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  ::std::mutex mns;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::shared_mutex sm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'shared_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_mutex rm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_timed_mutex rtm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_timed_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  my::mutex mym;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::mutex_suffix m1;
+  std::prefix_mutex m2;
+  ns::mutex m3;
+  mutex m4;
+  my::non_mutex myn;
+
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mns.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mym.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  myn.lock();
+
+  m.lock_suffix();
+  m.prefix_lock();
+
+  std::unique_lock lock(m);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'unique_lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::unique_lock l1(m1);
+  std::unique_lock l2(m2);
+  std::unique_lock l3(m3);
+  std::unique_lock l4(m4);
+
+  nonlock nonlock;
+}
+
+void sleep(int);
+void nanosleep(int);
+void usleep(int);
+void xsleep(int);
+void sleepx(int);
+void system(const char *);
+int wait(int *);
+int waitpid(int, int *, int);
+int waitid(int idtype, int id, int *infop, int options);
+
+struct rusage {};
+using pid_t = int;
+pid_t wait3(int *status, int options,
+struct rusage *rusage);
+
+pid_t wait4(pid_t pid, int *status, int options,
+struct rusage *rusage);
+
+namespace std {
+namespace this_thread {
+void yield();
+
+template 
+void sleep_for(Duration

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 322695.
segoon added a comment.
Herald added a subscriber: nullptr.cpp.

- explicitly state in docs that the check must be used only for async code


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h
  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/concurrency-async-fs.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s concurrency-async-fs %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-fs.FunctionsExtra", value: "::my::block"},{key: "concurrency-async-fs.TypesExtra", value: "::my::file"}]}'
+
+void chdir(const char *);
+
+namespace std {
+namespace filesystem {
+bool exists(const char *);
+
+void copy(const char *, const char *);
+
+class directory_iterator {
+public:
+  directory_iterator(const char *);
+
+  bool operator!=(const directory_iterator &) const;
+
+  directory_iterator &operator++();
+
+  int operator*() const;
+};
+
+directory_iterator begin(directory_iterator iter) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+directory_iterator end(const directory_iterator &) noexcept;
+
+} // namespace filesystem
+
+template 
+class basic_fstream {};
+
+template 
+class basic_ofstream {};
+
+template 
+class basic_ifstream {};
+
+typedef basic_fstream fstream;
+typedef basic_ofstream ofstream;
+typedef basic_ifstream ifstream;
+
+} // namespace std
+
+void copy();
+
+void test_core() {
+  chdir("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'chdir' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::exists("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'exists' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::copy("/a", "/b");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'copy' may block on filesystem access [concurrency-async-fs]
+
+  copy();
+
+  for (const auto &f : std::filesystem::directory_iterator("/")) {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+  }
+}
+
+void test_fstream() {
+  std::fstream fs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ofstream of;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ofstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ifstream ifs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ifstream' may access filesystem in a blocking way [concurrency-async-fs]
+
+  std::basic_fstream bfs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+}
+
+namespace my {
+class file {};
+
+void block();
+} // namespace my
+
+void test_user() {
+  my::file f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'file' may access filesystem in a blocking way [concurrency-async-fs]
+
+  my::block();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'block' may block on filesystem access [concurrency-async-fs]
+}
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
@@ -139,6 +139,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-async-fs `_,
`concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - concurrency-async-fs
+
+concurrency-async-fs
+
+
+Finds filesystem accesses that might block current system thread.
+Asynchronous code may deal with it in numerous ways, the most widespread
+ways are the following:
+
+* use asynchronous API like `aio` or `io_uring`
+* delegate all filesystem access to a thread pool
+
+Some pr

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 322696.
segoon added a comment.
Herald added a subscriber: nullptr.cpp.

- explicitly state in docs that the check must be used only for async code


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.h
  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/concurrency-async-no-new-threads.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.c
  
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s concurrency-async-no-new-threads %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-no-new-threads.FunctionsExtra", value: "::my::create_thread"},{key: "concurrency-async-no-new-threads.TypesExtra", value: "::my::thread"}]}'
+
+namespace std {
+
+class thread {
+public:
+  void join() {}
+};
+
+class jthread {};
+
+class nothread {};
+
+namespace execution {
+
+class sequenced_policy {};
+
+class parallel_policy {};
+
+class parallel_unsequenced_policy {};
+
+class unsequenced_policy {};
+
+sequenced_policy seq;
+parallel_policy par;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+parallel_unsequenced_policy par_unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+unsequenced_policy unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+
+} // namespace execution
+
+void async(int (*)()) {
+}
+
+template 
+void sort(T1 &&, T2, T3, T4);
+
+} // namespace std
+
+int pthread_create(int *thread, const int *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int thrd_create(int *thr, int func, void *arg);
+
+int fork();
+
+int vfork();
+
+int clone(int (*fn)(void *), void *child_stack,
+  int flags, void *arg);
+
+long clone3(int *cl_args, int size);
+
+namespace boost {
+
+class thread {};
+
+class scoped_thread {};
+
+void async(int (*)()) {}
+
+namespace compute {
+class device {};
+
+} // namespace compute
+
+} // namespace boost
+
+void test_threads() {
+  std::thread t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+
+  std::jthread j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'jthread' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::parallel_policy pp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::sequenced_policy sp;
+
+  std::sort(std::execution::par, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 'par' creates new threads [concurrency-async-no-new-threads]
+
+  boost::thread bt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+  boost::scoped_thread bst;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'scoped_thread' creates new threads [concurrency-async-no-new-threads]
+
+  boost::compute::device bcd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'device' creates new threads [concurrency-async-no-new-threads]
+
+  std::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  boost::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  pthread_create(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'pthread_create' creates new threads [concurrency-async-no-new-threads]
+
+  thrd_create(0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'thrd_create' creates new threads [concurrency-async-no-new-threads]
+
+  fork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'fork' creates new threads [concurrency-async-no-new-threads]
+
+  vfork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'vfork' creates new threads [concurrency-async-no-new-threads]
+
+  clone(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone' creates new threads [concurrency-async-no-new-threads]
+
+  clone3(0, 0);
+  // CHECK-MESSAGES: :[[@LIN

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a reviewer: alexfh_.
segoon added a comment.

alexfh, aaron.ballman, hi! Any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

alexfh, aaron.ballman, hi! Any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

alexfh, aaron.ballman, hi! Any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:22
+/* C++ std */
+"::std::async", //
+

aaron.ballman wrote:
> The trailing comment markers don't really add much.
it's a hack for clang-format, otherwise it contatenates the lines, creating 
unmaintainable mess of strings. "One line - one name" is much more suitable.



Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:33
+
+/* Linux */
+"::fork", //

aaron.ballman wrote:
> If we're going to add these, we should probably also add ones for Win32 and 
> Mac OS as well, like `CreateThread`, `CreateRemoteThread`, `_beginthread`, 
> `_beginthreadex`, etc.
I don't mind, but I'm not an expert in WinAPI or Windows programming. So, this 
part should be by someone with expertise of Windows in separate patches.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/concurrency-async-no-new-threads.rst:8-10
+functions and types. E.g. if the code uses C++ coroutines, it is expected
+that only new coroutines or coroutine-based primitives are created
+instead of heavy system threads.

aaron.ballman wrote:
> FWIW, this suggests to me that what you really want is a way for APIs to opt 
> into this behavior. There's no reason why you wouldn't have a complex system 
> that has both threads and coroutines in it, but it does stand to reason that 
> you may want to say "this function, and everything called within this 
> function, should not create any system threads" in some situations.
> 
> The note below helps call out the expectations from the check, but it 
> requires the developer to restructure the way they write code pretty 
> drastically in order to make the checking behavior more reasonable, which 
> does not seem ideal.
I think it is a complex problem, so it should be separated into smaller tasks.

Step one - checks with hardcoded functions/types with user-guided enabling on a 
per-file basis. A semi-automated check.

Step two - try to solve other parts of the puzzle. Maybe try to add 
[clang:coroutine_safe] tag and teach clang static analyzer to deduce coroutine 
safety property and use it for enabling/disabling the cheks. Maybe reuse other 
(not yet implemented) heuristics from static analyzer (or other tools) to 
identify coroutine functions and check only these functions. I'm not an expert 
in static analyzer, so other LLVM developers might find a clever heuristics 
when to enable/disable these checks or maybe how to deduce 
blacklisted/whitelisted functions/types lists (e.g. for 
concurrency-async-{fs,blocking}).

Indeed, the current approach has its own limitations. But it may be a first 
step in the right direction.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:22
+/* C++ std */
+"::std::async", //
+

aaron.ballman wrote:
> segoon wrote:
> > aaron.ballman wrote:
> > > The trailing comment markers don't really add much.
> > it's a hack for clang-format, otherwise it contatenates the lines, creating 
> > unmaintainable mess of strings. "One line - one name" is much more suitable.
> Ah, I didn't know that'd change the behavior of clang-format, that's neat!
> 
> FWIW, we usually don't do a whole lot of markup to avoid clang-format issues 
> (such as the clang-format: on/off markers). Instead, we usually just ignore 
> the LINT warnings in code review and check the code in as-is. This helps 
> reduce clutter in the code base. In this case, the comments aren't adding a 
> ton of clutter so maybe they're fine. But they definitely look odd as a 
> reader of the code, which is a bit distracting.
IMO having an ability to run clang-format for sources is very handy. It makes 
me sad if running clang-format against the source forces me to set some markers 
or reverting a part of clang-format changes in the source parts I have never 
touched. An ability to simply rerun checks/linters is very powerful.



Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:33
+
+/* Linux */
+"::fork", //

aaron.ballman wrote:
> segoon wrote:
> > aaron.ballman wrote:
> > > If we're going to add these, we should probably also add ones for Win32 
> > > and Mac OS as well, like `CreateThread`, `CreateRemoteThread`, 
> > > `_beginthread`, `_beginthreadex`, etc.
> > I don't mind, but I'm not an expert in WinAPI or Windows programming. So, 
> > this part should be by someone with expertise of Windows in separate 
> > patches.
> I don't think it needs to be done in separate patches. I can give you the 
> full list of things I care about.
> ```
> CreateThread 
> (https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread)
> CreateRemoteThread 
> (https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createremotethread)
> CreateRemoteThreadEx 
> (https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createremotethreadex)
> _beginthread
> _beginthreadex 
> (https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/beginthread-beginthreadex?view=msvc-160)
> ```
Oh, great, thank you! Added.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/concurrency-async-no-new-threads.rst:8-10
+functions and types. E.g. if the code uses C++ coroutines, it is expected
+that only new coroutines or coroutine-based primitives are created
+instead of heavy system threads.

aaron.ballman wrote:
> segoon wrote:
> > aaron.ballman wrote:
> > > FWIW, this suggests to me that what you really want is a way for APIs to 
> > > opt into this behavior. There's no reason why you wouldn't have a complex 
> > > system that has both threads and coroutines in it, but it does stand to 
> > > reason that you may want to say "this function, and everything called 
> > > within this function, should not create any system threads" in some 
> > > situations.
> > > 
> > > The note below helps call out the expectations from the check, but it 
> > > requires the developer to restructure the way they write code pretty 
> > > drastically in order to make the checking behavior more reasonable, which 
> > > does not seem ideal.
> > I think it is a complex problem, so it should be separated into smaller 
> > tasks.
> > 
> > Step one - checks with hardcoded functions/types with user-guided enabling 
> > on a per-file basis. A semi-automated check.
> > 
> > Step two - try to solve other parts of the puzzle. Maybe try to add 
> > [clang:coroutine_safe] tag and teach clang static analyzer to deduce 
> > coroutine safety property and use it for enabling/disabling the cheks. 
> > Maybe reuse other (not yet implemented) heuristics from static analyzer (or 
> > other tools) to identify coroutine functions and check only these 
> > functions. I'm not an expert in static analyzer, so other LLVM developers 
> > might find a clever heuristics when to enable/disable these checks or maybe 
> > how to deduce blacklisted/whitelisted functions/types lists (e.g. for 
> > concurrency-async-{fs,blocking}).
> > 
> > Indeed, the current approach has its own limitations. But it may be a first 
> > step in the right direction.
> My fear with the current approach is that I don't think projects usually 
> split their code in the way this check requires, and so the check will not be 
> very useful in practice because it will be too chatty. Have you tried running 
> this check over some large projects that use coroutines to see what the 
> diagnostic results look like?
I haven't conducted a survey, bu

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 323006.
segoon added a comment.

- add WinAPI support


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.h
  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/concurrency-async-no-new-threads.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.c
  
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s concurrency-async-no-new-threads %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-no-new-threads.FunctionsExtra", value: "::my::create_thread"},{key: "concurrency-async-no-new-threads.TypesExtra", value: "::my::thread"}]}'
+
+namespace std {
+
+class thread {
+public:
+  void join() {}
+};
+
+class jthread {};
+
+class nothread {};
+
+namespace execution {
+
+class sequenced_policy {};
+
+class parallel_policy {};
+
+class parallel_unsequenced_policy {};
+
+class unsequenced_policy {};
+
+sequenced_policy seq;
+parallel_policy par;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+parallel_unsequenced_policy par_unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+unsequenced_policy unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+
+} // namespace execution
+
+void async(int (*)()) {
+}
+
+template 
+void sort(T1 &&, T2, T3, T4);
+
+} // namespace std
+
+int pthread_create(int *thread, const int *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int thrd_create(int *thr, int func, void *arg);
+
+int fork();
+
+int vfork();
+
+int clone(int (*fn)(void *), void *child_stack,
+  int flags, void *arg);
+
+long clone3(int *cl_args, int size);
+
+namespace boost {
+
+class thread {};
+
+class scoped_thread {};
+
+void async(int (*)()) {}
+
+namespace compute {
+class device {};
+
+} // namespace compute
+
+} // namespace boost
+
+void test_threads() {
+  std::thread t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+
+  std::jthread j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'jthread' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::parallel_policy pp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::sequenced_policy sp;
+
+  std::sort(std::execution::par, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 'par' creates new threads [concurrency-async-no-new-threads]
+
+  boost::thread bt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+  boost::scoped_thread bst;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'scoped_thread' creates new threads [concurrency-async-no-new-threads]
+
+  boost::compute::device bcd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'device' creates new threads [concurrency-async-no-new-threads]
+
+  std::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  boost::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  pthread_create(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'pthread_create' creates new threads [concurrency-async-no-new-threads]
+
+  thrd_create(0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'thrd_create' creates new threads [concurrency-async-no-new-threads]
+
+  fork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'fork' creates new threads [concurrency-async-no-new-threads]
+
+  vfork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'vfork' creates new threads [concurrency-async-no-new-threads]
+
+  clone(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone' creates new threads [concurrency-async-no-new-threads]
+
+  clone3(0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone3' creates new threads [concurrency-async-no-new-threads]
+}

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 323008.
segoon added a comment.

-fix typo


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.h
  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/concurrency-async-no-new-threads.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.c
  
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s concurrency-async-no-new-threads %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-no-new-threads.FunctionsExtra", value: "::my::create_thread"},{key: "concurrency-async-no-new-threads.TypesExtra", value: "::my::thread"}]}'
+
+namespace std {
+
+class thread {
+public:
+  void join() {}
+};
+
+class jthread {};
+
+class nothread {};
+
+namespace execution {
+
+class sequenced_policy {};
+
+class parallel_policy {};
+
+class parallel_unsequenced_policy {};
+
+class unsequenced_policy {};
+
+sequenced_policy seq;
+parallel_policy par;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+parallel_unsequenced_policy par_unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+unsequenced_policy unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+
+} // namespace execution
+
+void async(int (*)()) {
+}
+
+template 
+void sort(T1 &&, T2, T3, T4);
+
+} // namespace std
+
+int pthread_create(int *thread, const int *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int thrd_create(int *thr, int func, void *arg);
+
+int fork();
+
+int vfork();
+
+int clone(int (*fn)(void *), void *child_stack,
+  int flags, void *arg);
+
+long clone3(int *cl_args, int size);
+
+namespace boost {
+
+class thread {};
+
+class scoped_thread {};
+
+void async(int (*)()) {}
+
+namespace compute {
+class device {};
+
+} // namespace compute
+
+} // namespace boost
+
+void test_threads() {
+  std::thread t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+
+  std::jthread j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'jthread' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::parallel_policy pp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::sequenced_policy sp;
+
+  std::sort(std::execution::par, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 'par' creates new threads [concurrency-async-no-new-threads]
+
+  boost::thread bt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+  boost::scoped_thread bst;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'scoped_thread' creates new threads [concurrency-async-no-new-threads]
+
+  boost::compute::device bcd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'device' creates new threads [concurrency-async-no-new-threads]
+
+  std::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  boost::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  pthread_create(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'pthread_create' creates new threads [concurrency-async-no-new-threads]
+
+  thrd_create(0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'thrd_create' creates new threads [concurrency-async-no-new-threads]
+
+  fork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'fork' creates new threads [concurrency-async-no-new-threads]
+
+  vfork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'vfork' creates new threads [concurrency-async-no-new-threads]
+
+  clone(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone' creates new threads [concurrency-async-no-new-threads]
+
+  clone3(0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone3' creates new threads [concurrency-async-no-new-threads]
+}
+
+namespac

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

It seems that the related file structure (IOW, "separated coroutine code and 
non-coroutine code" rule) is indeed specific to Yandex.Taxi. I asked multiple 
developers in different companies and no one could confirm they align with the 
rule. The checks in the current form seem to have lower value outside of 
Yandex.Taxi than I thought. So, I'll close PRs with the checks in the current 
form (unless someone tell me what part of the check(s) could be used in another 
way). Thanks for all who reviewed the patches and made suggestions :)

FWIW, we'll still use the checks in Yandex.Taxi. A clang static analyzer check 
or some other graph analysis / manual attributes marking approach would fit 
better for the LLVM upstream.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon abandoned this revision.
segoon added a comment.

see https://reviews.llvm.org/D94622#2617417


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon abandoned this revision.
segoon added a comment.

see https://reviews.llvm.org/D94622#2617417


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon created this revision.
Herald added a subscriber: jfb.
segoon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The preemptive functions/types can be separated into the following categories:

- explicit sleep(3)-like functions
- sleeping/waiting synchronization primitives

The check searches for:

- C++ synchronization primitives
- C11 synchronization primitives
- POSIX synchronization primitives
- some POSIX blocking functions
- some blocking Linux syscalls
- some Boost.Thread synchronization primitives

There are AST matchers for both sync primitives creation and blocking methods 
calls - we want
the former for user-created mutexes and the latter for mutex usage passed from 
outside (e.g. via
library global variable or function parameter).

Atomic code is WIP, will be done before the merge. TODOs in tests are marked 
with CHECKT-MESSAGES.

The check doesn't include the following:

- Io and filesystem operations. It will be included in a separate checker 
concurrency-async-fs as a user might have a different policy (e.g. no FS thread 
pool, so nothing can be done).
- Creation of new threads. Same here, will be implemented in 
concurrency-async-no-new-threads, it is OK for some projects and must be 
changed to async code in others.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp


Index: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
===
--- clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
+++ clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
@@ -314,12 +314,6 @@
 }
   }
 
-  const auto *L = Result.Nodes.getNodeAs("lock");
-  if (L) {
-diag(L->getBeginLoc(), "type may sleep and is not coroutine-safe")
-<< SourceRange(L->getBeginLoc(), L->getEndLoc());
-  }
-
   const auto *Atomic = Result.Nodes.getNodeAs(kAtomic);
   if (Atomic) {
 const auto *Lockfree = Result.Nodes.getNodeAs(kLockfree);


Index: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
===
--- clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
+++ clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
@@ -314,12 +314,6 @@
 }
   }
 
-  const auto *L = Result.Nodes.getNodeAs("lock");
-  if (L) {
-diag(L->getBeginLoc(), "type may sleep and is not coroutine-safe")
-<< SourceRange(L->getBeginLoc(), L->getEndLoc());
-  }
-
   const auto *Atomic = Result.Nodes.getNodeAs(kAtomic);
   if (Atomic) {
 const auto *Lockfree = Result.Nodes.getNodeAs(kLockfree);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 314092.
segoon added a comment.

up


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp


Index: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
===
--- clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
+++ clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
@@ -314,12 +314,6 @@
 }
   }
 
-  const auto *L = Result.Nodes.getNodeAs("lock");
-  if (L) {
-diag(L->getBeginLoc(), "type may sleep and is not coroutine-safe")
-<< SourceRange(L->getBeginLoc(), L->getEndLoc());
-  }
-
   const auto *Atomic = Result.Nodes.getNodeAs(kAtomic);
   if (Atomic) {
 const auto *Lockfree = Result.Nodes.getNodeAs(kLockfree);


Index: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
===
--- clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
+++ clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
@@ -314,12 +314,6 @@
 }
   }
 
-  const auto *L = Result.Nodes.getNodeAs("lock");
-  if (L) {
-diag(L->getBeginLoc(), "type may sleep and is not coroutine-safe")
-<< SourceRange(L->getBeginLoc(), L->getEndLoc());
-  }
-
   const auto *Atomic = Result.Nodes.getNodeAs(kAtomic);
   if (Atomic) {
 const auto *Lockfree = Result.Nodes.getNodeAs(kLockfree);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 314093.
segoon retitled this revision from "Add a check for blocking types and 
functions." to "[clang-tidy] Add a check for blocking types and functions.".
segoon added a comment.
Herald added a reviewer: jfb.
Herald added subscribers: xazax.hun, mgorny.
Herald added a reviewer: jfb.

fix the mess


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.h
  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/concurrency-async-blocking.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.c
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
@@ -0,0 +1,450 @@
+// RUN: %check_clang_tidy %s concurrency-async-blocking %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.WaitableExtra", value: "my::Future;my::cv"}, {key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.FunctionsExtra", value: "my_sleep;my::sleep"}, {key: "concurrency-async-blocking.TypesExtra", value: "my::big_lock;my::other_lock"}]}'
+
+/* Poor man's declaration of std::mutex and friends */
+namespace std {
+namespace chrono {
+class seconds {
+public:
+  seconds(int);
+};
+} // namespace chrono
+
+class mutex {
+public:
+  void lock();
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+
+  template 
+  void try_lock_for(Duration);
+};
+class recursive_mutex {};
+class recursive_timed_mutex {};
+class shared_mutex {};
+class shared_timed_mutex {};
+class mutex_suffix {};
+class prefix_mutex {};
+
+template 
+class unique_lock {
+public:
+  unique_lock(Lock &);
+
+  void lock();
+  template 
+  void try_lock_for(Duration);
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+};
+
+} // namespace std
+
+namespace ns {
+class mutex {};
+} // namespace ns
+
+class mutex {};
+
+template 
+class nonlock {};
+
+namespace my {
+class mutex {
+public:
+  void lock();
+};
+class shared_mutex {};
+class non_mutex {
+public:
+  void lock();
+};
+} // namespace my
+
+void test_lockable() {
+  std::mutex m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type std::mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+  ::std::mutex mns;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type ::std::mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::shared_mutex sm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type std::shared_mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_mutex rm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type std::recursive_mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_timed_mutex rtm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type std::recursive_timed_mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+  my::mutex mym;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type my::mutex may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::mutex_suffix m1;
+  std::prefix_mutex m2;
+  ns::mutex m3;
+  mutex m4;
+  my::non_mutex myn;
+
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method lock may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mns.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method lock may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mym.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method lock may sleep and is not coroutine-safe [concurrency-async-blocking]
+  myn.lock();
+
+  m.lock_suffix();
+  m.prefix_lock();
+
+  std::unique_lock lock(m);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type std::unique_lock may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::unique_lock l1(m1);
+  std::unique_lock l2(m2);
+  std::unique_lock l3(m3);
+  std::unique_lock l4(m4);
+
+  nonlock nonlock;
+}
+
+void sleep(int);
+void nanosleep(int);
+void usleep(int);
+void xsleep(int);
+void sleepx(int);
+void system(const char *);
+int wait(int *);
+int waitpid(int, int *, int);
+int waitid(int idtype, int id, int *infop, int options);
+
+struct rusage {};
+using pid_t = int;
+pid_t wait3(int *status, int options,
+struct rusage *rusage);
+
+pid_t wait4(pi

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 314136.
segoon marked 19 inline comments as done.
segoon added a comment.

review fixes


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.h
  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/concurrency-async-blocking.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.c
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
@@ -0,0 +1,449 @@
+// RUN: %check_clang_tidy %s concurrency-async-blocking %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.WaitableExtra", value: "my::Future;my::cv"}, {key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.FunctionsExtra", value: "my_sleep;my::sleep"}, {key: "concurrency-async-blocking.TypesExtra", value: "my::big_lock;my::other_lock"}]}'
+
+/* Poor man's declaration of std::mutex and friends */
+namespace std {
+namespace chrono {
+class seconds {
+public:
+  seconds(int);
+};
+} // namespace chrono
+
+class mutex {
+public:
+  void lock();
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+
+  template 
+  void try_lock_for(Duration);
+};
+class recursive_mutex {};
+class recursive_timed_mutex {};
+class shared_mutex {};
+class shared_timed_mutex {};
+class mutex_suffix {};
+class prefix_mutex {};
+
+template 
+class unique_lock {
+public:
+  unique_lock(Lock &);
+
+  void lock();
+  template 
+  void try_lock_for(Duration);
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+};
+
+} // namespace std
+
+namespace ns {
+class mutex {};
+} // namespace ns
+
+class mutex {};
+
+template 
+class nonlock {};
+
+namespace my {
+class mutex {
+public:
+  void lock();
+};
+class shared_mutex {};
+class non_mutex {
+public:
+  void lock();
+};
+} // namespace my
+
+void test_lockable() {
+  std::mutex m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  ::std::mutex mns;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::shared_mutex sm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'shared_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_mutex rm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_timed_mutex rtm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_timed_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  my::mutex mym;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::mutex_suffix m1;
+  std::prefix_mutex m2;
+  ns::mutex m3;
+  mutex m4;
+  my::non_mutex myn;
+
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mns.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mym.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  myn.lock();
+
+  m.lock_suffix();
+  m.prefix_lock();
+
+  std::unique_lock lock(m);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'unique_lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::unique_lock l1(m1);
+  std::unique_lock l2(m2);
+  std::unique_lock l3(m3);
+  std::unique_lock l4(m4);
+
+  nonlock nonlock;
+}
+
+void sleep(int);
+void nanosleep(int);
+void usleep(int);
+void xsleep(int);
+void sleepx(int);
+void system(const char *);
+int wait(int *);
+int waitpid(int, int *, int);
+int waitid(int idtype, int id, int *infop, int options);
+
+struct rusage {};
+using pid_t = int;
+pid_t wait3(int *status, int options,
+struct rusage *rusage);
+
+pid_t wait4(pid_t pid, int *status, int options,
+struct rusage *rusage);
+
+namespace std {
+namespace this_thread {
+void yield();
+
+template 
+void sleep_for(Duration);
+
+template 
+void sleep_until(Duration);
+} // namespace

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added inline comments.



Comment at: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp:35
+/* C++ std */
+"std::mutex", //
+"std::timed_mutex",   //

njames93 wrote:
> Is it wise to fully qualify these?
> `::std::mutex`
> 
> Also whats with the comments at the end of each line, they don't seem to add 
> anything.
the comments signal clang-format not to join multiple items into a single line


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp:450
+
+// TODO: remove CHECKT-MESSAGES

njames93 wrote:
> Whats this for?
WIP tests for not yet ready atomic::is_always_lock_free check


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 314631.
segoon edited the summary of this revision.
segoon added a comment.
Herald added a subscriber: lxfind.

- review fixes
- drop of atomic::is_always_lock_free check


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.h
  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/concurrency-async-blocking.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.c
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-blocking.cpp
@@ -0,0 +1,417 @@
+// RUN: %check_clang_tidy %s concurrency-async-blocking %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.WaitableExtra", value: "my::Future;my::cv"}, {key: "concurrency-async-blocking.LockableExtra", value: "my::mutex;my::shared_mutex"}, {key: "concurrency-async-blocking.FunctionsExtra", value: "my_sleep;my::sleep"}, {key: "concurrency-async-blocking.TypesExtra", value: "my::big_lock;my::other_lock"}]}'
+
+/* Poor man's declaration of std::mutex and friends */
+namespace std {
+namespace chrono {
+class seconds {
+public:
+  seconds(int);
+};
+} // namespace chrono
+
+class mutex {
+public:
+  void lock();
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+
+  template 
+  void try_lock_for(Duration);
+};
+class recursive_mutex {};
+class recursive_timed_mutex {};
+class shared_mutex {};
+class shared_timed_mutex {};
+class mutex_suffix {};
+class prefix_mutex {};
+
+template 
+class unique_lock {
+public:
+  unique_lock(Lock &);
+
+  void lock();
+  template 
+  void try_lock_for(Duration);
+
+  // non-std methods
+  void lock_suffix();
+  void prefix_lock();
+};
+
+} // namespace std
+
+namespace ns {
+class mutex {};
+} // namespace ns
+
+class mutex {};
+
+template 
+class nonlock {};
+
+namespace my {
+class mutex {
+public:
+  void lock();
+};
+class shared_mutex {};
+class non_mutex {
+public:
+  void lock();
+};
+} // namespace my
+
+void test_lockable() {
+  std::mutex m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  ::std::mutex mns;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::shared_mutex sm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'shared_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_mutex rm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  std::recursive_timed_mutex rtm;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'recursive_timed_mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  my::mutex mym;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'mutex' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::mutex_suffix m1;
+  std::prefix_mutex m2;
+  ns::mutex m3;
+  mutex m4;
+  my::non_mutex myn;
+
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mns.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  mym.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+  myn.lock();
+
+  m.lock_suffix();
+  m.prefix_lock();
+
+  std::unique_lock lock(m);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'unique_lock' may sleep and is not coroutine-safe [concurrency-async-blocking]
+
+  std::unique_lock l1(m1);
+  std::unique_lock l2(m2);
+  std::unique_lock l3(m3);
+  std::unique_lock l4(m4);
+
+  nonlock nonlock;
+}
+
+void sleep(int);
+void nanosleep(int);
+void usleep(int);
+void xsleep(int);
+void sleepx(int);
+void system(const char *);
+int wait(int *);
+int waitpid(int, int *, int);
+int waitid(int idtype, int id, int *infop, int options);
+
+struct rusage {};
+using pid_t = int;
+pid_t wait3(int *status, int options,
+struct rusage *rusage);
+
+pid_t wait4(pid_t pid, int *status, int options,
+struct rusage *rusage);
+
+namespace std {
+namespace this_thread {
+void yield();
+
+template 
+

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

friendly ping, any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94621: docs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon created this revision.
segoon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94621

Files:
  clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst


Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -24,6 +24,9 @@
 * Linux syscalls
 * Boost.Filesystem, Boost.Nowide
 
+Options
+---
+
 .. option:: FunctionsExtra
 
   Specifies additional functions to search for, separated with semicolon.


Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -24,6 +24,9 @@
 * Linux syscalls
 * Boost.Filesystem, Boost.Nowide
 
+Options
+---
+
 .. option:: FunctionsExtra
 
   Specifies additional functions to search for, separated with semicolon.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 316460.
segoon retitled this revision from "docs" to "[clang-tidy] add 
concurrency-async-fs".
segoon edited the summary of this revision.
segoon added a comment.
Herald added subscribers: ormris, xazax.hun, mgorny.

fix


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h
  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/concurrency-async-fs.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s concurrency-async-fs %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-fs.FunctionsExtra", value: "::my::block"},{key: "concurrency-async-fs.TypesExtra", value: "::my::file"}]}'
+
+void chdir(const char *);
+
+namespace std {
+namespace filesystem {
+bool exists(const char *);
+
+void copy(const char *, const char *);
+
+class directory_iterator {
+public:
+  directory_iterator(const char *);
+
+  bool operator!=(const directory_iterator &) const;
+
+  directory_iterator &operator++();
+
+  int operator*() const;
+};
+
+directory_iterator begin(directory_iterator iter) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+directory_iterator end(const directory_iterator &) noexcept;
+
+} // namespace filesystem
+
+template 
+class basic_fstream {};
+
+template 
+class basic_ofstream {};
+
+template 
+class basic_ifstream {};
+
+typedef basic_fstream fstream;
+typedef basic_ofstream ofstream;
+typedef basic_ifstream ifstream;
+
+} // namespace std
+
+void copy();
+
+void test_core() {
+  chdir("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'chdir' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::exists("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'exists' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::copy("/a", "/b");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'copy' may block on filesystem access [concurrency-async-fs]
+
+  copy();
+
+  for (const auto &f : std::filesystem::directory_iterator("/")) {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+  }
+}
+
+void test_fstream() {
+  std::fstream fs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ofstream of;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ofstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ifstream ifs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ifstream' may access filesystem in a blocking way [concurrency-async-fs]
+
+  std::basic_fstream bfs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+}
+
+namespace my {
+class file {};
+
+void block();
+} // namespace my
+
+void test_user() {
+  my::file f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'file' may access filesystem in a blocking way [concurrency-async-fs]
+
+  my::block();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'block' may block on filesystem access [concurrency-async-fs]
+}
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
@@ -139,6 +139,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-async-fs `_,
`concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - concurrency-async-fs
+
+concurrency-async-fs
+
+
+Search for filesystem accesses that might block current system thread.
+Asynchronous code may deal with it in numerous ways, the most widespread
+ways are the following:
+
+* use asynchronous API like aio

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

njames93, the purpose is to flag it indeed. This approach was found valueable 
in Yandex.Taxi, as it is very easy to forget that you're in a coroutine and may 
not use blocking API. The bug does affect performance (e.g. all coroutine 
threads wait for fs), it cannot be found by functional tests (as it is not a 
functional invariant violation) and may be rather tricky to debug (as the 
performance harm depends on many things like IO limits, page cache size, 
current load, etc.). It can be caught during code review, but it suffers from 
human errors. Rather than playing catch-me-if-you-can games, the check can be 
automated. As C/C++ standard libraries contain quite many blocking functions 
and C++20 gains official coroutine support, I find it valuable for the C++ 
community to have an already compiled list of such blocking functions and the 
check that uses it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 316664.
segoon edited the summary of this revision.
segoon added a comment.

- fix the first document line
- add default values


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.h
  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/concurrency-async-no-new-threads.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.c
  
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s concurrency-async-no-new-threads %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-no-new-threads.FunctionsExtra", value: "::my::create_thread"},{key: "concurrency-async-no-new-threads.TypesExtra", value: "::my::thread"}]}'
+
+namespace std {
+
+class thread {
+public:
+  void join() {}
+};
+
+class jthread {};
+
+class nothread {};
+
+namespace execution {
+
+class sequenced_policy {};
+
+class parallel_policy {};
+
+class parallel_unsequenced_policy {};
+
+class unsequenced_policy {};
+
+sequenced_policy seq;
+parallel_policy par;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+parallel_unsequenced_policy par_unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+unsequenced_policy unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+
+} // namespace execution
+
+void async(int (*)()) {
+}
+
+template 
+void sort(T1 &&, T2, T3, T4);
+
+} // namespace std
+
+int pthread_create(int *thread, const int *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int thrd_create(int *thr, int func, void *arg);
+
+int fork();
+
+int vfork();
+
+int clone(int (*fn)(void *), void *child_stack,
+  int flags, void *arg);
+
+long clone3(int *cl_args, int size);
+
+namespace boost {
+
+class thread {};
+
+class scoped_thread {};
+
+void async(int (*)()) {}
+
+namespace compute {
+class device {};
+
+} // namespace compute
+
+} // namespace boost
+
+void test_threads() {
+  std::thread t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+
+  std::jthread j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'jthread' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::parallel_policy pp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::sequenced_policy sp;
+
+  std::sort(std::execution::par, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 'par' creates new threads [concurrency-async-no-new-threads]
+
+  boost::thread bt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+  boost::scoped_thread bst;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'scoped_thread' creates new threads [concurrency-async-no-new-threads]
+
+  boost::compute::device bcd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'device' creates new threads [concurrency-async-no-new-threads]
+
+  std::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  boost::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  pthread_create(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'pthread_create' creates new threads [concurrency-async-no-new-threads]
+
+  thrd_create(0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'thrd_create' creates new threads [concurrency-async-no-new-threads]
+
+  fork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'fork' creates new threads [concurrency-async-no-new-threads]
+
+  vfork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'vfork' creates new threads [concurrency-async-no-new-threads]
+
+  clone(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone' creates new threads [concurrency-async-no-new-threads]
+
+  clone3(0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: fu

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 316667.
segoon added a comment.

- use back-ticks
- fix the first document line
- add default values


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h
  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/concurrency-async-fs.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s concurrency-async-fs %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-fs.FunctionsExtra", value: "::my::block"},{key: "concurrency-async-fs.TypesExtra", value: "::my::file"}]}'
+
+void chdir(const char *);
+
+namespace std {
+namespace filesystem {
+bool exists(const char *);
+
+void copy(const char *, const char *);
+
+class directory_iterator {
+public:
+  directory_iterator(const char *);
+
+  bool operator!=(const directory_iterator &) const;
+
+  directory_iterator &operator++();
+
+  int operator*() const;
+};
+
+directory_iterator begin(directory_iterator iter) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+directory_iterator end(const directory_iterator &) noexcept;
+
+} // namespace filesystem
+
+template 
+class basic_fstream {};
+
+template 
+class basic_ofstream {};
+
+template 
+class basic_ifstream {};
+
+typedef basic_fstream fstream;
+typedef basic_ofstream ofstream;
+typedef basic_ifstream ifstream;
+
+} // namespace std
+
+void copy();
+
+void test_core() {
+  chdir("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'chdir' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::exists("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'exists' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::copy("/a", "/b");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'copy' may block on filesystem access [concurrency-async-fs]
+
+  copy();
+
+  for (const auto &f : std::filesystem::directory_iterator("/")) {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+  }
+}
+
+void test_fstream() {
+  std::fstream fs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ofstream of;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ofstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ifstream ifs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ifstream' may access filesystem in a blocking way [concurrency-async-fs]
+
+  std::basic_fstream bfs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+}
+
+namespace my {
+class file {};
+
+void block();
+} // namespace my
+
+void test_user() {
+  my::file f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'file' may access filesystem in a blocking way [concurrency-async-fs]
+
+  my::block();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'block' may block on filesystem access [concurrency-async-fs]
+}
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
@@ -139,6 +139,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-async-fs `_,
`concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - concurrency-async-fs
+
+concurrency-async-fs
+
+
+Finds filesystem accesses that might block current system thread.
+Asynchronous code may deal with it in numerous ways, the most widespread
+ways are the following:
+
+* use asynchronous API like `aio` or `io_uring`
+* delegate all filesystem access to a thread pool
+
+Some projects may consider filesystem access from asyn

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

Eugene.Zelenko, thanks for the review! fixed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

Eugene.Zelenko, thanks for the review! fixed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

Eugene.Zelenko, njames93, any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94622/new/

https://reviews.llvm.org/D94622

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

Eugene.Zelenko, njames93, any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94621/new/

https://reviews.llvm.org/D94621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

Eugene.Zelenko, njames93, any comments on the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93940/new/

https://reviews.llvm.org/D93940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits