Index: clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule CppCoreGuidelinesTidyModule.cpp + InterfacesGlobalInitCheck.cpp ProBoundsArrayToPointerDecayCheck.cpp ProBoundsConstantArrayIndexCheck.cpp ProBoundsPointerArithmeticCheck.cpp Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "../misc/AssignOperatorSignatureCheck.h" +#include "InterfacesGlobalInitCheck.h" #include "ProBoundsArrayToPointerDecayCheck.h" #include "ProBoundsConstantArrayIndexCheck.h" #include "ProBoundsPointerArithmeticCheck.h" @@ -30,6 +31,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "cppcoreguidelines-interfaces-global-init"); CheckFactories.registerCheck( "cppcoreguidelines-pro-bounds-array-to-pointer-decay"); CheckFactories.registerCheck( Index: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h @@ -0,0 +1,35 @@ +//===--- InterfacesGlobalInitCheck.h - clang-tidy----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INTERFACES_GLOBAL_INIT_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INTERFACES_GLOBAL_INIT_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Flags possible initialization order issues of static variables. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.html +class InterfacesGlobalInitCheck : public ClangTidyCheck { +public: + InterfacesGlobalInitCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INTERFACES_GLOBAL_INIT_H Index: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp @@ -0,0 +1,60 @@ +//===--- InterfacesGlobalInitCheck.cpp - clang-tidy------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "InterfacesGlobalInitCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +void InterfacesGlobalInitCheck::registerMatchers(MatchFinder *Finder) { + const auto IsGlobal = + allOf(hasGlobalStorage(), + hasDeclContext(anyOf(translationUnitDecl(), // Global scope. + namespaceDecl(), // Namespace scope. + recordDecl())), // Class scope. + unless(isConstexpr())); + const auto IsNotDefined = unless(isDefinition()); + + const auto ReferencesUndefinedGlobalVar = declRefExpr( + hasDeclaration(varDecl(IsGlobal, IsNotDefined).bind("referencee"))); + + Finder->addMatcher( + varDecl(IsGlobal, isDefinition(), + hasInitializer(expr(hasDescendant(ReferencesUndefinedGlobalVar)))) + .bind("var"), + this); +} + +void InterfacesGlobalInitCheck::check(const MatchFinder::MatchResult &Result) { + const auto *const Var = Result.Nodes.getNodeAs("var"); + // For now assume that people who write macros know what they're doing. + if (Var->getLocation().isMacroID()) + return; + const auto *const Referencee = Result.Nodes.getNodeAs("referencee"); + // If the variable has been defined, we're good. + const auto *const ReferenceeDef = Referencee->getDefinition(); + if (ReferenceeDef != nullptr && + Result.SourceManager->isBeforeInTranslationUnit( + ReferenceeDef->getLocation(), Var->getLocation())) { + return; + } + diag(Var->getLocation(), + "initializing non-local variable with non-const expression depending on " + "uninitialized non-local variable %0") + << Referencee; +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -153,6 +153,13 @@ Finds unnecessary string initializations. +- New `cppcoreguidelines-interfaces-global-init + `_ check + + Flags initializers of globals that access extern objects, and therefore can + lead to order-of-initialization problems. + + Fixed bugs: Crash when running on compile database with relative source files paths. Index: docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst @@ -0,0 +1,14 @@ +.. title:: clang-tidy - cppcoreguidelines-interfaces-global-init + +cppcoreguidelines-interfaces-global-init +======================================== + +This check flags initializers of globals that access extern objects, +and therefore can lead to order-of-initialization problems. + +This rule is part of the "Interfaces" profile of the C++ Core Guidelines, see +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global-init + +Note that currently this does not flag calls to non-constexpr functions, and +therefore globals could still be accessed from functions themselves. + Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -16,6 +16,7 @@ cert-fio38-c (redirects to misc-non-copyable-objects) cert-flp30-c cert-oop11-cpp (redirects to misc-move-constructor-init) + cppcoreguidelines-interfaces-global-init cppcoreguidelines-pro-bounds-array-to-pointer-decay cppcoreguidelines-pro-bounds-constant-array-index cppcoreguidelines-pro-bounds-pointer-arithmetic Index: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp @@ -0,0 +1,77 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-interfaces-global-init %t + +constexpr int makesInt() { return 3; } +constexpr int takesInt(int i) { return i + 1; } +constexpr int takesIntPtr(int *i) { return *i; } + +extern int ExternGlobal; +static int GlobalScopeBadInit1 = ExternGlobal; +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' +static int GlobalScopeBadInit2 = takesInt(ExternGlobal); +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' +static int GlobalScopeBadInit3 = takesIntPtr(&ExternGlobal); +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' +static int GlobalScopeBadInit4 = 3 * (ExternGlobal + 2); +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' + +namespace ns { +static int NamespaceScope = makesInt(); +static int NamespaceScopeBadInit = takesInt(ExternGlobal); +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' + +struct A { + static int ClassScope; + static int ClassScopeBadInit; +}; + +int A::ClassScopeBadInit = takesInt(ExternGlobal); +// CHECK-MESSAGES: [[@LINE-1]]:8: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal' + +static int FromClassBadInit = takesInt(A::ClassScope); +// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ClassScope' +} // namespace ns + +// "const int B::I;" is fine, it just ODR-defines B::I. See [9.4.3] Static +// members [class.static]. However the ODR-definitions are not in the right +// order (C::I after C::J, see [3.6.2.3]). +class B1 { + static const int I = 0; + static const int J = I; +}; +const int B1::J; +// CHECK-MESSAGES: [[@LINE-1]]:15: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'I' +const int B1::I; + +void f() { + // This is fine, it's executed after dynamic initialization occurs. + static int G = takesInt(ExternGlobal); +} + +// Defined global variables are fine: +static int GlobalScope = makesInt(); +static int GlobalScopeBadInit = takesInt(GlobalScope); +static int GlobalScope2 = takesInt(ns::NamespaceScope); +// Enums are fine. +enum Enum { kEnumValue = 1 }; +static int GlobalScopeFromEnum = takesInt(kEnumValue); + +// Leave constexprs alone. +extern constexpr int GlobalScopeConstexpr = makesInt(); +static constexpr int GlobalScopeConstexprOk = + takesInt(GlobalScopeConstexpr); + +// This is a pretty common instance: People are usually not using constexpr, but +// this is what they should write: +static constexpr const char kValue[] = "value"; +constexpr int Fingerprint(const char *value) { return 0; } +static int kFingerprint = Fingerprint(kValue); + +// This is fine because the ODR-definitions are in the right order (C::J after +// C::I). +class B2 { + static const int I = 0; + static const int J = I; +}; +const int B2::I; +const int B2::J; +