[BZ,#18080] S390: Fix setcontext/swapcontext which are not restoring sigmask.
Commit Message
Hi,
on s390/s390x, a call to setcontext or swapcontext does not restore the
signal mask.
This can be reproduced with the following pseudocode:
-getcontext()
-block signal with sigprocmask
-setcontext()
-signal shouldn´t be blocked anymore.
The corresponding sigprocmask calls in files:
sysdeps/unix/sysv/linux/s390/s390-32/setcontext.S
sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S
sysdeps/unix/sysv/linux/s390/s390-64/setcontext.S
sysdeps/unix/sysv/linux/s390/s390-64/swapcontext.S
are using SIG_BLOCK instead of SIG_SETMASK.
This patch fixes this behaviour and adds a new testcase,
which checks signal mask after setcontext()/swapcontext() calls
and checks, if a pending signal was delivered after those calls.
Testsuite runs without regression on s390 and checked if new testcase
passes on x86_64.
Ok to commit?
Bye
Stefan
---
2015-03-04 Stefan Liebler <stli@linux.vnet.ibm.com>
[BZ #18080]
* sysdeps/unix/sysv/linux/s390/s390-32/setcontext.S
(__setcontext): Use SIG_SETMASK instead of SIG_BLOCK.
* sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S
(__swapcontext): Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/setcontext.S
(__setcontext): Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/swapcontext.S
(__swapcontext): Likewise.
* stdlib/Makefile (tests): Add new testcase tst-setcontext2.
* stdlib/tst-setcontext2.c: New file.
Comments
Stefan Liebler <stli@linux.vnet.ibm.com> writes:
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S b/sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S
> index c40f465..c385110 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/swapcontext.S
> @@ -75,7 +75,7 @@ ENTRY(__swapcontext)
> stm %r0,%r15,SC_GPRS(%r1)
>
> /* sigprocmask (SIG_SETMASK, &sc->sc_mask, NULL). */
> - la %r2,SIG_BLOCK
> + la %r2,SIG_SETMASK
> lr %r5,%r0
> la %r3,SC_MASK(%r5)
> slr %r4,%r4
There is no need to call sigprocmask twice, you can swap the mask with a
single call. Also, %r2 is clobbered here.
Andreas.
On 03/04/2015 12:26 PM, Stefan Liebler wrote:
> +volatile int handlerCalled;
I think this should use sigatomic_t.
There are a few missing spaces in function calls:
> +check(const char *funcName)
> + sigprocmask(SIG_BLOCK, NULL, &set);
> + if (sigismember(&set, SIGUSR2) != 0)
> + if (sigismember(&set, SIGUSR1) == 0)
> + if (sigismember(&set, SIGUSR2) == 0)
> + if (sigismember(&oldctx.uc_sigmask, SIGUSR2) == 0)
> + if (sigismember(&oldctx.uc_sigmask, SIGUSR1) == 0)
Sorry, no other comments at this time.
The first line of a new file (stdlib/tst-setcontext2.c in this case)
should be the comment describing that file. The copyright notice should
come after that, not be the first line.
@@ -66,11 +66,11 @@ test-srcs := tst-fmtmsg
tests := tst-strtol tst-strtod testmb testrand testsort testdiv \
test-canon test-canon2 tst-strtoll tst-environ \
tst-xpg-basename tst-random tst-random2 tst-bsearch \
- tst-limits tst-rand48 bug-strtod tst-setcontext \
- test-a64l tst-qsort tst-system testmb2 bug-strtod2 \
- tst-atof1 tst-atof2 tst-strtod2 tst-strtod3 tst-rand48-2 \
- tst-makecontext tst-strtod4 tst-strtod5 tst-qsort2 \
- tst-makecontext2 tst-strtod6 tst-unsetenv1 \
+ tst-limits tst-rand48 bug-strtod tst-setcontext \
+ tst-setcontext2 test-a64l tst-qsort tst-system testmb2 \
+ bug-strtod2 tst-atof1 tst-atof2 tst-strtod2 tst-strtod3 \
+ tst-rand48-2 tst-makecontext tst-strtod4 tst-strtod5 \
+ tst-qsort2 tst-makecontext2 tst-strtod6 tst-unsetenv1 \
tst-makecontext3 bug-getcontext bug-fmtmsg1 \
tst-secure-getenv tst-strtod-overflow tst-strtod-round \
tst-tininess tst-strtod-underflow tst-tls-atexit
new file mode 100644
@@ -0,0 +1,219 @@
+/* Copyright (C) 2015 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+volatile int global;
+volatile int handlerCalled;
+
+static void
+check(const char *funcName)
+{
+ sigset_t set;
+
+ /* check if SIGUSR2 is unblocked after setcontext-call. */
+ sigprocmask(SIG_BLOCK, NULL, &set);
+
+ if (sigismember(&set, SIGUSR2) != 0)
+ {
+ printf ("FAIL: SIGUSR2 is blocked after %s.\n", funcName);
+ exit (1);
+ }
+
+ if (sigismember(&set, SIGUSR1) == 0)
+ {
+ printf ("FAIL: SIGUSR1 is not blocked after %s.\n", funcName);
+ exit (1);
+ }
+}
+
+static void
+signalmask (int how, int signum)
+{
+ sigset_t set;
+ sigemptyset (&set);
+ sigaddset (&set, signum);
+ if (sigprocmask (how, &set, NULL) != 0)
+ {
+ printf ("FAIL: sigprocmaks (%d, %d, NULL): %m\n", how, signum);
+ exit (1);
+ }
+}
+
+static void
+signalpending (int signum, const char *msg)
+{
+ sigset_t set;
+ sigemptyset (&set);
+ if (sigpending (&set) != 0)
+ {
+ printf ("FAIL: sigpending: %m\n");
+ exit (1);
+ }
+ if (sigismember(&set, SIGUSR2) == 0)
+ {
+ printf ("FAIL: Signal %d is not pending %s\n", signum, msg);
+ exit (1);
+ }
+}
+
+static void
+handler (int __attribute__ ((unused)) signum)
+{
+ handlerCalled ++;
+}
+
+static int
+do_test (void)
+{
+ /* This test checks, if setcontext, swapcontext restores sig-mask. */
+
+ ucontext_t ctx, oldctx;
+ struct sigaction action;
+ pid_t pid;
+
+ pid = getpid ();
+
+ /* unblock SIGUSR2 */
+ signalmask (SIG_UNBLOCK, SIGUSR2);
+
+ /* block SIGUSR1 */
+ signalmask (SIG_BLOCK, SIGUSR1);
+
+ /* register handler for SIGUSR2 */
+ action.sa_flags = 0;
+ action.sa_handler = handler;
+ sigemptyset (&action.sa_mask);
+ sigaction (SIGUSR2, &action, NULL);
+
+ if (getcontext (&ctx) != 0)
+ {
+ printf ("FAIL: getcontext: %m\n");
+ exit (1);
+ }
+
+ global++;
+
+ if (global == 1)
+ {
+ puts ("after getcontext");
+
+ /* block SIGUSR2 */
+ signalmask (SIG_BLOCK, SIGUSR2);
+
+ /* send SIGUSR2 to me */
+ handlerCalled = 0;
+ kill (pid, SIGUSR2);
+
+ /* was SIGUSR2 handler called? */
+ if (handlerCalled != 0)
+ {
+ puts ("FAIL: signal handler was called, but signal was blocked.");
+ exit (1);
+ }
+
+ /* is SIGUSR2 pending? */
+ signalpending (SIGUSR2, "before setcontext");
+
+ /* SIGUSR2 will be unblocked by setcontext-call. */
+ if (setcontext (&ctx) != 0)
+ {
+ printf ("FAIL: setcontext: %m\n");
+ exit (1);
+ }
+ }
+ else if (global == 2)
+ {
+ puts ("after setcontext");
+
+ /* check SIGUSR1/2 */
+ check ("setcontext");
+
+ /* was SIGUSR2 handler called? */
+ if (handlerCalled != 1)
+ {
+ puts ("FAIL: signal handler was not called after setcontext.");
+ exit (1);
+ }
+
+ /* block SIGUSR2 */
+ signalmask (SIG_BLOCK, SIGUSR2);
+
+ /* send SIGUSR2 to me */
+ handlerCalled = 0;
+ kill (pid, SIGUSR2);
+
+ /* was SIGUSR2 handler called? */
+ if (handlerCalled != 0)
+ {
+ puts ("FAIL: signal handler was called, but signal was blocked.");
+ exit (1);
+ }
+
+ /* is SIGUSR2 pending? */
+ signalpending (SIGUSR2, "before swapcontext");
+
+ if (swapcontext (&oldctx, &ctx) != 0)
+ {
+ printf ("FAIL: swapcontext: %m\n");
+ exit (1);
+ }
+
+ puts ("FAIL: returned from swapcontext");
+ exit (1);
+ }
+ else if ( global != 3 )
+ {
+ puts ("FAIL: 'global' not incremented three times");
+ exit (1);
+ }
+
+ puts ("after swapcontext");
+ /* check SIGUSR1/2 */
+ check ("swapcontext");
+
+ /* was SIGUSR2 handler called? */
+ if (handlerCalled != 1)
+ {
+ puts ("FAIL: signal handler was not called after swapcontext.");
+ exit (1);
+ }
+
+ /* check sigmask in old context of swapcontext-call */
+ if (sigismember(&oldctx.uc_sigmask, SIGUSR2) == 0)
+ {
+ puts ("FAIL: SIGUSR2 is not blocked in oldctx.uc_sigmask.");
+ exit (1);
+ }
+
+ if (sigismember(&oldctx.uc_sigmask, SIGUSR1) == 0)
+ {
+ puts ("FAIL: SIGUSR1 is not blocked in oldctx.uc_sigmaks.");
+ exit (1);
+ }
+
+ puts ("test succeeded");
+ return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
@@ -34,7 +34,7 @@ ENTRY(__setcontext)
lr %r1,%r2
/* rt_sigprocmask (SIG_SETMASK, &sc->sc_mask, NULL, sigsetsize). */
- la %r2,SIG_BLOCK
+ la %r2,SIG_SETMASK
la %r3,SC_MASK(%r1)
slr %r4,%r4
lhi %r5,_NSIG8
@@ -75,7 +75,7 @@ ENTRY(__swapcontext)
stm %r0,%r15,SC_GPRS(%r1)
/* sigprocmask (SIG_SETMASK, &sc->sc_mask, NULL). */
- la %r2,SIG_BLOCK
+ la %r2,SIG_SETMASK
lr %r5,%r0
la %r3,SC_MASK(%r5)
slr %r4,%r4
@@ -34,7 +34,7 @@ ENTRY(__setcontext)
lgr %r1,%r2
/* sigprocmask (SIG_SETMASK, &sc->sc_mask, NULL). */
- la %r2,SIG_BLOCK
+ la %r2,SIG_SETMASK
la %r3,SC_MASK(%r1)
slgr %r4,%r4
lghi %r5,_NSIG8
@@ -75,7 +75,7 @@ ENTRY(__swapcontext)
stmg %r0,%r15,SC_GPRS(%r1)
/* rt_sigprocmask (SIG_SETMASK, &sc->sc_mask, NULL, sigsetsize). */
- la %r2,SIG_BLOCK
+ la %r2,SIG_SETMASK
lgr %r5,%r0
la %r3,SC_MASK(%r5)
lghi %r5,_NSIG8