Re: git: 6e824f371301 - main - time: siginfo_recvd needs to be marked volatile

On Mon, Jun 10, 2024 at 10:13:45PM -0500, Kyle Evans wrote:
> On 5/31/24 14:52, Konstantin Belousov wrote:
> > On Wed, May 22, 2024 at 05:37:56AM +0000, Kyle Evans wrote:
> > > The branch main has been updated by kevans:
> > > 
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=6e824f3713011f7955a4f88fb16445e8e2cbe72c
> > > 
> > > commit 6e824f3713011f7955a4f88fb16445e8e2cbe72c
> > > Author:     Kyle Evans <kevans@FreeBSD.org>
> > > AuthorDate: 2024-05-22 05:36:29 +0000
> > > Commit:     Kyle Evans <kevans@FreeBSD.org>
> > > CommitDate: 2024-05-22 05:37:41 +0000
> > > 
> > >      time: siginfo_recvd needs to be marked volatile
> > I do not think so.  It happens to work with the compilers we currently use.
> > 
> 
> Sorry, I meant to respond but it got lost somewhere in the stack.  Can you
> expand on this specifically a bit more?  I was under the impression that
> we'd generally need volatility due to write from SIGINFO handler / read
> repeatedly from main(), which serves to demonstrate my ignorance here.  Is
> it that there are sufficient side-effect-y things between loads in the
> wait4() loop that the compiler won't do anything hinky, or more that it's
> insufficient and should have been fenced as well?
Signal fence is provided by standard explicitly to handle the interaction
between signal handler and 'main' code.

On the other hand, description of the volatile semantic is vague and
references possible side effects caused by something not known to the
compiler, like memory-mapped device state.

It happens that both gcc and clang effectively implement volatile
accesses as relaxed atomics loads and stores.  This is enough to ensure
that the loop you wrote correctly interacts with the signal handler you
wrote.  But it is compiler-depended.

It might be interesting to read gcc documentation about its implementation-
dependent semantic of volaties, 'C implementation defined behavior->
Qualifiers' in the texinfo doc.

> 
> > >      sig_atomic_t does not imply volatility, we must do it ourselves to avoid
> > >      caching of siginfo_recvd loads.
> > For this purpose, standard provides atomic_signal_fence().
> > 
> > You would need to put it before read of siginfo_recvd (after wait4()), and
> > after update of siginfo_recvd in the signal handler.