[PATCH] Cygwin: setjmp/longjmp: decrement incyg after signal handling

[Takashi Yano]

Hi Corinna,

On Wed,  4 Dec 2024 13:54:47 +0100
Corinna Vinschen wrote:
> From: Corinna Vinschen <corinna@vinschen.de>
> 
> Commit 0b6fbd396ca2f ("* exceptions.cc (_cygtls::interrupt_now): Revert
> to checking for "spinning" when choosing to defer signal.") introduced
> a bug in the loop inside the stabilize_sig_stack subroutine:
> 
> First, stabilize_sig_stack grabs the stacklock. The _cygtls::incyg
> flag is then incremented before checking if a signal has to be handled
> for the current thread.
> 
> If no signal waits, the code simply jumps out, decrements _cygtls::incyg
> and returns to the caller, which eventually releases the stacklock.
> 
> However, if a signal is waiting, stabilize_sig_stack releases the
> stacklock, calls _cygtls::call_signal_handler(), and returns to
> the start of the subroutine, trying to grab the lock.
> 
> After grabbing the lock, it increments _cygtls::incyg... wait...
> again?
> 
> The loop does not decrement _cygtls::incyg after
> _cygtls::call_signal_handler(), which returns with _cygtls::incyg
> set to 1.  So it increments incyg to 2.  If no other signal is
> waiting, stabilize_sig_stack jumps out and decrements _cygtls::incyg
> to 1.  Eventually, setjmp or longjmp both will return to user
> code with _cygtls::incyg set to 1.  This *may* be fixed at some later
> point when signals arrive, but there will be a time when the application
> runs in user code with broken signal handling.
> 
> Fixes: 0b6fbd396ca2f ("* exceptions.cc (_cygtls::interrupt_now): Revert to checking for "spinning" when choosing to defer signal.")
> Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> ---
>  winsup/cygwin/scripts/gendef | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/winsup/cygwin/scripts/gendef b/winsup/cygwin/scripts/gendef
> index 7e14f69cf71c..377ceb59b2c8 100755
> --- a/winsup/cygwin/scripts/gendef
> +++ b/winsup/cygwin/scripts/gendef
> @@ -344,6 +344,7 @@ stabilize_sig_stack:
>  	movq	\$_cygtls.start_offset,%rcx	# point to beginning
>  	addq	%r12,%rcx			#  of tls block
>  	call	_ZN7_cygtls19call_signal_handlerEv
> +	decl	_cygtls.incyg(%r12)
>  	jmp	1b
>  3:	decl	_cygtls.incyg(%r12)
>  	addq	\$0x20,%rsp
> -- 
> 2.47.0
> 

I tested this patch with Christian's longjmp test case, but
the problem does not seem to be fixed.

However, if additional patch attached as well as this patch are
applied, the problem does not happen anymore. The additional
patch removes the spinning flag completely.

What do you think?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: no-spinning.patch
URL: <https://cygwin.com/pipermail/cygwin-patches/attachments/20241205/7bbff4e1/attachment.ksh>