Avoid calling caa_container_of on NULL pointer in cds_lfht macros
The cds_lfht_for_each_entry and cds_lfht_for_each_entry_duplicate macros
would call caa_container_of() macro on NULL pointer. This is not a
problem under normal circumstances as the check in the for loop fails
and the loop-statement is not called with invalid (pos) value.
However AddressSanitizer doesn't like that and complains about this:
Move the cds_lfht_iter_get_node(iter) != NULL from the cond-expression
of the for loop into both init-clause and iteration-expression as
conditional operator and check for (pos) value in the cond-expression
instead. Introduce the cds_lfht_entry() macro to eliminate code
duplication.
Michael Jeanson [Thu, 23 Mar 2023 18:23:55 +0000 (14:23 -0400)]
fix: warning 'noreturn' function does return on ppc
On a ppc64 system with gcc 9.5.0 I get the following error when building
with -O0 :
/usr/include/urcu/uatomic/generic.h: In function 'void _uatomic_link_error()':
/usr/include/urcu/uatomic/generic.h:53:1: warning: 'noreturn' function does return
53 | }
| ^
Split the inline function in 2 variants and apply the noreturn attribute
only on the builtin_trap one.
Change-Id: I5ae8e764c4cc27af0463924a653b9eaa9f698c34 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Ondřej Surý [Fri, 17 Mar 2023 15:44:10 +0000 (16:44 +0100)]
Fix: use __noreturn__ for C11-compatibility
The noreturn convenience macro provided by stdnoreturn.h might get
included before urcu headers, use __noreturn__ for better compatibility
with code using <stdnoreturn.h> header.
Fix: call_rcu: teardown default call_rcu worker on application exit
Teardown the default call_rcu worker thread if there are no queued
callbacks on process exit. This prevents leaking memory.
Here is how an application can ensure graceful teardown of this
worker thread:
- An application queuing call_rcu callbacks should invoke
rcu_barrier() before it exits.
- When chaining call_rcu callbacks, the number of calls to
rcu_barrier() on application exit must match at least the maximum
number of chained callbacks.
- If an application chains callbacks endlessly, it would have to be
modified to stop chaining callbacks when it detects an application
exit (e.g. with a flag), and wait for quiescence with rcu_barrier()
after setting that flag.
- The statements above apply to a library which queues call_rcu
callbacks, only it needs to invoke rcu_barrier in its library
destructor.
Fix a deadlock for auto-resize hash tables when cds_lfht_destroy
is called with RCU read-side lock held.
Example stack track of a hang:
Thread 2 (Thread 0x7f21ba876700 (LWP 26114)):
#0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1 0x00007f21beba7aa0 in futex (val3=0, uaddr2=0x0, timeout=0x0, val=-1, op=0, uaddr=0x7f21bedac308 <urcu_memb_gp+8>) at ../include/urcu/futex.h:81
#2 futex_noasync (timeout=0x0, uaddr2=0x0, val3=0, val=-1, op=0, uaddr=0x7f21bedac308 <urcu_memb_gp+8>) at ../include/urcu/futex.h:90
#3 wait_gp () at urcu.c:265
#4 wait_for_readers (input_readers=input_readers@entry=0x7f21ba8751b0, cur_snap_readers=cur_snap_readers@entry=0x0,
qsreaders=qsreaders@entry=0x7f21ba8751c0) at urcu.c:357
#5 0x00007f21beba8339 in urcu_memb_synchronize_rcu () at urcu.c:498
#6 0x00007f21be99f93f in fini_table (last_order=<optimized out>, first_order=13, ht=0x5651cec75400) at rculfhash.c:1489
#7 _do_cds_lfht_shrink (new_size=<optimized out>, old_size=<optimized out>, ht=0x5651cec75400) at rculfhash.c:2001
#8 _do_cds_lfht_resize (ht=ht@entry=0x5651cec75400) at rculfhash.c:2023
#9 0x00007f21be99fa26 in do_resize_cb (work=0x5651e20621a0) at rculfhash.c:2063
#10 0x00007f21be99dbfd in workqueue_thread (arg=0x5651cec74a00) at workqueue.c:234
#11 0x00007f21bd7c06db in start_thread (arg=0x7f21ba876700) at pthread_create.c:463
#12 0x00007f21bd4e961f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Thread 1 (Thread 0x7f21bf285300 (LWP 26098)):
#0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1 0x00007f21be99d8b7 in futex (val3=0, uaddr2=0x0, timeout=0x0, val=-1, op=0, uaddr=0x5651d8b38584) at ../include/urcu/futex.h:81
#2 futex_async (timeout=0x0, uaddr2=0x0, val3=0, val=-1, op=0, uaddr=0x5651d8b38584) at ../include/urcu/futex.h:113
#3 futex_wait (futex=futex@entry=0x5651d8b38584) at workqueue.c:135
#4 0x00007f21be99e2c8 in urcu_workqueue_wait_completion (completion=completion@entry=0x5651d8b38580) at workqueue.c:423
#5 0x00007f21be99e3f9 in urcu_workqueue_flush_queued_work (workqueue=0x5651cec74a00) at workqueue.c:452
#6 0x00007f21be9a0c83 in cds_lfht_destroy (ht=0x5651d8b2fcf0, attr=attr@entry=0x0) at rculfhash.c:1906
This deadlock is easy to reproduce when rapidly adding a large number of
entries in the cds_lfht, removing them, and calling cds_lfht_destroy().
The deadlock will occur if the call to cds_lfht_destroy() takes place
while a resize of the hash table is ongoing.
Fix this by moving the teardown of the lfht worker thread to libcds
library destructor, so it does not have to wait on synchronize_rcu from
a resize callback from within a read-side critical section. As a
consequence, the atfork callbacks are left registered within each urcu
flavor for which a resizeable hash table is created until the end of the
executable lifetime.
The other part of the fix is to move the hash table destruction to the
worker thread for auto-resize hash tables. This prevents having to wait
for resize callbacks from RCU read-side critical section. This is
guaranteed by the fact that the worker thread serializes previously
queued resize callbacks before the destroy callback.
Christopher Ng [Fri, 3 Feb 2023 12:16:06 +0000 (12:16 +0000)]
Fix building on MSYS2
Update cygwin libtool config in `configure.ac` to match MSYS2 build
environments as well. MSYS2 is also a Windows build environment that
produces DLLs.
Signed-off-by: Christopher Ng <facboy@gmail.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I48ca648123fd40b8003c72c0447c70a8b4bde6d6
Eric Wong [Sun, 2 Oct 2022 16:13:43 +0000 (12:13 -0400)]
Fix: Always check pthread_create for failures
pthread_create may fail with EAGAIN (which is no fault of the
programmer), so don't allow the check to be compiled out.
Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ia2695ea6953b589ac8ab8b444fb668daee06a614
Simon Marchi [Wed, 17 Aug 2022 15:24:25 +0000 (11:24 -0400)]
Fix: change method used by _rcu_dereference to strip type constness
Commit 1e41ec3b07e4 ("Make temporary variable in _rcu_dereference
non-const") used the trick to add 0 to the pointer passed as a parameter
to the macro to get rid of its constness, should it be const (with the
end goal of avoiding compiler warnings). This is problematic (as shown
in [1]) if it is a pointer to an opaque type though, as the compiler
cannot perform pointer arithmetic on such a pointer (even though it
wouldn't really need to here, as we add 0).
Change it to use another trick to strip away the constness, that
shouldn't hit this problem. It was found in the same stackoverflow post
as the original trick [2]. It consists of using a statement expression
like so:
__typeof__(({ const int foo; foo; }))
The statement expression yields a value of type `int`. Statement
expressions are extensions to the C language, but we already use them
here.
Simon Marchi [Wed, 17 Aug 2022 17:11:21 +0000 (13:11 -0400)]
Fix: remove type constness in URCU_FORCE_CAST's C++ version
The test added by the following patch wouldn't compile, when built
without _LGPL_SOURCE:
CXX test_build_dynlink_cxx-test_build_cxx.o
In file included from ../../include/urcu/arch.h:25,
from /home/simark/src/urcu/tests/unit/test_build.c:28,
from /home/simark/src/urcu/tests/unit/test_build_cxx.cpp:3:
/home/simark/src/urcu/tests/unit/test_build.c: In function ‘void test_build_rcu_dereference()’:
/home/simark/src/urcu/include/urcu/compiler.h:85:42: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
85 | #define URCU_FORCE_CAST(type, arg) (reinterpret_cast<type>(arg))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/urcu/include/urcu/pointer.h:71:49: note: in expansion of macro ‘URCU_FORCE_CAST’
71 | __typeof__(p) _________p1 = URCU_FORCE_CAST(__typeof__(p), \
| ^~~~~~~~~~~~~~~
/home/simark/src/urcu/tests/unit/test_build.c:133:9: note: in expansion of macro ‘rcu_dereference’
133 | rcu_dereference(opaque_const);
| ^~~~~~~~~~~~~~~
The compiler complains that we do a cast to a const type, equivalent to:
reinterpret_cast<const int>(arg)
... and that the const is meaningless in this context.
Use std::remove_cv to strip away any const or volatile qualifiers from
the type (using a volatile type would result in the same warning).
Change-Id: I94e79fcccfc2108021752f65977e1548084c646a Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Simon Marchi [Wed, 17 Aug 2022 16:49:50 +0000 (12:49 -0400)]
Move extern "C" down in include/urcu/urcu-bp.h
A following patch adds a <type_traits> include in
urcu/compiler.h. However, compiler.h gets included by urcu/pointer.h,
which gets included by urcu/urcu-bp.h inside an extern "C" scope.
Including the C++ header file <type_traits> inside an extern "C" scope
doesn't work:
In file included from /home/simark/src/urcu/include/urcu/compiler.h:25,
from /home/simark/src/urcu/include/urcu/pointer.h:29,
from /home/simark/src/urcu/include/urcu/urcu-bp.h:58,
from /home/simark/src/urcu/include/urcu-bp.h:2,
from /home/simark/src/urcu/tests/unit/test_urcu_multiflavor-bp.c:28,
from /home/simark/src/urcu/tests/unit/test_urcu_multiflavor-bp_cxx.cpp:3:
/usr/include/c++/12.1.1/type_traits:44:3: error: template with C linkage
44 | template<typename _Tp>
| ^~~~~~~~
/home/simark/src/urcu/include/urcu/urcu-bp.h:41:1: note: ‘extern "C"’ linkage started here
41 | extern "C" {
| ^~~~~~~~~~
Move the extern "C" in urcu-bp.h down, so that the includes are not
inside it. Each header file is responsible to use extern "C" where
relevant, and we should avoid including files inside such a scope.
Change-Id: I42bdfa6ab445e8c40f5bcac1c1ae0786d443626c Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Mon, 15 Aug 2022 15:11:54 +0000 (11:11 -0400)]
fix: ifdef linux specific cpu count compat
Expand the '#ifdef __linux__' block in src/compat-cpu.h to all static
inline functions related to sysfs since they are only useful on Linux
and fail to build on some non-Linux platforms. This issue was reported
on QNX.
Thanks to Elad Lahav <e2lahav@gmail.com> for reporting this issue.
Change-Id: I17c88a9a2fb5b9be6cf5325234a18ff40788cd09 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Wed, 27 Jul 2022 14:44:00 +0000 (10:44 -0400)]
fix: sysconf(_SC_NPROCESSORS_CONF) can be less than max cpu id
We rely on sysconf(_SC_NPROCESSORS_CONF) to get the maximum possible
number of CPUs that can be attached to the system for the lifetime of an
application.
As such we expect that the highest possible CPU id would be one less
than the number returned by sysconf(_SC_NPROCESSORS_CONF) which is
unfortunatly not always the case and can vary across libc
implementations and versions.
Glibc up to 2.35 will count the number of "cpuX" directories in
"/sys/devices/system/cpu" which doesn't include CPUS that were
hot-unplugged.
This information is however provided by the Linux kernel in
"/sys/devices/system/cpu/possible" in the form of a mask listing all the
CPUs that could possibly be hot-plugged in the system.
This patch replaces sysconf(_SC_NPROCESSORS_CONF) with an internal
function that first tries parsing the possible CPU mask to extract the
highest possible value and if this fails fallback to the previous
behavior.
Change-Id: I68dfed42ebbab02728a02eeefd4a395a22bb1bea Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Shao-Tse Hung [Tue, 2 Aug 2022 17:44:00 +0000 (01:44 +0800)]
Fix: revise obsolete command in README.md
The obsolete command `make bench` was replaced by `make short_bench` and
`make long_bench` in 2015. However, this command wasn't revised in
README, so I follow the previous commit and rewrite it.
The urcu-qsbr wait_gp() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_gp can cause wait_gp to return with a
urcu_qsbr_gp.futex state of -1, which is unexpected. It would cause the
following loops in wait_for_readers() to decrement the
urcu_qsbr_gp.futex to values below -1, thus actively using CPU as values
will be decremented to very low negative values until it reaches 0
through underflow, or until the input_readers list is found to be empty.
The state is restored to 0 when the input_readers list is found to be
empty, which restores the futex state to a correct state for the
following calls to wait_for_readers().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu wait_gp() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_gp can cause wait_gp to return with a
rcu_gp.futex state of -1, which is unexpected. It would cause the
following loops in wait_for_readers() to decrement the
rcu_gp.futex to values below -1, thus actively using CPU as values
will be decremented to very low negative values until it reaches 0
through underflow, or until the input_readers list is found to be empty.
The state is restored to 0 when the input_readers list is found to be
empty, which restores the futex state to a correct state for the
following calls to wait_for_readers().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu-wait urcu_adaptative_busy_wait() implements a futex wait/wakeup
scheme similar to the workqueue code, which has an issue with spurious
wakeups.
A spurious wakeup on urcu_adaptative_busy_wait can cause
urcu_adaptative_busy_wait to reach label skip_futex_wait with a
wait->state state of URCU_WAIT_WAITING, which is unexpected. It would
cause busy-waiting on URCU_WAIT_TEARDOWN state to start early. The
wait-teardown stage is done with URCU_WAIT_ATTEMPTS active attempts,
following by attempts spaced by 10ms sleeps. I do not expect that these
spurious wakeups will cause user-observable effects other than being
slightly less efficient that it should be.
urcu-wait is used by all urcu flavor's synchronize_rcu() to implement
the grace period batching scheme.
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from
URCU_WAIT_WAITING in user-space after the call to FUTEX_WAIT returns 0.
The urcu-defer wait_defer() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_defer can cause wait_defer to return with a
defer_thread_futex state of -1, which is unexpected. It would cause the
following loops in thr_defer() to decrement the defer_thread_futex to
values below -1, thus actively using CPU as values will be decremented
to very low negative values until it reaches 0 through underflow, or
until callbacks are eventually queued. The state is restored to 0 when
callbacks are found, which restores the futex state to a correct state
for the following calls to wait_defer().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu call_rcu() and rcu_barrier() each implement a futex wait/wakeup
scheme identical to the workqueue code, which has an issue with spurious
wakeups.
* call_rcu
A spurious wakeup on call_rcu_wait can cause call_rcu_wait to return
with a crdp->futex state of -1, which is unexpected. It would cause the
following loops in call_rcu_thread() to decrement the crdp->futex to
values below -1, thus actively using CPU time as values will be
decremented to very low negative values until the futex value underflows
back to 0. The state is *not* restored to 0 when the callback list is
found to be non-empty, so this unexpected state will persist until the
crdp->futex state underflows back to 0, or until the call_rcu_thread is
stopped. What prevents this from having too much user-observable effects
is that the call rcu thread has a 10ms sleep between loops, to favor
batching of callbacks. Therefore, rather than being a purely 100% active
busy-wait, this scenario leads to a busy-wait which is paced by 10ms
sleeps.
Therefore the observed issue will be that the call_rcu_thread will
unexpectedly wake up the CPU each 10ms after this spurious wakeup
happens.
* rcu_barrier
A spurious wakeup on call_rcu_completion_wait can cause
call_rcu_completion_wait to return with a completion->futex state of -1,
which is unexpected. It would cause the following loops in rcu_barrier()
to decrement the completion->futex to values below -1, thus actively
using CPU time as values will be decremented to very low negative values
until either the barrier count reaches 0 or until the futex value
underflows to 0.
Therefore the observed issue will be that rcu_barrier() will
unexpectedly use a lot of CPU time when this spurious wakeup happens.
These issues will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The workqueue thread futex_wait() returns with a workqueue->futex state
of -1, which is unexpected. In this situation, the workqueue thread is
observed to use 99% of CPU as workqueue->futex values are decremented to
very low negative values while the workqueue is empty.
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
Michael Jeanson [Tue, 7 Dec 2021 19:42:26 +0000 (14:42 -0500)]
fix: properly detect 'cmpxchg' on x86-32
We wrongly assumed that on x86-32 when '__i386__' is defined but none of
'__i486__', '__i586__' or '__i686__' that the target arch is a literal
i386 cpu without the cmpxchg instructions. However, when building with
'-march=core2' we get '__i386__' but none of the others even if the arch
is newer than an i686.
Change the compat code to use the '__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4'
builtin define to detect an x86-32 system without the cmpxchg
instructions.
Since this builtin define was introduced in GCC 4.3 and Clang 3.3,
building with older compilers on any x86-32 system will enable the
compat layer regardless of the availability of the instructions.
Change-Id: I8329431e55d778405b2ca7007d90c2c6e5cdd426 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Simon Marchi [Fri, 30 Jul 2021 03:06:11 +0000 (23:06 -0400)]
Make temporary variable in _rcu_dereference non-const
When building the lttng-tools project with Ubuntu's gcc 11, I get the
following error:
CC agent.lo
In file included from /tmp/lttng/include/urcu/arch.h:25,
from /tmp/lttng/include/urcu/uatomic.h:23,
from /home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c:11:
/home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c: In function ‘agent_update’:
/tmp/lttng/include/urcu/static/pointer.h:96:33: error: argument 2 of ‘__atomic_load’ discards ‘const’ qualifier [-Werror=incompatible-pointer-types]
96 | __atomic_load(&(p), &_________p1, __ATOMIC_CONSUME); \
| ^~~~~~~~~~~~~
/tmp/lttng/include/urcu/compiler.h:69:70: note: in definition of macro ‘caa_container_of’
69 | const __typeof__(((type *) NULL)->member) * __ptr = (ptr); \
| ^~~
/tmp/lttng/include/urcu/rculist.h:87:20: note: in expansion of macro ‘cds_list_entry’
87 | for (pos = cds_list_entry(rcu_dereference((head)->next), __typeof__(*(pos)), member); \
| ^~~~~~~~~~~~~~
/tmp/lttng/include/urcu/pointer.h:47:33: note: in expansion of macro ‘_rcu_dereference’
47 | #define rcu_dereference _rcu_dereference
| ^~~~~~~~~~~~~~~~
/tmp/lttng/include/urcu/rculist.h:87:35: note: in expansion of macro ‘rcu_dereference’
87 | for (pos = cds_list_entry(rcu_dereference((head)->next), __typeof__(*(pos)), member); \
| ^~~~~~~~~~~~~~~
/home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c:1551:9: note: in expansion of macro ‘cds_list_for_each_entry_rcu’
1551 | cds_list_for_each_entry_rcu(ctx, &agt->app_ctx_list, list_node) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
This is because the pointer passed to _rcu_dereference is const (the
pointer itself is const, IIUC, not necessarily the data it points to),
so the temporary _________p1 is also declared as const. We therefore
can't pass a non-const pointer to it to a function that modifies it.
I applied the trick found here [1] with success to get rid of the
constness of the variable. With this change, lttng-tools compiles
successfully with gcc 11.
There may be other spots in the headers where this would be needed, but
it is hard to spot them. I think we would need to write some test file
that pass const pointers to all macros of the API and see if they
compile.
Fix: x86 and s390 uatomic: __hp() macro warning with gcc 11
The __hp() macro used in the x86 and s390 uatomic code generates the
following warning with gcc-11:
In file included from ../include/urcu/uatomic.h:27,
from ../include/urcu/static/wfcqueue.h:35,
from ../include/urcu/wfcqueue.h:133,
from workqueue.c:39:
workqueue.c: In function ‘workqueue_thread’:
../include/urcu/uatomic/x86.h:155:17: warning: array subscript ‘struct __uatomic_dummy[0]’ is partly outside array bounds of ‘struct cds_wfcq_tail[1]’ [-Warray-bounds]
155 | __asm__ __volatile__(
| ^~~~~~~
workqueue.c:184:38: note: while referencing ‘cbs_tmp_tail’
184 | struct cds_wfcq_tail cbs_tmp_tail;
| ^~~~~~~~~~~~
The (previously undocumented) reason for this macro is to allow passing the
"void *" parameter as "m" or "+m" operand to the inline assembly. That
motivation was explained in commit 53b8ed6836363 ("s390 uatomic arch fix").
The out of bound access is detected by gcc because struct
__uatomic_dummy's length is quite large: an array of 10 unsigned long,
which is larger than the size pointed to by the void pointer.
So rather than using a fixed-size type, cast to a structure containing
an array of characters of a size matching the @addr input argument.
While we are at it and digging out git archeology, properly document the
__hp() macro for posterity.
Michael Jeanson [Wed, 2 Jun 2021 14:55:22 +0000 (10:55 -0400)]
Add serialized ABI definition files
This commit contains the serialized ABI definitions for a typical build
of the liburcu librairies. This information is extracted using
libabigail (https://sourceware.org/libabigail/).
The artefacts used to generate these were built with CFLAGS="-O0 -ggdb".
You can compare the serialized ABI with a shared object to check for
breaking changes. For example, here we compare an in-tree built version
of liburcu-memb.so with the serialized ABI of stable-0.13 :
Michael Jeanson [Tue, 1 Jun 2021 21:01:49 +0000 (17:01 -0400)]
bump SONAME major to 8
In URCU 0.11, we introduced new symbols to clean up the library symbol
namespacing, using the "alias" attribute to keep emitting the old
symbols, expecting to preserve ABI backward compatibility.
Unfortunately, it turns out that even though it works well for function
symbols, it is broken for public global variables due to the way ELF
copy relocation works.
When building a non-PIC executable that uses an extern variable, a .bss
symbol is emitted in the executable. This will take precedence over the
symbol implemented within the library in the Global Symbol Table.
Unfortunately, the alias within the library will not be aware that the
actual GST symbol differs from its alias within the library, and the
addresses for the symbol and its alias will differ at runtime.
Considering that this compatibility issue affects official library
releases, there is little we can do beyond documenting this issue, and
bumping the Userspace RCU major soname for the next (0.13) release.
Change-Id: I0ca8407dcffd871f025814923c6e329ec260133a Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Fri, 7 May 2021 15:34:33 +0000 (11:34 -0400)]
fix: clock_gettime on macOs
Newer version of macOs have an implementation of clock_gettime() that
requires additionnal setup, move the platform specific code first so it
is always used.
Change-Id: I12fcdeff6c0ae59bc1a13f4e2cd7f4ebcedfc253 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: rculist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix rculist.h to follow this coding style.
Fix: rcuhlist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix rcuhlist.h to follow this coding style.
Fix: hlist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix hlist.h to follow this coding style.
Fix: list.h: use parenthesis around macro parameters, caa_container_of()
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix list.h to follow this coding style.
Use caa_container_of() for cds_list_entry rather than open-code the
pointer arithmetic.
Comparing an offset from an object with NULL is undefined behavior
and the compiler may assume that this is never true.
This is indeed what is observed with gcc-10 miscompiling
cds_hlist_for_each_entry_rcu_2().
Fix this by introducing cds_hlist_entry_safe() rather than open-coding
the NULL check comparisons, and move cds_hlist_for_each_entry_2()
and cds_hlist_for_each_entry_safe_2() to this scheme as well.
Fix: use __atomic_load() rather than atomic load explicit
Use __atomic_load (gcc extension) rather than atomic load explict
(C11/C++11) for rcu_dereference because it does not require the input
type to be _Atomic. This fixes a regression with clang introduced by
commit 380f4b19052 ("Fix: use atomic load memory_order_consume for
rcu_dereference on C11/C++11").
Note that the cmm_smp_read_barrier_depends is removed when using
__ATOMIC_CONSUME because their memory ordering effect is redundant.
Michael Jeanson [Tue, 13 Apr 2021 20:19:06 +0000 (16:19 -0400)]
fix: HAVE_SCHED_SETAFFINITY is not defined
Use '#ifdef' instead of '#if' to test if HAVE_SCHED_SETAFFINITY is
defined. Both work but using '#if' on an undefined macro will generate a
warning with '-Wundef'.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ieb46ddab9ba033a5c552dbe78ac398cea0a641e8
Michael Jeanson [Thu, 1 Apr 2021 18:39:01 +0000 (14:39 -0400)]
cleanup: explicitly mark unused parameters (-Wunused-parameter)
Add the 'unused' attribute to function parameters that are unused to
allow turning on -Wunused-parameter and distinguish unused parameters
that are actual errors.
Change-Id: Ie585e37f9d38718543a31aee2e7ab3428cdfd0a5 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Thu, 18 Mar 2021 23:53:28 +0000 (19:53 -0400)]
Introduce AE_FEATURE to manage configure features
The new AE_FEATURE set of macros are wrappers over autoconf's
AC_ARG_ENABLE. The main objective is to make the m4sh code more readable
to the less seasoned autotools enthusiast among us and reduce the
duplication of code with its associated bugs.
The AE prefix was chosen to mean "Autotools EfficiOS" and is part of an
effort to standardize our custom macros across all our autotools based
projects.
Change-Id: Ief565473b38150fe2104492c6339bac73efba895 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Thu, 18 Mar 2021 21:50:48 +0000 (17:50 -0400)]
tests: Move tap-driver.sh out of the autotools aux directory
We have made local modifications to this script, move it to the test
suite directory so it doesn't get overwritten by an updated version from
autotools.
This is part of an effort to standardise our autotools setup across
project to simplify maintenance.
Change-Id: I4bc0bdbff05d03d8e4c0f9dcda51cf6a6cf7fec4 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Mon, 25 Jan 2021 18:36:24 +0000 (13:36 -0500)]
fix: exclude clang from GCC version blacklists
URCU_GCC_VERSION is used to blacklist specific GCC versions with known
bugs, clang also defines these macros to an equivalent GCC version it
claims to support, so exclude it.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Idf0980fddca6533313a3367601ddda8d8e13bfdf
The newly-released autoconf 2.70 introduces a number of breaking
changes [1] and is being rolled-out by some distros.
Amongst those changes, the AC_PROG_CC_STDC macro is marked as obsolete
and was merged into AC_PROG_CC, which we already use. On 2.70, this
results in a warning which we handle as an error.
A version check is added to invoke the AC_PROG_CC_STDC macro only when
running a pre-2.70 version of autoconf, fixing the issue.
Also, the AX_PTHREAD macro makes use of the $as_echo built-in shell
variable which no longer exists in 2.70. A patch was submitted to the
GNU Autoconf archive in March, but there have been no signs of life
given since then [2].
As such, our local copy is updated to the latest version and the patch
(which looks fairly straight-forward / safe) is applied. This should
minimize changes once we go back to an "official" version of the macro.
Michael Jeanson [Fri, 11 Dec 2020 18:52:10 +0000 (13:52 -0500)]
Don't force a target and optimization level on ARMv7
We shouldn't force a specific target cpu for the compiler on ARMv7 but
let the system or the user choose it. If some of our code depends on a
specific target CPU features, it should be compile tested.
Also remove the default optimisation level of O1, it's potentially a
workaround to an early armv7 compiler performance problem and anyway
most builds will have an optimisation level flag set in the CFLAGS which
will override this one.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Acked-by: Paul E. McKenney <paulmck@kernel.org> Cc: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I1d1bb5cc0fa0be8f8b1d6a9ad7bf063809be1aef
Michael Jeanson [Thu, 10 Dec 2020 21:52:30 +0000 (16:52 -0500)]
Use DMB only on ARMv7
Remove the configure time CONFIG_RCU_ARM_HAVE_DMB option and replace it
by compile time detection based on the ARM ISA version. This makes sure
we unconditionnaly use the DMB instruction only on ARMv7 where it's part
of the baseline ISA.
This will change the behavior on ARMv6 platform that possibly have this
instruction but it was probably already broken since we use the 'ISH'
option which doesn't seem to be valid on this ISA.
This will also allow sharing headers in a multi-arch environment and
reduce the build system complexity.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Cc: Jason Wessel <jason.wessel@windriver.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I8e56ada55148d8e0f198c3d2e741ea414de5fef2
Michael Jeanson [Wed, 9 Dec 2020 17:05:31 +0000 (12:05 -0500)]
fix: bump tests thread limit to 4096
Machines with more than 128 CPUs are becomming more common. A
future-proof fix here would be to dynamically allocate the array, but in
the meantime bump the limit to 4096 to fix the problem on a 160 CPUs
ppc64el system where this was reported.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Cc: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ib3cb5d8cb4515e6f626be33c2685fa38cb081782
Michael Jeanson [Tue, 17 Nov 2020 23:28:20 +0000 (18:28 -0500)]
Compile time futex syscall detection
The futex syscall availability is detected at configure time using a
simple compile test. Replace this with compile time detection using the
same test. This would allow sharing headers in a multi-arch environment
and reduce the build system complexity.
Change-Id: Ief709429e8908d0fd2d268d42f4a89d9fc14b91b Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Tue, 17 Nov 2020 23:08:49 +0000 (18:08 -0500)]
Static arch and uatomic headers
Replace the somewhat hackish symlinking of architecture specific headers
at configure time with compile time detection using compiler defines.
This would allow sharing headers in a multi-arch environment and reduce
the build system complexity.
Change-Id: I4900a18dab70bb0e51d1a8500581a445acd44cdf Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
projects / userspace-rcu.git / log
