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

#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;
}
-- 
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

Reply via email to