diff --git a/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt b/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangTidyConcurrencyModule ConcurrencyTidyModule.cpp + MtUnsafeCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp b/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp --- a/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "MtUnsafeCheck.h" namespace clang { namespace tidy { @@ -16,7 +17,10 @@ class ConcurrencyModule : public ClangTidyModule { public: - void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {} + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "concurrency-mt-unsafe"); + } }; } // namespace concurrency diff --git a/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h @@ -0,0 +1,43 @@ +//===--- 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_CONCURRENCY_MTUNSAFECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_MTUNSAFECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace concurrency { + +/// Checks that non-thread-safe functions are not used. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/threads-mt-unsafe.html +class MtUnsafeCheck : public ClangTidyCheck { +public: + MtUnsafeCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + + enum class FunctionSet { + Posix, + Glibc, + Any, + }; + +private: + const FunctionSet FuncSet; +}; + +} // namespace concurrency +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_MTUNSAFECHECK_H diff --git a/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp @@ -0,0 +1,316 @@ +//===--- MtUnsafeCheck.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 "MtUnsafeCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +// Initial list was extracted from gcc documentation +static const clang::StringRef GlibcFunctions[] = { + "::argp_error", + "::argp_help", + "::argp_parse", + "::argp_state_help", + "::argp_usage", + "::asctime", + "::clearenv", + "::crypt", + "::ctime", + "::cuserid", + "::drand48", + "::ecvt", + "::encrypt", + "::endfsent", + "::endgrent", + "::endhostent", + "::endnetent", + "::endnetgrent", + "::endprotoent", + "::endpwent", + "::endservent", + "::endutent", + "::endutxent", + "::erand48", + "::error_at_line", + "::exit", + "::fcloseall", + "::fcvt", + "::fgetgrent", + "::fgetpwent", + "::gammal", + "::getchar_unlocked", + "::getdate", + "::getfsent", + "::getfsfile", + "::getfsspec", + "::getgrent", + "::getgrent_r", + "::getgrgid", + "::getgrnam", + "::gethostbyaddr", + "::gethostbyname", + "::gethostbyname2", + "::gethostent", + "::getlogin", + "::getmntent", + "::getnetbyaddr", + "::getnetbyname", + "::getnetent", + "::getnetgrent", + "::getnetgrent_r", + "::getopt", + "::getopt_long", + "::getopt_long_only", + "::getpass", + "::getprotobyname", + "::getprotobynumber", + "::getprotoent", + "::getpwent", + "::getpwent_r", + "::getpwnam", + "::getpwuid", + "::getservbyname", + "::getservbyport", + "::getservent", + "::getutent", + "::getutent_r", + "::getutid", + "::getutid_r", + "::getutline", + "::getutline_r", + "::getutxent", + "::getutxid", + "::getutxline", + "::getwchar_unlocked", + "::glob", + "::glob64", + "::gmtime", + "::hcreate", + "::hdestroy", + "::hsearch", + "::innetgr", + "::jrand48", + "::l64a", + "::lcong48", + "::lgammafNx", + "::localeconv", + "::localtime", + "::login", + "::login_tty", + "::logout", + "::logwtmp", + "::lrand48", + "::mallinfo", + "::mallopt", + "::mblen", + "::mbrlen", + "::mbrtowc", + "::mbsnrtowcs", + "::mbsrtowcs", + "::mbtowc", + "::mcheck", + "::mprobe", + "::mrand48", + "::mtrace", + "::muntrace", + "::nrand48", + "::__ppc_get_timebase_freq", + "::ptsname", + "::putchar_unlocked", + "::putenv", + "::pututline", + "::pututxline", + "::putwchar_unlocked", + "::qecvt", + "::qfcvt", + "::register_printf_function", + "::seed48", + "::setenv", + "::setfsent", + "::setgrent", + "::sethostent", + "::sethostid", + "::setkey", + "::setlocale", + "::setlogmask", + "::setnetent", + "::setnetgrent", + "::setprotoent", + "::setpwent", + "::setservent", + "::setutent", + "::setutxent", + "::siginterrupt", + "::sigpause", + "::sigprocmask", + "::sigsuspend", + "::sleep", + "::srand48", + "::strerror", + "::strsignal", + "::strtok", + "::tcflow", + "::tcsendbreak", + "::tmpnam", + "::ttyname", + "::unsetenv", + "::updwtmp", + "::utmpname", + "::utmpxname", + "::valloc", + "::vlimit", + "::wcrtomb", + "::wcsnrtombs", + "::wcsrtombs", + "::wctomb", + "::wordexp", +}; + +static const clang::StringRef PosixFunctions[] = { + "::asctime", + "::basename", + "::catgets", + "::crypt", + "::ctime", + "::dbm_clearerr", + "::dbm_close", + "::dbm_delete", + "::dbm_error", + "::dbm_fetch", + "::dbm_firstkey", + "::dbm_nextkey", + "::dbm_open", + "::dbm_store", + "::dirname", + "::dlerror", + "::drand48", + "::encrypt", + "::endgrent", + "::endpwent", + "::endutxent", + "::ftw", + "::getc_unlocked", + "::getchar_unlocked", + "::getdate", + "::getenv", + "::getgrent", + "::getgrgid", + "::getgrnam", + "::gethostent", + "::getlogin", + "::getnetbyaddr", + "::getnetbyname", + "::getnetent", + "::getopt", + "::getprotobyname", + "::getprotobynumber", + "::getprotoent", + "::getpwent", + "::getpwnam", + "::getpwuid", + "::getservbyname", + "::getservbyport", + "::getservent", + "::getutxent", + "::getutxid", + "::getutxline", + "::gmtime", + "::hcreate", + "::hdestroy", + "::hsearch", + "::inet_ntoa", + "::l64a", + "::lgamma", + "::lgammaf", + "::lgammal", + "::localeconv", + "::localtime", + "::lrand48", + "::mrand48", + "::nftw", + "::nl_langinfo", + "::ptsname", + "::putc_unlocked", + "::putchar_unlocked", + "::putenv", + "::pututxline", + "::rand", + "::readdir", + "::setenv", + "::setgrent", + "::setkey", + "::setpwent", + "::setutxent", + "::strerror", + "::strsignal", + "::strtok", + "::system", + "::ttyname", + "::unsetenv", + "::wcstombs", + "::wctomb", +}; + +namespace clang { +namespace tidy { + +template <> struct OptionEnumMapping { + static llvm::ArrayRef< + std::pair> + getEnumMapping() { + static constexpr std::pair + Mapping[] = {{concurrency::MtUnsafeCheck::FunctionSet::Posix, "posix"}, + {concurrency::MtUnsafeCheck::FunctionSet::Glibc, "glibc"}, + {concurrency::MtUnsafeCheck::FunctionSet::Any, "any"}}; + return makeArrayRef(Mapping); + } +}; + +namespace concurrency { + +static ast_matchers::internal::Matcher +hasAnyMtUnsafeNames(MtUnsafeCheck::FunctionSet libc) { + switch (libc) { + case MtUnsafeCheck::FunctionSet::Posix: + return hasAnyName(PosixFunctions); + case MtUnsafeCheck::FunctionSet::Glibc: + return hasAnyName(GlibcFunctions); + case MtUnsafeCheck::FunctionSet::Any: + return anyOf(hasAnyName(PosixFunctions), hasAnyName(GlibcFunctions)); + } + llvm_unreachable("invalid FunctionSet"); +} + +MtUnsafeCheck::MtUnsafeCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + FuncSet(Options.get("FunctionSet", MtUnsafeCheck::FunctionSet::Any)) {} + +void MtUnsafeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "FunctionSet", FuncSet); +} + +void MtUnsafeCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(callee(functionDecl(hasAnyMtUnsafeNames(FuncSet)))) + .bind("mt-unsafe"), + this); +} + +void MtUnsafeCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs("mt-unsafe"); + assert(Call && "Unhandled binding in the Matcher"); + + diag(Call->getBeginLoc(), "function is not thread safe"); +} + +} // namespace concurrency +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -117,6 +117,12 @@ Finds condition variables in nested ``if`` statements that were also checked in the outer ``if`` statement and were not changed. +- New :doc:`concurrency-mt-unsafe ` + check. + + Finds thread-unsafe functions usage. Currently knows about POSIX and + Glibc function sets. + - New :doc:`bugprone-signal-handler ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst b/clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst new file mode 100644 --- /dev/null +++ b/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 (e.g. gmtime(3)) or utilize signals in a racy way. +The set of functions to check is specified with the `FunctionSet` option. + +Note that using some thread-unsafe functions may be still valid in +concurrent programming if only a single thread is used (e.g. setenv(3)), +however, some functions may track a state in global variables which +would be clobbered by subsequent (non-parallel, but concurrent) calls to +a related function. E.g. the following code suffers from unprotected +accesses to a global state: + +.. code-block:: c++ + + // getnetent(3) maintains global state with DB connection, etc. + // If a concurrent green thread calls getnetent(3), the global state is corrupted. + netent = getnetent(); + yield(); + netent = getnetent(); + + +Examples: + +.. code-block:: c++ + + tm = gmtime(timep); // uses a global buffer + + sleep(1); // implementation may use SIGALRM + +.. option:: FunctionSet + + 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 with a search for MT-Safe tag: + https://www.gnu.org/software/libc/manual/html_node/POSIX-Safety-Concepts.html + + If you want to identify thread-unsafe API for at least one libc or + unsure which libc will be used, use `any` (default). + diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -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" diff --git a/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp b/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-any.cpp new file mode 100644 --- /dev/null +++ b/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] +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp b/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-glibc.cpp new file mode 100644 --- /dev/null +++ b/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); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp b/clang-tools-extra/test/clang-tidy/checkers/concurrency-mt-unsafe-posix.cpp new file mode 100644 --- /dev/null +++ b/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] +}