[v2,1/2] setjmp: Use BSD sematic as default for setjmp

POSIX relaxed the relation of setjmp/longjmp and the signal mask
save/restore, meaning that setjmp does not require to be routed to
_setjmp to be standard compliant.

This is done to avoid breakage of SIGABRT handlers, since to fully
make abort AS-safe, it is required to remove the recurisve lock
used to unblock SIGABRT prior raised the signal.

Also, it allows caller to actually use setjmp, since from
7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
routed to _setjmp.

Checked on x86_64-linux-gnu.
---
 NEWS                                |  4 +++-
 manual/setjmp.texi                  | 14 ++++----------
 nptl/pthread_create.c               |  3 ++-
 setjmp/setjmp.h                     |  5 -----
 sysdeps/nptl/libc_start_call_main.h |  3 ++-
 5 files changed, 11 insertions(+), 18 deletions(-)

* Adhemerval Zanella:

> POSIX relaxed the relation of setjmp/longjmp and the signal mask
> save/restore, meaning that setjmp does not require to be routed to
> _setjmp to be standard compliant.
>
> This is done to avoid breakage of SIGABRT handlers, since to fully
> make abort AS-safe, it is required to remove the recurisve lock
> used to unblock SIGABRT prior raised the signal.
>
> Also, it allows caller to actually use setjmp, since from
> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
> routed to _setjmp.

I still don't think making a compatibility-impacting change to fix such
an obscure bug is a good trade-off.

The kernel providing something like exit_signal (terminate the process
even if a signal handler is installed) would be a better way forward, in
my opinion.

Thanks,
Florian

On 23/07/24 10:49, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> POSIX relaxed the relation of setjmp/longjmp and the signal mask
>> save/restore, meaning that setjmp does not require to be routed to
>> _setjmp to be standard compliant.
>>
>> This is done to avoid breakage of SIGABRT handlers, since to fully
>> make abort AS-safe, it is required to remove the recurisve lock
>> used to unblock SIGABRT prior raised the signal.
>>
>> Also, it allows caller to actually use setjmp, since from
>> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
>> routed to _setjmp.
> 
> I still don't think making a compatibility-impacting change to fix such
> an obscure bug is a good trade-off.
> 
> The kernel providing something like exit_signal (terminate the process
> even if a signal handler is installed) would be a better way forward, in
> my opinion.

What do you mean by obscure bug? The abort not being async-signal-safe is
being triggered by users and incurring in workarounds and time consuming
analysis.

And I don't see the need of a new kernel interface for an issue that can
be complete solved in userspace (and I think kernel developers would say
the same thing, since this most likely will need a new syscall to solve a
very specific userland issue).

And user can still use the SysV setjmp, they just need to explicit say so.
I am not really following the reluctant to fix abort() async-signal-safeness.

On Tue, Jul 23, 2024, at 12:04 PM, Adhemerval Zanella Netto wrote:
> On 23/07/24 10:49, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> POSIX relaxed the relation of setjmp/longjmp and the signal mask
>>> save/restore, meaning that setjmp does not require to be routed to
>>> _setjmp to be standard compliant.
>>>
>>> This is done to avoid breakage of SIGABRT handlers, since to fully
>>> make abort AS-safe, it is required to remove the recurisve lock
>>> used to unblock SIGABRT prior raised the signal.
>>>
>>> Also, it allows caller to actually use setjmp, since from
>>> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
>>> routed to _setjmp.
>> 
>> I still don't think making a compatibility-impacting change to fix such
>> an obscure bug is a good trade-off.

I'm inclined to think that making setjmp be sigsetjmp by default is more
likely to _hide_ bugs than _expose_ them, but ...

>> The kernel providing something like exit_signal (terminate the process
>> even if a signal handler is installed) would be a better way forward, in
>> my opinion.
>
> And I don't see the need of a new kernel interface for an issue that can
> be complete solved in userspace (and I think kernel developers would say
> the same thing, since this most likely will need a new syscall to solve a
> very specific userland issue).

... adding exit_signal() would mean that abort() didn't need to do locking at all,
which might actually be a strong enough argument for the kernel people.

zw

On Tue, Jul 23, 2024, at 1:26 PM, Zack Weinberg wrote:
>> And I don't see the need of a new kernel interface for an issue that
>> can be complete solved in userspace (and I think kernel developers
>> would say the same thing, since this most likely will need a new
>> syscall to solve a very specific userland issue).
>
> ... adding exit_signal() would mean that abort() didn't need to do
> locking at all, which might actually be a strong enough argument for
> the kernel people.

I still kinda like the exit_signal idea, but here's another idea that
might make abort AS-safe without needing to change setjmp or add a
new kernel interface: add another internal cross-thread broadcast
message (like the existing one for euid changes) that means "force
all other threads to exit [not cancel!] immediately".  We'd do that
if the initial raise(SIGABRT) fails, thus permitting us to reset the
SIGABRT disposition and not have to worry about another thread changing
it back, which I believe is the only reason for the locking?

zw

On 23/07/24 14:26, Zack Weinberg wrote:
> On Tue, Jul 23, 2024, at 12:04 PM, Adhemerval Zanella Netto wrote:
>> On 23/07/24 10:49, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> POSIX relaxed the relation of setjmp/longjmp and the signal mask
>>>> save/restore, meaning that setjmp does not require to be routed to
>>>> _setjmp to be standard compliant.
>>>>
>>>> This is done to avoid breakage of SIGABRT handlers, since to fully
>>>> make abort AS-safe, it is required to remove the recurisve lock
>>>> used to unblock SIGABRT prior raised the signal.
>>>>
>>>> Also, it allows caller to actually use setjmp, since from
>>>> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
>>>> routed to _setjmp.
>>>
>>> I still don't think making a compatibility-impacting change to fix such
>>> an obscure bug is a good trade-off.
> 
> I'm inclined to think that making setjmp be sigsetjmp by default is more
> likely to _hide_ bugs than _expose_ them, but ...
> 
>>> The kernel providing something like exit_signal (terminate the process
>>> even if a signal handler is installed) would be a better way forward, in
>>> my opinion.
>>
>> And I don't see the need of a new kernel interface for an issue that can
>> be complete solved in userspace (and I think kernel developers would say
>> the same thing, since this most likely will need a new syscall to solve a
>> very specific userland issue).
> 
> ... adding exit_signal() would mean that abort() didn't need to do locking at all,
> which might actually be a strong enough argument for the kernel people.

Besides the time required to propagate this, and the issue that abort() would be
async-signal-safe only for certain kernels, I am not fully sure that exit_signal
make sense when kernel already provides exit_group.  The whole idea of abort()
is to execute default action of abnormal termination, otherwise one can just
call _Exit/_exit.

On 23/07/24 14:34, Zack Weinberg wrote:
> On Tue, Jul 23, 2024, at 1:26 PM, Zack Weinberg wrote:
>>> And I don't see the need of a new kernel interface for an issue that
>>> can be complete solved in userspace (and I think kernel developers
>>> would say the same thing, since this most likely will need a new
>>> syscall to solve a very specific userland issue).
>>
>> ... adding exit_signal() would mean that abort() didn't need to do
>> locking at all, which might actually be a strong enough argument for
>> the kernel people.
> 
> I still kinda like the exit_signal idea, but here's another idea that
> might make abort AS-safe without needing to change setjmp or add a
> new kernel interface: add another internal cross-thread broadcast
> message (like the existing one for euid changes) that means "force
> all other threads to exit [not cancel!] immediately".  We'd do that
> if the initial raise(SIGABRT) fails, thus permitting us to reset the
> SIGABRT disposition and not have to worry about another thread changing
> it back, which I believe is the only reason for the locking?

This is *way* more complex, since it requires register a new internal
signal (or remap one like we do for SIGTIMER, but this also adds more
complexity) on abort anyway as we on pthread_cancel.  It also requires
to synchronize on each thread for abnormal end of execution, which raises
the question on how to handle newly created threads during the signaling
(which most likely would likely required even more synchronization in
pthread_create).

On Tue, 23 Jul 2024, Zack Weinberg wrote:

> >> The kernel providing something like exit_signal (terminate the process
> >> even if a signal handler is installed) would be a better way forward, in
> >> my opinion.
> >
> > And I don't see the need of a new kernel interface for an issue that can
> > be complete solved in userspace (and I think kernel developers would say
> > the same thing, since this most likely will need a new syscall to solve a
> > very specific userland issue).
> 
> ... adding exit_signal() would mean that abort() didn't need to do locking at all,
> which might actually be a strong enough argument for the kernel people.

 And FWIW this interface already exists internally in the kernel, just not 
exposed for general use to the userland (it's `force_exit_sig').

  Maciej

* Zack Weinberg:

> On Tue, Jul 23, 2024, at 12:04 PM, Adhemerval Zanella Netto wrote:
>> On 23/07/24 10:49, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>> 
>>>> POSIX relaxed the relation of setjmp/longjmp and the signal mask
>>>> save/restore, meaning that setjmp does not require to be routed to
>>>> _setjmp to be standard compliant.
>>>>
>>>> This is done to avoid breakage of SIGABRT handlers, since to fully
>>>> make abort AS-safe, it is required to remove the recurisve lock
>>>> used to unblock SIGABRT prior raised the signal.
>>>>
>>>> Also, it allows caller to actually use setjmp, since from
>>>> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
>>>> routed to _setjmp.
>>> 
>>> I still don't think making a compatibility-impacting change to fix such
>>> an obscure bug is a good trade-off.
>
> I'm inclined to think that making setjmp be sigsetjmp by default is more
> likely to _hide_ bugs than _expose_ them, but ...

It's the impact of the extra system call that I'm worried about.  It may
preclude using setjmp for using setjmp/longjmp exception handling, for
performance reasons.

It only impacts applications as they are recompiled, but still.  People
will have to change their sources and switch to sigsetjmp.

Thanks,
Floria