m

Precedence: bulk
MIME-Version: 1.0
References: <64CE143F-78B6-4A90-A6DF-2D1C8361902D@everwhe.re>
 <mvkhetitsgpcyces7ctq6vvkp5ma2jni7j2ybl6sdrmyme2dxh@qhuw576ud7v6> <05F97E03-03EF-4B21-88B3-7F94018CCF6F@everwhe.re>
In-Reply-To: <05F97E03-03EF-4B21-88B3-7F94018CCF6F@everwhe.re>
From: Jonathan Wakely <jwakely.gcc@gmail.com>
Date: Sat, 1 Mar 2025 11:59:35 +0000
Message-ID: <CAH6eHdSoRGJ-g8yLymvNH6NKrNFgTFryshBunHgexxiep6NiUA@mail.gmail.com>
Subject: Re: Incorrect const in futex(2) example code
To: rahl <rahl@everwhe.re>
Cc: Alejandro Colomar <alx@kernel.org>, linux-man@vger.kernel.org, gcc-help@gcc.gnu.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, 1 Mar 2025 at 11:36, rahl <rahl@everwhe.re> wrote:
>
> On 28 February 2025 20:57:50 UTC, Alejandro Colomar <alx@kernel.org> wrot=
e:
> >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 introdu=
ced 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 futex.2 examples used to use the non-standard
__sync_bool_compare_and_swap, which was correct. It was changed to use
atomic_compare_exchange_strong by:

commit 915c4ba36f9f71db88e7e7913b845d046996f485
Author: Benjamin Peterson <benjamin@python.org>
Date:   Tue Nov 13 22:53:41 2018 -0800

   futex.2: Make the example use C11 atomics rather than GCC builtins

That change was wrong and broke the examples, because C11 atomics are
only usable on _Atomic variables. __sync_bool_compare_and_swap and
__atomic_compare_exchange are usable with plain non-_Atomic types.


> The documentation linked above should clarify this as well.
>
> It turns out these calls could be replaced with a compiler built-in (__at=
omic_compare_exchange_n) both for clang and gcc, which wouldn't have the ab=
ove 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 th=
e full output from git-format-patch, with the message being in what I belie=
ve is the expected format - having read that an attachment is also an accep=
table 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