[PATCH RFA] Propagate signals to non-clone PID on GNU/Linux

[Kevin Buettner <kevinb at redhat dot com>]

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;