sysdeps: Use C11 stdatomic.h where possible

On Unix, dbus has historically used gcc-specific lock-free atomic
intrinsics where available, falling back to a pthreads mutex where
possible. Meanwhile, on Windows, it has historically used
InterlockedIncrement() and similar library functions (in practice
wrappers around lock-free intrinsics on real Windows, but IPC calls into
wineserver on Wine).

ISO C11 provides a new header, stdatomic.h, with standardized support
for atomic operations. Exactly how these are implemented is a compiler
quality-of-implementation decision, but any reasonable compiler
implementation on a modern CPU should be using intrinsics. Let's use
this wherever possible, falling back to our old implementation only if
the C11 implementation is unsupported.

One concrete benefit that we get from this is that when compiling with
mingw-w64 gcc and running via Wine, this makes atomic reference counting
operations into a simple local operation, rather than IPC to wineserver
which can be very slow. This should make our CI tests considerably more
reliable.

In all vaguely modern gcc versions (gcc 5.5 or later) and in contemporary
versions of clang, the default compiler mode is C11 or later with GNU
extensions. We intentionally do not ask for any specific C standard, so
we can use C11 features like this one, as long as we do so conditionally.

The Microsoft Visual C compiler does not currently support this without
special options, so we still use the Interlocked family of functions
when compiling for Windows with MSVC.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie 2023-08-14 19:53:11 +01:00
parent e621e99241
commit 88b5749984
7 changed files with 62 additions and 12 deletions

View file

@ -21,6 +21,7 @@ check_include_file(linux/close_range.h HAVE_LINUX_CLOSE_RANGE_H)
check_include_file(linux/magic.h HAVE_LINUX_MAGIC_H)
check_include_file(locale.h HAVE_LOCALE_H)
check_include_file(signal.h HAVE_SIGNAL_H)
check_include_file(stdatomic.h HAVE_STDATOMIC_H)
check_include_file(stdio.h HAVE_STDIO_H) # dbus-sysdeps.h
check_include_files("stdint.h;sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
check_include_file(sys/inotify.h HAVE_SYS_INOTIFY_H)

View file

@ -113,6 +113,7 @@
/* Define to 1 if you have stdio.h */
#cmakedefine HAVE_STDIO_H 1
#cmakedefine HAVE_STDATOMIC_H 1
#cmakedefine HAVE_SYSLOG_H 1
#cmakedefine HAVE_SYS_EVENTS_H 1
#cmakedefine HAVE_SYS_INOTIFY_H 1

View file

@ -419,6 +419,7 @@ errno.h
linux/close_range.h
locale.h
signal.h
stdatomic.h
sys/prctl.h
sys/random.h
sys/resource.h

View file

@ -98,7 +98,7 @@
#include <systemd/sd-daemon.h>
#endif
#if !DBUS_USE_SYNC
#if !defined(HAVE_STDATOMIC_H) && !DBUS_USE_SYNC
#include <pthread.h>
#endif
@ -3141,7 +3141,7 @@ _dbus_pid_for_log (void)
return getpid ();
}
#if !DBUS_USE_SYNC
#if !defined(HAVE_STDATOMIC_H) && !DBUS_USE_SYNC
/* To be thread-safe by default on platforms that don't necessarily have
* atomic operations (notably Debian armel, which is armv4t), we must
* use a mutex that can be initialized statically, like this.
@ -3159,7 +3159,11 @@ static pthread_mutex_t atomic_mutex = PTHREAD_MUTEX_INITIALIZER;
dbus_int32_t
_dbus_atomic_inc (DBusAtomic *atomic)
{
#if DBUS_USE_SYNC
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "old = *atomic; *atomic += 1; return old" */
return atomic_fetch_add (&atomic->value, 1);
#elif DBUS_USE_SYNC
/* Atomic version of "*atomic += 1; return *atomic - 1" */
return __sync_add_and_fetch(&atomic->value, 1)-1;
#else
dbus_int32_t res;
@ -3182,7 +3186,11 @@ _dbus_atomic_inc (DBusAtomic *atomic)
dbus_int32_t
_dbus_atomic_dec (DBusAtomic *atomic)
{
#if DBUS_USE_SYNC
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "old = *atomic; *atomic -= 1; return old" */
return atomic_fetch_sub (&atomic->value, 1);
#elif DBUS_USE_SYNC
/* Atomic version of "*atomic -= 1; return *atomic + 1" */
return __sync_sub_and_fetch(&atomic->value, 1)+1;
#else
dbus_int32_t res;
@ -3206,7 +3214,10 @@ _dbus_atomic_dec (DBusAtomic *atomic)
dbus_int32_t
_dbus_atomic_get (DBusAtomic *atomic)
{
#if DBUS_USE_SYNC
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "return *atomic" */
return atomic_load (&atomic->value);
#elif DBUS_USE_SYNC
__sync_synchronize ();
return atomic->value;
#else
@ -3228,7 +3239,10 @@ _dbus_atomic_get (DBusAtomic *atomic)
void
_dbus_atomic_set_zero (DBusAtomic *atomic)
{
#if DBUS_USE_SYNC
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "*atomic = 0" */
atomic_store (&atomic->value, 0);
#elif DBUS_USE_SYNC
/* Atomic version of "*atomic &= 0; return *atomic" */
__sync_and_and_fetch (&atomic->value, 0);
#else
@ -3246,7 +3260,10 @@ _dbus_atomic_set_zero (DBusAtomic *atomic)
void
_dbus_atomic_set_nonzero (DBusAtomic *atomic)
{
#if DBUS_USE_SYNC
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "*atomic = 1" */
atomic_store (&atomic->value, 1);
#elif DBUS_USE_SYNC
/* Atomic version of "*atomic |= 1; return *atomic" */
__sync_or_and_fetch (&atomic->value, 1);
#else

View file

@ -3452,9 +3452,13 @@ _dbus_make_file_world_readable(const DBusString *filename,
dbus_int32_t
_dbus_atomic_inc (DBusAtomic *atomic)
{
// +/- 1 is needed here!
// no volatile argument with mingw
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "old = *atomic; *atomic += 1; return old" */
return atomic_fetch_add (&atomic->value, 1);
#else
/* Atomic version of "*atomic += 1; return *atomic - 1" */
return InterlockedIncrement (&atomic->value) - 1;
#endif
}
/**
@ -3467,9 +3471,13 @@ _dbus_atomic_inc (DBusAtomic *atomic)
dbus_int32_t
_dbus_atomic_dec (DBusAtomic *atomic)
{
// +/- 1 is needed here!
// no volatile argument with mingw
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "old = *atomic; *atomic -= 1; return old" */
return atomic_fetch_sub (&atomic->value, 1);
#else
/* Atomic version of "*atomic -= 1; return *atomic + 1" */
return InterlockedDecrement (&atomic->value) + 1;
#endif
}
/**
@ -3482,6 +3490,10 @@ _dbus_atomic_dec (DBusAtomic *atomic)
dbus_int32_t
_dbus_atomic_get (DBusAtomic *atomic)
{
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "return *atomic" */
return atomic_load (&atomic->value);
#else
/* In this situation, GLib issues a MemoryBarrier() and then returns
* atomic->value. However, mingw from mingw.org (not to be confused with
* mingw-w64 from mingw-w64.sf.net) does not have MemoryBarrier in its
@ -3495,6 +3507,7 @@ _dbus_atomic_get (DBusAtomic *atomic)
InterlockedExchange (&dummy, 1);
return atomic->value;
#endif
}
/**
@ -3505,7 +3518,12 @@ _dbus_atomic_get (DBusAtomic *atomic)
void
_dbus_atomic_set_zero (DBusAtomic *atomic)
{
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "*atomic = 0" */
atomic_store (&atomic->value, 0);
#else
InterlockedExchange (&atomic->value, 0);
#endif
}
/**
@ -3516,7 +3534,12 @@ _dbus_atomic_set_zero (DBusAtomic *atomic)
void
_dbus_atomic_set_nonzero (DBusAtomic *atomic)
{
#ifdef HAVE_STDATOMIC_H
/* Atomic version of "*atomic = 1" */
atomic_store (&atomic->value, 1);
#else
InterlockedExchange (&atomic->value, 1);
#endif
}
/**

View file

@ -38,6 +38,10 @@
#include <inttypes.h>
#endif
#ifdef HAVE_STDATOMIC_H
#include <stdatomic.h>
#endif
#include <dbus/dbus-errors.h>
#include <dbus/dbus-file.h>
#include <dbus/dbus-string.h>
@ -333,7 +337,9 @@ typedef struct DBusAtomic DBusAtomic;
*/
struct DBusAtomic
{
#ifdef DBUS_WIN
#ifdef HAVE_STDATOMIC_H
atomic_int value; /**< Value of the atomic integer. */
#elif defined(DBUS_WIN)
volatile long value; /**< Value of the atomic integer. */
#else
volatile dbus_int32_t value; /**< Value of the atomic integer. */

View file

@ -691,6 +691,7 @@ check_headers = [
'linux/magic.h',
'locale.h',
'signal.h',
'stdatomic.h',
'syslog.h',
'sys/prctl.h',
'sys/random.h',