Thanks, so I add a test: atomic_float/compare_exchange_padding.cc, which will fail due to timeout without the patch. --- libstdc++-v3/ChangeLog: * include/bits/atomic_base.h: add __builtin_clear_padding in __atomic_float constructor. * testsuite/lib/dg-options.exp: enable libatomic for IA32 and X86-64. * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New test. --- libstdc++-v3/include/bits/atomic_base.h | 7 ++- .../atomic_float/compare_exchange_padding.cc | 50 +++++++++++++++++++ libstdc++-v3/testsuite/lib/dg-options.exp | 1 + 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h index d3a2c4f3805..cbe3749e17f 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -1283,7 +1283,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr __atomic_float(_Fp __t) : _M_fp(__t) - { } + { +#if __cplusplus >= 201402L && __has_builtin(__builtin_clear_padding) + if _GLIBCXX17_CONSTEXPR (__atomic_impl::__maybe_has_padding<_Fp>()) + __builtin_clear_padding(std::__addressof(_M_fp)); +#endif + } __atomic_float(const __atomic_float&) = delete; __atomic_float& operator=(const __atomic_float&) = delete; diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc new file mode 100644 index 00000000000..9376ab22850 --- /dev/null +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc @@ -0,0 +1,50 @@ +// { dg-do run { target c++20 } } +// { dg-options "-O0" } +// { dg-timeout 10 } +// { dg-additional-options "-mlong-double-80" { target x86_64-*-* } } +// { dg-do run { target { ia32 || x86_64-*-* } } } +// { dg-add-options libatomic } + +#include +#include + +template +void __attribute__((noinline,noipa)) +fill_padding(T& f) +{ + T mask; + __builtin_memset(&mask, 0xff, sizeof(T)); + __builtin_clear_padding(&mask); + unsigned char* ptr_f = (unsigned char*)&f; + unsigned char* ptr_mask = (unsigned char*)&mask; + for (int i = 0; i < sizeof(T); i++) + { + if (ptr_mask[i] == 0x00) + { + ptr_f[i] = 0xff; + } + } +} + +void +test01() +{ + long double f = 0.5f; // long double may contains padding on X86 + fill_padding(f); + std::atomic as{ f }; // padding cleared on constructor + long double t = 1.5; + + as.fetch_add(t); + long double s = f + t; + t = as.load(); + VERIFY(s == t); // padding ignored on float comparing + fill_padding(s); + VERIFY(as.compare_exchange_weak(s, f)); // padding cleared on cmpexchg + fill_padding(f); + VERIFY(as.compare_exchange_strong(f, t)); // padding cleared on cmpexchg +} + +int main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp index bc387d17ed7..d9a19dadd7f 100644 --- a/libstdc++-v3/testsuite/lib/dg-options.exp +++ b/libstdc++-v3/testsuite/lib/dg-options.exp @@ -337,6 +337,7 @@ proc add_options_for_libatomic { flags } { || ([istarget powerpc*-*-*] && [check_effective_target_ilp32]) || [istarget riscv*-*-*] || ([istarget sparc*-*-linux-gnu] && [check_effective_target_ilp32]) + || ([istarget i?86-*-*] || [istarget x86_64-*-*]) } { global TOOL_OPTIONS -- 2.25.1 H.J. Lu 于2024年1月15日周一 11:46写道: > On Sun, Jan 7, 2024, 5:02 PM xndcn wrote: > >> Hi, I found __atomic_float constructor does not clear padding, >> while __compare_exchange assumes it as zeroed padding. So it is easy to >> reproducing a infinite loop in X86-64 with long double type like: >> --- >> -O0 -std=c++23 -mlong-double-80 >> #include >> #include >> >> #define T long double >> int main() { >> std::atomic t(0.5); >> t.fetch_add(0.5); >> float x = t; >> printf("%f\n", x); >> } >> --- >> >> So we should add __builtin_clear_padding in __atomic_float constructor, >> just like the generic atomic struct. >> >> regtested on x86_64-linux. Is it OK for trunk? >> >> --- >> libstdc++: atomic: Add missing clear_padding in __atomic_float >> constructor. >> >> libstdc++-v3/ChangeLog: >> >> * include/bits/atomic_base.h: add __builtin_clear_padding in >> __atomic_float constructor. >> --- >> libstdc++-v3/include/bits/atomic_base.h | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/libstdc++-v3/include/bits/atomic_base.h >> b/libstdc++-v3/include/bits/atomic_base.h >> index f4ce0fa53..d59c2209e 100644 >> --- a/libstdc++-v3/include/bits/atomic_base.h >> +++ b/libstdc++-v3/include/bits/atomic_base.h >> @@ -1283,7 +1283,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> >> constexpr >> __atomic_float(_Fp __t) : _M_fp(__t) >> - { } >> + { >> +#if __has_builtin(__builtin_clear_padding) >> + if _GLIBCXX17_CONSTEXPR (__atomic_impl::__maybe_has_padding<_Fp>()) >> + __builtin_clear_padding(std::__addressof(_M_fp)); >> +#endif >> + } >> >> __atomic_float(const __atomic_float&) = delete; >> __atomic_float& operator=(const __atomic_float&) = delete; >> -- >> 2.25.1 >> > > Can you add a testcase? > > Thanks. > > H.J. > >>