On Mar 8 12:07, Christian Franke via Cygwin wrote:
> Corinna Vinschen via Cygwin wrote:
> > On Mar 6 13:24, Christian Franke via Cygwin wrote:
> > > Found because 'stress-ng --context 1 ...' always hangs.
> > >
> > > The attached testcase uses the example from Linux swapcontext(3) to call
> > > the
> > > context functions.
> > Just tested with 3.5.3 and it doesn't work there, either.
> >
> > So yeah, it's a bug, but it's not a 3.6 regression and of minor
> > importance. We can look into that for 3.7.
> >
>
> This is possibly a regression introduced in 3.0.6. A comparison of strace
> outputs of signal handling before and after the swapcontext() calls reveals
> that 'incyg' is incorrectly set after swapcontext(). A revert of
> https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=c5f9eed1
> fixes both the testcase and 'stress-ng --context ...'.
Ahhh, good point. When I looked into get/setcontext last week, I was
wondering about this _my_tls.incyg = true, but didn't look into the
history.
I'm not sure if this was the right thing to do anyway. Unfortunately,
my commit message of the time was really really bad, and I don't see
anything in the mailing lists pointing out where the idea to set iscyg
was coming from.
In the light of the signal changes in Cygwin, it might be a good idea to
drop it.
> No patch provided because I'm not sure how to handle the (at least) two
> other use cases of setcontext() correctly:
> - the call from _cygtls::call_signal_handler(), and
I re-read the POSIX and Linux man pages on sigaction. None of them
claim that the context given to the signal handler is anything other
than read-only information given to the handler, and, worse, supposed
to be used by the signal mechanism after the signal handler returns.
So it seems calling setcontext() on the vague notion that the signal
handler expects this if it changed the context, was a bug from the start.
I'm inclined to apply this patch:
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 6d8985374b9e..a05883e3fc4f 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1850,14 +1850,6 @@ _cygtls::call_signal_handler ()
? context.uc_sigmask : this_oldmask);
if (this_errno >= 0)
set_errno (this_errno);
- if (this_sa_flags & SA_SIGINFO)
- {
- /* If more than just the sigmask in the context has been changed by
- the signal handler, call setcontext. */
- context_copy.uc_sigmask = context.uc_sigmask;
- if (memcmp (&context, &context_copy, sizeof context) != 0)
- setcontext (&context);
- }
}
/* FIXME: Since 2011 this return statement always returned 1 (meaning
@@ -1899,7 +1891,6 @@ setcontext (const ucontext_t *ucp)
{
PCONTEXT ctx = (PCONTEXT) &ucp->uc_mcontext;
set_signal_mask (_my_tls.sigmask, ucp->uc_sigmask);
- _my_tls.incyg = true;
RtlRestoreContext (ctx, NULL);
/* If we got here, something was wrong. */
set_errno (EINVAL);
It's not quite clear to me why signal handling should be broken if
setcontext is used inside a signal handler. The incyg flag is false
when running the signal handler and that's correct. Theoretically a
running signal handler is not different from other process code.
Do you have an STC, by any chance?
Thanks,
Corinna
--
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
