From aef872e2999a78522b6189a4b96ccd484d2226d4 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 16:21:30 +0100 Subject: [PATCH 01/10] Meson: Enable __atomic and __sync builtins also on Windows We can take advantage of __atomic and __sync builtins when compiling with GCC and CLang on Windows Unfortunately we can't use C11 atomics yet! We also use C++ on Windows, and compatibility between C _Atomic and C++ std::atomic is not guaranteed (see C++/N2741 [3]) References: 1. https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html 2. https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html 3. https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2741.htm --- meson.build | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/meson.build b/meson.build index 11525ea84..c7b0223d3 100644 --- a/meson.build +++ b/meson.build @@ -762,22 +762,27 @@ endforeach extra_link_args += pthread_link_args -# Atomics are an optional feature in C11. Also need to check that C11 atomics are lock free. -# On Windows we use the Interlocked family of functions -if host_machine.system() != 'windows' - if cc.links(files('meson-cc-tests/atomic-ops-c11.c'), name: 'Atomic ops: c11') - conf.set('HAVE_C11_ATOMIC_PRIMITIVES', 1) - elif cc.links(files('meson-cc-tests/atomic-ops-cxx11.c'), name: 'Atomic ops: cxx11') - conf.set('HAVE_CXX11_ATOMIC_PRIMITIVES', 1) - elif cc.links(files('meson-cc-tests/atomic-ops-gcc-legacy.c'), name: 'Atomic ops: gcc legacy') - conf.set('HAVE_GCC_LEGACY_ATOMICS', 1) - elif cc.has_header('atomic_ops.h') - conf.set('HAVE_LIB_ATOMIC_OPS', 1) - elif cc.has_header('libkern/OSAtomic.h') - conf.set('HAVE_OS_ATOMIC_OPS', 1) - else - warning('Atomic ops not supported.') - endif +cpp_enabled = host_machine.system() == 'windows' + +if not cpp_enabled and cc.links(files('meson-cc-tests/atomic-ops-c11.c'), name: 'Atomic ops: c11') + # Currently we avoid C11 atomics when using both C and C++. The standards + # do not guarantee compatibility between C11 atomics and C++11 std::atomic + # (though effort is underway, see C++/N2741). We can enable this for selected + # compilers over time. + # + # When not using C++, check if C11 atomics are available and whether atomic + # ints and pointers are lock-free. + conf.set('HAVE_C11_ATOMIC_PRIMITIVES', 1) +elif cc.links(files('meson-cc-tests/atomic-ops-cxx11.c'), name: 'Atomic ops: cxx11') + conf.set('HAVE_CXX11_ATOMIC_PRIMITIVES', 1) +elif cc.links(files('meson-cc-tests/atomic-ops-gcc-legacy.c'), name: 'Atomic ops: gcc legacy') + conf.set('HAVE_GCC_LEGACY_ATOMICS', 1) +elif host_machine.system() != 'windows' and cc.has_header('atomic_ops.h') + conf.set('HAVE_LIB_ATOMIC_OPS', 1) +elif host_machine.system() == 'darwin' and cc.has_header('libkern/OSAtomic.h') + conf.set('HAVE_OS_ATOMIC_OPS', 1) +else + warning('Atomic ops not supported.') endif test_mkdir_c_args = [] From f6da210c49f4fdafcb25e086f41604f9bfa2e212 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 16:24:52 +0100 Subject: [PATCH 02/10] Atomics: Make Interlocked-based atomics MSVC-specific Atomics based on Interlocked functions and MemoryBarrier are quite compiler-agnostic. However, the load / store functions use plain C reads / assignments, and those are not guaranteed to be atomic by the C/C++ standard. They are atomic on all major compilers, though ideally access should be mediated by volatile to make things work in corner cases. As such, unknown compilers should use mutex-based atomics and we should emit a warning. If issues arise, we can add compiler- specific implementations. --- meson.build | 2 +- src/cairo-atomic-private.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index c7b0223d3..5d56c87de 100644 --- a/meson.build +++ b/meson.build @@ -781,7 +781,7 @@ elif host_machine.system() != 'windows' and cc.has_header('atomic_ops.h') conf.set('HAVE_LIB_ATOMIC_OPS', 1) elif host_machine.system() == 'darwin' and cc.has_header('libkern/OSAtomic.h') conf.set('HAVE_OS_ATOMIC_OPS', 1) -else +elif not cc.has_define('_MSC_VER') warning('Atomic ops not supported.') endif diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index d8d805771..3796c331d 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -330,7 +330,7 @@ typedef intptr_t cairo_atomic_intptr_t; #endif /* HAVE_OS_ATOMIC_OPS */ -#if !defined(HAS_ATOMIC_OPS) && defined(_WIN32) +#if !defined(HAS_ATOMIC_OPS) && defined(_MSC_VER) #include #define HAS_ATOMIC_OPS 1 From 3d9e235236541f896c7ea024c66d9b07cf5fad9a Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 16:59:15 +0100 Subject: [PATCH 03/10] Atomics: Implement _cairo_atomic_int_cmpxchg_return_old for MSVC --- src/cairo-atomic-private.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index 3796c331d..eb4ca12a3 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -360,6 +360,14 @@ _cairo_atomic_int_cmpxchg (cairo_atomic_int_t *x, return InterlockedCompareExchange (x, (LONG)newv, (LONG)oldv) == oldv; } +static cairo_always_inline int +_cairo_atomic_int_cmpxchg_return_old (cairo_atomic_int_t *x, + int oldv, + int newv) +{ + return (int) InterlockedCompareExchange (x, (LONG)newv, (LONG)oldv); +} + static cairo_always_inline void * _cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) { From 2acf6eaf3e5f64f3c6c183a27fa2cb9460ceaccc Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 17:03:49 +0100 Subject: [PATCH 04/10] Atomics: Define macros to avoid fallbacks The four cmpxchg functions must be defined as macros to avoid using fallback definitons. --- src/cairo-atomic-private.h | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index eb4ca12a3..6a31d0b1e 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -353,21 +353,27 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x) # define _cairo_atomic_int_dec_and_test(x) (InterlockedDecrement (x) == 0) static cairo_always_inline cairo_bool_t -_cairo_atomic_int_cmpxchg (cairo_atomic_int_t *x, - int oldv, - int newv) +_cairo_atomic_int_cmpxchg_impl (cairo_atomic_int_t *x, + int oldv, + int newv) { return InterlockedCompareExchange (x, (LONG)newv, (LONG)oldv) == oldv; } +#define _cairo_atomic_int_cmpxchg(x, oldv, newv) \ + _cairo_atomic_int_cmpxchg_impl(x, oldv, newv) + static cairo_always_inline int -_cairo_atomic_int_cmpxchg_return_old (cairo_atomic_int_t *x, - int oldv, - int newv) +_cairo_atomic_int_cmpxchg_return_old_impl (cairo_atomic_int_t *x, + int oldv, + int newv) { return (int) InterlockedCompareExchange (x, (LONG)newv, (LONG)oldv); } +#define _cairo_atomic_int_cmpxchg_return_old(x, oldv, newv) \ + _cairo_atomic_int_cmpxchg_return_old_impl(x, oldv, newv) + static cairo_always_inline void * _cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) { @@ -376,17 +382,23 @@ _cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) } static cairo_always_inline cairo_bool_t -_cairo_atomic_ptr_cmpxchg (cairo_atomic_intptr_t *x, void *oldv, void *newv) +_cairo_atomic_ptr_cmpxchg_impl (cairo_atomic_intptr_t *x, void *oldv, void *newv) { return InterlockedCompareExchangePointer (x, newv, oldv) == oldv; } +#define _cairo_atomic_ptr_cmpxchg(x, oldv, newv) \ + _cairo_atomic_ptr_cmpxchg_impl(x, oldv, newv) + static cairo_always_inline void * -_cairo_atomic_ptr_cmpxchg_return_old (cairo_atomic_intptr_t *x, void *oldv, void *newv) +_cairo_atomic_ptr_cmpxchg_return_old_impl (cairo_atomic_intptr_t *x, void *oldv, void *newv) { return InterlockedCompareExchangePointer (x, newv, oldv); } +#define _cairo_atomic_ptr_cmpxchg_return_old(x, oldv, newv) \ + _cairo_atomic_ptr_cmpxchg_return_old_impl(x, oldv, newv) + #endif /* !defined(HAS_ATOMIC_OPS) && defined(_WIN32) */ From cd8ae260af3ab90c21e09e7760f209e4fb1860fd Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 18:36:37 +0100 Subject: [PATCH 05/10] Atomics: Enhance libatomic_ops-based implementations --- src/cairo-atomic-private.h | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index 6a31d0b1e..1f3ce1ffd 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -274,26 +274,35 @@ _cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) #endif /* HAVE_GCC_LEGACY_ATOMICS */ #if HAVE_LIB_ATOMIC_OPS + #include +#include #define HAS_ATOMIC_OPS 1 -typedef AO_t cairo_atomic_int_t; +typedef int cairo_atomic_int_t; -# define _cairo_atomic_int_get(x) (AO_load_full (x)) -# define _cairo_atomic_int_get_relaxed(x) (AO_load_full (x)) -# define _cairo_atomic_int_set_relaxed(x, val) (AO_store_full ((x), (val))) +/* Casts from signed to unsigned must not change representation */ +static_assert((unsigned)-1 == UINT_MAX, + "We require two's complement representation of signed integrals"); -# define _cairo_atomic_int_inc(x) ((void) AO_fetch_and_add1_full(x)) -# define _cairo_atomic_int_dec(x) ((void) AO_fetch_and_sub1_full(x)) -# define _cairo_atomic_int_dec_and_test(x) (AO_fetch_and_sub1_full(x) == 1) -# define _cairo_atomic_int_cmpxchg(x, oldv, newv) AO_compare_and_swap_full(x, oldv, newv) +# define _cairo_atomic_int_get(x) ((int)AO_int_load_full ((unsigned *)(x))) +# define _cairo_atomic_int_get_relaxed(x) ((int)AO_int_load ((unsigned *)(x))) +# define _cairo_atomic_int_set_relaxed(x, val) (AO_int_store ((unsigned *)(x), (unsigned)(val))) -typedef intptr_t cairo_atomic_intptr_t; +# define _cairo_atomic_int_inc(x) ((void) AO_int_fetch_and_add1_full((unsigned *)(x))) +# define _cairo_atomic_int_dec(x) ((void) AO_int_fetch_and_sub1_full((unsigned *)(x))) +# define _cairo_atomic_int_dec_and_test(x) (AO_int_fetch_and_sub1_full((unsigned *)(x)) == 1U) +# define _cairo_atomic_int_cmpxchg(x, oldv, newv) AO_int_compare_and_swap_full((unsigned *)x, (unsigned)(oldv), (unsigned)(newv)) +# define _cairo_atomic_int_cmpxchg_return_old(x, oldv, newv) AO_int_fetch_compare_and_swap_full((unsigned *)x, (unsigned)(oldv), (unsigned)(newv)) + +typedef AO_t cairo_atomic_intptr_t; + +static_assert (sizeof (AO_t) >= sizeof (void *), "AO_t cannot be used for pointers"); # define _cairo_atomic_ptr_get(x) _cairo_atomic_intptr_to_voidptr (AO_load_full (x)) -# define _cairo_atomic_ptr_cmpxchg(x, oldv, newv) \ - _cairo_atomic_int_cmpxchg ((cairo_atomic_intptr_t*)(x), (cairo_atomic_intptr_t)oldv, (cairo_atomic_intptr_t)newv) +# define _cairo_atomic_ptr_cmpxchg(x, oldv, newv) AO_compare_and_swap_full(x, (AO_t)oldv, (AO_t)newv) +# define _cairo_atomic_ptr_cmpxchg_return_old(x, oldv, newv) AO_fetch_compare_and_swap_full(x, (AO_t)oldv, (AO_t)newv) #endif From 5fb0538cce4350e6410201db49babc1882a079f1 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 23 Mar 2026 19:12:11 +0100 Subject: [PATCH 06/10] Meson: Link to libatomic_ops From https://github.com/bdwgc/libatomic_ops/blob/master/README_details.txt: Applications should include atomic_ops.h. Nearly all operations are implemented by header files included from it. It is sometimes necessary, and always recommended to also link against libatomic_ops.a --- meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 5d56c87de..4beed7888 100644 --- a/meson.build +++ b/meson.build @@ -777,7 +777,8 @@ elif cc.links(files('meson-cc-tests/atomic-ops-cxx11.c'), name: 'Atomic ops: cxx conf.set('HAVE_CXX11_ATOMIC_PRIMITIVES', 1) elif cc.links(files('meson-cc-tests/atomic-ops-gcc-legacy.c'), name: 'Atomic ops: gcc legacy') conf.set('HAVE_GCC_LEGACY_ATOMICS', 1) -elif host_machine.system() != 'windows' and cc.has_header('atomic_ops.h') +elif host_machine.system() != 'windows' and dependency('atomic_ops', required: false).found() + internal_deps += [dependency('atomic_ops')] conf.set('HAVE_LIB_ATOMIC_OPS', 1) elif host_machine.system() == 'darwin' and cc.has_header('libkern/OSAtomic.h') conf.set('HAVE_OS_ATOMIC_OPS', 1) From 9a5df2f0db56aaf46ad550a5f0fb16b087a97e81 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Tue, 24 Mar 2026 11:24:10 +0100 Subject: [PATCH 07/10] Atomics: Fix relaxed atomics for GCC-legacy, OSAtomic, MSVC, and mutex fallback Relaxed atomics really benefit from volatile access since otherwise the compiler can hoist loads / stores out of loops. For example, in the typical spin scenario: while (!_cairo_atomic_int_get_relaxed (&done)); The optimizer can do: val = _cairo_atomic_int_get_relaxed (&done); if (!val) { for (;;); } Unless told otherwise, e.g. with volatile or _Atomic, the optimizer assumes that variables are only accessed by a single thread of execution. That's also why data races are undefined behaviour in C11. For GCC and CLang we use an intermediate volatile cast. This works because GCC and CLang support the Linux Kernel Memory Model (LKMM) [1], ensuring that: 1. volatile can be applied to single accesses and not necessarily to the variable itself 2. volatile accesses are atomic for aligned and machine-word sized variables For MSVC we use the newer __iso_volatile intrinsics which are recommended over volatile accesses [2]. Mutex-based fallbacks must all go under the mutex, even relaxed ones, so we drop the ATOMIC_OP_NEEDS_MEMORY_BARRIER macro. References: 1. https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0124r6.html 2. https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-c4746 --- meson.build | 4 ---- src/cairo-atomic-private.h | 24 +++++++++++------------- src/cairo-atomic.c | 2 -- src/cairo-mutex-list-private.h | 2 +- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/meson.build b/meson.build index 4beed7888..263427d9d 100644 --- a/meson.build +++ b/meson.build @@ -803,10 +803,6 @@ else conf.set('HAVE_MKDIR', 0) endif -if not ['x86', 'x86_64'].contains(host_machine.cpu_family()) - conf.set('ATOMIC_OP_NEEDS_MEMORY_BARRIER', 1) -endif - have_ld_preload = ['linux', 'freebsd', 'darwin', 'dragonfly', 'sunos'].contains(host_machine.system()) if have_ld_preload and zlib_dep.found() and conf.get('CAIRO_HAS_REAL_PTHREAD', 0) == 1 and conf.get('CAIRO_HAS_DLSYM', 0) == 1 diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index 1f3ce1ffd..4f58b3a03 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -243,13 +243,13 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x) static cairo_always_inline int _cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x) { - return *x; + return *(volatile int *)x; } static cairo_always_inline void _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) { - *x = val; + *(volatile int *)x = val; } static cairo_always_inline void * @@ -315,8 +315,8 @@ typedef int32_t cairo_atomic_int_t; typedef intptr_t cairo_atomic_intptr_t; # define _cairo_atomic_int_get(x) (OSMemoryBarrier(), *(x)) -# define _cairo_atomic_int_get_relaxed(x) *(x) -# define _cairo_atomic_int_set_relaxed(x, val) *(x) = (val) +# define _cairo_atomic_int_get_relaxed(x) *(volatile int32_t *)(x) +# define _cairo_atomic_int_set_relaxed(x, val) *(volatile int32_t *)(x) = (val) # define _cairo_atomic_int_inc(x) ((void) OSAtomicIncrement32Barrier (x)) # define _cairo_atomic_int_dec(x) ((void) OSAtomicDecrement32Barrier (x)) @@ -340,7 +340,9 @@ typedef intptr_t cairo_atomic_intptr_t; #endif /* HAVE_OS_ATOMIC_OPS */ #if !defined(HAS_ATOMIC_OPS) && defined(_MSC_VER) + #include +#include #define HAS_ATOMIC_OPS 1 @@ -354,8 +356,8 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x) return *x; } -# define _cairo_atomic_int_get_relaxed(x) *(x) -# define _cairo_atomic_int_set_relaxed(x, val) *(x) = (val) +# define _cairo_atomic_int_get_relaxed(x) __iso_volatile_load32 ((__int32 *) x) +# define _cairo_atomic_int_set_relaxed(x, val) __iso_volatile_store32 ((__int32 *) x, (__int32) val) # define _cairo_atomic_int_inc(x) ((void) InterlockedIncrement (x)) # define _cairo_atomic_int_dec(x) ((void) InterlockedDecrement (x)) @@ -433,21 +435,17 @@ _cairo_atomic_ptr_cmpxchg_return_old_impl (cairo_atomic_intptr_t *x, void *oldv, #define _cairo_atomic_int_cmpxchg_return_old(x, oldv, newv) _cairo_atomic_int_cmpxchg_return_old_impl (x, oldv, newv) #define _cairo_atomic_ptr_cmpxchg_return_old(x, oldv, newv) _cairo_atomic_ptr_cmpxchg_return_old_impl (x, oldv, newv) -#ifdef ATOMIC_OP_NEEDS_MEMORY_BARRIER cairo_private int _cairo_atomic_int_get (cairo_atomic_int_t *x); + cairo_private int _cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x); + void _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val); + cairo_private void* _cairo_atomic_ptr_get(cairo_atomic_intptr_t *x); -#else -# define _cairo_atomic_int_get(x) (*x) -# define _cairo_atomic_int_get_relaxed(x) (*x) -# define _cairo_atomic_int_set_relaxed(x, val) (*x) = (val) -# define _cairo_atomic_ptr_get(x) (*x) -#endif #else diff --git a/src/cairo-atomic.c b/src/cairo-atomic.c index 26966d007..5130ef4fc 100644 --- a/src/cairo-atomic.c +++ b/src/cairo-atomic.c @@ -89,7 +89,6 @@ _cairo_atomic_ptr_cmpxchg_return_old_impl (cairo_atomic_intptr_t *x, void *oldv, return ret; } -#ifdef ATOMIC_OP_NEEDS_MEMORY_BARRIER int _cairo_atomic_int_get (cairo_atomic_int_t *x) { @@ -127,6 +126,5 @@ _cairo_atomic_ptr_get (void **x) return ret; } -#endif #endif diff --git a/src/cairo-mutex-list-private.h b/src/cairo-mutex-list-private.h index 48f74f2c3..8504c4a25 100644 --- a/src/cairo-mutex-list-private.h +++ b/src/cairo-mutex-list-private.h @@ -63,7 +63,7 @@ CAIRO_MUTEX_DECLARE (_cairo_xlib_display_mutex) CAIRO_MUTEX_DECLARE (_cairo_xcb_connections_mutex) #endif -#if !defined (HAS_ATOMIC_OPS) || defined (ATOMIC_OP_NEEDS_MEMORY_BARRIER) +#if !defined (HAS_ATOMIC_OPS) CAIRO_MUTEX_DECLARE (_cairo_atomic_mutex) #endif From 2e65e01d25575a686bc76273a1c1b3ec080afcf5 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Tue, 24 Mar 2026 11:32:11 +0100 Subject: [PATCH 08/10] Use cairo_init_once functions to get C_locale object --- src/cairo-misc.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/cairo-misc.c b/src/cairo-misc.c index 59cf76fc9..582206ffd 100644 --- a/src/cairo-misc.c +++ b/src/cairo-misc.c @@ -798,26 +798,18 @@ _cairo_get_locale_decimal_point (void) #if defined (HAVE_NEWLOCALE) && defined (HAVE_STRTOD_L) +static cairo_atomic_once_t C_locale_once; static locale_t C_locale; static locale_t get_C_locale (void) { - locale_t C; - -retry: - C = (locale_t) _cairo_atomic_ptr_get ((cairo_atomic_intptr_t *) &C_locale); - - if (unlikely (!C)) { - C = newlocale (LC_ALL_MASK, "C", NULL); - - if (!_cairo_atomic_ptr_cmpxchg ((cairo_atomic_intptr_t *) &C_locale, NULL, C)) { - freelocale (C_locale); - goto retry; - } + if (_cairo_atomic_init_once_enter (&C_locale_once)) { + C_locale = newlocale (LC_ALL_MASK, "C", NULL); + _cairo_atomic_init_once_leave (&C_locale_once); } - return C; + return C_locale; } double From bedec29b00bc0f671e4b7ec669b25143eb2a88eb Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Tue, 24 Mar 2026 11:56:15 +0100 Subject: [PATCH 09/10] Use more relaxed atomics Turns out all uses of _cairo_atomic_ptr_get can be turned to relaxed loads. So we rename _cairo_atomic_ptr_get to _cairo_atomic_ptr_get_relaxed and drop its memory-ordering constraints. In addition, most uses of _cairo_atomic_int_get can be turned to relaxed loads. We replace these calls of _cairo_atomic_int_get to _cairo_atomic_int_get_relaxed (which is already implemented) --- src/cairo-atomic-private.h | 34 ++++++++++++++++------------- src/cairo-atomic.c | 2 +- src/cairo-freed-pool-private.h | 2 +- src/cairo-reference-count-private.h | 2 +- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index 4f58b3a03..dd2b5e3c2 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -75,9 +75,9 @@ _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) } static cairo_always_inline void * -_cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) +_cairo_atomic_ptr_get_relaxed (cairo_atomic_intptr_t *x) { - return atomic_load_explicit (x, memory_order_seq_cst); + return atomic_load_explicit (x, memory_order_relaxed); } # define _cairo_atomic_int_inc(x) ((void) atomic_fetch_add_explicit(x, 1, memory_order_seq_cst)) @@ -169,9 +169,9 @@ _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) } static cairo_always_inline void * -_cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) +_cairo_atomic_ptr_get_relaxed (cairo_atomic_intptr_t *x) { - return (void*)__atomic_load_n(x, __ATOMIC_SEQ_CST); + return (void*)__atomic_load_n(x, __ATOMIC_RELAXED); } # define _cairo_atomic_int_inc(x) ((void) __atomic_fetch_add(x, 1, __ATOMIC_SEQ_CST)) @@ -253,10 +253,9 @@ _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) } static cairo_always_inline void * -_cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) +_cairo_atomic_ptr_get_relaxed (cairo_atomic_intptr_t *x) { - __sync_synchronize (); - return (void*)*x; + return *(void * volatile *)x; } # define _cairo_atomic_int_inc(x) ((void) __sync_fetch_and_add(x, 1)) @@ -300,7 +299,7 @@ typedef AO_t cairo_atomic_intptr_t; static_assert (sizeof (AO_t) >= sizeof (void *), "AO_t cannot be used for pointers"); -# define _cairo_atomic_ptr_get(x) _cairo_atomic_intptr_to_voidptr (AO_load_full (x)) +# define _cairo_atomic_ptr_get_relaxed(x) _cairo_atomic_intptr_to_voidptr (AO_load (x)) # define _cairo_atomic_ptr_cmpxchg(x, oldv, newv) AO_compare_and_swap_full(x, (AO_t)oldv, (AO_t)newv) # define _cairo_atomic_ptr_cmpxchg_return_old(x, oldv, newv) AO_fetch_compare_and_swap_full(x, (AO_t)oldv, (AO_t)newv) @@ -335,7 +334,7 @@ typedef intptr_t cairo_atomic_intptr_t; #error No matching integer pointer type #endif -# define _cairo_atomic_ptr_get(x) (OSMemoryBarrier(), *(x)) +# define _cairo_atomic_ptr_get_relaxed(x) (*(void * volatile *)(x)) #endif /* HAVE_OS_ATOMIC_OPS */ @@ -386,10 +385,15 @@ _cairo_atomic_int_cmpxchg_return_old_impl (cairo_atomic_int_t *x, _cairo_atomic_int_cmpxchg_return_old_impl(x, oldv, newv) static cairo_always_inline void * -_cairo_atomic_ptr_get (cairo_atomic_intptr_t *x) +_cairo_atomic_ptr_get_relaxed (cairo_atomic_intptr_t *x) { - MemoryBarrier (); - return (void *) *x; +#if SIZEOF_VOID_P == 4 + return (void *) __iso_volatile_load32 ((__int32 *) (void *) x); +#elif SIZEOF_VOID_P == 8 + return (void *) __iso_volatile_load64 ((__int64 *) (void *) x); +#else +#error "unknown pointer size" +#endif } static cairo_always_inline cairo_bool_t @@ -445,7 +449,7 @@ void _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val); cairo_private void* -_cairo_atomic_ptr_get(cairo_atomic_intptr_t *x); +_cairo_atomic_ptr_get_relaxed(cairo_atomic_intptr_t *x); #else @@ -462,7 +466,7 @@ _cairo_atomic_int_cmpxchg_return_old_fallback(cairo_atomic_int_t *x, int oldv, i int curr; do { - curr = _cairo_atomic_int_get (x); + curr = _cairo_atomic_int_get_relaxed (x); } while (curr == oldv && !_cairo_atomic_int_cmpxchg (x, oldv, newv)); return curr; @@ -474,7 +478,7 @@ _cairo_atomic_ptr_cmpxchg_return_old_fallback(cairo_atomic_intptr_t *x, void *ol void *curr; do { - curr = _cairo_atomic_ptr_get (x); + curr = _cairo_atomic_ptr_get_relaxed (x); } while (curr == oldv && !_cairo_atomic_ptr_cmpxchg (x, oldv, newv)); return curr; diff --git a/src/cairo-atomic.c b/src/cairo-atomic.c index 5130ef4fc..99b0efa57 100644 --- a/src/cairo-atomic.c +++ b/src/cairo-atomic.c @@ -116,7 +116,7 @@ _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) } void* -_cairo_atomic_ptr_get (void **x) +_cairo_atomic_ptr_get_relaxed (void **x) { void *ret; diff --git a/src/cairo-freed-pool-private.h b/src/cairo-freed-pool-private.h index ced1adb3d..9015cd3bc 100644 --- a/src/cairo-freed-pool-private.h +++ b/src/cairo-freed-pool-private.h @@ -60,7 +60,7 @@ _atomic_fetch (cairo_atomic_intptr_t *slot) void *ptr; do { - ptr = _cairo_atomic_ptr_get (slot); + ptr = _cairo_atomic_ptr_get_relaxed (slot); } while (! _cairo_atomic_ptr_cmpxchg (slot, ptr, NULL)); return ptr; diff --git a/src/cairo-reference-count-private.h b/src/cairo-reference-count-private.h index f19125c61..d72dbeefa 100644 --- a/src/cairo-reference-count-private.h +++ b/src/cairo-reference-count-private.h @@ -50,7 +50,7 @@ typedef struct { #define CAIRO_REFERENCE_COUNT_INIT(RC, VALUE) ((RC)->ref_count = (VALUE)) -#define CAIRO_REFERENCE_COUNT_GET_VALUE(RC) _cairo_atomic_int_get (&(RC)->ref_count) +#define CAIRO_REFERENCE_COUNT_GET_VALUE(RC) _cairo_atomic_int_get_relaxed (&(RC)->ref_count) #define CAIRO_REFERENCE_COUNT_INVALID_VALUE ((int) -1) #define CAIRO_REFERENCE_COUNT_INVALID {CAIRO_REFERENCE_COUNT_INVALID_VALUE} From a3b4c4204ae5b1636a668ed474c93ccb7d276db3 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Tue, 24 Mar 2026 12:25:19 +0100 Subject: [PATCH 10/10] Fix _cairo_atomic_int_get for GCC-legacy, OSAtomic, and MSVC _cairo_atomic_int_get must have acquire semantics. When implemented via a freestanding memory barrier (__sync_synchronize, etc.), the barrier must be placed after the read. Fixes #932 --- src/cairo-atomic-private.h | 40 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h index dd2b5e3c2..44fcf59ed 100644 --- a/src/cairo-atomic-private.h +++ b/src/cairo-atomic-private.h @@ -233,13 +233,6 @@ _cairo_atomic_ptr_cmpxchg_return_old_impl(cairo_atomic_intptr_t *x, void *oldv, typedef int cairo_atomic_int_t; typedef intptr_t cairo_atomic_intptr_t; -static cairo_always_inline int -_cairo_atomic_int_get (cairo_atomic_int_t *x) -{ - __sync_synchronize (); - return *x; -} - static cairo_always_inline int _cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x) { @@ -252,6 +245,15 @@ _cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, int val) *(volatile int *)x = val; } +static cairo_always_inline int +_cairo_atomic_int_get (cairo_atomic_int_t *x) +{ + int result = _cairo_atomic_int_get_relaxed (x); + __sync_synchronize (); + + return result; +} + static cairo_always_inline void * _cairo_atomic_ptr_get_relaxed (cairo_atomic_intptr_t *x) { @@ -313,10 +315,18 @@ static_assert (sizeof (AO_t) >= sizeof (void *), "AO_t cannot be used for pointe typedef int32_t cairo_atomic_int_t; typedef intptr_t cairo_atomic_intptr_t; -# define _cairo_atomic_int_get(x) (OSMemoryBarrier(), *(x)) # define _cairo_atomic_int_get_relaxed(x) *(volatile int32_t *)(x) # define _cairo_atomic_int_set_relaxed(x, val) *(volatile int32_t *)(x) = (val) +static cairo_always_inline int +_cairo_atomic_int_get (cairo_atomic_int_t *x) +{ + int result = _cairo_atomic_int_get_relaxed (x); + OSMemoryBarrier (); + + return result; +} + # define _cairo_atomic_int_inc(x) ((void) OSAtomicIncrement32Barrier (x)) # define _cairo_atomic_int_dec(x) ((void) OSAtomicDecrement32Barrier (x)) # define _cairo_atomic_int_dec_and_test(x) (OSAtomicDecrement32Barrier (x) == 0) @@ -348,15 +358,21 @@ typedef intptr_t cairo_atomic_intptr_t; typedef LONG cairo_atomic_int_t; typedef PVOID cairo_atomic_intptr_t; +# define _cairo_atomic_int_get_relaxed(x) __iso_volatile_load32 ((__int32 *) x) +# define _cairo_atomic_int_set_relaxed(x, val) __iso_volatile_store32 ((__int32 *) x, (__int32) val) + static cairo_always_inline int _cairo_atomic_int_get (cairo_atomic_int_t *x) { + int result = _cairo_atomic_int_get_relaxed (x); +#if defined (_M_IX86) || defined (_M_AMD64) + _ReadWriteBarrier (); /* compiler-only */ +#else MemoryBarrier (); - return *x; -} +#endif -# define _cairo_atomic_int_get_relaxed(x) __iso_volatile_load32 ((__int32 *) x) -# define _cairo_atomic_int_set_relaxed(x, val) __iso_volatile_store32 ((__int32 *) x, (__int32) val) + return result; +} # define _cairo_atomic_int_inc(x) ((void) InterlockedIncrement (x)) # define _cairo_atomic_int_dec(x) ((void) InterlockedDecrement (x))