On Fri, 11 Apr 2025 at 15:14, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Fri, Apr 11, 2025 at 2:25 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> On Fri, 11 Apr 2025 at 08:23, Tomasz Kaminski <tkami...@redhat.com> wrote:
>> >
>> >
>> >
>> > On Thu, Apr 10, 2025 at 6:27 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>> >>
>> >> The std::atomic constructor clears padding bits so that compare_exchange
>> >> will not fail due to differences in padding bits. But we can only do
>> >> that for C++14 and later, because in C++11 a constexpr constructor must
>> >> have an empty body. However, the code in compare_exchange_strong assumes
>> >> that padding is always cleared, and so it fails in C++11 due to non-zero
>> >> padding.
>> >>
>> >> Since we can't clear the padding in C++11 mode, we shouldn't assume it's
>> >> been cleared when in C++11 mode. This fixes the reported bug. However,
>> >> the fix fails to handle the case where the std::atomic is constructed in
>> >> C++11 code (and so doesn't zero padding) but the CAS happens in C++14
>> >> code (and so assumes padding has been zeroed). We might need to use the
>> >> same loop as atomic_ref::compare_exchange_strong to properly fix this
>> >> bug for that case.
>> >>
>> >> Although the mixed C++11 / C++14 case isn't fixed, this is still an
>> >> incremental improvement. It fixes the pure-C++11 case and doesn't
>> >> preclude a more comprehensive fix later.
>> >
>> > Wouldn't alternative comprehensive fix be equivalent to doing just:
>> > diff --git a/libstdc++-v3/include/std/atomic
>> > b/libstdc++-v3/include/std/atomic
>> > index 9b1aca0fc09..238cf739161 100644
>> > --- a/libstdc++-v3/include/std/atomic
>> > +++ b/libstdc++-v3/include/std/atomic
>> > @@ -345,7 +345,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> > compare_exchange_weak(_Tp& __e, _Tp __i, memory_order __s,
>> > memory_order __f) noexcept
>> > {
>> > - return __atomic_impl::__compare_exchange(_M_i, __e, __i, true,
>> > + return __atomic_impl::__compare_exchange<(__cplusplus <
>> > 201402L)>(_M_i, __e, __i, true,
>> > __s, __f);
>>
>> If the std::atomic constructor happens in a translation unit compiled
>> as C++11 but the call to compare_exchange_weak happens in a
>> translation unit compiled as C++14, then padding won't be cleared, but
>> this will call __compare_exchange<false> which expects padding to have
>> been cleared.
>
> I see. This seems to be the same drawback as in case of the current fix,
> however
Yes.
> my suggestion guarantees that padding bits are ignored/cleared in program
> compiled only in C++11 mode. As I understand the suggested change, make a
> comparison exchange
> prone to fail due padding bits.
Yes, that's true. We would reliably ignore padding bits for C++11 too.
OK, I'll experiment with that next week.
