diff options
Diffstat (limited to 'clang-tools-extra')
8 files changed, 193 insertions, 1 deletions
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 17c12533b7a..895bd03f32b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -16,6 +16,7 @@ #include "BranchCloneCheck.h" #include "CopyConstructorInitCheck.h" #include "DanglingHandleCheck.h" +#include "DynamicStaticInitializersCheck.h" #include "ExceptionEscapeCheck.h" #include "FoldInitTypeCheck.h" #include "ForwardDeclarationNamespaceCheck.h" @@ -73,6 +74,8 @@ public: "bugprone-copy-constructor-init"); CheckFactories.registerCheck<DanglingHandleCheck>( "bugprone-dangling-handle"); + CheckFactories.registerCheck<DynamicStaticInitializersCheck>( + "bugprone-dynamic-static-initializers"); CheckFactories.registerCheck<ExceptionEscapeCheck>( "bugprone-exception-escape"); CheckFactories.registerCheck<FoldInitTypeCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 4f5df11793d..705ee97514e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_library(clangTidyBugproneModule BugproneTidyModule.cpp CopyConstructorInitCheck.cpp DanglingHandleCheck.cpp + DynamicStaticInitializersCheck.cpp ExceptionEscapeCheck.cpp FoldInitTypeCheck.cpp ForwardDeclarationNamespaceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp new file mode 100644 index 00000000000..2c6f3b13fa7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp @@ -0,0 +1,68 @@ +//===--- DynamicStaticInitializersCheck.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 "DynamicStaticInitializersCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +AST_MATCHER(clang::VarDecl, hasConstantDeclaration) { + const Expr *Init = Node.getInit(); + if (Init && !Init->isValueDependent()) { + if (Node.isConstexpr()) + return true; + return Node.checkInitIsICE(); + } + return false; +} + +DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions(Options.getLocalOrGlobal( + "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) { + if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, ',')) { + llvm::errs() << "Invalid header file extension: " + << RawStringHeaderFileExtensions << "\n"; + } +} + +void DynamicStaticInitializersCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus || getLangOpts().ThreadsafeStatics) + return; + Finder->addMatcher( + varDecl(hasGlobalStorage(), unless(hasConstantDeclaration())).bind("var"), + this); +} + +void DynamicStaticInitializersCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var"); + SourceLocation Loc = Var->getLocation(); + if (!Loc.isValid() || !utils::isPresumedLocInHeaderFile(Loc, *Result.SourceManager, + HeaderFileExtensions)) + return; + // If the initializer is a constant expression, then the compiler + // doesn't have to dynamically initialize it. + diag(Loc, "static variable %0 may be dynamically initialized in this header file") + << Var; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h new file mode 100644 index 00000000000..06db940bd4e --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h @@ -0,0 +1,43 @@ +//===--- DynamicStaticInitializersCheck.h - clang-tidy ----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DYNAMIC_STATIC_INITIALIZERS_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DYNAMIC_STATIC_INITIALIZERS_CHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/HeaderFileExtensionsUtils.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds dynamically initialized static variables in header files. +/// +/// The check supports these options: +/// - `HeaderFileExtensions`: a comma-separated list of filename extensions of +/// header files (The filename extensions should not contain "." prefix). +/// "h,hh,hpp,hxx" by default. +/// For extension-less header files, using an empty string or leaving an +/// empty string between "," if there are other filename extensions. +class DynamicStaticInitializersCheck : public ClangTidyCheck { +public: + DynamicStaticInitializersCheck(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; + +private: + const std::string RawStringHeaderFileExtensions; + utils::HeaderFileExtensionsSet HeaderFileExtensions; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DYNAMIC_STATIC_INITIALIZERS_CHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4095f3d1bef..cdb14ff6f43 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,6 +67,12 @@ The improvements are... Improvements to clang-tidy -------------------------- +- New :doc:`bugprone-dynamic-static-initializers + <clang-tidy/checks/bugprone-dynamic-static-initializers>` check. + + Finds instances where variables with static storage are initialized + dynamically in header files. + - New :doc:`linuxkernel-must-use-errs <clang-tidy/checks/linuxkernel-must-use-errs>` check. @@ -79,7 +85,6 @@ Improvements to clang-tidy Finds uses of deprecated Googletest APIs with names containing ``case`` and replaces them with equivalent APIs with ``suite``. - Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst new file mode 100644 index 00000000000..3da36ce4665 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst @@ -0,0 +1,27 @@ +.. title:: clang-tidy - bugprone-dynamic-static-initializers + +bugprone-dynamic-static-initializers +==================================== + +Finds instances of static variables that are dynamically initialized +in header files. + +This can pose problems in certain multithreaded contexts. For example, +when disabling compiler generated synchronization instructions for +static variables initialized at runtime (e.g. by ``-fno-threadsafe-statics``), even if a particular project +takes the necessary precautions to prevent race conditions during +initialization by providing their own synchronization, header files included from other projects may +not. Therefore, such a check is helpful for ensuring that disabling +compiler generated synchronization for static variable initialization will not cause +problems. + +Consider the following code: + +-- code-block:: c + +int foo() { + static int k = bar(); + return k; +} + +When synchronization of static initialization is disabled, if two threads both call `foo` for the first time, there is the possibility that `k` will be double initialized, creating a race condition. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 26fc254f1fa..cd371a5ff17 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -44,6 +44,7 @@ Clang-Tidy Checks bugprone-branch-clone bugprone-copy-constructor-init bugprone-dangling-handle + bugprone-dynamic-static-initializers bugprone-exception-escape bugprone-fold-init-type bugprone-forward-declaration-namespace diff --git a/clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp b/clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp new file mode 100644 index 00000000000..647b2e57165 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp @@ -0,0 +1,44 @@ +// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t -- -- -fno-threadsafe-statics + +int fact(int n) { + return (n == 0) ? 1 : n * fact(n - 1); +} + +int static_thing = fact(5); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'static_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers] + +int sample() { + int x; + return x; +} + +int dynamic_thing = sample(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'dynamic_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers] + +int not_so_bad = 12 + 4942; // no warning + +extern int bar(); + +int foo() { + static int k = bar(); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'k' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers] + return k; +} + +int bar2() { + return 7; +} + +int foo2() { + // This may work fine when optimization is enabled because bar() can + // be turned into a constant 7. But without optimization, it can + // cause problems. Therefore, we must err on the side of conservatism. + static int x = bar2(); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'x' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers] + return x; +} + +int foo3() { + static int p = 7 + 83; // no warning + return p; +} |
