cygwin 3.6.0: No signals received after swapcontext() is used

[Christian Franke]

Corinna Vinschen via Cygwin wrote:
> On Mar  8 12:07, Christian Franke via Cygwin wrote:
>> ...
>> 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?

The attached testcase should test the following use cases of setcontext:
- call from regular user space
- call from a signal handler interrupting user space
- call from a signal handler interrupting a system call

It works as expected ... until the signal count reaches 256. Then 
signals are again only delivered from inside of a system call.

$ uname -r
3.6.0-0.427.g1d97f8aa4385.x86_64

$ gcc -o ctxloop ctxloop.c

$ ./ctxloop # and then press+hold ^C
0x0: sigcnt=0
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[SIGINT]
0x7ffffc4c0: sigcnt=1
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[SIGINT]
0x7ffffc440: sigcnt=2
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[SIGINT]
0x7ffffc440: sigcnt=3
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[SIGINT]
0x7ffffc400: sigcnt=4
0x7ffffcc40: main
0x7ffffcbb0: waste_time
...
[SIGINT]
0x7ffffc400: sigcnt=255
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[SIGINT]
0x7ffffc400: sigcnt=256
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[^C][^C][^C]
<return> to continue, 'q' to quit
[SIGINT]
0x7ffffc630: sigcnt=257
0x7ffffcc40: main
0x7ffffcbb0: waste_time
[^C][^C][^C]
<return> to continue, 'q' to quit
[SIGINT]
...

Interesting... Hmm... is there some 8-bit counter which overflows and 
then stucks at 0xff or 0x00?

-- 
Regards,
Christian

-------------- next part --------------
#include <math.h>
#include <signal.h>
#include <stdio.h>
#include <ucontext.h>
#include <unistd.h>

static ucontext_t uctx;
static volatile sig_atomic_t sigcnt;
static volatile void* sigframe;

static void sighandler(int sig)
{
  (void)sig;
  ++sigcnt;
  sigframe = __builtin_frame_address(0);
  write(1, "[SIGINT]\n", 9);
  if (setcontext(&uctx)) // goto loop;
    write(1, "[FAIL]\n", 7);
}

static void __attribute__ ((noinline)) waste_time()
{
  printf("%p: waste_time\n", __builtin_frame_address(0));
  volatile double x = 1.0;
  const int n = 45*1000*1000; // ~2.5s on i7-14700
  for (int i = 0; i < n; i++)
    x = asin(sin(x));
}

int main()
{
  signal(SIGINT, sighandler);
  if (getcontext(&uctx)) { // loop:
    perror("getcontext"); return 1;
  }
  printf("%p: sigcnt=%d\n", sigframe, sigcnt);
  printf("%p: main\n", __builtin_frame_address(0));

  waste_time(); // SIGINT hits user space

  printf("<return> to continue, 'q' to quit\n");
  char buf[80];
  int nr = read(1, buf, sizeof(buf)); // SIGINT hits "kernel" space
  if (nr > 0 && buf[0] == 'q')
    return 0;
  if (nr == -1)
    perror("read");

  setcontext(&uctx); // goto loop;
  perror("setcontext");
  return 1;
}