From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LDRmp-00059R-66 for qemu-devel@nongnu.org; Thu, 18 Dec 2008 17:56:07 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LDRmo-00058o-82 for qemu-devel@nongnu.org; Thu, 18 Dec 2008 17:56:06 -0500 Received: from [199.232.76.173] (port=41282 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LDRmn-00058j-Sl for qemu-devel@nongnu.org; Thu, 18 Dec 2008 17:56:05 -0500 Received: from hall.aurel32.net ([88.191.82.174]:36398) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LDRmn-0000cC-3X for qemu-devel@nongnu.org; Thu, 18 Dec 2008 17:56:05 -0500 Date: Thu, 18 Dec 2008 23:55:59 +0100 From: Aurelien Jarno Subject: Re: [Qemu-devel] [2/2] User-mode GDB stub improvements - handle signals Message-ID: <20081218225559.GL12048@volta.aurel32.net> References: <20081212190902.GB31647@caradoc.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20081212190902.GB31647@caradoc.them.org> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Paul Brook On Fri, Dec 12, 2008 at 02:09:02PM -0500, Daniel Jacobowitz wrote: > From: Daniel Jacobowitz > > Handle signals in the user-mode GDB stub. Report them to GDB, and > allow it to change or cancel them. Also correct the protocol numbering; > it happens to match Linux numbering for SIGINT and SIGTRAP, but that's > just good fortune. > > Signed-off-by: Daniel Jacobowitz Thanks, applied. > --- > > I find this makes the gdb stub a lot more useful. For example, > you'll get a chance to see SIGSEGV or SIGABRT before they take > down the simulated program. Guess no one who uses the gdbstub > writes programs that crash? :-) > > diff -u gdbstub.c gdbstub.c > --- gdbstub.c (working copy) > +++ gdbstub.c (working copy) > @@ -38,18 +38,213 @@ > #define MAX_PACKET_LENGTH 4096 > > #include "qemu_socket.h" > -#ifdef _WIN32 > -/* XXX: these constants may be independent of the host ones even for Unix */ > -#ifndef SIGTRAP > -#define SIGTRAP 5 > -#endif > -#ifndef SIGINT > -#define SIGINT 2 > -#endif > + > + > +enum { > + GDB_SIGNAL_0 = 0, > + GDB_SIGNAL_INT = 2, > + GDB_SIGNAL_TRAP = 5, > + GDB_SIGNAL_UNKNOWN = 143 > +}; > + > +#ifdef CONFIG_USER_ONLY > + > +/* Map target signal numbers to GDB protocol signal numbers and vice > + * versa. For user emulation's currently supported systems, we can > + * assume most signals are defined. > + */ > + > +static int gdb_signal_table[] = { > + 0, > + TARGET_SIGHUP, > + TARGET_SIGINT, > + TARGET_SIGQUIT, > + TARGET_SIGILL, > + TARGET_SIGTRAP, > + TARGET_SIGABRT, > + -1, /* SIGEMT */ > + TARGET_SIGFPE, > + TARGET_SIGKILL, > + TARGET_SIGBUS, > + TARGET_SIGSEGV, > + TARGET_SIGSYS, > + TARGET_SIGPIPE, > + TARGET_SIGALRM, > + TARGET_SIGTERM, > + TARGET_SIGURG, > + TARGET_SIGSTOP, > + TARGET_SIGTSTP, > + TARGET_SIGCONT, > + TARGET_SIGCHLD, > + TARGET_SIGTTIN, > + TARGET_SIGTTOU, > + TARGET_SIGIO, > + TARGET_SIGXCPU, > + TARGET_SIGXFSZ, > + TARGET_SIGVTALRM, > + TARGET_SIGPROF, > + TARGET_SIGWINCH, > + -1, /* SIGLOST */ > + TARGET_SIGUSR1, > + TARGET_SIGUSR2, > + TARGET_SIGPWR, > + -1, /* SIGPOLL */ > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + -1, > + __SIGRTMIN + 1, > + __SIGRTMIN + 2, > + __SIGRTMIN + 3, > + __SIGRTMIN + 4, > + __SIGRTMIN + 5, > + __SIGRTMIN + 6, > + __SIGRTMIN + 7, > + __SIGRTMIN + 8, > + __SIGRTMIN + 9, > + __SIGRTMIN + 10, > + __SIGRTMIN + 11, > + __SIGRTMIN + 12, > + __SIGRTMIN + 13, > + __SIGRTMIN + 14, > + __SIGRTMIN + 15, > + __SIGRTMIN + 16, > + __SIGRTMIN + 17, > + __SIGRTMIN + 18, > + __SIGRTMIN + 19, > + __SIGRTMIN + 20, > + __SIGRTMIN + 21, > + __SIGRTMIN + 22, > + __SIGRTMIN + 23, > + __SIGRTMIN + 24, > + __SIGRTMIN + 25, > + __SIGRTMIN + 26, > + __SIGRTMIN + 27, > + __SIGRTMIN + 28, > + __SIGRTMIN + 29, > + __SIGRTMIN + 30, > + __SIGRTMIN + 31, > + -1, /* SIGCANCEL */ > + __SIGRTMIN, > + __SIGRTMIN + 32, > + __SIGRTMIN + 33, > + __SIGRTMIN + 34, > + __SIGRTMIN + 35, > + __SIGRTMIN + 36, > + __SIGRTMIN + 37, > + __SIGRTMIN + 38, > + __SIGRTMIN + 39, > + __SIGRTMIN + 40, > + __SIGRTMIN + 41, > + __SIGRTMIN + 42, > + __SIGRTMIN + 43, > + __SIGRTMIN + 44, > + __SIGRTMIN + 45, > + __SIGRTMIN + 46, > + __SIGRTMIN + 47, > + __SIGRTMIN + 48, > + __SIGRTMIN + 49, > + __SIGRTMIN + 50, > + __SIGRTMIN + 51, > + __SIGRTMIN + 52, > + __SIGRTMIN + 53, > + __SIGRTMIN + 54, > + __SIGRTMIN + 55, > + __SIGRTMIN + 56, > + __SIGRTMIN + 57, > + __SIGRTMIN + 58, > + __SIGRTMIN + 59, > + __SIGRTMIN + 60, > + __SIGRTMIN + 61, > + __SIGRTMIN + 62, > + __SIGRTMIN + 63, > + __SIGRTMIN + 64, > + __SIGRTMIN + 65, > + __SIGRTMIN + 66, > + __SIGRTMIN + 67, > + __SIGRTMIN + 68, > + __SIGRTMIN + 69, > + __SIGRTMIN + 70, > + __SIGRTMIN + 71, > + __SIGRTMIN + 72, > + __SIGRTMIN + 73, > + __SIGRTMIN + 74, > + __SIGRTMIN + 75, > + __SIGRTMIN + 76, > + __SIGRTMIN + 77, > + __SIGRTMIN + 78, > + __SIGRTMIN + 79, > + __SIGRTMIN + 80, > + __SIGRTMIN + 81, > + __SIGRTMIN + 82, > + __SIGRTMIN + 83, > + __SIGRTMIN + 84, > + __SIGRTMIN + 85, > + __SIGRTMIN + 86, > + __SIGRTMIN + 87, > + __SIGRTMIN + 88, > + __SIGRTMIN + 89, > + __SIGRTMIN + 90, > + __SIGRTMIN + 91, > + __SIGRTMIN + 92, > + __SIGRTMIN + 93, > + __SIGRTMIN + 94, > + __SIGRTMIN + 95, > + -1, /* SIGINFO */ > + -1, /* UNKNOWN */ > + -1, /* DEFAULT */ > + -1, > + -1, > + -1, > + -1, > + -1, > + -1 > +}; > #else > -#include > +/* In system mode we only need SIGINT and SIGTRAP; other signals > + are not yet supported. */ > + > +enum { > + TARGET_SIGINT = 2, > + TARGET_SIGTRAP = 5 > +}; > + > +static int gdb_signal_table[] = { > + -1, > + -1, > + TARGET_SIGINT, > + -1, > + -1, > + TARGET_SIGTRAP > +}; > +#endif > + > +#ifdef CONFIG_USER_ONLY > +static int target_signal_to_gdb (int sig) > +{ > + int i; > + for (i = 0; i < ARRAY_SIZE (gdb_signal_table); i++) > + if (gdb_signal_table[i] == sig) > + return i; > + return GDB_SIGNAL_UNKNOWN; > +} > #endif > > +static int gdb_signal_to_target (int sig) > +{ > + if (sig < ARRAY_SIZE (gdb_signal_table)) > + return gdb_signal_table[sig]; > + else > + return -1; > +} > + > //#define DEBUG_GDB > > typedef struct GDBRegisterState { > @@ -1300,7 +1495,7 @@ > switch(ch) { > case '?': > /* TODO: Make this return the correct value for user-mode. */ > - snprintf(buf, sizeof(buf), "T%02xthread:%02x;", SIGTRAP, > + snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP, > s->c_cpu->cpu_index+1); > put_packet(s, buf); > /* Remove all the breakpoints when this query is issued, > @@ -1331,10 +1526,13 @@ > s->c_cpu->pc = addr; > #endif > } > + s->signal = 0; > gdb_continue(s); > return RS_IDLE; > case 'C': > - s->signal = strtoul(p, (char **)&p, 16); > + s->signal = gdb_signal_to_target (strtoul(p, (char **)&p, 16)); > + if (s->signal == -1) > + s->signal = 0; > gdb_continue(s); > return RS_IDLE; > case 'k': > @@ -1692,16 +1890,16 @@ > } > snprintf(buf, sizeof(buf), > "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";", > - SIGTRAP, env->cpu_index+1, type, > + GDB_SIGNAL_TRAP, env->cpu_index+1, type, > env->watchpoint_hit->vaddr); > put_packet(s, buf); > env->watchpoint_hit = NULL; > return; > } > tb_flush(env); > - ret = SIGTRAP; > + ret = GDB_SIGNAL_TRAP; > } else if (reason == EXCP_INTERRUPT) { > - ret = SIGINT; > + ret = GDB_SIGNAL_INT; > } else { > ret = 0; > } > @@ -1853,6 +2051,19 @@ > > #ifdef CONFIG_USER_ONLY > int > +gdb_queuesig (void) > +{ > + GDBState *s; > + > + s = gdbserver_state; > + > + if (gdbserver_fd < 0 || s->fd < 0) > + return 0; > + else > + return 1; > +} > + > +int > gdb_handlesig (CPUState *env, int sig) > { > GDBState *s; > @@ -1869,7 +2080,7 @@ > > if (sig != 0) > { > - snprintf(buf, sizeof(buf), "S%02x", sig); > + snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb (sig)); > put_packet(s, buf); > } > /* put_packet() might have detected that the peer terminated the > @@ -1915,6 +2126,19 @@ > put_packet(s, buf); > } > > +/* Tell the remote gdb that the process has exited due to SIG. */ > +void gdb_signalled(CPUState *env, int sig) > +{ > + GDBState *s; > + char buf[4]; > + > + s = gdbserver_state; > + if (gdbserver_fd < 0 || s->fd < 0) > + return; > + > + snprintf(buf, sizeof(buf), "X%02x", target_signal_to_gdb (sig)); > + put_packet(s, buf); > +} > > static void gdb_accept(void) > { > diff -u gdbstub.h gdbstub.h > --- gdbstub.h (working copy) > +++ gdbstub.h (working copy) > @@ -10,8 +10,10 @@ > int use_gdb_syscalls(void); > void gdb_set_stop_cpu(CPUState *env); > #ifdef CONFIG_USER_ONLY > +int gdb_queuesig (void); > int gdb_handlesig (CPUState *, int); > void gdb_exit(CPUState *, int); > +void gdb_signalled(CPUState *, int); > int gdbserver_start(int); > void gdbserver_fork(CPUState *); > #else > only in patch2: > unchanged: > --- linux-user/signal.c (revision 5995) > +++ linux-user/signal.c (working copy) > @@ -264,6 +264,26 @@ void target_to_host_siginfo(siginfo_t *i > (void *)(long)tswapl(tinfo->_sifields._rt._sigval.sival_ptr); > } > > +static int fatal_signal (int sig) > +{ > + switch (sig) { > + case TARGET_SIGCHLD: > + case TARGET_SIGURG: > + case TARGET_SIGWINCH: > + /* Ignored by default. */ > + return 0; > + case TARGET_SIGCONT: > + case TARGET_SIGSTOP: > + case TARGET_SIGTSTP: > + case TARGET_SIGTTIN: > + case TARGET_SIGTTOU: > + /* Job control signals. */ > + return 0; > + default: > + return 1; > + } > +} > + > void signal_init(void) > { > struct sigaction act; > @@ -298,10 +318,12 @@ void signal_init(void) > } > /* If there's already a handler installed then something has > gone horribly wrong, so don't even try to handle that case. */ > - /* Install some handlers for our own use. */ > - if (host_sig == SIGSEGV || host_sig == SIGBUS) { > + /* Install some handlers for our own use. We need at least > + SIGSEGV and SIGBUS, to detect exceptions. We can not just > + trap all signals because it affects syscall interrupt > + behavior. But do trap all default-fatal signals. */ > + if (fatal_signal (i)) > sigaction(host_sig, &act, NULL); > - } > } > } > > @@ -332,6 +354,7 @@ static void __attribute((noreturn)) forc > fprintf(stderr, "qemu: uncaught target signal %d (%s) - exiting\n", > sig, strsignal(host_sig)); > #if 1 > + gdb_signalled(thread_env, sig); > _exit(-host_sig); > #else > { > @@ -353,14 +376,16 @@ int queue_signal(CPUState *env, int sig, > struct emulated_sigtable *k; > struct sigqueue *q, **pq; > abi_ulong handler; > + int queue; > > #if defined(DEBUG_SIGNAL) > fprintf(stderr, "queue_signal: sig=%d\n", > sig); > #endif > k = &ts->sigtab[sig - 1]; > + queue = gdb_queuesig (); > handler = sigact_table[sig - 1]._sa_handler; > - if (handler == TARGET_SIG_DFL) { > + if (!queue && handler == TARGET_SIG_DFL) { > if (sig == TARGET_SIGTSTP || sig == TARGET_SIGTTIN || sig == TARGET_SIGTTOU) { > kill(getpid(),SIGSTOP); > return 0; > @@ -374,10 +399,10 @@ int queue_signal(CPUState *env, int sig, > } else { > return 0; /* indicate ignored */ > } > - } else if (handler == TARGET_SIG_IGN) { > + } else if (!queue && handler == TARGET_SIG_IGN) { > /* ignore signal */ > return 0; > - } else if (handler == TARGET_SIG_ERR) { > + } else if (!queue && handler == TARGET_SIG_ERR) { > force_sig(sig); > } else { > pq = &k->first; > @@ -417,7 +442,8 @@ static void host_signal_handler(int host > > /* the CPU emulator uses some host signals to detect exceptions, > we we forward to it some signals */ > - if (host_signum == SIGSEGV || host_signum == SIGBUS) { > + if ((host_signum == SIGSEGV || host_signum == SIGBUS) > + && info->si_code == SI_KERNEL) { > if (cpu_signal_handler(host_signum, info, puc)) > return; > } > @@ -544,7 +570,10 @@ int do_sigaction(int sig, const struct t > if (k->_sa_handler == TARGET_SIG_IGN) { > act1.sa_sigaction = (void *)SIG_IGN; > } else if (k->_sa_handler == TARGET_SIG_DFL) { > - act1.sa_sigaction = (void *)SIG_DFL; > + if (fatal_signal (sig)) > + act1.sa_sigaction = host_signal_handler; > + else > + act1.sa_sigaction = (void *)SIG_DFL; > } else { > act1.sa_sigaction = host_signal_handler; > } > @@ -3107,17 +3136,21 @@ void process_pending_signals(CPUState *c > > sig = gdb_handlesig (cpu_env, sig); > if (!sig) { > - fprintf (stderr, "Lost signal\n"); > - abort(); > + sa = NULL; > + handler = TARGET_SIG_IGN; > + } else { > + sa = &sigact_table[sig - 1]; > + handler = sa->_sa_handler; > } > > - sa = &sigact_table[sig - 1]; > - handler = sa->_sa_handler; > if (handler == TARGET_SIG_DFL) { > - /* default handler : ignore some signal. The other are fatal */ > - if (sig != TARGET_SIGCHLD && > - sig != TARGET_SIGURG && > - sig != TARGET_SIGWINCH) { > + /* default handler : ignore some signal. The other are job control or fatal */ > + if (sig == TARGET_SIGTSTP || sig == TARGET_SIGTTIN || sig == TARGET_SIGTTOU) { > + kill(getpid(),SIGSTOP); > + } else if (sig != TARGET_SIGCHLD && > + sig != TARGET_SIGURG && > + sig != TARGET_SIGWINCH && > + sig != TARGET_SIGCONT) { > force_sig(sig); > } > } else if (handler == TARGET_SIG_IGN) { > > -- > Daniel Jacobowitz > CodeSourcery > > > -- .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73 : :' : Debian developer | Electrical Engineer `. `' aurel32@debian.org | aurelien@aurel32.net `- people.debian.org/~aurel32 | www.aurel32.net