Index: clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h @@ -0,0 +1,38 @@ +//===--- AsyncFsCheck.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_ASYNCFSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_ASYNCFSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace concurrency { + +/// Finds filesystem accesses that might block current system thread. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/concurrency-async-fs.html +class AsyncFsCheck : public ClangTidyCheck { +public: + AsyncFsCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const std::string FunctionsExtra; + const std::string TypesExtra; +}; + +} // namespace concurrency +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CONCURRENCY_ASYNCFSCHECK_H Index: clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp @@ -0,0 +1,302 @@ +//===--- AsyncFsCheck.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 "AsyncFsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +static const char Function[] = "func"; +static const char TypeName[] = "type"; +static const char Name[] = "name"; + +static const clang::StringRef FsFunctions[] = { + /* C++ std */ + "::std::filesystem::absolute", // + "::std::filesystem::canonical", // + "::std::filesystem::weakly_canonical", // + "::std::filesystem::relative", // + "::std::filesystem::proximate", // + "::std::filesystem::copy", // + "::std::filesystem::copy_file", // + "::std::filesystem::copy_symlink", // + "::std::filesystem::create_directory", // + "::std::filesystem::create_directories", // + "::std::filesystem::create_hard_link", // + "::std::filesystem::create_symlink", // + "::std::filesystem::create_directory_symlink", // + "::std::filesystem::exists", // + "::std::filesystem::equivalent", // + "::std::filesystem::file_size", // + "::std::filesystem::hard_link_count", // + "::std::filesystem::last_write_time", // + "::std::filesystem::permissions", // + "::std::filesystem::read_symlink", // + "::std::filesystem::remove", // + "::std::filesystem::remove_all", // + "::std::filesystem::rename", // + "::std::filesystem::resize_file", // + "::std::filesystem::space", // + "::std::filesystem::status", // + "::std::filesystem::symlink_status", // + + "::std::filesystem::is_block_file", // + "::std::filesystem::is_character_file", // + "::std::filesystem::is_directory", // + "::std::filesystem::is_empty", // + "::std::filesystem::is_fifo", // + "::std::filesystem::is_other", // + "::std::filesystem::is_regular_file", // + "::std::filesystem::is_socket", // + "::std::filesystem::is_symlink", // + "::std::filesystem::status_known", // + + /* Boost.Filesystem */ + "::boost::filesystem::absolute", // + "::boost::filesystem::canonical", // + "::boost::filesystem::copy", // + "::boost::filesystem::copy_directory", // + "::boost::filesystem::copy_file", // + "::boost::filesystem::copy_symlink", // + "::boost::filesystem::create_directories", // + "::boost::filesystem::create_directory", // + "::boost::filesystem::create_hard_link", // + "::boost::filesystem::create_symlink", // + "::boost::filesystem::equivalent", // + "::boost::filesystem::exists", // + "::boost::filesystem::file_size", // + "::boost::filesystem::hard_link_count", // + "::boost::filesystem::last_write_time", // + "::boost::filesystem::permissions", // + "::boost::filesystem::proximate", // + "::boost::filesystem::read_symlink", // + "::boost::filesystem::relative", // + "::boost::filesystem::remove", // + "::boost::filesystem::remove_all", // + "::boost::filesystem::rename", // + "::boost::filesystem::resize_file", // + "::boost::filesystem::space", // + "::boost::filesystem::status", // + "::boost::filesystem::status_known", // + "::boost::filesystem::symlink_status", // + "::boost::filesystem::system_complete", // + "::boost::filesystem::temp_directory_path", // + "::boost::filesystem::unique_path", // + "::boost::filesystem::weakly_canonical", // + + "::boost::filesystem::is_block_file", // + "::boost::filesystem::is_character_file", // + "::boost::filesystem::is_directory", // + "::boost::filesystem::is_empty", // + "::boost::filesystem::is_fifo", // + "::boost::filesystem::is_other", // + "::boost::filesystem::is_regular_file", // + "::boost::filesystem::is_socket", // + "::boost::filesystem::is_symlink", // + "::boost::filesystem::status_known", // + + /* Boost.Nowide */ + "::boost::nowide::fopen", // + "::boost::nowide::freopen", // + "::boost::nowide::rename", // + "::boost::nowide::remove", // + "::boost::nowide::stat", // + + /* POSIX, unistd.h */ + "::access", // + "::chdir", // + "::chown", // + "::faccessat", // + "::fchdir", // + "::fchown", // + "::fchownat", // + "::fdatasync", // + "::fsync", // + "::getgroups", // + "::gethostname", // + "::lchown", // + "::link", // + "::linkat", // + "::readlink", // + "::readlinkat", // + "::rmdir", // + "::symlink", // + "::symlinkat", // + "::sync", // + "::truncate", // + "::ttyname", // + "::ttyname_r", // + "::unlink", // + "::unlinkat", // + + /* POSIX, dirent.h */ + "::opendir", // + "::readdir_r", // + "::rewinddir", // + "::scandir", // + "::seekdir", // + "::telldir", // + + /* POSIX, fcntl.h */ + "::open", // + "::openat", // + "::creat", // + "::fcntl", // + "::posix_fallocate", // + "::posix_fadvise", // + + /* POSIX, grp.h */ + "::getgrent", // + "::getgrgid", // + "::getgrgid_r", // + "::getgrnam", // + "::getgrnam_r", // + "::setgrent", // + + /* POSIX, stdio.h */ + "::fdopen", // + "::fopen", // + "::freopen", // + "::popen", // + "::tempnam", // + "::tmpfile", // + "::tmpnam", // + "::rename", // + "::renameat", // + + /* POSIX, stdlib.h */ + "::mkdtemp", // + "::mkstemp", // + "::posix_openpt", // + "::realpath", // + + /* POSIX, sys/mman.h */ + "::msync", // + "::mlock", // + "::shm_open", // + "::shm_unlink", // + + /* POSIX, sys/stat.h */ + "::chmod", // + "::fchmod", // + "::fchmodat", // + "::fstat", // + "::fstatat", // + "::lstat", // + "::futimens", // + "::mkdir", // + "::mkdirat", // + "::mkfifo", // + "::mkfifoat", // + "::mknod", // + "::mknodat", // + "::stat", // + "::utimensat", // + + /* POSIX, sys/statvfs.h */ + "::fstatvsf", "::statvsf", + + /* POSIX, utime.h */ + "::utime", + + /* POSIX, misc */ + "::dlopen", // + "::glob", // + "::setlocale", // + + /* Linux */ + "::statfs", // + "::fstatfs", // + "::syncfs", // + "::name_to_handle_at", // + "::open_by_handle_at", // + "::mount", // + "::umount", // + "::ustat", // + "::chroot", // + "::open_tree", // + "::move_mount", // + "::fsopen", // + "::fsconfig", // + "::fsmount", // + "::fspick", // +}; + +static const clang::StringRef FsTypes[] = { + /* C++ std */ + "::std::basic_ifstream", // + "::std::basic_ofstream", // + "::std::basic_fstream", // + + "::std::filesystem::directory_iterator", // + "::std::filesystem::recursive_directory_iterator", // + + /* Boost.Filesystem */ + "::boost::filesystem::directory_iterator", // + "::boost::filesystem::recursive_directory_iterator", // +}; + +static std::vector +toVector(llvm::ArrayRef Base, llvm::StringRef Extra) { + llvm::SmallVector Tmp{Base.begin(), Base.end()}; + if (!Extra.empty()) { + Extra.split(Tmp, ";"); + } + + return {Tmp.begin(), Tmp.end()}; +} + +namespace clang { +namespace tidy { +namespace concurrency { + +AsyncFsCheck::AsyncFsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + FunctionsExtra(Options.get("FunctionsExtra", "")), + TypesExtra(Options.get("TypesExtra", "")) {} + +void AsyncFsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "FunctionsExtra", FunctionsExtra); + Options.store(Opts, "TypesExtra", TypesExtra); +} + +void AsyncFsCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr( + callee(functionDecl(hasAnyName(toVector(FsFunctions, FunctionsExtra))) + .bind(Name))) + .bind(Function), + this); + + Finder->addMatcher( + valueDecl(hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( + namedDecl(hasAnyName(toVector(FsTypes, TypesExtra))) + .bind(Name)))))) + .bind(TypeName), + this); +} + +void AsyncFsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *N = Result.Nodes.getNodeAs(Name); + assert(N); + + if (const auto *CE = Result.Nodes.getNodeAs(Function)) { + diag(CE->getBeginLoc(), "function %0 may block on filesystem access") + << N << CE->getSourceRange(); + } else if (const auto *Decl = Result.Nodes.getNodeAs(TypeName)) { + diag(Decl->getBeginLoc(), "type %0 may access filesystem in a blocking way") + << N << Decl->getSourceRange(); + } else { + assert(false); + } +} + +} // namespace concurrency +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt +++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt @@ -4,6 +4,7 @@ ) add_clang_library(clangTidyConcurrencyModule + AsyncFsCheck.cpp ConcurrencyTidyModule.cpp MtUnsafeCheck.cpp Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp +++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AsyncFsCheck.h" #include "MtUnsafeCheck.h" namespace clang { @@ -18,6 +19,8 @@ class ConcurrencyModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "concurrency-async-fs"); CheckFactories.registerCheck( "concurrency-mt-unsafe"); } Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -121,6 +121,11 @@ Finds structs that are inefficiently packed or aligned, and recommends packing and/or aligning of said structs as needed. +- New :doc:`concurrency-async-fs + ` check. + + Finds filesystem accesses that might block current system thread. + - New :doc:`cppcoreguidelines-prefer-member-initializer ` check. 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 projects may consider filesystem access from asynchronous code as +a non-issue, e.g. it is known that all filesystem code doesn't block +system threads as it resides in memory (e.g. `tmpfs`), or blocking functions +are used in the startup code only, or asynchronous API/thread pool is not +acceptable for some reason. + +.. note:: + + The check doesn't identify synchronous and asynchronous code. Insead, it + assumes that all analyzed code is asynchronous and all blocking calls have to + be found. You should split the sync and async code at the filesystem level + and enable `concurrency-async-*` checks for files with asynchronous code + only. + +The check by default tries to find all fs-related types/functions from the +following list: + +* C++ std +* POSIX +* Linux syscalls +* Boost.Filesystem, Boost.Nowide + +Options +------- + +.. option:: FunctionsExtra + + Specifies additional functions to search for, separated with semicolon. + Defaults to empty string (no functions). + +.. option:: TypesExtra + + Specifies additional types to search for, separated with semicolon. + Defaults to empty string (no types). 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/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] +}