<atomic>: In pre-C++20 mode, the constructor should be trivial

[zeux]

Starting from VS 2019 16.6 (#336), std::atomic constructor is unconditionally initializing the value:

    constexpr atomic() noexcept(is_nothrow_default_constructible_v<_Ty>) : _Base() {}

Before this version. the constructor was trivial instead:

#ifdef __clang__ // TRANSITION, VSO-406237
    constexpr atomic() noexcept(is_nothrow_default_constructible_v<_Ty>) : _Base() {}
#else // ^^^ no workaround / workaround vvv
    atomic()                                  = default;
#endif // TRANSITION, VSO-406237

As far as I understand, this is a behavior change in C++20 standard. However, I would not expect to see this behavior without specifying /std:c++latest.

This is significant because it results in some cases where std::atomic in global scope now generates dynamic initializers. Notably, in this case:

struct Foo
{
    std::atomic<int> data[2];
};

Foo foo[2];

Before this change, the foo variable was placed into BSS with no dynamic initializer. After this change, the dynamic initializer is emitted. This is problematic because if any code in static constructors accesses the atomic, it may work on the atomic before it's initialized which means that the initialization will overwrite the value of the atomic to 0.

Thus this is a breaking change - which would be fine except that it's a silent breaking change, and I'd expect to have to opt into this change by using the latest standard version instead.

[zeux]

Here's a full repro:

#include <atomic>
#include <stdio.h>

struct Meow
{
	std::atomic<int> data[2];
	char padding[64];
};

int bar();

// SWAP THESE TWO LINES TO OBSERVE THE BUG
Meow cats[256];
int foo = bar();

int bar()
{
	// edit: can add atomic_init(&cats[0].data[0], 0); here to eliminate UB - doesn't change behavior
	return cats[0].data[0]++;
}

int main()
{
	printf("%d\n", cats[0].data[0].load());
}

With latest MSVC (19.26.28720.3), this program prints 1 as written, and 0 if you swap the two indicated lines.

[BillyONeal]

There's certainly a c1xx bug here, and maybe also a clang bug:

C:\Users\billy\Desktop>type repro.cpp
#include <assert.h>

struct FakeAtomic {
    int example;

    constexpr FakeAtomic() noexcept : example{} {}
    int operator++() { return ++example; }
    int operator++(int) { return example++; }
    int load() const { return example; }
};

struct Meow {
    FakeAtomic data[2];
    char padding[64];
};

int bar();

int foo = bar();
#ifdef WORKAROUND
Meow should_be_statically_initalized[256]{};
#else
Meow should_be_statically_initalized[256];
#endif

int bar() { return should_be_statically_initalized[0].data[0]++; }

int main() {
    assert(foo == 0);
    assert(should_be_statically_initalized[0].data[0].load() == 1);
}

C:\Users\billy\Desktop>cl /EHsc /W4 /WX .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.25.28611 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.25.28611.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Users\billy\Desktop>.\repro.exe
Assertion failed: should_be_statically_initalized[0].data[0].load() == 1, file .\repro.cpp, line 30

C:\Users\billy\Desktop>clang-cl /EHsc /W4 /WX .\repro.cpp

C:\Users\billy\Desktop>.\repro.exe
Assertion failed: should_be_statically_initalized[0].data[0].load() == 1, file .\repro.cpp, line 30

C:\Users\billy\Desktop>cl /DWORKAROUND /EHsc /W4 /WX .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.25.28611 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.25.28611.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Users\billy\Desktop>.\repro.exe
Assertion failed: should_be_statically_initalized[0].data[0].load() == 1, file .\repro.cpp, line 30

C:\Users\billy\Desktop>clang-cl /DWORKAROUND /EHsc /W4 /WX .\repro.cpp

C:\Users\billy\Desktop>.\repro.exe

C:\Users\billy\Desktop>

[BillyONeal]

As I mentioned on Twitter, implementing WG21-P0883 unconditionally is very much intentional because the previous C++11-C++17 rules leaves the atomic in a broken state before someone calls atomic_init. That is, the C++17 rule said that the atomic constructor left the atomic "uninitialized", meaning there is no int there to load or ++ or whatever, before atomic_init is called. This behavior was so user hostile that we never implemented the C++11 rules; we always default-initialized the value. As far as I am aware libc++ on Clang was the only library implementation that tried to comply with the C++11 rules.

Given that the old rules were broken, the C++20 rules have a high likelihood of doing what the user expected, and the C++20 rules are actually implementable for us, we implement them unconditionally. However, we do depend on the compilers getting static initialization right, and it looks like they didn't in this case.

[BillyONeal]

C1XX bug filed as DevCom-970302 (I don't believe we have a workaround) see workaround by Alex Guteniev (@AlexGuteniev) below
Clang bug filed as LLVM-45367 (Workaround by adding a {} initializer)

[BillyONeal]

Upon further reflection, [basic.start.static]/2 says that either constant initialization is performed, or zero initialization, but your example wants both, so perhaps clang is correct to emit a dynamic initializer here without the extra {}s. Will keep you posted with what they say.

[AlexGuteniev]

From what I understood so far, it is better not to mix default and zero initialization at all.

Here's workaround that works in MSVC:

struct Meow
{
	std::atomic<int> data[2];
	char padding[64] = {};
};

[BillyONeal]

Richard Smith, who works on Clang, has reposted this to the C++ committee's Core Working Group:

Consider:

struct X { int n = 1; constexpr X() {} };
struct Meow { X x; char padding[64]; };
Meow should_be_statically_initalized[256];

Prior to C++20, this example required dynamic initialization: the constructor of Meow was non->constexpr due to the uninitialized member.

P1331R2 changed that, and gave Meow a constexpr constructor. But we still don't get static >initialization here, because CWG2026 changed the rules for constant initialization so that it >replaces zero-initialization instead of happening after zero-initialization.

so it looks like the workaround Alex Guteniev (@AlexGuteniev) posted is the correct workaround for both c1xx and clang.

[StephanTLavavej]

Is this purely a pair of compiler bugs with no STL changes required? If so, should we close this as external resolved? (In any event, thanks for the report!)

[CaseyCarter]

There are a couple of issues here:

1. All specializations of std::atomic now have non-trivial default constructors. This is very much by design and not a bug.

2. Some objects that have specializations of std::atomic as subobjects which were previously constant-initialized are now being dynamically-initialized. This is a bug in the compilers and/or a defect in the C++ Standard, but it's not clear to me which.

I'd like to see the exact nature of the bug clarified before we close this.

[Neumann-A]

related: https://codereview.qt-project.org/c/qt/qtbase/+/305484

[BillyONeal]

Alexander Neumann (@Neumann-A) Thanks for running that down

[BillyONeal]

I stand by what I said above in #661 (comment) that I think we should implement the change unconditionally, but it has now caused regressions in both LLVM and Qt, and was independently reported as DevCom-1089003 . Should we consider trying to add a special case for C++17 and earlier mode, only for lock free atomics, which default initializes the T and thus can be trivial again?

Customers calling that are still breaking the standard's rules but the standard's rules were unreasonable.

[StephanTLavavej]

The purpose of Standard modes is to give customers time to adapt to source-breaking changes. While I don't think it's a great idea to provide default-initializing trivial constructors in C++14/17 mode (if those Standards didn't require triviality), this is exactly how I feel about compiling in C++14/17 mode at all.

Therefore, I am in favor of mitigating the source-breaking change by adding a special case for C++14/17 mode. Affected customers should be aware that we will never extend this to C++20 mode.