Re: [PATCH 2/2] mips: Remove rt_sigreturn usage on context function

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 24/06/2019 13:55, Maciej W. Rozycki wrote:
> On Mon, 3 Jun 2019, Adhemerval Zanella wrote:
> 
>>>  This is an obsolete interface, but FYI according to the relevant version 
>>> of SUS[1] you are supposed to be able to return from a signal handler by 
>>> calling setcontext(3) similarly to siglongjmp(3) and by removing this code 
>>> you make the context not to be fully restored in that case (on the MIPS 
>>> target the distinction is due to the DSP register state).
>>>
>>>  What do you mean by: "setcontext is not async-signal-safe"?  I think such 
>>> an assertion (if indeed true) should have been explained in the commit 
>>> message even if it was previously discussed (where?).
>>
>> It is in fact blurry on POSIX 2001, since it defines on '3. Definitions' [1] that
>> function *is not* async-cancel-safe unless explicitly described as such and
>> later on '2.4 Signal Concepts' [2] it defines the function set that should be
>> async-signal-safe and it does not list 'setcontext' as one.
> 
>  Well, it says specifically that:
> 
> "[An async-cancel-safe function is] A function that may be invoked, 
> without restriction, from signal-catching functions."
> 
> which in my opinion does not preclude a non-async-cancel-safe function to 
> be called with some restrictions.  And the description of the function 
> clearly states it is intended to be used in a signal handler.  So while we 
> may not be required by the standard to produce a predictable result, we 
> certainly are allowed to if feasible.

My understanding is an implementation detail that a function not required to
be a async-signal-safe might be safe to be called from a signal handler.
And the problem is exactly what is 'predictable' and ' feasible', since 
it requires some kernel support along with some out of the scope choices
(ABI boundary and which state is saved/restored). That's why I stated is
is 'blurry' on POSIX 2001 and if the symbols were not deprecated I think
it would require to open a defect report to Austin Group make it clear.

> 
>  NB `siglongjmp' isn't listed as an async-cancel-safe function either and 
> I think we have no doubt as to whether it can be called in a signal 
> handler and produce a predictable result.

My understanding is POSIX.1-2008 Technical Corrigendum 2 added both longjmp and 
siglongjmp to the list of async-signal-safe functions. And it also adds some
issues regarding its usage [1]:

"Note that although longjmp() and siglongjmp() are in the list of async-signal-safe 
functions, there are restrictions on subsequent behavior after the function is 
called from a signal-catching function. This is because the code executing after 
longjmp() or siglongjmp() can call any unsafe functions with the same danger as calling 
those unsafe functions directly from the signal handler. Applications that use longjmp() 
or siglongjmp() out of signal handlers require rigorous protection in order to be 
portable. Many of the other functions that are excluded from the list are traditionally
implemented using either the C language malloc() or free() functions or the ISO C standard
I/O library, both of which traditionally use data structures in a non-async-signal-safe 
manner. Because any combination of different functions using a common data structure
can cause async-signal-safety problems, POSIX.1 does not define the behavior when any
unsafe function is called in (or after a longjmp() or siglongjmp() out of) a signal handler 
that interrupts any unsafe function or the non-async-signal-safe processing equivalent 
to exit() that is performed after return from the initial call to main()."

Basically it is undefined if you call siglongjmp on a signal handler that has
interrupted a non-asignal-safe function.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html
 
> 
>> It is even more complicated because this is not portable over architectures on
>> Linux, since:
>>
>>     1. not every one supports rt_sigaction. It means that either that architecture
>>        does not really require additional context that userland might not
>>        save/restore or that architecture extensions save/restore are not really 
>>        supported.
>>        For instance, it seems that newer architectures (like RiscV) follow this
>>        approach and just really support synchronous by saving/restore the preserved
>>        state. It is the same with some newer ABI extension, such as AArch64 SVE.
>>
>>     2. Not every architecture passes an mcontext_t and third argument for signal
>>        handlers with SA_SIGINFO (sparc and ia64 for instance). 
>>
>>     3. Even for architecture that does support passing an mcontext_t, userland
>>        can not have enough information that some context is lazy-saved (such some
>>        architecture extension such as vector register) by the kernel.
>>
>> So that's why I think setcontext is not really aync-signal-safe and it is really
>> hard to get it right in Linux (check powerpc complexity over time to support 
>> Altivec and later VSX) and while it is support is very architecture-specific.  
> 
>  Perhaps Power would have to define and use a set of syscalls to aid the 
> userland in selecting the set of registers to save and restore.  It was 
> proposed for the MIPS/Linux port too, but it turned out unnecessary.

POWER does that setting the uc_mcontext.gp_regs[PT_MSR] with expected state
(MSR_VEC, MSR_VSX).

> 
>  I see no need or sense to remove what used to just work.
> 
>> I am not really against reverting the patch, and it might be better outcome for an 
>> already deprecated symbol, but my idea was to make it somewhat more sane and 
>> portable on Linux.
> 
>  In what sense portable?

Besides the issues POSIX itself alerts by using setcontext/sig*jmp in a signal
handler, also the fact most architecture only defines a rational subset of the
CPU state is set/restore instead of trying to support all possible extensions
(which most likely requires kernel support). So, for instance an application that
use some vector extension might add arch-specific handle to either check or
not expect the fully state to be set/restored.

> 
>   Maciej
>