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>

diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake
index b138b36706945adc34e368f52c7f1ea4a16f508d..b7f37020bfb9bb98d484b2ff4ada6c52ff6258bd 100644 (file)
--- 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 16e737f3a3e96ea7a7bfd73b1d1370168e2747dd..77fc19c398587774613f730971e013135760ef93 100644 (file)
--- 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 389b27964f676414f3d207e59423f29b05590e9a..6b717e75b94329c6a60775b393eb25b02886e204 100644 (file)
--- 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 c66096e076e62ac17b32c06a0a7d784eded490d7..60bdc6f710ae4f6b8d86deb55d9c1b95390f0c2c 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -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

diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index fa40832861ad67941234380c66e130dc814bdec1..2f9273173d9d6e262da6245a96cbc7166ea592f3 100644 (file)
--- 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 e97371790924d4e79a8d4614b342ba33aa41ac0d..91b6016f67f07167c50aceb8b69b72e21d87d2ec 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -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. */

diff --git a/meson.build b/meson.build
index 4b2d9b9008416a7ae3b459cb51fd79499f0e6eae..da3c0d20ad380ee879dff75d71f83c81765c2543 100644 (file)
--- 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',