DMARC-Filter: OpenDMARC Filter v1.4.2 delorie.com 52DMlvIL495970 Authentication-Results: delorie.com; dmarc=pass (p=none dis=none) header.from=cygwin.com Authentication-Results: delorie.com; spf=pass smtp.mailfrom=cygwin.com DKIM-Filter: OpenDKIM Filter v2.11.0 delorie.com 52DMlvIL495970 Authentication-Results: delorie.com; dkim=pass (1024-bit key, unprotected) header.d=cygwin.com header.i=@cygwin.com header.a=rsa-sha256 header.s=default header.b=aM5AVB9h X-Recipient: archive-cygwin AT delorie DOT com DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7DB283857BB0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cygwin.com; s=default; t=1741906075; bh=2jl5OtiLpJygMf2aYklw+4PTJ0dQVDIYlte/jB6dUCs=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=aM5AVB9hSypPHCTRubxu+3kXt98JBBYi5gz7o1Y5e+fg+tHGPTuwCInCyg20Kg1dY +HwYfRhkbhy6MP0lHLMeMBb5ff2MSpB9GNDN79bmLFBLC82CxDK3SLPvyMJhs7D6El s1tPMmv6bA6FLbmpLdZeNvcDvKKSdqZs6LlyYyaE= X-Original-To: cygwin AT cygwin DOT com Delivered-To: cygwin AT cygwin DOT com DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9972E3858D33 Date: Thu, 13 Mar 2025 23:46:49 +0100 To: cygwin AT cygwin DOT com Subject: Re: cygwin 3.6.0: No signals received after swapcontext() is used Message-ID: Mail-Followup-To: cygwin AT cygwin DOT com References: <373993a3-9f0f-9750-60a0-950f83b3b0b5 AT t-online DOT de> <20250313204252 DOT e340f0de50838f161b0e8323 AT nifty DOT ne DOT jp> <20250313213148 DOT 6c2cb65f5e692005f28d3d2c AT nifty DOT ne DOT jp> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="5RNPtvaBxYEccfIb" Content-Disposition: inline In-Reply-To: X-BeenThere: cygwin AT cygwin DOT com X-Mailman-Version: 2.1.30 Precedence: list List-Id: General Cygwin discussions and problem reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Corinna Vinschen via Cygwin Reply-To: cygwin AT cygwin DOT com Cc: Corinna Vinschen Errors-To: cygwin-bounces~archive-cygwin=delorie DOT com AT cygwin DOT com Sender: "Cygwin" --5RNPtvaBxYEccfIb Content-Type: text/plain; charset=utf-8 Content-Disposition: inline On Mar 13 17:30, Corinna Vinschen via Cygwin wrote: > On Mar 13 21:31, Takashi Yano via Cygwin wrote: > > What about following patch instead of your sigdelayed patch? > > [...] > > @@ -1834,6 +1841,26 @@ _cygtls::call_signal_handler () > > signal handler. */ > > thisfunc (thissig, &thissi, thiscontext); > > > > + lock (); > > + if (stackptr == ptr) > > + push (retaddr1); > > + else if (stackptr == ptr + 1) > > + { > > + DWORD64 retaddr3 = pop(); > > + push (retaddr1); > > + push (retaddr3); > > + } > > + else if (stackptr == ptr - 1) > > + { > > + if (retaddr2) > > + push (retaddr2); > > + else > > + stackptr++; > > + } > > + else > > + api_fatal ("Signal stack corrupted?."); > > + unlock (); > > + > > This... looks confusing and desperately needs comments (or at least > I need comments). > > stackptr == ptr + 1 occurs if another signal arrived while the handler > was running, but isn't there a chance that sigdelayed has been pushed > as well, i.e., stackptr == ptr + 2? > > I have no idea how the stackptr == ptr - 1 situation is supposed to > happen, though. `else stackptr++;' looks weird. If you don't push a > known address, what do you expect retaddr() pointing to, afterwards? I have a slighty changed version. This one treats anything other than 0, 1 or 2 new addresses on the stack as bug. I really made an effort trying to come up with a situation where the signal stack underflows, but I just couldn't. If I'm missing something, please explain how this may happen. Apart from that, I attached my patch proposal. Thanks, Corinna --5RNPtvaBxYEccfIb Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename=0001-Cygwin-signals-pop-return-address-from-signal-stack-.patch From 588d51915e6e49bbec3ab30aff7f9fe3d4e82744 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Thu, 13 Mar 2025 23:28:53 +0100 Subject: [PATCH] Cygwin: signals: pop return address from signal stack earlier Commit a942476236b5 ("Cygwin: sigdelayed: pop return address from signal stack earlier") failed to take two facts into account: - _cygtls::call_signal_handler() potentially needs the return address as well, and - the signal handler may be interrupted by another signal. Revert the change in sigdelayed() and handle the signal stack manipulation in _cygtls::call_signal_handler() instead. Fixes: a942476236b5 ("Cygwin: sigdelayed: pop return address from signal stack earlier") Co-authored-by: Corinna Vinschen Signed-off-by: Takashi Yano Signed-off-by: Corinna Vinschen --- winsup/cygwin/exceptions.cc | 27 +++++++++++++++++++++++++++ winsup/cygwin/scripts/gendef | 24 ++++++------------------ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index c9fe6a38693c..2e25aa214a2c 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -1758,6 +1758,12 @@ _cygtls::call_signal_handler () reset_signal_arrived (); incyg = false; current_sig = 0; /* Flag that we can accept another signal */ + + /* We have to fetch the original return address from the signal stack + prior to calling the signal handler. This avoids filling up the + signal stack if the signal handler longjumps (longjmp/setcontext). */ + __tlsstack_t orig_retaddr = pop (); + __tlsstack_t *orig_stackptr = stackptr; unlock (); /* unlock signal stack */ /* Alternate signal stack requested for this signal and alternate signal @@ -1834,6 +1840,27 @@ _cygtls::call_signal_handler () signal handler. */ thisfunc (thissig, &thissi, thiscontext); + lock (); + switch (stackptr - orig_stackptr) + { + case 2: /* sigdelayed + added retaddr, pop sigdelayed */ + pop (); + fallthrough; + case 1: /* added retaddr */ + { + __tlsstack_t added_retaddr = pop(); + push (orig_retaddr); + push (added_retaddr); + } + break; + case 0: + push (orig_retaddr); + break; + default: + api_fatal ("Signal stack corrupted (%D)?", stackptr - orig_stackptr); + } + unlock (); + incyg = true; set_signal_mask (_my_tls.sigmask, (this_sa_flags & SA_SIGINFO) diff --git a/winsup/cygwin/scripts/gendef b/winsup/cygwin/scripts/gendef index e3bcae5b7351..521550175f98 100755 --- a/winsup/cygwin/scripts/gendef +++ b/winsup/cygwin/scripts/gendef @@ -161,7 +161,7 @@ _sigbe: # return here after cygwin syscall jz 2f # if so pause jmp 1b # and loop -2: movq \$-8,%r11 # decrement signal stack +2: movq \$-8,%r11 # now decrement aux stack xaddq %r11,_cygtls.stackptr(%r10) # and get pointer movq -8(%r11),%r11 # get return address from signal stack decl _cygtls.incyg(%r10) @@ -250,16 +250,6 @@ sigdelayed: movq %gs:8,%r12 # get tls movl _cygtls.saved_errno(%r12),%r15d # temporarily save saved_errno - - # We have to fetch the original return address from the signal stack - # prior to calling the signal handler. This avoids filling up the - # signal stack if the signal handler longjumps (longjmp/setcontext). - # Store the return address in a callee-saved register (r13). - movq \$-8,%r11 # decrement signal stack - xaddq %r11,_cygtls.stackptr(%r12) # and get pointer - xorq %r13,%r13 - xchgq %r13,-8(%r11) # get return address from signal stack - movq \$_cygtls.start_offset,%rcx # point to beginning of tls block addq %r12,%rcx # and store as first arg to method call _ZN7_cygtls19call_signal_handlerEv # call handler @@ -270,13 +260,15 @@ sigdelayed: jz 2f # if so pause jmp 1b # and loop - 2: testl %r15d,%r15d # was saved_errno < 0 jl 3f # yup. ignore it movq _cygtls.errno_addr(%r12),%r11 movl %r15d,(%r11) - -3: xorl %r11d,%r11d +3: movq \$-8,%r11 # now decrement aux stack + xaddq %r11,_cygtls.stackptr(%r12) # and get pointer + xorq %r10,%r10 + xchgq %r10,-8(%r11) # get return address from signal stack + xorl %r11d,%r11d movl %r11d,_cygtls.incyg(%r12) movl %r11d,_cygtls.stacklock(%r12) # release lock @@ -293,10 +285,6 @@ sigdelayed: movl 0x24(%rsp),%ebx addq %rbx,%rsp - # Before restoring callee-saved registers, move return address from - # callee-saved r13 to caller-saved r10. - movq %r13, %r10 - popq %rax popq %rbx popq %rcx -- 2.48.1 --5RNPtvaBxYEccfIb Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation: https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple --5RNPtvaBxYEccfIb--