Use 64-bit atomic on sem_t with 8-byte alignment [BZ #33632]

commit 7fec8a5de6826ef9ae440238d698f0fe5a5fb372
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Thu Nov 13 14:26:08 2025 -0300

    Revert __HAVE_64B_ATOMICS configure check

uses 64-bit atomic operations on sem_t if 64-bit atomics are supported.
But sem_t may be aligned to 32-bit on 32-bit architectures.

1. Add a macro, SEM_T_ALIGN, for sem_t alignment.
2. Add a macro, HAVE_UNALIGNED_64B_ATOMICS.  Define it if unaligned 64-bit
atomic operations are supported.
3. Add a macro, USE_64B_ATOMICS_ON_SEM_T.  Define to 1 if 64-bit atomic
operations are supported and SEM_T_ALIGN is at least 8-byte aligned or
HAVE_UNALIGNED_64B_ATOMICS is defined.
4. Assert that size and alignment of sem_t are not lower than those of
the internal struct new_sem.
5. Check USE_64B_ATOMICS_ON_SEM_T, instead of USE_64B_ATOMICS, when using
64-bit atomic operations on sem_t.

This fixes BZ #33632.

Reviewed-by: Wilco Dijkstra  <Wilco.Dijkstra@arm.com>

diff --git a/config.h.in b/config.h.in
index a7cc17df8ede5c803ed73dfff5c1b52828a48b39..6fa3b4e2cd111fef70b9b7e727f6ecc8b8f72ed8 100644 (file)
--- a/config.h.in
+++ b/config.h.in
@@ -296,6 +296,9 @@
 /* Define if SFrame v2 is enabled.  */
 #define ENABLE_SFRAME 0
 
+/* Define if unaligned 64-bit atomic operations are supported.  */
+#undef HAVE_UNALIGNED_64B_ATOMICS
+
 /* The default value of x86 CET control.  */
 #define DEFAULT_DL_X86_CET_CONTROL cet_elf_property
 

diff --git a/csu/Makefile b/csu/Makefile
index 6d1c6b28cb92138b351c2ab55464e33e2b343548..028f171dea6d151b444af7311c9e00529f13f88f 100644 (file)
--- a/csu/Makefile
+++ b/csu/Makefile
@@ -137,7 +137,7 @@ before-compile += $(objpfx)abi-tag.h
 generated += abi-tag.h
 
 # Put it here to generate it earlier.
-gen-as-const-headers += rtld-sizes.sym
+gen-as-const-headers += rtld-sizes.sym sem_t-align.sym
 
 # These are the special initializer/finalizer files.  They are always the
 # first and last file in the link.  crti.o ... crtn.o define the global

diff --git a/csu/sem_t-align.sym b/csu/sem_t-align.sym
new file mode 100644 (file)
index 0000000..39c2405
--- /dev/null
+++ b/csu/sem_t-align.sym
@@ -0,0 +1,4 @@
+#include <semaphore.h>
+
+--
+SEM_T_ALIGN    __alignof (sem_t)

diff --git a/htl/pt-internal.h b/htl/pt-internal.h
index 63dcc0a6ba93442587b4694e3327c1a1ee57a79e..5002793b085fc9047f7a77f1a0cb7e156fca7b53 100644 (file)
--- a/htl/pt-internal.h
+++ b/htl/pt-internal.h
@@ -38,6 +38,8 @@
 #endif
 
 #include <tls.h>
+#include <semaphore.h>
+#include <atomic-sem_t.h>
 
 /* Thread state.  */
 enum pthread_state
@@ -343,7 +345,7 @@ libc_hidden_proto (__pthread_default_condattr)
    See nptl implementation for the details.  */
 struct new_sem
 {
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   /* The data field holds both value (in the least-significant 32 bits) and
      nwaiters.  */
 # if __BYTE_ORDER == __LITTLE_ENDIAN
@@ -370,6 +372,12 @@ struct new_sem
 #endif
 };
 
+_Static_assert (sizeof (sem_t) >= sizeof (struct new_sem),
+               "sizeof (sem_t) >= sizeof (struct new_sem)");
+
+_Static_assert (__alignof (sem_t) >= __alignof (struct new_sem),
+               "__alignof (sem_t) >= __alignof (struct new_sem)");
+
 extern int __sem_waitfast (struct new_sem *isem, int definitive_result);
 
 #endif /* pt-internal.h */

diff --git a/nptl/sem_getvalue.c b/nptl/sem_getvalue.c
index 80b00be545131e4bab89a11ff8959bcf5b824b37..c5291b464fa57192a3409136b5243a1969da6c91 100644 (file)
--- a/nptl/sem_getvalue.c
+++ b/nptl/sem_getvalue.c
@@ -33,7 +33,7 @@ __new_sem_getvalue (sem_t *sem, int *sval)
      necessary, use a stronger MO here and elsewhere (e.g., potentially
      release MO in all places where we consume a token).  */
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   *sval = atomic_load_relaxed (&isem->data) & SEM_VALUE_MASK;
 #else
   *sval = atomic_load_relaxed (&isem->value) >> SEM_VALUE_SHIFT;

diff --git a/nptl/sem_init.c b/nptl/sem_init.c
index 9cd81803af134fa38cbf8b84b2888aa8d97f8609..2b4b7afec43860e4202ccc77ba07a940fbff4262 100644 (file)
--- a/nptl/sem_init.c
+++ b/nptl/sem_init.c
@@ -38,7 +38,7 @@ __new_sem_init (sem_t *sem, int pshared, unsigned int value)
   struct new_sem *isem = (struct new_sem *) sem;
 
   /* Use the values the caller provided.  */
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   isem->data = value;
 #else
   isem->value = value << SEM_VALUE_SHIFT;

diff --git a/nptl/sem_post.c b/nptl/sem_post.c
index e2845dd1aa07eeeb20ef57a1c826ae380bdac176..2ccbc7dd3f517138c6d0c3ca693f509169cd92be 100644 (file)
--- a/nptl/sem_post.c
+++ b/nptl/sem_post.c
@@ -34,7 +34,7 @@ __new_sem_post (sem_t *sem)
   struct new_sem *isem = (struct new_sem *) sem;
   int private = isem->private;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   /* Add a token to the semaphore.  We use release MO to make sure that a
      thread acquiring this token synchronizes with us and other threads that
      added tokens before (the release sequence includes atomic RMW operations

diff --git a/nptl/sem_waitcommon.c b/nptl/sem_waitcommon.c
index a58df013280e541ae534c6f9e7a700e8f3038cce..409c0ecdfb2a286a9914dddb9ded985d5c35f8d5 100644 (file)
--- a/nptl/sem_waitcommon.c
+++ b/nptl/sem_waitcommon.c
@@ -77,7 +77,7 @@
    requirement because the semaphore must not be destructed while any sem_wait
    is still executing.  */
 
-#if !USE_64B_ATOMICS
+#if !USE_64B_ATOMICS_ON_SEM_T
 static void
 __sem_wait_32_finish (struct new_sem *sem);
 #endif
@@ -87,7 +87,7 @@ __sem_wait_cleanup (void *arg)
 {
   struct new_sem *sem = (struct new_sem *) arg;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   /* Stop being registered as a waiter.  See below for MO.  */
   atomic_fetch_add_relaxed (&sem->data, -((uint64_t) 1 << SEM_NWAITERS_SHIFT));
 #else
@@ -107,7 +107,7 @@ do_futex_wait (struct new_sem *sem, clockid_t clockid,
 {
   int err;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   err = __futex_abstimed_wait_cancelable64 (
       (unsigned int *) &sem->data + SEM_VALUE_OFFSET, 0,
       clockid, abstime,
@@ -132,7 +132,7 @@ __new_sem_wait_fast (struct new_sem *sem, int definitive_result)
      synchronize memory); thus, relaxed MO is sufficient for the initial load
      and the failure path of the CAS.  If the weak CAS fails and we need a
      definitive result, retry.  */
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   uint64_t d = atomic_load_relaxed (&sem->data);
   do
     {
@@ -166,7 +166,7 @@ __new_sem_wait_slow64 (struct new_sem *sem, clockid_t clockid,
 {
   int err = 0;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   /* Add a waiter.  Relaxed MO is sufficient because we can rely on the
      ordering provided by the RMW operations we use.  */
   uint64_t d = atomic_fetch_add_relaxed (&sem->data,
@@ -280,7 +280,7 @@ __new_sem_wait_slow64 (struct new_sem *sem, clockid_t clockid,
          /* If there is no token, wait.  */
          if ((v >> SEM_VALUE_SHIFT) == 0)
            {
-             /* See USE_64B_ATOMICS variant.  */
+             /* See USE_64B_ATOMICS_ON_SEM_T variant.  */
              err = do_futex_wait (sem, clockid, abstime);
              if (err == ETIMEDOUT || err == EINTR)
                {
@@ -313,7 +313,7 @@ error:
 }
 
 /* Stop being a registered waiter (non-64b-atomics code only).  */
-#if !USE_64B_ATOMICS
+#if !USE_64B_ATOMICS_ON_SEM_T
 static void
 __sem_wait_32_finish (struct new_sem *sem)
 {

diff --git a/nptl/semaphoreP.h b/nptl/semaphoreP.h
index 36498529f178c79510c22df8ac4d77c5570e0b70..5ab72abbac7c8d1843520cd1e7d54b7ebfaf96ba 100644 (file)
--- a/nptl/semaphoreP.h
+++ b/nptl/semaphoreP.h
@@ -24,7 +24,7 @@
 
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   sem->data = value;
 #else
   sem->value = value << SEM_VALUE_SHIFT;

diff --git a/nptl/tst-sem11.c b/nptl/tst-sem11.c
index 7c946c30d114031dbf69e6bd0f7f46345b060135..cda9aae3208f543c801e78e5be5f7085bf459ae1 100644 (file)
--- a/nptl/tst-sem11.c
+++ b/nptl/tst-sem11.c
@@ -52,7 +52,7 @@ do_test (void)
       puts ("sem_init failed");
       return 1;
     }
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   if ((u.ns.data >> SEM_NWAITERS_SHIFT) != 0)
 #else
   if (u.ns.nwaiters != 0)
@@ -89,7 +89,7 @@ do_test (void)
       goto again;
     }
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   if ((u.ns.data >> SEM_NWAITERS_SHIFT) != 0)
 #else
   if (u.ns.nwaiters != 0)

diff --git a/nptl/tst-sem13.c b/nptl/tst-sem13.c
index 17067dd572e6446d47e6e4304cc1a7ecb6aba98c..2b7d7ceae9321243b4a569b777e5d5508fe0c099 100644 (file)
--- a/nptl/tst-sem13.c
+++ b/nptl/tst-sem13.c
@@ -30,7 +30,7 @@ do_test_wait (waitfn_t waitfn, const char *fnname)
   TEST_VERIFY_EXIT (waitfn (&u.s, &ts) < 0);
   TEST_COMPARE (errno, EINVAL);
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   unsigned int nwaiters = (u.ns.data >> SEM_NWAITERS_SHIFT);
 #else
   unsigned int nwaiters = u.ns.nwaiters;
@@ -42,7 +42,7 @@ do_test_wait (waitfn_t waitfn, const char *fnname)
   errno = 0;
   TEST_VERIFY_EXIT (waitfn (&u.s, &ts) < 0);
   TEST_COMPARE (errno, ETIMEDOUT);
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   nwaiters = (u.ns.data >> SEM_NWAITERS_SHIFT);
 #else
   nwaiters = u.ns.nwaiters;

diff --git a/sysdeps/generic/atomic-sem_t.h b/sysdeps/generic/atomic-sem_t.h
new file mode 100644 (file)
index 0000000..6205cc4
--- /dev/null
+++ b/sysdeps/generic/atomic-sem_t.h
@@ -0,0 +1,26 @@
+/* Copyright (C) 2025 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <atomic-machine.h>
+#include <sem_t-align.h>
+
+#if USE_64B_ATOMICS && (SEM_T_ALIGN >= 8 \
+                       || defined HAVE_UNALIGNED_64B_ATOMICS)
+# define USE_64B_ATOMICS_ON_SEM_T 1
+#else
+# define USE_64B_ATOMICS_ON_SEM_T 0
+#endif

diff --git a/sysdeps/htl/sem-destroy.c b/sysdeps/htl/sem-destroy.c
index dbf61b885e402e9fced58f32029dd1220cd5e6b4..b6d75d6340331a6804d40e7d790c097e3d314b6b 100644 (file)
--- a/sysdeps/htl/sem-destroy.c
+++ b/sysdeps/htl/sem-destroy.c
@@ -28,7 +28,7 @@ __sem_destroy (sem_t *sem)
 {
   struct new_sem *isem = (struct new_sem *) sem;
   if (
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
       atomic_load_relaxed (&isem->data) >> SEM_NWAITERS_SHIFT
 #else
       atomic_load_relaxed (&isem->value) & SEM_NWAITERS_MASK

diff --git a/sysdeps/htl/sem-getvalue.c b/sysdeps/htl/sem-getvalue.c
index 73bb96576decb5d51d9ebffa343cd71d91ce65dc..2e1b2159840603fb11aab0e273ea350b5b9f885c 100644 (file)
--- a/sysdeps/htl/sem-getvalue.c
+++ b/sysdeps/htl/sem-getvalue.c
@@ -25,7 +25,7 @@ __sem_getvalue (sem_t *restrict sem, int *restrict value)
 {
   struct new_sem *isem = (struct new_sem *) sem;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   *value = atomic_load_relaxed (&isem->data) & SEM_VALUE_MASK;
 #else
   *value = atomic_load_relaxed (&isem->value) >> SEM_VALUE_SHIFT;

diff --git a/sysdeps/htl/sem-post.c b/sysdeps/htl/sem-post.c
index f42fb2867018d02e6771003f376fb273af6fd385..1852848e7cb634b923451e1daf75e6ed37de8d48 100644 (file)
--- a/sysdeps/htl/sem-post.c
+++ b/sysdeps/htl/sem-post.c
@@ -31,7 +31,7 @@ __sem_post (sem_t *sem)
   struct new_sem *isem = (struct new_sem *) sem;
   int flags = isem->pshared ? GSYNC_SHARED : 0;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   uint64_t d = atomic_load_relaxed (&isem->data);
 
   do

diff --git a/sysdeps/htl/sem-timedwait.c b/sysdeps/htl/sem-timedwait.c
index 755d502f8c862d92ffacd2c0f5191618cac74277..f69ccc49665e12e8735d08dfb8a17700a69b6592 100644 (file)
--- a/sysdeps/htl/sem-timedwait.c
+++ b/sysdeps/htl/sem-timedwait.c
@@ -27,7 +27,7 @@
 #include <pt-internal.h>
 #include <shlib-compat.h>
 
-#if !USE_64B_ATOMICS
+#if !USE_64B_ATOMICS_ON_SEM_T
 static void
 __sem_wait_32_finish (struct new_sem *isem);
 #endif
@@ -37,7 +37,7 @@ __sem_wait_cleanup (void *arg)
 {
   struct new_sem *isem = arg;
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   atomic_fetch_add_relaxed (&isem->data, -((uint64_t) 1 << SEM_NWAITERS_SHIFT));
 #else
   __sem_wait_32_finish (isem);
@@ -60,7 +60,7 @@ __sem_timedwait_internal (sem_t *restrict sem,
 
   int cancel_oldtype = LIBC_CANCEL_ASYNC();
 
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   uint64_t d = atomic_fetch_add_relaxed (&isem->data,
                 (uint64_t) 1 << SEM_NWAITERS_SHIFT);
 
@@ -170,7 +170,7 @@ error:
   return ret;
 }
 
-#if !USE_64B_ATOMICS
+#if !USE_64B_ATOMICS_ON_SEM_T
 /* Stop being a registered waiter (non-64b-atomics code only).  */
 static void
 __sem_wait_32_finish (struct new_sem *isem)

diff --git a/sysdeps/htl/sem-waitfast.c b/sysdeps/htl/sem-waitfast.c
index 3ee4d417c877d634919c179d7774f70db98f036c..af847fc378b252abaf49cde5ec9999db274b7b64 100644 (file)
--- a/sysdeps/htl/sem-waitfast.c
+++ b/sysdeps/htl/sem-waitfast.c
@@ -24,7 +24,7 @@
 int
 __sem_waitfast (struct new_sem *isem, int definitive_result)
 {
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   uint64_t d = atomic_load_relaxed (&isem->data);
 
   do

diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h
index 73ddd44872f4f94b28b8994a3732817fd522bdd3..9169453cc9347b4f8a9a427ea30bf93a2de6d62b 100644 (file)
--- a/sysdeps/nptl/internaltypes.h
+++ b/sysdeps/nptl/internaltypes.h
@@ -21,6 +21,8 @@
 #include <stdint.h>
 #include <atomic.h>
 #include <endian.h>
+#include <semaphore.h>
+#include <atomic-sem_t.h>
 
 
 struct pthread_attr
@@ -159,7 +161,7 @@ struct pthread_key_struct
 /* Semaphore variable structure.  */
 struct new_sem
 {
-#if USE_64B_ATOMICS
+#if USE_64B_ATOMICS_ON_SEM_T
   /* The data field holds both value (in the least-significant 32 bits) and
      nwaiters.  */
 # if __BYTE_ORDER == __LITTLE_ENDIAN
@@ -184,6 +186,12 @@ struct new_sem
 #endif
 };
 
+_Static_assert (sizeof (sem_t) >= sizeof (struct new_sem),
+               "sizeof (sem_t) >= sizeof (struct new_sem)");
+
+_Static_assert (__alignof (sem_t) >= __alignof (struct new_sem),
+               "__alignof (sem_t) >= __alignof (struct new_sem)");
+
 struct old_sem
 {
   unsigned int value;