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

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

On 25/06/2019 15:12, Maciej W. Rozycki wrote:
> On Mon, 24 Jun 2019, Adhemerval Zanella wrote:
> 
>>>  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.
> 
>  Agreed as to the defect report.  The situation WRT the kernel is not 
> really any different from POSIX threads support however, as they also 
> require assistance from the underlying OS kernel.

Yes, but at least Linux went on safe way to handle threads a different
process regarding state save/restore. It means it is up to kernel to
correctly handle the required state and allows some clever optimization
like lazy-save cpu extensions, such as floating-point or vector register.

> 
>>>  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
> 
>  At which point the `*context' functions have already been obsoleted and 
> hence nobody bothered to do anything about them, or I presume the same 
> concerns would apply to `setcontext' and `swapcontext' as do to 
> `siglongjmp'.
> 
>>>> 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.
> 
>  About the only non-trivial sensible use I can envisage for the `*context' 
> functions is to have an in-process multitasking execution environment 
> where multiple contexts of execution are switched between in a scheduler 
> based around SIGALRM or a similar signal arranged to be raised at regular 
> intervals (a poor man's threads implementation).  For this to work at all 
> obviously all the user registers have to be saved and restored as the 
> scheduling signal can trigger at any time with respect to linear code 
> execution, and the relevant functions, especially `swapcontext', have to 
> be invocable from within a signal handler.
> 
>  While I agree we are not bound by the standard (anymore) to implement 
> this feature for supported architectures we have not before, I believe the 
> MIPS implementation used to fulfil the purpose for the register sets 
> defined at the time (i.e. the extended FP context of the later MSA ASE is 
> not supported) and as I say I see no reason to remove it.  It remains up 
> to the user of the API to avoid the use of non-async-signal-safe 
> functions, including `malloc' and `free' in particular.

My understanding is this scenario only safe if for every non as-safe call
you actually unmask/mask all signals, and even though my previous remarks
still hold that this is not really portable in the sense that depending
of the computation you won't get reliable results over the architectures
(it might depend of the compiler flags or compiler that might generate
extension usage that might not be saved/restored).

Not sure how this scenario is used in real-world cases and if the computation
involved does require to correct save/restore all possible state.

> 
>  I had a small test case included with the original submission of MIPS 
> `*context' support that covered the use of `*context' functions in a 
> signal handler, which however was dropped as the change was applied.  I 
> meant to write a more proper one with a signal-based scheduler as outlined 
> above cycling across a few contexts a few times, however I ran out of time 
> and never got back to it.

The main problem now is with compiler aggressively using more architecture
extension and with *context definition not being correctly defined, it support
is really arch-specific. And I think we should move away of such behaviour,
unless it is really the intention (such ABI arch-specific ABIs or libraries
that leverages some extension such as libmvec).