From: Simon McVittie Date: Mon, 14 Aug 2023 18:53:11 +0000 (+0100) Subject: sysdeps: Use C11 stdatomic.h where possible X-Git-Tag: dbus-1.15.8~12^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88b57499846a1de4d97e8eff377132dfc1e0b7de;p=thirdparty%2Fdbus.git 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 --- diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake index b138b3670..b7f37020b 100644 --- a/cmake/ConfigureChecks.cmake +++ b/cmake/ConfigureChecks.cmake @@ -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) diff --git a/cmake/config.h.cmake b/cmake/config.h.cmake index 16e737f3a..77fc19c39 100644 --- a/cmake/config.h.cmake +++ b/cmake/config.h.cmake @@ -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 diff --git a/configure.ac b/configure.ac index 389b27964..6b717e75b 100644 --- a/configure.ac +++ b/configure.ac @@ -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 diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index c66096e07..60bdc6f71 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -98,7 +98,7 @@ #include #endif -#if !DBUS_USE_SYNC +#if !defined(HAVE_STDATOMIC_H) && !DBUS_USE_SYNC #include #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 diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index fa4083286..2f9273173 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -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 } /** diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index e97371790..91b6016f6 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -38,6 +38,10 @@ #include #endif +#ifdef HAVE_STDATOMIC_H +#include +#endif + #include #include #include @@ -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. */ diff --git a/meson.build b/meson.build index 4b2d9b900..da3c0d20a 100644 --- a/meson.build +++ b/meson.build @@ -691,6 +691,7 @@ check_headers = [ 'linux/magic.h', 'locale.h', 'signal.h', + 'stdatomic.h', 'syslog.h', 'sys/prctl.h', 'sys/random.h',