Re: [PATCH] signal handlers: volatile sigatomic_t, not volatile OR sigatomic_t

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry, that's not at all what I meant. I used sleep() just to simulate
the main function doing something while signals arrive asynchronously.
What you gave in your example is exactly the situation that does
require volatile, per my previous email, as the main function
constantly polls the shared value without any function calls in reads.

--Elad

On Wed, Mar 20, 2024 at 8:34 PM Guilherme Janczak
<guilherme.janczak@xxxxxxxxxx> wrote:
>
> In the case in your example, sleep() doesn't need to be implemented
> using SIGALRM. It isn't in glibc and OpenBSD that I know of:
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/sleep.c;h=3df79097c4464510f0197138a00a0c9772a5e83e;hb=3ab9b88e2ac91062b6d493fe32bd101a55006c6a
> https://man.openbsd.org/sleep.3
>
> I managed to make GCC and Clang optimize out a sig_atomic_t store:
> ```
> #define _POSIX_C_SOURCE 200809L
>
> #include <sys/time.h>
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <signal.h>
> #include <string.h>
>
> static void catch_sigalrm(int);
>
> #ifdef VOLATILE
> volatile sig_atomic_t msg;
> static const char *decl = "volatile sig_atomic_t msg";
> #else
> sig_atomic_t msg;
> static const char *decl = "sig_atomic_t msg";
> #endif
>
> volatile sig_atomic_t reply;
> volatile sig_atomic_t replied;
>
> int
> main(void)
> {
>         const struct sigaction act = { .sa_handler = catch_sigalrm };
>         const struct itimerval timer = {
>                 .it_value.tv_usec = 1,
>                 .it_interval.tv_usec = 1,
>         };
>         sigaction(SIGALRM, &act, NULL);
>         setitimer(ITIMER_REAL, &timer, NULL);
>         while (!replied) {
>                 msg = 1;
>         }
>         printf("%s == %d\n", decl, (int)reply);
> }
>
> static void
> catch_sigalrm(int unused)
> {
>         int n = msg;
>         if (!n)
>                 return;
>         reply = n;
>         replied = 1;
> }
> ```
>
> This runs forever:
> ```
> $ cc -O2 test2.c && ./a.out
> ```
> This terminates:
> ```
> $ clang -O2 -DVOLATILE test2.c && ./a.out
> volatile sig_atomic_t msg == 1
> ```
>
> Without volatile in its declaration, msg is never set, so the program
> spins forever because the signal handler never tells the loop to stop.
> There's probably a simpler way to do this.
>
> On Wed, Mar 20, 2024 at 07:20:03PM -0400, Elad Lahav wrote:
> > Actually, in the example I cited, there are multiple function calls
> > (printf, raise) between the two reads. How can the compiler optimize
> > out reading the value a second time?
> >
> > My view, which is subject to change at any moment and without notice,
> > is that you need volatile in the cases where you normally need
> > volatile, rather than inherently whenever you use sig_atomic_t. In the
> > following example I would not expect you to need it:
> >
> > static sig_atomic_t value;
> >
> > void
> > sig_handler(int signum)
> > {
> >     value++;
> > }
> >
> > void
> > func(void)
> > {
> >     value = 0;
> >     sleep(1);
> >     printf("There have been %d signals while I was napping\n", value);
> > }
> >
> > --Elad
> >
> > On Wed, Mar 20, 2024 at 7:05 PM Elad Lahav <e2lahav@xxxxxxxxx> wrote:
> > >
> > > No problem...
> > > Yes, in your example the issue is that the type is not atomic, and
> > > thus subject to partial updates that can be interrupted.
> > > Looking at the example given in
> > > https://en.cppreference.com/w/c/program/sig_atomic_t the volatile is
> > > needed to let the compiler know that the value can be updated in
> > > between two reads by the main function. That makes sense, especially
> > > if you have code that loops waiting for the value to change.
> > >
> > > --Elad
> > >
> > > On Wed, Mar 20, 2024 at 6:39 PM Guilherme Janczak
> > > <guilherme.janczak@xxxxxxxxxx> wrote:
> > > >
> > > > Actually, uh, I misread your reply, forget the previous reply I sent.
> > > >
> > > > You don't need the volatile with lock-free atomics, but the standard
> > > > says you do need it with sig_atomic_t. I don't know of a case that would
> > > > break a plain `sig_atomic_t` variable with no `volatile`, however.
> > > >
> > > > On Wed, Mar 20, 2024 at 04:44:33PM -0400, Elad Lahav wrote:
> > > > > Do you really need volatile?
> > > > > There are two cases to consider. Either your code synchronizes updates
> > > > > to the shared value with the signal handler (e.g., by blocking and
> > > > > then unblocking the signal), in which case I believe the compiler
> > > > > cannot ignore updates to the value; or you don't, and you can't depend
> > > > > on the variable having any specific value in the signal handler. The
> > > > > only thing you want to prevent in the latter case is the handler
> > > > > observing a partial update to the variable, which I presume is where
> > > > > the other requirements originate. (In practice, there should be little
> > > > > or no concern with any primitive type on modern hardware).
> > > > >
> > > > > --Elad
> > > > >
> > > > > On Wed, Mar 20, 2024 at 4:32 PM Guilherme Janczak
> > > > > <guilherme.janczak@xxxxxxxxxx> wrote:
> > > > > >
> > > > > > Variables shared with signal handlers must be of type `volatile
> > > > > > sigatomic_t`, not `volatile` or `sigatomic_t` as the current text says,
> > > > > > according to a C11 draft:
> > > > > >
> > > > > >     When ... interrupted by ... a signal, values of objects that are
> > > > > >     neither lock-free atomic objects nor of type volatile sig_atomic_t
> > > > > >     are unspecified.
> > > > > >
> > > > > > Ref: https://www.iso-9899.info/n1570.html#5.1.2.3p5
> > > > > > Signed-off-by: Guilherme Janczak <guilherme.janczak@xxxxxxxxxx>
> > > > > > ---
> > > > > >  memorder/memorder.tex | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/memorder/memorder.tex b/memorder/memorder.tex
> > > > > > index 5c50d42d..873c3424 100644
> > > > > > --- a/memorder/memorder.tex
> > > > > > +++ b/memorder/memorder.tex
> > > > > > @@ -1317,8 +1317,8 @@ from the viewpoint of the interrupted thread, at least at the
> > > > > >  assembly-language level.
> > > > > >  However, the C and C++ languages do not define the results of handlers
> > > > > >  and interrupted threads sharing plain variables.
> > > > > > -Instead, such shared variables must be \co{sig_atomic_t}, lock-free
> > > > > > -atomics, or \co{volatile}.
> > > > > > +Instead, such shared variables must be \co{volatile sig_atomic_t} or
> > > > > > +lock-free atomics.
> > > > > >
> > > > > >  On the other hand, because the handler executes within the interrupted
> > > > > >  thread's context, the memory ordering used to synchronize communication
> > > > > > --
> > > > > > 2.42.0
> > > > > >
> > > > > >
> > > > >





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux