raise() marked __leaf__ is not C-compliant?

raise() marked __leaf__ is not C-compliant?

[Tadeus Prastowo]

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

Hello,

To quote C18 on the signal() function
(https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf),
Section 7.14.1.1 Paragraph 5:

If the signal occurs other than as the result of calling the abort or
raise function, the behavior is undefined if the signal handler refers
to any object with static or thread storage duration that is not a
lock-free atomic object other than by assigning a value to an object
declared as volatile sig_atomic_t

End quote.

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.

Otherwise, why glibc 2.30 implements raise() on x86_64 by using the
GCC function attribute __leaf__ when GCC says
(https://gcc.gnu.org/onlinedocs/gcc-9.3.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes):

Note that leaf functions might indirectly run a signal handler defined
in the current compilation unit that uses static variables. Similarly,
when lazy symbol resolution is in effect, leaf functions might invoke
indirect functions whose resolver function or implementation function
is defined in the current compilation unit and uses static variables.
There is no standard-compliant way to write such a signal handler,
resolver function, or implementation function, and the best that you
can do is to remove the leaf attribute or mark all such static
variables volatile.

End quote.

I see the use of the __leaf__ attribute by glibc 2.30 by reading
/usr/include/signal.h, finding __THROW, and then reading
/usr/include/sys/cdefs.h for __THROW definition, which expands to
include the __leaf__ attribute.

With glibc-2.30 raise() being marked __leaf__, the attached C program
has an undefined behavior because it terminates when compiled with GCC
using either -O0 or -Os but enters an infinite loop when using -O2.

Should glibc 2.30 be compiled in a specific way to have the defined
behavior guaranteed by the C standard?  If yes, what configure options
should be used?

Thank you.

-- 
Best regards,
Tadeus

[-- Attachment #2: raise.c --]
[-- Type: text/x-csrc, Size: 1069 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.  Unfortunately, glibc-2.30 raise() is marked __leaf__, and so, the
 * execution won't terminate on a x86_64 machine.
 */

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

static 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;
}

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

[Adhemerval Zanella]

On 27/10/2020 13:57, Tadeus Prastowo via Libc-help wrote:
> Hello,
> 
> To quote C18 on the signal() function
> (https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf),
> Section 7.14.1.1 Paragraph 5:
> 
> If the signal occurs other than as the result of calling the abort or
> raise function, the behavior is undefined if the signal handler refers
> to any object with static or thread storage duration that is not a
> lock-free atomic object other than by assigning a value to an object
> declared as volatile sig_atomic_t
> 
> End quote.
> 
> 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').

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

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

[Tadeus Prastowo]

[-- 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;
}

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

[Tadeus Prastowo]

Sorry that I closed this thread too soon and reopen it again now after
re-reading GCC documentation because the GCC documentation thinks that
raise() should not be marked __leaf__.

On Wed, Oct 28, 2020 at 6:47 AM Tadeus Prastowo <0x66726565@gmail.com> wrote:
>
> 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.

After re-reading the GCC documentation, I think marking raise() with
__leaf__ makes raise() non-compliant with the C standard, unless the C
standard says that calling a signal handler synchronously using
raise() on a normal execution path is an undefined behavior.

The GCC documentation on __leaf__
(https://gcc.gnu.org/onlinedocs/gcc-9.3.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes)
says (words in square brackets are added to fit our context):

Note that leaf functions might indirectly run a signal handler defined
in the current compilation unit that uses static variables.  [...]
There is no standard-compliant way to write such [an] implementation
function [, for example, glibc-2.30 raise()], and the best that you
can do is to remove the leaf attribute or mark all such static
variables volatile.

End quote.

Glibc-2.30 raise() definitely runs a signal handler, and the signal
handler can be defined in the current compilation unit using static
variables.  So, unless the C standard says that calling a signal
handler synchronously using raise() on a normal execution path is an
undefined behavior, the marking of raise() with __leaf__ makes raise()
non-compliant with the C standard.

May I know your opinion, please?

Thank you.

-- 
Best regards,
Tadeus

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

[Tadeus Prastowo]

Allow me to fix some mistakes in my previous e-mails:

On Wed, Oct 28, 2020 at 7:13 AM Tadeus Prastowo <0x66726565@gmail.com> wrote:
>
> Sorry that I closed this thread too soon and reopen it again now after
> re-reading GCC documentation because the GCC documentation thinks that
> raise() should not be marked __leaf__.
>
> On Wed, Oct 28, 2020 at 6:47 AM Tadeus Prastowo <0x66726565@gmail.com> wrote:
> >
> > 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 meant to say: I have studied it but found no pointer to parts of the
C standard that says that it is an undefined behavior to access a
non-volatile object with static storage duration from within a signal
handler that is called synchronously using raise() on a normal
execution path (i.e., the note "undefined behavior 131" only talks
about an undefined behavior that results from calling raise() from
within a signal handler).

> > 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.

I meant to say: That is why I ask for a pointer to parts of the C
standard if indeed accessing a non-volatile object with static storage
duration from within a signal handler that is called 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.
>
> After re-reading the GCC documentation, I think marking raise() with
> __leaf__ makes raise() non-compliant with the C standard, unless the C
> standard says that calling a signal handler synchronously using
> raise() on a normal execution path is an undefined behavior.

I meant to say: After re-reading the GCC documentation, I think
marking raise() with __leaf__ makes raise() non-compliant with the C
standard, unless the C standard says that accessing a non-volatile
object with static storage duration from within a signal handler that
is called synchronously using raise() on a normal execution path is an
undefined behavior.

> The GCC documentation on __leaf__
> (https://gcc.gnu.org/onlinedocs/gcc-9.3.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes)
> says (words in square brackets are added to fit our context):
>
> Note that leaf functions might indirectly run a signal handler defined
> in the current compilation unit that uses static variables.  [...]
> There is no standard-compliant way to write such [an] implementation
> function [, for example, glibc-2.30 raise()], and the best that you
> can do is to remove the leaf attribute or mark all such static
> variables volatile.
>
> End quote.
>
> Glibc-2.30 raise() definitely runs a signal handler, and the signal
> handler can be defined in the current compilation unit using static
> variables.  So, unless the C standard says that calling a signal
> handler synchronously using raise() on a normal execution path is an
> undefined behavior, the marking of raise() with __leaf__ makes raise()
> non-compliant with the C standard.

I meant to say: Glibc-2.30 raise() definitely runs a signal handler,
and the signal handler can be defined in the current compilation unit
to use static variables.  So, unless the C standard says that
accessing a non-volatile object with static storage duration from
within a signal handler that is called synchronously using raise() on
a normal execution path is an undefined behavior, the marking of
raise() with __leaf__ makes raise() non-compliant with the C standard.

> May I know your opinion, please?

Thank you for your patience.

-- 
Best regards,
Tadeus

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

[Adhemerval Zanella]

On 28/10/2020 04:33, Tadeus Prastowo wrote:
> Allow me to fix some mistakes in my previous e-mails:
> 
> On Wed, Oct 28, 2020 at 7:13 AM Tadeus Prastowo <0x66726565@gmail.com> wrote:
>>
>> Sorry that I closed this thread too soon and reopen it again now after
>> re-reading GCC documentation because the GCC documentation thinks that
>> raise() should not be marked __leaf__.
>>
>> On Wed, Oct 28, 2020 at 6:47 AM Tadeus Prastowo <0x66726565@gmail.com> wrote:
>>>
>>> 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 meant to say: I have studied it but found no pointer to parts of the
> C standard that says that it is an undefined behavior to access a
> non-volatile object with static storage duration from within a signal
> handler that is called synchronously using raise() on a normal
> execution path (i.e., the note "undefined behavior 131" only talks
> about an undefined behavior that results from calling raise() from
> within a signal handler).
> 
>>> 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.
> 
> I meant to say: That is why I ask for a pointer to parts of the C
> standard if indeed accessing a non-volatile object with static storage
> duration from within a signal handler that is called 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.
>>
>> After re-reading the GCC documentation, I think marking raise() with
>> __leaf__ makes raise() non-compliant with the C standard, unless the C
>> standard says that calling a signal handler synchronously using
>> raise() on a normal execution path is an undefined behavior.
> 
> I meant to say: After re-reading the GCC documentation, I think
> marking raise() with __leaf__ makes raise() non-compliant with the C
> standard, unless the C standard says that accessing a non-volatile
> object with static storage duration from within a signal handler that
> is called synchronously using raise() on a normal execution path is an
> undefined behavior.
> 
>> The GCC documentation on __leaf__
>> (https://gcc.gnu.org/onlinedocs/gcc-9.3.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes)
>> says (words in square brackets are added to fit our context):
>>
>> Note that leaf functions might indirectly run a signal handler defined
>> in the current compilation unit that uses static variables.  [...]
>> There is no standard-compliant way to write such [an] implementation
>> function [, for example, glibc-2.30 raise()], and the best that you
>> can do is to remove the leaf attribute or mark all such static
>> variables volatile.
>>
>> End quote.
>>
>> Glibc-2.30 raise() definitely runs a signal handler, and the signal
>> handler can be defined in the current compilation unit using static
>> variables.  So, unless the C standard says that calling a signal
>> handler synchronously using raise() on a normal execution path is an
>> undefined behavior, the marking of raise() with __leaf__ makes raise()
>> non-compliant with the C standard.
> 
> I meant to say: Glibc-2.30 raise() definitely runs a signal handler,
> and the signal handler can be defined in the current compilation unit
> to use static variables.  So, unless the C standard says that
> accessing a non-volatile object with static storage duration from
> within a signal handler that is called synchronously using raise() on
> a normal execution path is an undefined behavior, the marking of
> raise() with __leaf__ makes raise() non-compliant with the C standard.
> 
>> May I know your opinion, please?

The sentence "raise() definitely runs a signal handler" is not really
valid in a portable sense. Afaik neither C nor POSIX states which signals
should be delivered synchronously or asynchronously (although some do 
only make sense to be delivered synchronously such as SIGSEGV).

However, Linux does ran some signals synchronously and I agree that using
leaf attribute is incorrect and lead to this kind of problems. My point is
to be fully portable, you need to assume any signal might be delivered 
asynchronously (and C standard specifies the volatile sig_atomic_t for
such cases).

Florian, what other leaf functions do you think are problematic? I am
wondering if you would be better to just avoid this micro-optimization 
altogether.

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

[Tadeus Prastowo]

On Wed, Oct 28, 2020 at 12:53 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> On 28/10/2020 04:33, Tadeus Prastowo wrote:
> >
> > I meant to say: Glibc-2.30 raise() definitely runs a signal handler,
> > and the signal handler can be defined in the current compilation unit
> > to use static variables.  So, unless the C standard says that
> > accessing a non-volatile object with static storage duration from
> > within a signal handler that is called synchronously using raise() on
> > a normal execution path is an undefined behavior, the marking of
> > raise() with __leaf__ makes raise() non-compliant with the C standard.
> >
> >> May I know your opinion, please?
>
> The sentence "raise() definitely runs a signal handler" is not really
> valid in a portable sense. Afaik neither C nor POSIX states which signals
> should be delivered synchronously or asynchronously (although some do
> only make sense to be delivered synchronously such as SIGSEGV).
>
> However, Linux does ran some signals synchronously and I agree that using
> leaf attribute is incorrect and lead to this kind of problems. My point is
> to be fully portable, you need to assume any signal might be delivered
> asynchronously (and C standard specifies the volatile sig_atomic_t for
> such cases).

Thank you for your response.

However, C, including C99, C11, and the latest C18 [1], says: "If a
signal handler is called, the raise function shall not return until
after the signal handler does."  And, POSIX [2] says: "If a signal
handler is called, the raise() function shall not return until after
the signal handler does."  So, the sentence "raise() definitely runs a
signal handler" is valid in a portable sense as required by the
standards, no?

[1] Page 195 of
https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
[2] The raise section of Chapter 3 "System Interfaces" of IEEE
Standard for Information Technology---POSIX, Volume 2 "System
Interfaces", Issue 7.

-- 
Best regards,
Tadeus

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

[Adhemerval Zanella]

On 28/10/2020 10:19, Tadeus Prastowo wrote:
> On Wed, Oct 28, 2020 at 12:53 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> On 28/10/2020 04:33, Tadeus Prastowo wrote:
>>>
>>> I meant to say: Glibc-2.30 raise() definitely runs a signal handler,
>>> and the signal handler can be defined in the current compilation unit
>>> to use static variables.  So, unless the C standard says that
>>> accessing a non-volatile object with static storage duration from
>>> within a signal handler that is called synchronously using raise() on
>>> a normal execution path is an undefined behavior, the marking of
>>> raise() with __leaf__ makes raise() non-compliant with the C standard.
>>>
>>>> May I know your opinion, please?
>>
>> The sentence "raise() definitely runs a signal handler" is not really
>> valid in a portable sense. Afaik neither C nor POSIX states which signals
>> should be delivered synchronously or asynchronously (although some do
>> only make sense to be delivered synchronously such as SIGSEGV).
>>
>> However, Linux does ran some signals synchronously and I agree that using
>> leaf attribute is incorrect and lead to this kind of problems. My point is
>> to be fully portable, you need to assume any signal might be delivered
>> asynchronously (and C standard specifies the volatile sig_atomic_t for
>> such cases).
> 
> Thank you for your response.
> 
> However, C, including C99, C11, and the latest C18 [1], says: "If a
> signal handler is called, the raise function shall not return until
> after the signal handler does."  And, POSIX [2] says: "If a signal
> handler is called, the raise() function shall not return until after
> the signal handler does."  So, the sentence "raise() definitely runs a
> signal handler" is valid in a portable sense as required by the
> standards, no?

My understanding is it allows synchronous signals, not enforce it;
and if the signal is synchronous then it should complete prior hand.

> 
> [1] Page 195 of
> https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
> [2] The raise section of Chapter 3 "System Interfaces" of IEEE
> Standard for Information Technology---POSIX, Volume 2 "System
> Interfaces", Issue 7.
> 

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

[Tadeus Prastowo]

On Wed, Oct 28, 2020 at 6:34 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> On 28/10/2020 10:19, Tadeus Prastowo wrote:
> > On Wed, Oct 28, 2020 at 12:53 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >> The sentence "raise() definitely runs a signal handler" is not really
> >> valid in a portable sense. Afaik neither C nor POSIX states which signals
> >> should be delivered synchronously or asynchronously (although some do
> >> only make sense to be delivered synchronously such as SIGSEGV).
> >>
> >> However, Linux does ran some signals synchronously and I agree that using
> >> leaf attribute is incorrect and lead to this kind of problems. My point is
> >> to be fully portable, you need to assume any signal might be delivered
> >> asynchronously (and C standard specifies the volatile sig_atomic_t for
> >> such cases).
> >
> > Thank you for your response.
> >
> > However, C, including C99, C11, and the latest C18 [1], says: "If a
> > signal handler is called, the raise function shall not return until
> > after the signal handler does."  And, POSIX [2] says: "If a signal
> > handler is called, the raise() function shall not return until after
> > the signal handler does."  So, the sentence "raise() definitely runs a
> > signal handler" is valid in a portable sense as required by the
> > standards, no?
>
> My understanding is it allows synchronous signals, not enforce it;
> and if the signal is synchronous then it should complete prior hand.

I understand your point as the C standard says:

[...] distinct values that are the signal numbers, each corresponding
to the specified condition:
SIGABRT [...]
SIGFPE [...]
SIGILL [...]
SIGINT [...]
SIGSEGV [...]
SIGTERM [...]
An implementation need not generate any of these signals, except as a
result of explicit calls to the raise function.  [...]  The complete
set of signals, their semantics, and their default handling is
implementation-defined; all signal numbers shall be positive.

[...]

void (*signal(int sig, void (*func)(int)))(int);
Description
[...]
When a signal occurs and func points to a function, it is
implementation-defined whether the equivalent of signal(sig, SIG_DFL);
is executed [...]; in the case of SIGILL, the implementation may
alternatively define that no action is taken.  Then the equivalent of
(*func)(sig); is executed.

End quote.

So, yes, you are right that the sentence "raise() definitely runs a
signal handler" is inaccurate because, as quoted above, C standard
allows an implementation to not run the handler that the user has
designated to handle the signal being generated by raise().  However,
if the implementation decides that "the equivalent of (*func)(sig); is
executed", then the C standard requires that "the raise function shall
not return until after the signal handler does."

To conclude, in my earlier post, I meant to say: Glibc-2.30 raise()
may run a signal handler that is defined in the current compilation
unit to use static variables.  So, unless the C standard says that it
is an undefined behavior to access a non-volatile object with static
storage duration from within a signal handler that is called
synchronously by raise() on a normal execution path, the marking of
raise() with __leaf__ makes raise() non-compliant with the C standard.

> > [1] Page 195 of
> > https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
> > [2] The raise section of Chapter 3 "System Interfaces" of IEEE
> > Standard for Information Technology---POSIX, Volume 2 "System
> > Interfaces", Issue 7.

Thank you for your patience in allowing me to make myself clear.

-- 
Best regards,
Tadeus

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

[Adhemerval Zanella]

On 28/10/2020 16:23, Tadeus Prastowo wrote:
> On Wed, Oct 28, 2020 at 6:34 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> On 28/10/2020 10:19, Tadeus Prastowo wrote:
>>> On Wed, Oct 28, 2020 at 12:53 PM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>> The sentence "raise() definitely runs a signal handler" is not really
>>>> valid in a portable sense. Afaik neither C nor POSIX states which signals
>>>> should be delivered synchronously or asynchronously (although some do
>>>> only make sense to be delivered synchronously such as SIGSEGV).
>>>>
>>>> However, Linux does ran some signals synchronously and I agree that using
>>>> leaf attribute is incorrect and lead to this kind of problems. My point is
>>>> to be fully portable, you need to assume any signal might be delivered
>>>> asynchronously (and C standard specifies the volatile sig_atomic_t for
>>>> such cases).
>>>
>>> Thank you for your response.
>>>
>>> However, C, including C99, C11, and the latest C18 [1], says: "If a
>>> signal handler is called, the raise function shall not return until
>>> after the signal handler does."  And, POSIX [2] says: "If a signal
>>> handler is called, the raise() function shall not return until after
>>> the signal handler does."  So, the sentence "raise() definitely runs a
>>> signal handler" is valid in a portable sense as required by the
>>> standards, no?
>>
>> My understanding is it allows synchronous signals, not enforce it;
>> and if the signal is synchronous then it should complete prior hand.
> 
> I understand your point as the C standard says:
> 
> [...] distinct values that are the signal numbers, each corresponding
> to the specified condition:
> SIGABRT [...]
> SIGFPE [...]
> SIGILL [...]
> SIGINT [...]
> SIGSEGV [...]
> SIGTERM [...]
> An implementation need not generate any of these signals, except as a
> result of explicit calls to the raise function.  [...]  The complete
> set of signals, their semantics, and their default handling is
> implementation-defined; all signal numbers shall be positive.
> 
> [...]
> 
> void (*signal(int sig, void (*func)(int)))(int);
> Description
> [...]
> When a signal occurs and func points to a function, it is
> implementation-defined whether the equivalent of signal(sig, SIG_DFL);
> is executed [...]; in the case of SIGILL, the implementation may
> alternatively define that no action is taken.  Then the equivalent of
> (*func)(sig); is executed.
> 
> End quote.
> 
> So, yes, you are right that the sentence "raise() definitely runs a
> signal handler" is inaccurate because, as quoted above, C standard
> allows an implementation to not run the handler that the user has
> designated to handle the signal being generated by raise().  However,
> if the implementation decides that "the equivalent of (*func)(sig); is
> executed", then the C standard requires that "the raise function shall
> not return until after the signal handler does."

I agree this does make sense for *synchronous* signal triggering.  My
point ir neither C not POSIX specify which signal should be synchronous. 
In fact, afaik neither standard does make a clearly distinction nor 
specific the expected semantic for signal "synchronicity" (it is really 
implementation defined).

> 
> To conclude, in my earlier post, I meant to say: Glibc-2.30 raise()
> may run a signal handler that is defined in the current compilation
> unit to use static variables.  So, unless the C standard says that it
> is an undefined behavior to access a non-volatile object with static
> storage duration from within a signal handler that is called
> synchronously by raise() on a normal execution path, the marking of
> raise() with __leaf__ makes raise() non-compliant with the C standard.

I am not a language lawyer here, and I agree with you that adding leaf
attribute on 'raise' is wrong. But because glibc can't guarantee how
the underlying kernel will trigger the signal in a raise syscall.

I still think that once you can't assume the signal will be triggered
synchronously, you can't also assume that accessing non-volatile object
with static storage duration from within a signal handler might not
trigger undefined behavior.

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

[Tadeus Prastowo]

On Wed, Oct 28, 2020 at 9:17 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> On 28/10/2020 16:23, Tadeus Prastowo wrote:
> > On Wed, Oct 28, 2020 at 6:34 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >> On 28/10/2020 10:19, Tadeus Prastowo wrote:
> >>>
> >>> However, C, including C99, C11, and the latest C18 [1], says: "If a
> >>> signal handler is called, the raise function shall not return until
> >>> after the signal handler does."  And, POSIX [2] says: "If a signal
> >>> handler is called, the raise() function shall not return until after
> >>> the signal handler does."  So, the sentence "raise() definitely runs a
> >>> signal handler" is valid in a portable sense as required by the
> >>> standards, no?
> >>
> >> My understanding is it allows synchronous signals, not enforce it;
> >> and if the signal is synchronous then it should complete prior hand.
> >
> > I understand your point as the C standard says:
> >
> > [...] distinct values that are the signal numbers, each corresponding
> > to the specified condition:
> > SIGABRT [...]
> > SIGFPE [...]
> > SIGILL [...]
> > SIGINT [...]
> > SIGSEGV [...]
> > SIGTERM [...]
> > An implementation need not generate any of these signals, except as a
> > result of explicit calls to the raise function.  [...]  The complete
> > set of signals, their semantics, and their default handling is
> > implementation-defined; all signal numbers shall be positive.
> >
> > [...]
> >
> > void (*signal(int sig, void (*func)(int)))(int);
> > Description
> > [...]
> > When a signal occurs and func points to a function, it is
> > implementation-defined whether the equivalent of signal(sig, SIG_DFL);
> > is executed [...]; in the case of SIGILL, the implementation may
> > alternatively define that no action is taken.  Then the equivalent of
> > (*func)(sig); is executed.
> >
> > End quote.
> >
> > So, yes, you are right that the sentence "raise() definitely runs a
> > signal handler" is inaccurate because, as quoted above, C standard
> > allows an implementation to not run the handler that the user has
> > designated to handle the signal being generated by raise().  However,
> > if the implementation decides that "the equivalent of (*func)(sig); is
> > executed", then the C standard requires that "the raise function shall
> > not return until after the signal handler does."
>
> I agree this does make sense for *synchronous* signal triggering.  My
> point ir neither C not POSIX specify which signal should be synchronous.
> In fact, afaik neither standard does make a clearly distinction nor
> specific the expected semantic for signal "synchronicity" (it is really
> implementation defined).

I am sorry that after I re-read the quoted C standard, the correct
understanding is that the implementation is required to execute "the
equivalent of (*func)(sig);" if the registration of `func' using
signal() is successful (i.e., signal() does not return SIG_ERR).  This
is because the C standard says: "When a signal occurs and func points
to a function, it is implementation-defined whether [...].  Then the
equivalent of (*func)(sig); is executed."  Hence, if an implementation
decides not to execute `func' for a certain implementation-defined
signal (e.g., SIGKILL), then the call signal(SIGKILL, func) must
return SIG_ERR, which is already the case.  Otherwise, the C standard
requires the implementation to run the signal handler `func'.
However, before executing `func', the C standard allows the
implementation to perform some setup, such as executing the equivalent
of signal(sig, SIG_DFL);, but no matter what the setup does, the
outcome of the setup will not cancel the execution of `func' owing to
the fact that, by signal() not returning SIG_ERR, `func' is already
required to be executed after the setup, which is already the case as
demonstrated by the following program on SIGILL on which the C
standard allows an implementation to perform no setup at all.

#include <signal.h>
static int val;
void handler(int signo) {
  val = 1;
}
int main(int argc, char **argv) {
  signal(SIGILL, handler);
  raise(SIGILL);
  return val;
}

> > To conclude, in my earlier post, I meant to say: Glibc-2.30 raise()
> > may run a signal handler that is defined in the current compilation
> > unit to use static variables.  So, unless the C standard says that it
> > is an undefined behavior to access a non-volatile object with static
> > storage duration from within a signal handler that is called
> > synchronously by raise() on a normal execution path, the marking of
> > raise() with __leaf__ makes raise() non-compliant with the C standard.

Hence, while it is indeed true that the sentence "raise() definitely
runs a signal handler" is inaccurate, it is inaccurate not because the
implementation is allowed by the C standard to not run the handler
that is successfully registered using signal() but because no handler
has been registered successfully using signal() in the first place.

> I am not a language lawyer here, and I agree with you that adding leaf
> attribute on 'raise' is wrong.

Thank you for your agreement.

> But because glibc can't guarantee how
> the underlying kernel will trigger the signal in a raise syscall.

By striving to be comformant to the C standard, glibc has to guarantee
the behavior that the C standard does not state as
undefined/unspecified/implementation-defined, which in this case is
the behavior that, after a signal handler is successfully registered
using signal() and then the handled signal is generated using raise(),
"the raise function shall not return until after the signal handler
does", doesn't it?

> I still think that once you can't assume the signal will be triggered
> synchronously, you can't also assume that accessing non-volatile object
> with static storage duration from within a signal handler might not
> trigger undefined behavior.

Isn't that by the C standard, which makes no distinction between
synchronous and asynchronous signals and so allows me to not make any
assumption about them, no undefined behavior results from accessing a
non-volatile object with static storage duration from within a signal
handler whose signal is generated using raise() on the normal
execution path?

Thank you for your patience in perusing my message.

-- 
Best regards,
Tadeus

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

[Florian Weimer]

* Tadeus Prastowo via Libc-help:

> With glibc-2.30 raise() being marked __leaf__, the attached C program
> has an undefined behavior because it terminates when compiled with GCC
> using either -O0 or -Os but enters an infinite loop when using -O2.

Yes, I think the leaf attribute is incorrect for raise and similar
functions.  throw()/noexcept is questionable as well because the GNU
implementation supports throwing from a synchronous signal handler, as
an extension.

I can't find a Bugzilla bug for this right now, but I think we discussed
it before.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

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

[Tadeus Prastowo]

On Wed, Oct 28, 2020 at 9:21 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Tadeus Prastowo via Libc-help:
>
> > With glibc-2.30 raise() being marked __leaf__, the attached C program
> > has an undefined behavior because it terminates when compiled with GCC
> > using either -O0 or -Os but enters an infinite loop when using -O2.
>
> Yes, I think the leaf attribute is incorrect for raise and similar
> functions.  throw()/noexcept is questionable as well because the GNU
> implementation supports throwing from a synchronous signal handler, as
> an extension.

Thank you for your response.

> I can't find a Bugzilla bug for this right now, but I think we discussed
> it before.

I will open one.

Thank you.

> Thanks,
> Florian

-- 
Best regards,
Tadeus

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

[Tadeus Prastowo]

On Wed, Oct 28, 2020 at 1:58 PM Tadeus Prastowo <0x66726565@gmail.com> wrote:
>
> On Wed, Oct 28, 2020 at 9:21 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * Tadeus Prastowo via Libc-help:
> >
> > > With glibc-2.30 raise() being marked __leaf__, the attached C program
> > > has an undefined behavior because it terminates when compiled with GCC
> > > using either -O0 or -Os but enters an infinite loop when using -O2.
> >
> > Yes, I think the leaf attribute is incorrect for raise and similar
> > functions.  throw()/noexcept is questionable as well because the GNU
> > implementation supports throwing from a synchronous signal handler, as
> > an extension.
>
> Thank you for your response.
>
> > I can't find a Bugzilla bug for this right now, but I think we discussed
> > it before.
>
> I will open one.

I have opened it: https://sourceware.org/bugzilla/show_bug.cgi?id=26802

> Thank you.
>
> > Thanks,
> > Florian

-- 
Best regards,
Tadeus