[llvm-bugs] [Bug 49027] ManagedStatic init-order fiasco bug since Visual Studio 14.27 because of std::atomic value-init constructor

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=49027

Reid Kleckner <rnk at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|NEW                         |RESOLVED

--- Comment #3 from Reid Kleckner <rnk at google.com> ---
(In reply to Antonio Maiorano from comment #2)
> Is it a coincidence that for MSC_VER < 1925, the non-constexpr ctor path
> works because std::atomic is trivially_constructible?

It's intentional, not a coincidence. :) We had to back and forth on this
condition a few times to make it work for the various release of MSVC that we
supported at the time. We're trying to use the old codepath if std::atomic is
trivially constructible, and the new codepath if it has a constexpr default
constructor.

> Perhaps more importantly, this constexpr ctor path is only used  by Clang
> and MSVC >= 1925, but not by GCC. This would imply that once libc(std)++
> adopts the C++20 std::atomic ctor change, GCC builds will result in the same
> problem I had with MSVC (dynamic initializer for std::atomic). To avoid
> this, I think it would be worth adding a static_assert to the non-constexpr
> ctor path like:

GCC should be using a constexpr constructor here, it should be using the new
code path. The condition is if !defined(_MSC_VER) || defined(__clang__), so as
long as GCC does not define _MSC_VER, it should use the constexpr constructor.

Basically, constexpr construction is the future, and we should be able to scrap
these ifdefs in a few years and forget that this unfortunate mess ever
happened. =D

There is an attribute that can help us here:
https://clang.llvm.org/docs/AttributeReference.html#require-constant-initialization-constinit-c-20

LLVM already uses it on one ManagedStatic instance here:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/CommandLine.cpp#L489

Right now the attribute doesn't help us find bugs because it only works for
clang. However, we could try to use the C++20 spelling if it is available here:
https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/Compiler.h#L283

I guess this was prototyped with a descriptive name,
[[clang::require_constant_initialization]], and standardized as a contextual
keyword constinit. That's new to me.

---

Anyway, I think we can close as a duplicate. Re-reading the old issue, I see it
got reopened and fixed again. HTH

*** This bug has been marked as a duplicate of bug 41367 ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210204/4fd06e3b/attachment.html>