This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[gdbserver/linux] several fixes for detaching/killing running processes support


(The actually started out as splitting a very small patch
from a larger patch that prepared the way to being able
to transparently pause/unpause lwps at any time...)

Killing a running process no longer worked, we were now hanging,
waiting for the lwp to stop before PTRACE_KILLing it would work,
due to a recent `stop_expected' flag semantic change.

Another issue is that disabling event reporting or deleting the thread
event breakpoint was silently failing, because we were trying to delete
breakpoints/poke at memory of possibly running lwp.
I ended up getting rid of the blind `delete_all_breakpoints'
call in linux_detach, replaced by having thread_db.c explicitly delete
the thread event breakpoint.  GDB deletes GDB breakpoints before requesting
a detach, and, we stop tracing (which removes internal trap breakpoints)
before detaching too.  There should be no other breakpoints listed by the
time we get to actually detaching.  Keeping things under stricter control
hopefully prevents masking out bugs until too late (field).  It's also
nicer to follow the rule that code that inserts breakpoints, and as such
may hold a breakpoint reference, should take care of deleting its own
breakpoints, so that we don't end up with cases of one module accessing a
dangling breakpoint reference, due to some other module deleting
breakpoints behind the first module's back.

You'll see that the patch also makes the linux target not
immediately delete the last lwp of a process that has just
exited, but instead defers it to mourning time, similarly to
what gdb/linux-nat.c does.  This allows still referring to
current_inferior/current_process between reporting the process
exit to gdbserver core, and mourning the process.

I'd like to add a test that covers this detach/kill a
running process, but, I'm avoiding that for now, because
it doesn't still work correctly with native/gdbserver
linux/ptrace (what we can test in non-stop mode here in the
FSF tree) in some trivial use cases, like when we have a
user breakpoint set before requesting the detach (removing
the breakpoint fails, due to not being able to touch memory
of a running process).

-- 
Pedro Alves

2010-05-02  Pedro Alves  <pedro@codesourcery.com>

	* linux-low.c (linux_kill_one_lwp): Assume the lwp is stopped.
	(linux_kill): Stop all lwps here.  Don't delete the main lwp here.
	(linux_detach_one_lwp): Assume the lwp is stopped.
	(any_thread_of): Delete.
	(linux_detach): Stop all lwps here.  Don't blindly delete all
	breakpoints.
	(delete_lwp_callback): New.
	(linux_mourn): Delete all lwps of the process that is gone.
	(linux_wait_1): Don't delete the last lwp of the process here.
	* mem-break.h (mark_breakpoints_out): Declare.
	* mem-break.c (mark_breakpoints_out): New.
	(free_all_breakpoints): Use it.
	* server.c (handle_target_event): If the process is gone, mark
	breakpoints out.
	* thread-db.c (struct thread_db) <create_bp>: New field.
	(thread_db_enable_reporting): Fix prototype.  Store a thread event
	breakpoint reference in the thread_db struct.
	(thread_db_load_search): Clear the thread_db object.
	(try_thread_db_load_1): Ditto.
	(switch_to_process): New.
	(disable_thread_event_reporting): Use it.
	(remove_thread_event_breakpoints): New.
	(thread_db_detach, thread_db_mourn): Use it.

---
 gdb/gdbserver/linux-low.c |   90 +++++++++++++++++-----------------------------
 gdb/gdbserver/mem-break.c |   14 +++++--
 gdb/gdbserver/mem-break.h |    4 ++
 gdb/gdbserver/server.c    |    5 ++
 gdb/gdbserver/thread-db.c |   66 ++++++++++++++++++++++++++++-----
 5 files changed, 109 insertions(+), 70 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2010-05-01 16:59:14.000000000 +0100
+++ src/gdb/gdbserver/linux-low.c	2010-05-02 01:18:22.000000000 +0100
@@ -739,11 +739,6 @@ linux_kill_one_lwp (struct inferior_list
       return 0;
     }
 
-  /* If we're killing a running inferior, make sure it is stopped
-     first, as PTRACE_KILL will not work otherwise.  */
-  if (!lwp->stopped)
-    send_sigstop (lwp);
-
   do
     {
       ptrace (PTRACE_KILL, lwpid_of (lwp), 0, 0);
@@ -768,6 +763,10 @@ linux_kill (int pid)
   if (process == NULL)
     return -1;
 
+  /* If we're killing a running inferior, make sure it is stopped
+     first, as PTRACE_KILL will not work otherwise.  */
+  stop_all_lwps ();
+
   find_inferior (&all_threads, linux_kill_one_lwp, &pid);
 
   /* See the comment in linux_kill_one_lwp.  We did not kill the first
@@ -779,11 +778,6 @@ linux_kill (int pid)
     fprintf (stderr, "lk_1: killing lwp %ld, for pid: %d\n",
 	     lwpid_of (lwp), pid);
 
-  /* If we're killing a running inferior, make sure it is stopped
-     first, as PTRACE_KILL will not work otherwise.  */
-  if (!lwp->stopped)
-    send_sigstop (lwp);
-
   do
     {
       ptrace (PTRACE_KILL, lwpid_of (lwp), 0, 0);
@@ -792,9 +786,11 @@ linux_kill (int pid)
       lwpid = linux_wait_for_event (lwp->head.id, &wstat, __WALL);
     } while (lwpid > 0 && WIFSTOPPED (wstat));
 
-  delete_lwp (lwp);
-
   the_target->mourn (process);
+
+  /* Since we presently can only stop all lwps of all processes, we
+     need to unstop lwps of other processes.  */
+  unstop_all_lwps (NULL);
   return 0;
 }
 
@@ -808,28 +804,6 @@ linux_detach_one_lwp (struct inferior_li
   if (ptid_get_pid (entry->id) != pid)
     return 0;
 
-  /* If we're detaching from a running inferior, make sure it is
-     stopped first, as PTRACE_DETACH will not work otherwise.  */
-  if (!lwp->stopped)
-    {
-      int lwpid = lwpid_of (lwp);
-
-      stopping_threads = 1;
-      send_sigstop (lwp);
-
-      /* If this detects a new thread through a clone event, the new
-	 thread is appended to the end of the lwp list, so we'll
-	 eventually detach from it.  */
-      wait_for_sigstop (&lwp->head);
-      stopping_threads = 0;
-
-      /* If LWP exits while we're trying to stop it, there's nothing
-	 left to do.  */
-      lwp = find_lwp_pid (pid_to_ptid (lwpid));
-      if (lwp == NULL)
-	return 0;
-    }
-
   /* If this process is stopped but is expecting a SIGSTOP, then make
      sure we take care of that now.  This isn't absolutely guaranteed
      to collect the SIGSTOP, but is fairly likely to.  */
@@ -838,8 +812,7 @@ linux_detach_one_lwp (struct inferior_li
       int wstat;
       /* Clear stop_expected, so that the SIGSTOP will be reported.  */
       lwp->stop_expected = 0;
-      if (lwp->stopped)
-	linux_resume_one_lwp (lwp, 0, 0, NULL);
+      linux_resume_one_lwp (lwp, 0, 0, NULL);
       linux_wait_for_event (lwp->head.id, &wstat, __WALL);
     }
 
@@ -855,17 +828,6 @@ linux_detach_one_lwp (struct inferior_li
 }
 
 static int
-any_thread_of (struct inferior_list_entry *entry, void *args)
-{
-  int *pid_p = args;
-
-  if (ptid_get_pid (entry->id) == *pid_p)
-    return 1;
-
-  return 0;
-}
-
-static int
 linux_detach (int pid)
 {
   struct process_info *process;
@@ -874,17 +836,37 @@ linux_detach (int pid)
   if (process == NULL)
     return -1;
 
+  /* Stop all threads before detaching.  First, ptrace requires that
+     the thread is stopped to sucessfully detach.  Second, thread_db
+     may need to uninstall thread event breakpoints from memory, which
+     only works with a stopped process anyway.  */
+  stop_all_lwps ();
+
 #ifdef USE_THREAD_DB
   thread_db_detach (process);
 #endif
 
-  current_inferior =
-    (struct thread_info *) find_inferior (&all_threads, any_thread_of, &pid);
-
-  delete_all_breakpoints ();
   find_inferior (&all_threads, linux_detach_one_lwp, &pid);
 
   the_target->mourn (process);
+
+  /* Since we presently can only stop all lwps of all processes, we
+     need to unstop lwps of other processes.  */
+  unstop_all_lwps (NULL);
+  return 0;
+}
+
+/* Remove all LWPs that belong to process PROC from the lwp list.  */
+
+static int
+delete_lwp_callback (struct inferior_list_entry *entry, void *proc)
+{
+  struct lwp_info *lwp = (struct lwp_info *) entry;
+  struct process_info *process = proc;
+
+  if (pid_of (lwp) == pid_of (process))
+    delete_lwp (lwp);
+
   return 0;
 }
 
@@ -897,6 +879,8 @@ linux_mourn (struct process_info *proces
   thread_db_mourn (process);
 #endif
 
+  find_inferior (&all_lwps, delete_lwp_callback, process);
+
   /* Freeing all private data.  */
   priv = process->private;
   free (priv->arch_private);
@@ -1695,10 +1679,6 @@ retry:
     {
       if (WIFEXITED (w) || WIFSIGNALED (w))
 	{
-	  delete_lwp (event_child);
-
-	  current_inferior = NULL;
-
 	  if (WIFEXITED (w))
 	    {
 	      ourstatus->kind = TARGET_WAITKIND_EXITED;
Index: src/gdb/gdbserver/mem-break.c
===================================================================
--- src.orig/gdb/gdbserver/mem-break.c	2010-05-01 16:59:02.000000000 +0100
+++ src/gdb/gdbserver/mem-break.c	2010-05-02 01:18:22.000000000 +0100
@@ -716,16 +716,24 @@ delete_all_breakpoints (void)
     delete_breakpoint_1 (proc, proc->breakpoints);
 }
 
-/* Release all breakpoints, but do not try to un-insert them from the
-   inferior.  */
+/* Clear the "inserted" flag in all breakpoints.  */
 
 void
-free_all_breakpoints (struct process_info *proc)
+mark_breakpoints_out (struct process_info *proc)
 {
   struct raw_breakpoint *raw_bp;
 
   for (raw_bp = proc->raw_breakpoints; raw_bp != NULL; raw_bp = raw_bp->next)
     raw_bp->inserted = 0;
+}
+
+/* Release all breakpoints, but do not try to un-insert them from the
+   inferior.  */
+
+void
+free_all_breakpoints (struct process_info *proc)
+{
+  mark_breakpoints_out (proc);
 
   /* Note: use PROC explicitly instead of deferring to
      delete_all_breakpoints --- CURRENT_INFERIOR may already have been
Index: src/gdb/gdbserver/mem-break.h
===================================================================
--- src.orig/gdb/gdbserver/mem-break.h	2010-05-01 16:59:03.000000000 +0100
+++ src/gdb/gdbserver/mem-break.h	2010-05-02 01:18:22.000000000 +0100
@@ -103,6 +103,10 @@ void set_breakpoint_data (const unsigned
 
 void delete_all_breakpoints (void);
 
+/* Clear the "inserted" flag in all breakpoints of PROC.  */
+
+void mark_breakpoints_out (struct process_info *proc);
+
 /* Delete all breakpoints, but do not try to un-insert them from the
    inferior.  */
 
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2010-05-01 16:59:02.000000000 +0100
+++ src/gdb/gdbserver/server.c	2010-05-01 16:59:16.000000000 +0100
@@ -3031,7 +3031,10 @@ handle_target_event (int err, gdb_client
 
       if (last_status.kind == TARGET_WAITKIND_EXITED
 	  || last_status.kind == TARGET_WAITKIND_SIGNALLED)
-	mourn_inferior (process);
+	{
+	  mark_breakpoints_out (process);
+	  mourn_inferior (process);
+	}
 
       if (forward_event)
 	{
Index: src/gdb/gdbserver/thread-db.c
===================================================================
--- src.orig/gdb/gdbserver/thread-db.c	2010-05-01 16:59:02.000000000 +0100
+++ src/gdb/gdbserver/thread-db.c	2010-05-02 01:18:32.000000000 +0100
@@ -52,6 +52,16 @@ struct thread_db
   void *handle;
 #endif
 
+  /* Thread creation event breakpoint.  The code at this location in
+     the child process will be called by the pthread library whenever
+     a new thread is created.  By setting a special breakpoint at this
+     location, GDB can detect when a new thread is created.  We obtain
+     this location via the td_ta_event_addr call.  Note that if the
+     running kernel supports tracing clones, then we don't need to use
+     (and in fact don't use) this magic thread event breakpoint to
+     learn about threads.  */
+  struct breakpoint *td_create_bp;
+
   /* Addresses of libthread_db functions.  */
   td_err_e (*td_ta_new_p) (struct ps_prochandle * ps, td_thragent_t **ta);
   td_err_e (*td_ta_event_getmsg_p) (const td_thragent_t *ta,
@@ -205,7 +215,7 @@ thread_db_create_event (CORE_ADDR where)
 }
 
 static int
-thread_db_enable_reporting ()
+thread_db_enable_reporting (void)
 {
   td_thr_events_t events;
   td_notify_t notify;
@@ -239,8 +249,9 @@ thread_db_enable_reporting ()
 	       thread_db_err_str (err));
       return 0;
     }
-  set_breakpoint_at ((CORE_ADDR) (unsigned long) notify.u.bptaddr,
-		     thread_db_create_event);
+  thread_db->td_create_bp
+    = set_breakpoint_at ((CORE_ADDR) (unsigned long) notify.u.bptaddr,
+			 thread_db_create_event);
 
   return 1;
 }
@@ -501,6 +512,8 @@ thread_db_load_search (void)
   if (proc->private->thread_db != NULL)
     fatal ("unexpected: proc->private->thread_db != NULL");
 
+  memset (&tdb, 0, sizeof (tdb));
+
   tdb.td_ta_new_p = &td_ta_new;
 
   /* Attempt to open a connection to the thread library.  */
@@ -544,6 +557,8 @@ try_thread_db_load_1 (void *handle)
   if (proc->private->thread_db != NULL)
     fatal ("unexpected: proc->private->thread_db != NULL");
 
+  memset (&tdb, 0, sizeof (tdb));
+
   tdb.handle = handle;
 
   /* Initialize pointers to the dynamic library functions we will use.
@@ -766,6 +781,16 @@ any_thread_of (struct inferior_list_entr
   return 0;
 }
 
+static void
+switch_to_process (struct process_info *proc)
+{
+  int pid = pid_of (proc);
+
+  current_inferior =
+    (struct thread_info *) find_inferior (&all_threads,
+					  any_thread_of, &pid);
+}
+
 /* Disconnect from libthread_db and free resources.  */
 
 static void
@@ -785,15 +810,10 @@ disable_thread_event_reporting (struct p
 
       if (td_ta_clear_event_p != NULL)
 	{
-	  struct thread_info *saved_inferior;
+	  struct thread_info *saved_inferior = current_inferior;
 	  td_thr_events_t events;
-	  int pid;
 
-	  pid = pid_of (proc);
-	  saved_inferior = current_inferior;
-	  current_inferior =
-	    (struct thread_info *) find_inferior (&all_threads,
-						  any_thread_of, &pid);
+	  switch_to_process (proc);
 
 	  /* Set the process wide mask saying we aren't interested
 	     in any events anymore.  */
@@ -805,10 +825,34 @@ disable_thread_event_reporting (struct p
     }
 }
 
+static void
+remove_thread_event_breakpoints (struct process_info *proc)
+{
+  struct thread_db *thread_db = proc->private->thread_db;
+
+  if (thread_db->td_create_bp != NULL)
+    {
+      struct thread_info *saved_inferior = current_inferior;
+
+      switch_to_process (proc);
+
+      delete_breakpoint (thread_db->td_create_bp);
+      thread_db->td_create_bp = NULL;
+
+      current_inferior = saved_inferior;
+    }
+}
+
 void
 thread_db_detach (struct process_info *proc)
 {
-  disable_thread_event_reporting (proc);
+  struct thread_db *thread_db = proc->private->thread_db;
+
+  if (thread_db)
+    {
+      disable_thread_event_reporting (proc);
+      remove_thread_event_breakpoints (proc);
+    }
 }
 
 /* Disconnect from libthread_db and free resources.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]