On 28 February 2025 20:57:50 UTC, Alejandro Colomar <alx@xxxxxxxxxx> wrote: >On Fri, Feb 28, 2025 at 10:13:04AM +0000, rahl wrote: >> I noticed that 'const' is used for both 'one' and 'zero' variables in >> the Examples section demo code of manpage 'futex(2)'. >> >> The variables are both used in calls to >> 'atomic_compare_exchange_strong()' where the 'const' is discarded as >> it may write to the 'expected' parameter during a "failure" case. > >I don't understand what that function is. It doesn't have a manual >page, and it's neither in /usr/include. It doesn't appear in the GCC >manual either. And it's not described in ISO C. > >What is that function? > >I'd like to understand what we're calling to be able to understand how >the calling code is wrong. atomic_compare_exchange_XXX() are defined in stdatomic.h and were introduced in C11 (ISO/IEC 9899:2011 I believe). The main online documentation for these that I'm currently aware of is at cppreference. <https://en.cppreference.com/w/c/atomic/atomic_compare_exchange> >BTW, there seem to be other important bugs in that example program, >which I don't understand either. Would you mind having a look at those >(I'm assuming that you seem familiar with these atomic APIs)? See: You're right, there are more bugs. I'm however not so familiar with these functions, but I do have some help. The errors in question relate to a missing _Atomic qualifier for several variables and function parameters. The documentation linked above should clarify this as well. It turns out these calls could be replaced with a compiler built-in (__atomic_compare_exchange_n) both for clang and gcc, which wouldn't have the above problem, and would also allow for the removal of 'stdatomic.h'. However, this didn't feel too in keeping with manpage example code. The gcc docs are here: <https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html> >> Attached is the trivial patch to remove the offending qualifiers. > >Please provide a complete patch, with a commit message. See the files >under <CONTRIBUTING.d/>. Rather than submit a separate email, I have attached a file containing the full output from git-format-patch, with the message being in what I believe is the expected format - having read that an attachment is also an acceptable approach, albeit perhaps not in email form. If for some reason you feel it should be submitted as a separate email I can do so. This way seemed more prudent for now as this is my first such patch and I'd like to get it right before having multiple threads in the ML. Let me know if there are any further issues. Cheers, rahl
From cf6f0122481ee8e8b3dc88532a4ccb6b4513e09d Mon Sep 17 00:00:00 2001
From: rahl <rahl@xxxxxxxxxx>
Date: Sat, 1 Mar 2025 10:40:18 +0000
Subject: [PATCH] man/man2/futex.2: Fix demo code example
To: Alejandro Colomar <alx@xxxxxxxxxx>
Cc: <linux-man@xxxxxxxxxxxxxxx>
This began by noticing that the use of const for the futex# variables was
incorrect given that their values may be altered by calls to
atomic_compare_exchange_strong().
Further, while gcc is silent on the matter, clang's implementation of
__c11_atomic_exchange_compare_strong() (called via the function macro
atomic_exchange_compare_strong) throws an error due to it requiring an _Atomic
type.
This patch is an attempt to make the minimal amount of change to fix these
issues.
During implementation, NRK pointed out that the 'one' variable need only exist
within the relevant 'while' loop as it's value would need to be reinitialised
on every iteration (having been altered after a failed call to
atomic_compare_exchange_strong).
Co-authored-by: nrk <nrk@xxxxxxxxxxx>
Signed-off-by: rahl <rahl@xxxxxxxxxx>
---
man/man2/futex.2 | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/man/man2/futex.2 b/man/man2/futex.2
index 70c3956ab..5432ba316 100644
--- a/man/man2/futex.2
+++ b/man/man2/futex.2
@@ -1806,10 +1806,11 @@ Child (18535) 4
#include <sys/wait.h>
#include <unistd.h>
\&
-static uint32_t *futex1, *futex2, *iaddr;
+static _Atomic uint32_t *futex1, *futex2;
+static uint32_t *iaddr;
\&
static int
-futex(uint32_t *uaddr, int futex_op, uint32_t val,
+futex(_Atomic uint32_t *uaddr, int futex_op, uint32_t val,
const struct timespec *timeout, uint32_t *uaddr2, uint32_t val3)
{
return syscall(SYS_futex, uaddr, futex_op, val,
@@ -1820,20 +1821,25 @@ futex(uint32_t *uaddr, int futex_op, uint32_t val,
become 1, and then set the value to 0. */
\&
static void
-fwait(uint32_t *futexp)
+fwait(_Atomic uint32_t *futexp)
{
- long s;
- const uint32_t one = 1;
+ long s;
\&
- /* atomic_compare_exchange_strong(ptr, oldval, newval)
+ /* atomic_compare_exchange_strong(T *obj, T *expected, T desired)
atomically performs the equivalent of:
\&
- if (*ptr == *oldval)
- *ptr = newval;
-\&
- It returns true if the test yielded true and *ptr was updated. */
+ if (memcmp(obj, expected, sizeof *obj) == 0) {
+ memcpy(obj, &desired, sizeof *obj);
+ return true;
+ } else {
+ memcpy(expected, obj, sizeof *obj);
+ return false;
+ }
+ */
\&
while (1) {
+\&
+ uint32_t one = 1;
\&
/* Is the futex available? */
if (atomic_compare_exchange_strong(futexp, &one, 0))
@@ -1852,10 +1858,10 @@ fwait(uint32_t *futexp)
so that if the peer is blocked in fwait(), it can proceed. */
\&
static void
-fpost(uint32_t *futexp)
+fpost(_Atomic uint32_t *futexp)
{
- long s;
- const uint32_t zero = 0;
+ long s;
+ uint32_t zero = 0;
\&
/* atomic_compare_exchange_strong() was described
in comments above. */
@@ -1887,8 +1893,8 @@ main(int argc, char *argv[])
if (iaddr == MAP_FAILED)
err(EXIT_FAILURE, "mmap");
\&
- futex1 = &iaddr[0];
- futex2 = &iaddr[1];
+ futex1 = (_Atomic uint32_t *)&iaddr[0];
+ futex2 = (_Atomic uint32_t *)&iaddr[1];
\&
*futex1 = 0; /* State: unavailable */
*futex2 = 1; /* State: available */
--
2.48.1
Attachment:
signature.asc
Description: PGP signature
