On Thu, 13 Mar 2025 23:46:49 +0100
Corinna Vinschen wrote:
> 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.
I think the following is the right thing. This version pulls return
addresses completely (not only one) before calling signal handler.
I think, stackptr - orig_stackptr can be larger than 2 when
user code
signal handler 1
signal handler 2
signal handler 3
signal handler 4
ret
ret
ret
HERE <= stackptr - orig_stackptr == 3
ret
Is this right?
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index c9fe6a386..453f9fea3 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1758,6 +1758,14 @@ _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 retaddrs[TLS_STACK_SIZE];
+ int n1 = stackptr - stack;
+ memcpy (retaddrs, stack, n1 * sizeof (__tlsstack_t));
+ stackptr = stack;
unlock (); /* unlock signal stack */
/* Alternate signal stack requested for this signal and alternate signal
@@ -1834,6 +1842,15 @@ _cygtls::call_signal_handler ()
signal handler. */
thisfunc (thissig, &thissi, thiscontext);
+ lock ();
+ int n2 = stackptr - stack;
+ if (n1 + n2 > TLS_STACK_SIZE)
+ api_fatal ("Signal stack exhausted.");
+ memcpy (retaddrs + n1, stack, n2 * sizeof (__tlsstack_t));
+ memcpy (stack, retaddrs, (n1 + n2) * sizeof (__tlsstack_t));
+ stackptr += n1;
+ unlock ();
+
incyg = true;
set_signal_mask (_my_tls.sigmask, (this_sa_flags & SA_SIGINFO)
--
Takashi Yano <[email protected]>
--
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
