From 3b06e633b7c97ed8fb68877ae2e96c21dd820f84 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Wed, 24 Apr 2019 20:13:23 +0000 Subject: [PATCH] Try once more to ensure constant initializaton of ManagedStatics First, use the old style of linker initialization for MSVC 2019 in addition to 2017. MSVC 2019 emits a dynamic initializer for ManagedStatic when compiled in debug mode, and according to zturner, also sometimes in release mode. I wasn't able to reproduce that, but it seems best to stick with the old code that works. When clang is using the MSVC STL, we have to give ManagedStatic a constexpr constructor that fully zero initializes all fields, otherwise it emits a dynamic initializer. The MSVC STL implementation of std::atomic has a non-trivial (but constexpr) default constructor that zero initializes the atomic value. Because one of the fields has a non-trivial constructor, ManagedStatic ends up with a non-trivial ctor. The ctor is not constexpr, so clang ends up emitting a dynamic initializer, even though it simply does zero initialization. To make it constexpr, we must initialize all fields of the ManagedStatic. However, while the constructor that takes a pointer is marked constexpr, clang says it does not evaluate to a constant because it contains a cast from a pointer to an integer. I filed this as: https://developercommunity.visualstudio.com/content/problem/545566/stdatomic-value-constructor-is-not-actually-conste.html Once we do that, we can add back the LLVM_REQUIRE_CONSTANT_INITIALIZATION marker, and so far as I'm aware it compiles successfully on all supported targets. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@359135 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/ManagedStatic.h | 24 ++++++++++++++---------- lib/Support/CommandLine.cpp | 1 + 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/include/llvm/Support/ManagedStatic.h b/include/llvm/Support/ManagedStatic.h index 30c3daa5ba5..e65bb051f18 100644 --- a/include/llvm/Support/ManagedStatic.h +++ b/include/llvm/Support/ManagedStatic.h @@ -32,20 +32,24 @@ template struct object_deleter { static void call(void *Ptr) { delete[](T *)Ptr; } }; -// If the current compiler is MSVC 2017 or earlier, then we have to work around -// a bug where MSVC emits code to perform dynamic initialization even if the -// class has a constexpr constructor. Instead, fall back to the C++98 strategy -// where there are no constructors or member initializers. We can remove this -// when MSVC 2019 (19.20+) is our minimum supported version. -#if !defined(__clang__) && defined(_MSC_VER) && _MSC_VER < 1920 -#define LLVM_AVOID_CONSTEXPR_CTOR +// ManagedStatic must be initialized to zero, and it must *not* have a dynamic +// initializer because managed statics are often created while running other +// dynamic initializers. In standard C++11, the best way to accomplish this is +// with a constexpr default constructor. However, different versions of the +// Visual C++ compiler have had bugs where, even though the constructor may be +// constexpr, a dynamic initializer may be emitted depending on optimization +// settings. For the affected versions of MSVC, use the old linker +// initialization pattern of not providing a constructor and leaving the fields +// uninitialized. +#if !defined(_MSC_VER) || defined(__clang__) +#define LLVM_USE_CONSTEXPR_CTOR #endif /// ManagedStaticBase - Common base class for ManagedStatic instances. class ManagedStaticBase { protected: -#ifndef LLVM_AVOID_CONSTEXPR_CTOR - mutable std::atomic Ptr{nullptr}; +#ifdef LLVM_USE_CONSTEXPR_CTOR + mutable std::atomic Ptr{}; mutable void (*DeleterFn)(void *) = nullptr; mutable const ManagedStaticBase *Next = nullptr; #else @@ -59,7 +63,7 @@ protected: void RegisterManagedStatic(void *(*creator)(), void (*deleter)(void*)) const; public: -#ifndef LLVM_AVOID_CONSTEXPR_CTOR +#ifdef LLVM_USE_CONSTEXPR_CTOR constexpr ManagedStaticBase() = default; #endif diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp index 2f3dc4d5092..7da3862302c 100644 --- a/lib/Support/CommandLine.cpp +++ b/lib/Support/CommandLine.cpp @@ -412,6 +412,7 @@ void OptionCategory::registerCategory() { // that this ManagedStatic uses constant initailization and not dynamic // initialization because it is referenced from cl::opt constructors, which run // dynamically in an arbitrary order. +LLVM_REQUIRE_CONSTANT_INITIALIZATION ManagedStatic llvm::cl::TopLevelSubCommand; // A special subcommand that can be used to put an option into all subcommands. -- 2.50.1