From ca541c6b8c6ce1c166329fa1c19345d8f7794a5a Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 23 Jun 2026 14:02:10 -0400 Subject: [PATCH] urcu-pointer: Add rcu_dereference_sym2() taking the pointer address The non-LGPL rcu_dereference() macro passed the RCU-protected pointer to rcu_dereference_sym() by value, forcing the load of the pointer to happen in the caller as a plain (non-atomic, non-volatile) access. The previous commit added a barrier to recover the publication *ordering*, but a barrier cannot recover the *atomicity* of the load: it still happens in the caller, where the compiler may tear, fuse, duplicate, refetch or value-speculate it, and it remains a data race reported by ThreadSanitizer (in builds that do not define _LGPL_SOURCE). Introduce rcu_dereference_sym2(), which takes the *address* of the RCU-protected pointer and performs the memory_order_consume load on the shared location itself, mirroring rcu_{set,xchg,cmpxchg}_pointer_sym(). Point the rcu_dereference() macro at the new symbol, passing &(p). The load is now a single atomic consume load on the actual location, which fixes both problems at once: the access is atomic (no tearing, fusing, duplication or value speculation) and correctly ordered, and the data race is gone. No barrier is needed. rcu_dereference_sym() is kept exported and unchanged (with its barrier) for ABI compatibility with code already built against the old macro. This adds a new interface to the liburcu libraries, so the libtool version-info is bumped (current and age incremented). The soname is unchanged since the addition is backward compatible. Signed-off-by: Mathieu Desnoyers Change-Id: I972218ee8ff6c76edc3563921fdb0720477c02af --- configure.ac | 4 ++-- include/urcu/compiler.h | 13 ++++++++++++- include/urcu/pointer.h | 12 +++++++++++- src/urcu-pointer.c | 23 ++++++++++++++++++++++- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 743e57546..8d8c423d0 100644 --- a/configure.ac +++ b/configure.ac @@ -14,9 +14,9 @@ m4_define([urcu_version], urcu_version_major[.]urcu_version_minor[.]urcu_version # Library version information of "liburcu" # Following the numbering scheme proposed by libtool for the library version # http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html -m4_define([urcu_lib_version_current], [9]) +m4_define([urcu_lib_version_current], [10]) m4_define([urcu_lib_version_revision], [0]) -m4_define([urcu_lib_version_age], [1]) +m4_define([urcu_lib_version_age], [2]) m4_define([urcu_lib_version], urcu_lib_version_current[:]urcu_lib_version_revision[:]urcu_lib_version_age) diff --git a/include/urcu/compiler.h b/include/urcu/compiler.h index aa260f72b..263f22148 100644 --- a/include/urcu/compiler.h +++ b/include/urcu/compiler.h @@ -150,7 +150,18 @@ #define __rcu #ifdef __cplusplus -#define URCU_FORCE_CAST(_type, arg) (reinterpret_cast::type>(arg)) +/* + * Force a cast between pointer types, matching the C "(type) arg" behavior of + * disregarding any cv-qualifier on the source. Stripping the source qualifiers + * is required e.g. for rcu_dereference() of a const-qualified pointer lvalue, + * where &(p) has type "T * const *" and must be passed as "void **": a plain + * reinterpret_cast cannot cast away that inner const. All users of this macro + * cast between (object) pointer types. + */ +#define URCU_FORCE_CAST(_type, arg) \ + (reinterpret_cast::type>( \ + const_cast( \ + reinterpret_cast(arg)))) #else #define URCU_FORCE_CAST(type, arg) ((type) (arg)) #endif diff --git a/include/urcu/pointer.h b/include/urcu/pointer.h index 7a673d619..723d285d9 100644 --- a/include/urcu/pointer.h +++ b/include/urcu/pointer.h @@ -50,12 +50,22 @@ extern "C" { #else /* !(defined(_LGPL_SOURCE) || defined(URCU_INLINE_SMALL_FUNCTIONS)) */ +/* + * rcu_dereference_sym() (taking the pointer by value) is kept exported for + * ABI compatibility only: the load of the RCU-protected pointer happened in + * the caller as a racy plain access, losing the memory_order_consume + * ordering. New code passes the address of the pointer to + * rcu_dereference_sym2() so the consume load is performed on the shared + * location itself. As with the inlined _rcu_dereference(), @p must be an + * lvalue. + */ extern void *rcu_dereference_sym(void *p); +extern void *rcu_dereference_sym2(void **p); #define rcu_dereference(p) \ __extension__ \ ({ \ __typeof__(p) _________p1 = URCU_FORCE_CAST(__typeof__(p), \ - rcu_dereference_sym(URCU_FORCE_CAST(void *, p))); \ + rcu_dereference_sym2(URCU_FORCE_CAST(void **, &(p)))); \ (_________p1); \ }) diff --git a/src/urcu-pointer.c b/src/urcu-pointer.c index 0b42f20c1..425315a5b 100644 --- a/src/urcu-pointer.c +++ b/src/urcu-pointer.c @@ -18,6 +18,9 @@ extern void synchronize_rcu(void); /* + * Kept for ABI compatibility with code built against liburcu releases that + * emitted calls to this symbol. + * * This wrapper receives the RCU-protected pointer by *value*: the load of * the pointer therefore already happened in the caller as a plain * (non-atomic) access, before this function is even entered. As a result @@ -36,7 +39,14 @@ extern void synchronize_rcu(void); * pointer load is already ordered before any dependent access, and the * bug cannot manifest. (TSO permits only store-to-load reordering, which * is irrelevant here since the pointer access is a load.) Elide the - * barrier there to avoid penalising the common case. + * barrier there to avoid penalising the common case. Note that the plain + * load itself stays in the caller, so it remains a data race reported by + * ThreadSanitizer. + * + * New code emitted by the rcu_dereference() macro calls + * rcu_dereference_sym2() instead, which receives the address of the + * pointer and performs the consume load on the shared location itself, + * avoiding both the ordering and the data-race problems. */ void *rcu_dereference_sym(void *p) { @@ -46,6 +56,17 @@ void *rcu_dereference_sym(void *p) return p; } +/* + * Receives the address of the RCU-protected pointer, so the + * memory_order_consume load is performed on the actual shared location + * rather than on a local copy. This preserves the dependency-ordering + * guarantee and is free of the data race affecting rcu_dereference_sym(). + */ +void *rcu_dereference_sym2(void **p) +{ + return _rcu_dereference(*p); +} + void *rcu_set_pointer_sym(void **p, void *v) { uatomic_store(p, v, CMM_RELEASE);