Replace all setjmp()/longjmp() with sigsetjmp()/siglongjmp()

The setjmp() function doesn't specify whether signal masks are saved and
restored; on Linux they are not, but on BSD (including MacOSX) they are.
We want to have consistent behaviour across platforms, so we should
always use "don't save/restore signal mask" (this is also generally
going to be faster). This also works around a bug in MacOSX where the
signal-restoration on longjmp() affects the signal mask for a completely
different thread, not just the mask for the thread which did the longjmp.
The most visible effect of this was that ctrl-C was ignored on MacOSX
because the CPU thread did a longjmp which resulted in its signal mask
being applied to every thread, so that all threads had SIGINT and SIGTERM
blocked.

The POSIX-sanctioned portable way to do a jump without affecting signal
masks is to siglongjmp() to a sigjmp_buf which was created by calling
sigsetjmp() with a zero savemask parameter, so change all uses of
setjmp()/longjmp() accordingly. [Technically POSIX allows sigsetjmp(buf, 0)
to save the signal mask; however the following siglongjmp() must not
restore the signal mask, so the pair can be effectively considered as
"sigjmp/longjmp which don't touch the mask".]

For Windows we provide a trivial sigsetjmp/siglongjmp in terms of
setjmp/longjmp -- this is OK because no user will ever pass a non-zero
savemask.

The setjmp() uses in tests/tcg/test-i386.c and tests/tcg/linux-test.c
are left untouched because these are self-contained singlethreaded
test programs intended to be run under QEMU's Linux emulation, so they
have neither the portability nor the multithreading issues to deal with.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Tested-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

9 files changed, 52 insertions, 42 deletions

diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
index e37ebac9c4..1fb41c9f14 100644
--- a/coroutine-sigaltstack.c
+++ b/coroutine-sigaltstack.c
@@ -45,7 +45,7 @@ static unsigned int pool_size;
 typedef struct {
     Coroutine base;
     void *stack;
-    jmp_buf env;
+    sigjmp_buf env;
 } CoroutineUContext;
 
 /**
@@ -59,7 +59,7 @@ typedef struct {
     CoroutineUContext leader;
 
     /** Information for the signal handler (trampoline) */
-    jmp_buf tr_reenter;
+    sigjmp_buf tr_reenter;
     volatile sig_atomic_t tr_called;
     void *tr_handler;
 } CoroutineThreadState;
@@ -115,8 +115,8 @@ static void __attribute__((constructor)) coroutine_init(void)
 static void coroutine_bootstrap(CoroutineUContext *self, Coroutine *co)
 {
     /* Initialize longjmp environment and switch back the caller */
-    if (!setjmp(self->env)) {
-        longjmp(*(jmp_buf *)co->entry_arg, 1);
+    if (!sigsetjmp(self->env, 0)) {
+        siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
     }
 
     while (true) {
@@ -145,14 +145,14 @@ static void coroutine_trampoline(int signal)
     /*
      * Here we have to do a bit of a ping pong between the caller, given that
      * this is a signal handler and we have to do a return "soon". Then the
-     * caller can reestablish everything and do a longjmp here again.
+     * caller can reestablish everything and do a siglongjmp here again.
      */
-    if (!setjmp(coTS->tr_reenter)) {
+    if (!sigsetjmp(coTS->tr_reenter, 0)) {
         return;
     }
 
     /*
-     * Ok, the caller has longjmp'ed back to us, so now prepare
+     * Ok, the caller has siglongjmp'ed back to us, so now prepare
      * us for the real machine state switching. We have to jump
      * into another function here to get a new stack context for
      * the auto variables (which have to be auto-variables
@@ -179,7 +179,7 @@ static Coroutine *coroutine_new(void)
 
     /* The way to manipulate stack is with the sigaltstack function. We
      * prepare a stack, with it delivering a signal to ourselves and then
-     * put setjmp/longjmp where needed.
+     * put sigsetjmp/siglongjmp where needed.
      * This has been done keeping coroutine-ucontext as a model and with the
      * pth ideas (GNU Portable Threads). See coroutine-ucontext for the basics
      * of the coroutines and see pth_mctx.c (from the pth project) for the
@@ -220,7 +220,7 @@ static Coroutine *coroutine_new(void)
 
     /*
      * Now transfer control onto the signal stack and set it up.
-     * It will return immediately via "return" after the setjmp()
+     * It will return immediately via "return" after the sigsetjmp()
      * was performed. Be careful here with race conditions.  The
      * signal can be delivered the first time sigsuspend() is
      * called.
@@ -261,8 +261,8 @@ static Coroutine *coroutine_new(void)
      * type-conversion warnings related to the `volatile' qualifier and
      * the fact that `jmp_buf' usually is an array type.
      */
-    if (!setjmp(old_env)) {
-        longjmp(coTS->tr_reenter, 1);
+    if (!sigsetjmp(old_env, 0)) {
+        siglongjmp(coTS->tr_reenter, 1);
     }
 
     /*
@@ -311,9 +311,9 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
 
     s->current = to_;
 
-    ret = setjmp(from->env);
+    ret = sigsetjmp(from->env, 0);
     if (ret == 0) {
-        longjmp(to->env, action);
+        siglongjmp(to->env, action);
     }
     return ret;
 }
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index a9c30e9df4..bd20e384b7 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -46,7 +46,7 @@ static unsigned int pool_size;
 typedef struct {
     Coroutine base;
     void *stack;
-    jmp_buf env;
+    sigjmp_buf env;
 
 #ifdef CONFIG_VALGRIND_H
     unsigned int valgrind_stack_id;
@@ -130,8 +130,8 @@ static void coroutine_trampoline(int i0, int i1)
     co = &self->base;
 
     /* Initialize longjmp environment and switch back the caller */
-    if (!setjmp(self->env)) {
-        longjmp(*(jmp_buf *)co->entry_arg, 1);
+    if (!sigsetjmp(self->env, 0)) {
+        siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
     }
 
     while (true) {
@@ -145,14 +145,15 @@ static Coroutine *coroutine_new(void)
     const size_t stack_size = 1 << 20;
     CoroutineUContext *co;
     ucontext_t old_uc, uc;
-    jmp_buf old_env;
+    sigjmp_buf old_env;
     union cc_arg arg = {0};
 
-    /* The ucontext functions preserve signal masks which incurs a system call
-     * overhead.  setjmp()/longjmp() does not preserve signal masks but only
-     * works on the current stack.  Since we need a way to create and switch to
-     * a new stack, use the ucontext functions for that but setjmp()/longjmp()
-     * for everything else.
+    /* The ucontext functions preserve signal masks which incurs a
+     * system call overhead.  sigsetjmp(buf, 0)/siglongjmp() does not
+     * preserve signal masks but only works on the current stack.
+     * Since we need a way to create and switch to a new stack, use
+     * the ucontext functions for that but sigsetjmp()/siglongjmp() for
+     * everything else.
      */
 
     if (getcontext(&uc) == -1) {
@@ -178,8 +179,8 @@ static Coroutine *coroutine_new(void)
     makecontext(&uc, (void (*)(void))coroutine_trampoline,
                 2, arg.i[0], arg.i[1]);
 
-    /* swapcontext() in, longjmp() back out */
-    if (!setjmp(old_env)) {
+    /* swapcontext() in, siglongjmp() back out */
+    if (!sigsetjmp(old_env, 0)) {
         swapcontext(&old_uc, &uc);
     }
     return &co->base;
@@ -242,9 +243,9 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
 
     s->current = to_;
 
-    ret = setjmp(from->env);
+    ret = sigsetjmp(from->env, 0);
     if (ret == 0) {
-        longjmp(to->env, action);
+        siglongjmp(to->env, action);
     }
     return ret;
 }
diff --git a/cpu-exec.c b/cpu-exec.c
index 9fcfe9e0db..afbe4977ab 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -35,7 +35,7 @@ void cpu_loop_exit(CPUArchState *env)
     CPUState *cpu = ENV_GET_CPU(env);
 
     cpu->current_tb = NULL;
-    longjmp(env->jmp_env, 1);
+    siglongjmp(env->jmp_env, 1);
 }
 
 /* exit the current TB from a signal handler. The host registers are
@@ -47,7 +47,7 @@ void cpu_resume_from_signal(CPUArchState *env, void *puc)
     /* XXX: restore cpu registers saved in host registers */
 
     env->exception_index = -1;
-    longjmp(env->jmp_env, 1);
+    siglongjmp(env->jmp_env, 1);
 }
 #endif
 
@@ -234,7 +234,7 @@ int cpu_exec(CPUArchState *env)
 
     /* prepare setjmp context for exception handling */
     for(;;) {
-        if (setjmp(env->jmp_env) == 0) {
+        if (sigsetjmp(env->jmp_env, 0) == 0) {
             /* if an exception is pending, we execute it here */
             if (env->exception_index >= 0) {
                 if (env->exception_index >= EXCP_INTERRUPT) {
diff --git a/disas/i386.c b/disas/i386.c
index dbecf1ffb5..73cc06f1c3 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -226,7 +226,7 @@ struct dis_private {
   bfd_byte the_buffer[MAX_MNEM_SIZE];
   bfd_vma insn_start;
   int orig_sizeflag;
-  jmp_buf bailout;
+  sigjmp_buf bailout;
 };
 
 enum address_mode
@@ -303,7 +303,7 @@ fetch_data2(struct disassemble_info *info, bfd_byte *addr)
 	 STATUS.  */
       if (priv->max_fetched == priv->the_buffer)
 	(*info->memory_error_func) (status, start, info);
-      longjmp (priv->bailout, 1);
+      siglongjmp(priv->bailout, 1);
     }
   else
     priv->max_fetched = addr;
@@ -3661,7 +3661,7 @@ print_insn (bfd_vma pc, disassemble_info *info)
   start_codep = priv.the_buffer;
   codep = priv.the_buffer;
 
-  if (setjmp (priv.bailout) != 0)
+  if (sigsetjmp(priv.bailout, 0) != 0)
     {
       const char *name;
 
diff --git a/disas/m68k.c b/disas/m68k.c
index c950241f79..cc0db96cae 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -624,7 +624,7 @@ struct private
   bfd_byte *max_fetched;
   bfd_byte the_buffer[MAXLEN];
   bfd_vma insn_start;
-  jmp_buf bailout;
+  sigjmp_buf bailout;
 };
 
 /* Make sure that bytes from INFO->PRIVATE_DATA->BUFFER (inclusive)
@@ -644,7 +644,7 @@ fetch_data2(struct disassemble_info *info, bfd_byte *addr)
   if (status != 0)
     {
       (*info->memory_error_func) (status, start, info);
-      longjmp (priv->bailout, 1);
+      siglongjmp(priv->bailout, 1);
     }
   else
     priv->max_fetched = addr;
@@ -1912,9 +1912,10 @@ print_insn_m68k (bfd_vma memaddr, disassemble_info *info)
   priv.max_fetched = priv.the_buffer;
   priv.insn_start = memaddr;
 
-  if (setjmp (priv.bailout) != 0)
-    /* Error return.  */
-    return -1;
+  if (sigsetjmp(priv.bailout, 0) != 0) {
+      /* Error return.  */
+      return -1;
+  }
 
   switch (info->mach)
     {
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index ae64590cdf..3dc96568ac 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -184,7 +184,7 @@ typedef struct CPUWatchpoint {
     struct GDBRegisterState *gdb_regs;                                  \
                                                                         \
     /* Core interrupt code */                                           \
-    jmp_buf jmp_env;                                                    \
+    sigjmp_buf jmp_env;                                                 \
     int exception_index;                                                \
                                                                         \
     CPUArchState *next_cpu; /* next CPU sharing TB cache */                 \
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index bf9edeb9ab..71f5fa0a91 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -63,6 +63,14 @@
 # undef setjmp
 # define setjmp(env) _setjmp(env, NULL)
 #endif
+/* QEMU uses sigsetjmp()/siglongjmp() as the portable way to specify
+ * "longjmp and don't touch the signal masks". Since we know that the
+ * savemask parameter will always be zero we can safely define these
+ * in terms of setjmp/longjmp on Win32.
+ */
+#define sigjmp_buf jmp_buf
+#define sigsetjmp(env, savemask) setjmp(env)
+#define siglongjmp(env, val) longjmp(env, val)
 
 /* Declaration of ffs() is missing in MinGW's strings.h. */
 int ffs(int i);
diff --git a/monitor.c b/monitor.c
index 6a0f2573f5..32a6e74fd9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2740,7 +2740,7 @@ static const mon_cmd_t qmp_cmds[] = {
 /*******************************************************************/
 
 static const char *pch;
-static jmp_buf expr_env;
+static sigjmp_buf expr_env;
 
 #define MD_TLONG 0
 #define MD_I32   1
@@ -3135,7 +3135,7 @@ static const MonitorDef monitor_defs[] = {
 static void expr_error(Monitor *mon, const char *msg)
 {
     monitor_printf(mon, "%s\n", msg);
-    longjmp(expr_env, 1);
+    siglongjmp(expr_env, 1);
 }
 
 /* return 0 if OK, -1 if not found */
@@ -3345,7 +3345,7 @@ static int64_t expr_sum(Monitor *mon)
 static int get_expr(Monitor *mon, int64_t *pval, const char **pp)
 {
     pch = *pp;
-    if (setjmp(expr_env)) {
+    if (sigsetjmp(expr_env, 0)) {
         *pp = pch;
         return -1;
     }
diff --git a/user-exec.c b/user-exec.c
index c71acbc503..71bd6c531c 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -70,7 +70,7 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
 #endif
     }
     env1->exception_index = -1;
-    longjmp(env1->jmp_env, 1);
+    siglongjmp(env1->jmp_env, 1);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is