[Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Riku Voipio]

The proper exit code for dieing from an uncaught signal is -<signal>.
The kernel doesn't allow exit() or _exit() to pass a negative value.
To get the proper exit code we need to actually die from an uncaught
signal.

A default signal handler is installed, we send ourself a signal
and we wait for it to arrive.

Patch originates from Scratchbox

Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
---
 linux-user/signal.c |   37 +++++++++++++++++++++++++------------
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index e0f6aaf..dac9933 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -24,6 +24,7 @@
 #include <unistd.h>
 #include <signal.h>
 #include <errno.h>
+#include <assert.h>
 #include <sys/ucontext.h>
 
 #include "qemu.h"
@@ -328,21 +329,33 @@ static inline void free_sigqueue(CPUState *env, struct sigqueue *q)
 static void __attribute((noreturn)) force_sig(int sig)
 {
     int host_sig;
+    struct sigaction act;
     host_sig = target_to_host_signal(sig);
     fprintf(stderr, "qemu: uncaught target signal %d (%s) - exiting\n",
             sig, strsignal(host_sig));
-#if 1
-    _exit(-host_sig);
-#else
-    {
-        struct sigaction act;
-        sigemptyset(&act.sa_mask);
-        act.sa_flags = SA_SIGINFO;
-        act.sa_sigaction = SIG_DFL;
-        sigaction(SIGABRT, &act, NULL);
-        abort();
-    }
-#endif
+
+    /* The proper exit code for dieing from an uncaught signal is
+     * -<signal>.  The kernel doesn't allow exit() or _exit() to pass
+     * a negative value.  To get the proper exit code we need to
+     * actually die from an uncaught signal.  Here the default signal
+     * handler is installed, we send ourself a signal and we wait for
+     * it to arrive. */
+    sigfillset(&act.sa_mask);
+    act.sa_handler = SIG_DFL;
+    sigaction(host_sig, &act, NULL);
+
+    /* For some reason raise(host_sig) doesn't send the signal when
+     * statically linked on x86-64. */
+    kill(getpid(), host_sig);
+
+    /* Make sure the signal isn't masked (just reuse the mask inside
+    of act) */
+    sigdelset(&act.sa_mask, host_sig);
+    sigsuspend(&act.sa_mask);
+
+    /* unreachable */
+    assert(0);
+
 }
 
 /* queue a signal so that it will be send to the virtual CPU as soon
-- 
1.5.6.5


-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Thiemo Seufer]

Riku Voipio wrote:
> The proper exit code for dieing from an uncaught signal is -<signal>.
> The kernel doesn't allow exit() or _exit() to pass a negative value.
> To get the proper exit code we need to actually die from an uncaught
> signal.
> 
> A default signal handler is installed, we send ourself a signal
> and we wait for it to arrive.
> 
> Patch originates from Scratchbox

So, who holds the copyright (and deserves the credit)?


Thiemo

> Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
> ---
>  linux-user/signal.c |   37 +++++++++++++++++++++++++------------
>  1 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index e0f6aaf..dac9933 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -24,6 +24,7 @@
>  #include <unistd.h>
>  #include <signal.h>
>  #include <errno.h>
> +#include <assert.h>
>  #include <sys/ucontext.h>
>  
>  #include "qemu.h"
> @@ -328,21 +329,33 @@ static inline void free_sigqueue(CPUState *env, struct sigqueue *q)
>  static void __attribute((noreturn)) force_sig(int sig)
>  {
>      int host_sig;
> +    struct sigaction act;
>      host_sig = target_to_host_signal(sig);
>      fprintf(stderr, "qemu: uncaught target signal %d (%s) - exiting\n",
>              sig, strsignal(host_sig));
> -#if 1
> -    _exit(-host_sig);
> -#else
> -    {
> -        struct sigaction act;
> -        sigemptyset(&act.sa_mask);
> -        act.sa_flags = SA_SIGINFO;
> -        act.sa_sigaction = SIG_DFL;
> -        sigaction(SIGABRT, &act, NULL);
> -        abort();
> -    }
> -#endif
> +
> +    /* The proper exit code for dieing from an uncaught signal is
> +     * -<signal>.  The kernel doesn't allow exit() or _exit() to pass
> +     * a negative value.  To get the proper exit code we need to
> +     * actually die from an uncaught signal.  Here the default signal
> +     * handler is installed, we send ourself a signal and we wait for
> +     * it to arrive. */
> +    sigfillset(&act.sa_mask);
> +    act.sa_handler = SIG_DFL;
> +    sigaction(host_sig, &act, NULL);
> +
> +    /* For some reason raise(host_sig) doesn't send the signal when
> +     * statically linked on x86-64. */
> +    kill(getpid(), host_sig);
> +
> +    /* Make sure the signal isn't masked (just reuse the mask inside
> +    of act) */
> +    sigdelset(&act.sa_mask, host_sig);
> +    sigsuspend(&act.sa_mask);
> +
> +    /* unreachable */
> +    assert(0);
> +
>  }
>  
>  /* queue a signal so that it will be send to the virtual CPU as soon
> -- 
> 1.5.6.5
> 
> 
> -- 
> "rm -rf" only sounds scary if you don't have backups
> 

Re: [Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Jamie Lokier]

> > The proper exit code for dieing from an uncaught signal is -<signal>.
> > The kernel doesn't allow exit() or _exit() to pass a negative value.
> > To get the proper exit code we need to actually die from an uncaught
> > signal.

It's nothing like -<signal>, so the comment should be changed.

The general principle of sending yourself a signal to get the right
exit status is good.

> > +    sigfillset(&act.sa_mask);
> > +    act.sa_handler = SIG_DFL;
> > +    sigaction(host_sig, &act, NULL);

What if the SIG_DFL _host_ behaviour is not to terminate the host
process, but it has terminated the guest process?  Awkward one.



> > +    /* For some reason raise(host_sig) doesn't send the signal when
> > +     * statically linked on x86-64. */
> > +    kill(getpid(), host_sig);

Is getpid() always right here, and should tgkill() or tkill() be used when
clone is supported?

-- Jamie

Re: [Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Riku Voipio]

On Thu, Nov 27, 2008 at 12:16:15PM +0000, Jamie Lokier wrote:
> > > The proper exit code for dieing from an uncaught signal is -<signal>.
> > > The kernel doesn't allow exit() or _exit() to pass a negative value.
> > > To get the proper exit code we need to actually die from an uncaught
> > > signal.
> 
> It's nothing like -<signal>, so the comment should be changed.

Something like:

Proper exit code for dieing from an uncaught signal differs from normal
exit, so applications using WISIGNALED/WTERMSIG don't get the expected
result. The proper way is to actually die from an uncaught signal.

> The general principle of sending yourself a signal to get the right
> exit status is good.

> > > +    sigfillset(&act.sa_mask);
> > > +    act.sa_handler = SIG_DFL;
> > > +    sigaction(host_sig, &act, NULL);
> 
> What if the SIG_DFL _host_ behaviour is not to terminate the host
> process, but it has terminated the guest process?  Awkward one.

Could this happen on Linux or is this a portability issue?

> > > +    /* For some reason raise(host_sig) doesn't send the signal when
> > > +     * statically linked on x86-64. */
> > > +    kill(getpid(), host_sig);

> Is getpid() always right here, and should tgkill() or tkill() be used when
> clone is supported?

I'll have to look into this. The thought that this code needs to
do multithreaded signal handling (preferredly in a portable fashion)
feels like I'm heading towards endless swamplands..

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] linux-user: Proper exit code for uncaught signals

[Riku Voipio]

On Thu, Nov 27, 2008 at 12:42:33PM +0100, Thiemo Seufer wrote:
> Riku Voipio wrote:
> > The proper exit code for dieing from an uncaught signal is -<signal>.
> > The kernel doesn't allow exit() or _exit() to pass a negative value.
> > To get the proper exit code we need to actually die from an uncaught
> > signal.
> > 
> > A default signal handler is installed, we send ourself a signal
> > and we wait for it to arrive.
> > 
> > Patch originates from Scratchbox
> 
> So, who holds the copyright (and deserves the credit)?

The qemu tarball[1] provided on scratchbox site doesn't
document who wrote patches and who has the copyright. Most
likely a contractor for Nokia wrote it, and the copyright is
Nokia's.

Lauro might know as the patch is also at the qemu-arm-eabi
sf project site.

[1] http://scratchbox.org/download/files/sbox-releases/stable/src/scratchbox-devkit-cputransp-qemu-arm-cvs-m-1.0.7/qemu-arm-0108.tar.gz

> Thiemo
> 
> > Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
> > ---
> >  linux-user/signal.c |   37 +++++++++++++++++++++++++------------
> >  1 files changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/linux-user/signal.c b/linux-user/signal.c
> > index e0f6aaf..dac9933 100644
> > --- a/linux-user/signal.c
> > +++ b/linux-user/signal.c
> > @@ -24,6 +24,7 @@
> >  #include <unistd.h>
> >  #include <signal.h>
> >  #include <errno.h>
> > +#include <assert.h>
> >  #include <sys/ucontext.h>
> >  
> >  #include "qemu.h"
> > @@ -328,21 +329,33 @@ static inline void free_sigqueue(CPUState *env, struct sigqueue *q)
> >  static void __attribute((noreturn)) force_sig(int sig)
> >  {
> >      int host_sig;
> > +    struct sigaction act;
> >      host_sig = target_to_host_signal(sig);
> >      fprintf(stderr, "qemu: uncaught target signal %d (%s) - exiting\n",
> >              sig, strsignal(host_sig));
> > -#if 1
> > -    _exit(-host_sig);
> > -#else
> > -    {
> > -        struct sigaction act;
> > -        sigemptyset(&act.sa_mask);
> > -        act.sa_flags = SA_SIGINFO;
> > -        act.sa_sigaction = SIG_DFL;
> > -        sigaction(SIGABRT, &act, NULL);
> > -        abort();
> > -    }
> > -#endif
> > +
> > +    /* The proper exit code for dieing from an uncaught signal is
> > +     * -<signal>.  The kernel doesn't allow exit() or _exit() to pass
> > +     * a negative value.  To get the proper exit code we need to
> > +     * actually die from an uncaught signal.  Here the default signal
> > +     * handler is installed, we send ourself a signal and we wait for
> > +     * it to arrive. */
> > +    sigfillset(&act.sa_mask);
> > +    act.sa_handler = SIG_DFL;
> > +    sigaction(host_sig, &act, NULL);
> > +
> > +    /* For some reason raise(host_sig) doesn't send the signal when
> > +     * statically linked on x86-64. */
> > +    kill(getpid(), host_sig);
> > +
> > +    /* Make sure the signal isn't masked (just reuse the mask inside
> > +    of act) */
> > +    sigdelset(&act.sa_mask, host_sig);
> > +    sigsuspend(&act.sa_mask);
> > +
> > +    /* unreachable */
> > +    assert(0);
> > +
> >  }
> >  
> >  /* queue a signal so that it will be send to the virtual CPU as soon
> > -- 
> > 1.5.6.5
> > 
> > 
> > -- 
> > "rm -rf" only sounds scary if you don't have backups
> > 

-- 
"rm -rf" only sounds scary if you don't have backups