Re: raise() marked __leaf__ is not C-compliant?

[Tadeus Prastowo <0x66726565@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

On Tue, Oct 27, 2020 at 7:50 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> On 27/10/2020 13:57, Tadeus Prastowo via Libc-help wrote:
> >
> > My understanding is that if my single-threaded program installs a
> > signal handler using signal() and the handler is executed as a result
> > of calling raise(), then the handler has a defined behavior when the
> > handler accesses any object with static storage duration even though
> > the object is not qualified using volatile and not of type
> > sig_atomic_t.
> >
> > If my understanding is incorrect, I would like to have some pointer to
> > parts of the C standard that say so.
>
> Unfortunately this is not fully correct, specially if you accessing the
> object outside the signal handler.  As you have noticed in your example,
> compiler can assume the variable 'terminated' won't be modified outside
> the function and apply optimization that avoid read its value from
> memory (most likely GCC will optimize out the '!terminated').

However, the compiler cannot assume that the variable `terminated'
won't be modified outside the loop if the loop calls a function whose
definition the compiler does not see (e.g., the raise() function
defined by glibc).  Otherwise, the compiler is wrong if the loop calls
a function defined in another translation unit because the function
may modify `terminated'.

> One of the best didactic explanation I have for this is from [1].
>
> [1] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Thank you for the reference.

I have studied it but found no pointer to parts of the C standard that
says that it is an undefined behavior to call a signal handler
synchronously using raise() on a normal execution path (i.e., not from
within another signal handler, which certainly is undefined behavior
as noted in "undefined behavior 131").

I know that the safest thing to do is always to qualify such a
variable with `volatile', but I think it is valid to not do so for the
cases that are not stated as
undefined/unspecified/implementation-defined behavior by the C
standard.  That is why I ask for a pointer to parts of the C standard
if indeed calling a signal handler synchronously using raise() on a
normal execution path is an undefined behavior.

After further thought, I found out that the problem is not glibc-2.30
marking raise() with __leaf__ but the file-scope limitation of
`terminated'.  As I already mentioned, since `terminated' has a
file-scope, the compiler can assume that `terminated' will not be
modified by a function defined in another translation unit.  And
hence, the compiler is correct in optimizing out the `!terminated'.
Once `terminated' has a global scope, the compiler correctly does not
optimize it out as exemplified by the attached program.  So, marking
raise() with __leaf__ does not make raise() non-compliant with the C
standard.

Thank you for answering my question.

-- 
Best regards,
Tadeus

[-- Attachment #2: raise.c --]
[-- Type: text/x-csrc, Size: 945 bytes --]

/* Compile this as follows:
 *   gcc -O0 -o raise raise.c
 * The C standard on signal() guarantees that the execution of raise will
 * terminate.
 *
 * Compile this as follows:
 *   gcc -Os -o raise raise.c
 * The C standard on signal() guarantees that the execution of raise will
 * terminate.
 *
 * Compile this as follows:
 *   gcc -O2 -o raise raise.c
 * The C standard on signal() guarantees that the execution of raise will
 * terminate.
 */

#include <signal.h>
#include <stdlib.h>
#include <time.h>

int terminated;

static void handler(int signo)
{
  terminated = 1;
}

int main(int argc, char **argv) {
  unsigned total = 0, delta = drand48() >= argc ? 1 : 0;
  /* At runtime, delta will be 0 as argc is one */

  signal(SIGUSR1, handler);

  /* This is an infinite loop unless terminated is 1 as delta is surely 0 */
  for (int i = 0; !terminated && i < 100; i += delta) {
    total += i;
    raise(SIGUSR1);
  }
  
  return total;
}