From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by sourceware.org (Postfix) with ESMTPS id 862303858D1E for ; Tue, 16 Jan 2024 09:54:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 862303858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 862303858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::129 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705398850; cv=none; b=oTpWI1AdWyd4erAcOIBfr+Y42g0SPEiMaY6JrFEpxTCtKwmEEyvVJsmTfb2AXh2KCbHgDzZOxbBVDoRIje4PcQIMbGNWSj5yQfmqI+K1Qu1Ear4SIX3TAxmHjsuV7rnFVPWMttyzpgZ9NbgpoyzgQEGHR56+zOGr5p/cn5LMYjw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705398850; c=relaxed/simple; bh=kIitMhYhQfgY+rVrJzY+GnVczjHFgc8f32oM8k9F6Wk=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=WY4b/tGCH3H+/d/Axbn9wb7jzyJ99C+IzKvuQmt7CU8CUCqV+zXefOHvP/piJASfKbyNkEA6PPf/0lvJxbzJjPvhlyRu9BFS1aAhrW6/DSCyjIM9TMrB2w3hs1ddL6shPR55g+D+noobWVgSYT/SvtymU9FoaVBLdomeH/KpfTU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x129.google.com with SMTP id 2adb3069b0e04-50edf4f478eso7645714e87.3 for ; Tue, 16 Jan 2024 01:54:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705398846; x=1706003646; darn=gcc.gnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=zneUcsp3xVThZPRXug9PGUTCHHHkPCCNfUnVMCM8dr4=; b=FMNAIAp/wXtlnHS2FyMBPliE00Prp+z2bHJsIa/x2ZNq9oV4+xwUe0ZwVayqON5SIO 66rYd6oGEofjNuQZYoJMkizbevFhQnR6C9YzltphI5+Bia4gi99aKbWmTq6uZmg0snHh 7A5JtMY35CepppGzx2E1CRDuqlQv7S0CuCi/CxxmtnDOScbum8oEjTa5shlD+7cudZRW f9/SPEHf2IMPjFteze67SER4GdnoCMr9MPzLAFD1NB/mLUyYK11Xa+oY3l3C9WmNg6nW 1sfTd/XeOOL/x5HDL95myLGNTj50Rk5QcudFnW9GnhH/Qo5JQBzTXArp3QGKvPWiarDS Zffg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705398846; x=1706003646; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zneUcsp3xVThZPRXug9PGUTCHHHkPCCNfUnVMCM8dr4=; b=YutT7C0joD38dzCUK/nr65jrB6uoQy802DhsBRFRkH9tjUfm4dNff4hBXqdB6PpU52 3Ne7+2nPlNIV8KBgL/Q7pJ+3d6xwGhRdZj5/HlLtb9h0/hY+QldNju9MZSbkbYuaxrbG oOA3AQWKGHprCr8zLidNhgOagjzglr4NEywl1MOPzm9/YrTUwRTyM4yfsHKTFErkiVRY tUb+F45awM3jvAAG5SSBFquKEOcYtBrsKqKwIWXLfGQDtamDyIm9+jd+ac13NtU47Y30 /Ybb47vnEqIKkOyDzdtYW58OIDuX4qZnaBr68n/ePP2j1/cn3UD4KZtd35naeqjn8gCo NVvQ== X-Gm-Message-State: AOJu0YyBXP1D7LT10jTi9NN/B1LLLyTp64vEgoMZwPdyRn8jgIYQL/e+ JsXmhu7z6SU29NbFlemQyg3XhrgHpgn5gtt3vI0= X-Google-Smtp-Source: AGHT+IEpkh2X09qPZd8c55OeTYZ9NZECoXk4IJ6Ssfe4puDlVGfIDtpHBwAdNMTGiJa0xXjItt67heN6b88fRkmTX6c= X-Received: by 2002:a05:6512:1116:b0:50e:88d0:447b with SMTP id l22-20020a056512111600b0050e88d0447bmr3714733lfg.31.1705398845614; Tue, 16 Jan 2024 01:54:05 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: xndcn Date: Tue, 16 Jan 2024 17:53:54 +0800 Message-ID: Subject: Re: [PATCH] libstdc++: atomic: Add missing clear_padding in __atomic_float constructor To: "H.J. Lu" Cc: GCC Patches , Jakub Jelinek Content-Type: multipart/alternative; boundary="00000000000064aea7060f0d1c9f" X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --00000000000064aea7060f0d1c9f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 >=3D 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&) =3D delete; __atomic_float& operator=3D(const __atomic_float&) =3D 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 =3D (unsigned char*)&f; + unsigned char* ptr_mask =3D (unsigned char*)&mask; + for (int i =3D 0; i < sizeof(T); i++) + { + if (ptr_mask[i] =3D=3D 0x00) + { + ptr_f[i] =3D 0xff; + } + } +} + +void +test01() +{ + long double f =3D 0.5f; // long double may contains padding on X86 + fill_padding(f); + std::atomic as{ f }; // padding cleared on constructor + long double t =3D 1.5; + + as.fetch_add(t); + long double s =3D f + t; + t =3D as.load(); + VERIFY(s =3D=3D 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 --=20 2.25.1 H.J. Lu =E4=BA=8E2024=E5=B9=B41=E6=9C=8815=E6=97=A5= =E5=91=A8=E4=B8=80 11:46=E5=86=99=E9=81=93=EF=BC=9A > 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=3Dc++23 -mlong-double-80 >> #include >> #include >> >> #define T long double >> int main() { >> std::atomic t(0.5); >> t.fetch_add(0.5); >> float x =3D 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&) =3D delete; >> __atomic_float& operator=3D(const __atomic_float&) =3D delete; >> -- >> 2.25.1 >> > > Can you add a testcase? > > Thanks. > > H.J. > >> --00000000000064aea7060f0d1c9f--