From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14932 invoked by alias); 28 Mar 2002 07:59:46 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 14919 invoked from network); 28 Mar 2002 07:59:41 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 28 Mar 2002 07:59:41 -0000 Received: from cse.cygnus.com (cse.cygnus.com [205.180.230.236]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id XAA07755; Wed, 27 Mar 2002 23:59:38 -0800 (PST) Received: (from kev@localhost) by cse.cygnus.com (8.11.6/8.11.6) id g2S7xLX25567; Thu, 28 Mar 2002 00:59:21 -0700 Date: Wed, 27 Mar 2002 23:59:00 -0000 From: Kevin Buettner Message-Id: <1020328075921.ZM25566@localhost.localdomain> To: Mark Kettenis , gdb-patches@sources.redhat.com Subject: [PATCH RFA] Propagate signals to non-clone PID on GNU/Linux MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-03/txt/msg00577.txt.bz2 These days, thanks to Mark, it's possible to attach to any LWP when debugging a multi-threaded program on GNU/Linux. However, there are at least two undesirable consequences that may occur when doing so. The first problem is that GDB will propagate signals to the the pid to which GDB was initially attached. If this pid is a clone, the signal is ignored. (If you look at /proc/pid/status for any of the clones, you'll see that the signal mask is set so that most signals are blocked.) This is a problem because it may be impossible for the user to regain control of gdb (via Ctrl-C) if the the program is continued. The second (potential) problem is that memory reads and writes will occur with respect to the PID to which the user is attached. This will only be a problem though if the LWP in question should choose to exit before the user has finished debugging the multi-threaded program. The patch below fixes the first problem, but not the second. It wouldn't be too hard to "fix" the second problem as well, but I'm not yet convinced that it's desirable to do so. After all, the user may have intentionally attached to one of the clones (maybe the manager thread) knowing that the non-clone lwp is expected to exit. Yes, it seems unlikely, but you never know. In any case, one of the things my patch does is warn the user when attaching to an LWP which is a clone. It even suggests the correct pid to reattach to if the user decides that this may be a problem. This patch addresses the first problem by augmenting the parameter list of set_sigint_trap() to now include the pid to which to send the signal to. The status quo (in which the signal is propagated to inferior_ptid) is maintained if zero is passed as the pid parameter to set_sigint_trap(). lin-lwp.c makes use of this new parameter to pass the pid of the non-clone process to set_sigint_trap() when it detects that an attach was done to a clone *and* a non-clone also exists. I made some adjustments to LWP initialization. The "overall" process id (which is merely the pid to which the user attached) is added to the LWP table in child_wait(). I had previously committed a patch which belatedly performed some of the initialization in lin_lwp_attach_lwp(), but at this point, we no longer are able to determine whether or not the pid in question represents a clone. Therefore, I've removed this code in favor of doing the initialization in child_wait(). This also fixes a more subtle bug in which the ``cloned'' flag would be set incorrectly for the clone to which gdb is attached. This is a problem because lin-lwp.c sometimes uses this flag (e.g, in stop_wait_callback()) in order to know whether to use the __WCLONE flag in a call to waitpid(). In order to reproduce the bug that this patch fixes, do the following: 1) Start linux-dp (without gdb). 2) In another window, do ps -axuw | grep linux-dp or similar to determine the pids (LWPs) associated with the linux-dp program that you've started in step 1. 3) Invoke gdb on linux-dp and attach to one of the clone LWPs. The lowest numbered pid will generally be the initial, non-clone pid, and the rest will be clones. 4) At the gdb prompt, immediately continue. 5) Attempt to interrupt the program with Cntrl-C from GDB. If GDB is not built with the patch below, it will be impossible to regain control. In fact, it will probably be necessary to kill both linux-dp and gdb externally. (I'd appreciate it if someone would give me some hints on how to turn this into a test that we can add to the testsuite.) Okay to commit? (I'd like approval for the lin-lwp.c portion, though if anyone has objections to the rest of the patch, please speak up!) * inferior.h (set_sigint_trap): Add new parameter representing pid to send signal to. Also, update copyright. * inflow.c (set_sigint_trap): Likewise. (sigint_pid): New static global variable. (pass_signal): Use ``sigint_pid'' to determine process to send signal to. (clear_sigint_trap): Clear ``sigint_pid''. * inftarg.c (set_sigint_trap): Adjust all callers. Update copyright. * lynx-nat.c (set_sigint_trap): Likewise. * symm-nat.c (set_sigint_trap): Likewise. * lin-lwp.c (set_sigint_trap): Likewise. (signal_pid): New static global. (set_up_clone_as_overall_process_id): New function. (lin_lwp_attach_lwp): Remove code which handles the overall process being stopped in another layer. Detect existence of a non-cloned process as a subsiderary LWP. (lin_lwp_detach, lin_lwp_mourn_inferior): Clear signal_pid. (child_wait): Mark LWP as stopped. Also set flag indicating whether LWP is a cloned process or not. (lin_lwp_wait): Pass ``signal_pid'' to set_sigint_trap(). Index: inferior.h =================================================================== RCS file: /cvs/src/src/gdb/inferior.h,v retrieving revision 1.24 diff -u -p -r1.24 inferior.h --- inferior.h 2001/11/22 00:23:12 1.24 +++ inferior.h 2002/03/28 06:48:38 @@ -1,7 +1,7 @@ /* Variables that describe the inferior process running under GDB: Where it is, why it stopped, and how to step it. Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, - 1998, 1999, 2000, 2001 Free Software Foundation, Inc. + 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -83,7 +83,11 @@ extern int ptid_equal (ptid_t p1, ptid_t pointer needed for later doing the cleanup. */ extern struct cleanup * save_inferior_ptid (void); -extern void set_sigint_trap (void); +/* Establish a SIGINT signal handler which will propagate SIGINT to the + inferior. A non-zero value PID designates the process to deliver + the signal to. Otherwise, the signal will be sent to the process + designated by the pid component of inferior_ptid. */ +extern void set_sigint_trap (int pid); extern void clear_sigint_trap (void); Index: inflow.c =================================================================== RCS file: /cvs/src/src/gdb/inflow.c,v retrieving revision 1.13 diff -u -p -r1.13 inflow.c --- inflow.c 2002/03/27 21:35:35 1.13 +++ inflow.c 2002/03/28 06:48:40 @@ -1,6 +1,6 @@ /* Low level interface to ptrace, for GDB when running under Unix. Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, - 1996, 1998, 1999, 2000, 2001 Free Software Foundation, Inc. + 1996, 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -598,6 +598,10 @@ kill_command (char *arg, int from_tty) } } +/* PID to signal when passing SIGINT. Set to 0 to use pid associated + with inferior_ptid. */ +static int sigint_pid; + /* Call set_sigint_trap when you need to pass a signal on to an attached process when handling SIGINT */ @@ -606,18 +610,27 @@ static void pass_signal (int signo) { #ifndef _WIN32 - kill (PIDGET (inferior_ptid), SIGINT); + int pid; + + if (sigint_pid) + pid = sigint_pid; + else + pid = PIDGET (inferior_ptid); + + kill (pid, SIGINT); #endif } +/* Original SIGINT signal handler. */ static void (*osig) (); void -set_sigint_trap (void) +set_sigint_trap (int pid) { if (attach_flag || inferior_thisrun_terminal) { osig = (void (*)()) signal (SIGINT, pass_signal); + sigint_pid = pid; } } @@ -627,6 +640,7 @@ clear_sigint_trap (void) if (attach_flag || inferior_thisrun_terminal) { signal (SIGINT, osig); + sigint_pid = 0; } } Index: inftarg.c =================================================================== RCS file: /cvs/src/src/gdb/inftarg.c,v retrieving revision 1.8 diff -u -p -r1.8 inftarg.c --- inftarg.c 2002/01/09 00:36:58 1.8 +++ inftarg.c 2002/03/28 06:48:40 @@ -131,7 +131,7 @@ child_wait (ptid_t ptid, struct target_w do { - set_sigint_trap (); /* Causes SIGINT to be passed on to the + set_sigint_trap (0); /* Causes SIGINT to be passed on to the attached process. */ set_sigio_trap (); Index: lin-lwp.c =================================================================== RCS file: /cvs/src/src/gdb/lin-lwp.c,v retrieving revision 1.33 diff -u -p -r1.33 lin-lwp.c --- lin-lwp.c 2002/03/27 21:35:35 1.33 +++ lin-lwp.c 2002/03/28 06:48:42 @@ -1,5 +1,5 @@ /* Multi-threaded debugging support for GNU/Linux (LWP layer). - Copyright 2000, 2001 Free Software Foundation, Inc. + Copyright 2000, 2001, 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -120,6 +120,16 @@ static int threaded; #define is_lwp(ptid) (GET_LWP (ptid) != 0) #define BUILD_LWP(lwp, pid) ptid_build (pid, lwp, 0) +/* An alternate pid to use for passing signals to the inferior. Normally, + signals are just passed to PIDGET (inferior_ptid), but it's not + desirable to do this when the user has attached to a cloned + process. In that case, we set ``signal_pid'' to the pid of a + non-cloned process. Note, however, that a zero-valued + ``signal_pid'' indicates that the normal mechanism should be used. + Note: This pid should also probably be used for reading/writing + memory. */ +static int signal_pid; + /* If the last reported event was a SIGTRAP, this variable is set to the process id of the LWP/thread that got it. */ ptid_t trap_ptid; @@ -341,6 +351,55 @@ lin_lwp_open (char *args, int from_tty) } #endif +/* Warn user that the process id to which GDB is attached is a clone + and arrange for signals to be sent to non-cloned process. PTID + repesents the non-cloned process found during the process of + attaching to LWPs other than the overall process id. */ + +static void +set_up_clone_as_overall_process_id (ptid_t ptid) +{ + struct lwp_info *lp; + struct lwp_info *olp; + + gdb_assert (GET_PID (ptid) != GET_LWP (ptid)); + + /* Find LWP associated with ptid. */ + lp = find_lwp_pid (ptid); + gdb_assert (lp != NULL); + gdb_assert (!lp->cloned); + + /* Find the LWP associated with the overall PID. */ + olp = find_lwp_pid (BUILD_LWP (GET_PID (ptid), GET_PID (ptid))); + gdb_assert (olp != NULL); + + if (!olp->cloned) + { + /* Overall process id isn't a clone. But the reason that + we're in this function at all is that we've found another + non-clone process id. This is likely an error of some + kind, so warn the user about it. We could also generate + an error here, but that seems rather rude, particularly if + the user is attempting to use gdb to help find out why there + are two (or more) non-clone processes. */ + + warning ("LWP %d and LWP %ld are both non-cloned processes. (This is " + "probably an error, either in gdb or in the thread library.)", + GET_PID (ptid), GET_LWP (ptid)); + } + else + { + /* Overall process id is a clone, and PTID isn't. This + means that the user probably should've attached to + GET_LWP (PTID) instead. Issue a warning. */ + + warning ("Attached to a cloned process. You may wish to detach " + "and attach to LWP %ld instead.", GET_LWP (ptid)); + signal_pid = GET_LWP (ptid); + } + +} + /* Attach to the LWP specified by PID. If VERBOSE is non-zero, print a message telling the user that a new LWP has been added to the process. */ @@ -390,16 +449,9 @@ lin_lwp_attach_lwp (ptid_t ptid, int ver && WIFSTOPPED (status) && WSTOPSIG (status)); lp->stopped = 1; - } - else - { - /* We assume that the LWP representing the original process - is already stopped. Mark it as stopped in the data structure - that the lin-lwp layer uses to keep track of threads. Note - that this won't have already been done since the main thread - will have, we assume, been stopped by an attach from a - different layer. */ - lp->stopped = 1; + + if (!lp->cloned) + set_up_clone_as_overall_process_id (ptid); } } @@ -488,6 +540,7 @@ lin_lwp_detach (char *args, int from_tty gdb_assert (num_lwps == 1); trap_ptid = null_ptid; + signal_pid = 0; /* Destroy LWP info; it's no longer valid. */ init_lwp_list (); @@ -953,17 +1006,26 @@ child_wait (ptid_t ptid, struct target_w int save_errno; int status; pid_t pid; + int cloned; + struct lwp_info *lp; + ptid_t lwp_id; do { - set_sigint_trap (); /* Causes SIGINT to be passed on to the + set_sigint_trap (0); /* Causes SIGINT to be passed on to the attached process. */ set_sigio_trap (); pid = waitpid (GET_PID (ptid), &status, 0); if (pid == -1 && errno == ECHILD) - /* Try again with __WCLONE to check cloned processes. */ - pid = waitpid (GET_PID (ptid), &status, __WCLONE); + { + /* Try again with __WCLONE to check cloned processes. */ + pid = waitpid (GET_PID (ptid), &status, __WCLONE); + cloned = 1; + } + else + cloned = 0; + save_errno = errno; clear_sigio_trap (); @@ -981,6 +1043,16 @@ child_wait (ptid_t ptid, struct target_w return minus_one_ptid; } + /* Mark the lwp as stopped. */ + lwp_id = BUILD_LWP (pid, pid); + lp = find_lwp_pid (lwp_id); + if (lp == NULL) + { + lp = add_lwp (lwp_id); + lp->cloned = cloned; + } + lp->stopped = 1; + store_waitstatus (ourstatus, status); return pid_to_ptid (pid); } @@ -1027,7 +1099,7 @@ lin_lwp_wait (ptid_t ptid, struct target status_to_str (status), GET_LWP (lp->ptid)); } - /* But if we don't fine one, we'll have to wait, and check both + /* But if we don't find one, we'll have to wait, and check both cloned and uncloned processes. We start with the cloned processes. */ options = __WCLONE | WNOHANG; @@ -1079,8 +1151,8 @@ lin_lwp_wait (ptid_t ptid, struct target stop_wait_callback (lp, NULL); } - set_sigint_trap (); /* Causes SIGINT to be passed on to the - attached process. */ + set_sigint_trap (signal_pid); /* Causes SIGINT to be passed on to the + attached process. */ set_sigio_trap (); while (status == 0) @@ -1334,6 +1406,7 @@ static void lin_lwp_mourn_inferior (void) { trap_ptid = null_ptid; + signal_pid = 0; /* Destroy LWP info; it's no longer valid. */ init_lwp_list (); Index: lynx-nat.c =================================================================== RCS file: /cvs/src/src/gdb/lynx-nat.c,v retrieving revision 1.9 diff -u -p -r1.9 lynx-nat.c --- lynx-nat.c 2001/06/15 23:50:46 1.9 +++ lynx-nat.c 2002/03/28 06:48:43 @@ -1,5 +1,5 @@ /* Native-dependent code for LynxOS. - Copyright 1993, 1994, 1995, 1996, 1999, 2000, 2001 + Copyright 1993, 1994, 1995, 1996, 1999, 2000, 2001, 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -598,7 +598,7 @@ child_wait (ptid_t ptid, struct target_w { int sig; - set_sigint_trap (); /* Causes SIGINT to be passed on to the + set_sigint_trap (0); /* Causes SIGINT to be passed on to the attached process. */ pid = wait (&status); Index: symm-nat.c =================================================================== RCS file: /cvs/src/src/gdb/symm-nat.c,v retrieving revision 1.11 diff -u -p -r1.11 symm-nat.c --- symm-nat.c 2002/01/08 00:59:31 1.11 +++ symm-nat.c 2002/03/28 06:48:44 @@ -1,6 +1,6 @@ /* Sequent Symmetry host interface, for GDB when running under Unix. Copyright 1986, 1987, 1989, 1991, 1992, 1993, 1994, 1995, 1999, 2000, - 2001 + 2001, 2002 Free Software Foundation, Inc. This file is part of GDB. @@ -441,7 +441,7 @@ child_wait (ptid_t ptid, struct target_w do { - set_sigint_trap (); /* Causes SIGINT to be passed on to the + set_sigint_trap (0); /* Causes SIGINT to be passed on to the attached process. */ save_errno = errno;