Re: [patch] Fix for PR gdb/10838

[Pedro Alves <pedro@codesourcery.com>]

On Saturday 14 November 2009 02:04:41, Paul Pluzhnikov wrote:

> Program terminated with signal 11, Segmentation fault.
> 
> #0  0x0000000000408b75 in look_up_one_symbol (name=0x7f6dc44ee3a2
> "__nptl_threads_events", addrp=0x7fffcce720c8) at
> ../../../src/gdb/gdbserver/remote-utils.c:1374
> 1374      for (sym = proc->symbol_cache; sym; sym = sym->next)
> 
> proc is NULL.
> Called from:
> 
> #0  0x0000000000408b75 in look_up_one_symbol () at
> ../../../src/gdb/gdbserver/remote-utils.c:1374
> #1  0x000000000041ae0e in ps_pglobal_lookup () at
> ../../../src/gdb/gdbserver/proc-service.c:68
> #2  0x00007f6dc44ed60d in td_ta_clear_event () from /lib/libthread_db.so.1
> #3  0x000000000041ac8b in thread_db_free (proc=0x6311a0) at
> ../../../src/gdb/gdbserver/thread-db.c:773
> #4  0x0000000000411b44 in linux_remove_process (process=0x6311a0) at
> ../../../src/gdb/gdbserver/linux-low.c:267

Having look_up_one_symbol handle the null process case
tastes like papering over an earlier issue.

> Attached patch fixes that (tested with gdbserver on Linux/x86_64 this time).

Why are we calling td_ta_clear_event at all if the
process is gone?  I think we should do that only if detaching,
like this (untested)?  This is what GDB does too.  The fact
that td_ta_clear_event wants to poke at a dead inferior is
a good indication of something not right.  If the process is
gone (or we didn't actually manage to connect to
this thread_db), there's no event mask to clear.

Also, thread_db_free assumes its PROC argument is the
current inferior (due to the fact that proc-service.c
relies on current_inferior currently).  This assumption
could/should be removed, but I'll leave that for
another day.

-- 
Pedro Alves

2009-11-16  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* linux-low.c (linux_remove_process): Add `detaching' parameter.
	Pass it to thread_db_free.
	(linux_kill, linux_detach, linux_wait_1): Adjust to pass the
	proper `detaching' argument to linux_remove_process.
	* linux-low.h (thread_db_free): Add `detaching' parameter.
	* thread-db.c (thread_db_init): Pass true as `detaching' argument
	to thread_db_free.
	(thread_db_free): Add `detaching' parameter.  Only
	call td_ta_clear_event if detaching from process.

---
 gdb/gdbserver/linux-low.c |   10 +++++-----
 gdb/gdbserver/linux-low.h |    2 +-
 gdb/gdbserver/thread-db.c |   26 +++++++++++++-------------
 3 files changed, 19 insertions(+), 19 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2009-11-16 02:07:20.000000000 +0000
+++ src/gdb/gdbserver/linux-low.c	2009-11-16 14:31:45.000000000 +0000
@@ -259,12 +259,12 @@ linux_add_process (int pid, int attached
    also freeing all private data.  */
 
 static void
-linux_remove_process (struct process_info *process)
+linux_remove_process (struct process_info *process, int detaching)
 {
   struct process_info_private *priv = process->private;
 
 #ifdef USE_THREAD_DB
-  thread_db_free (process);
+  thread_db_free (process, detaching);
 #endif
 
   free (priv->arch_private);
@@ -663,7 +663,7 @@ linux_kill (int pid)
     } while (lwpid > 0 && WIFSTOPPED (wstat));
 
   delete_lwp (lwp);
-  linux_remove_process (process);
+  linux_remove_process (process, 0);
   return 0;
 }
 
@@ -752,7 +752,7 @@ linux_detach (int pid)
 
   delete_all_breakpoints ();
   find_inferior (&all_threads, linux_detach_one_lwp, &pid);
-  linux_remove_process (process);
+  linux_remove_process (process, 1);
   return 0;
 }
 
@@ -1378,7 +1378,7 @@ retry:
 	  struct process_info *process = find_process_pid (pid);
 
 	  delete_lwp (lwp);
-	  linux_remove_process (process);
+	  linux_remove_process (process, 0);
 
 	  current_inferior = NULL;
 
Index: src/gdb/gdbserver/linux-low.h
===================================================================
--- src.orig/gdb/gdbserver/linux-low.h	2009-11-16 02:07:20.000000000 +0000
+++ src/gdb/gdbserver/linux-low.h	2009-11-16 12:51:09.000000000 +0000
@@ -201,7 +201,7 @@ struct lwp_info *find_lwp_pid (ptid_t pt
 
 /* From thread-db.c  */
 int thread_db_init (int use_events);
-void thread_db_free (struct process_info *);
+void thread_db_free (struct process_info *, int detaching);
 int thread_db_handle_monitor_command (char *);
 int thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
 			       CORE_ADDR load_module, CORE_ADDR *address);
Index: src/gdb/gdbserver/thread-db.c
===================================================================
--- src.orig/gdb/gdbserver/thread-db.c	2009-11-16 12:51:06.000000000 +0000
+++ src/gdb/gdbserver/thread-db.c	2009-11-16 13:51:22.000000000 +0000
@@ -737,7 +737,7 @@ thread_db_init (int use_events)
       if (use_events && thread_db_enable_reporting () == 0)
 	{
 	  /* Keep trying; maybe event reporting will work later.  */
-	  thread_db_free (proc);
+	  thread_db_free (proc, 0);
 	  return 0;
 	}
       thread_db_find_new_threads ();
@@ -752,38 +752,38 @@ thread_db_init (int use_events)
 /* Disconnect from libthread_db and free resources.  */
 
 void
-thread_db_free (struct process_info *proc)
+thread_db_free (struct process_info *proc, int detaching)
 {
   struct thread_db *thread_db = proc->private->thread_db;
   if (thread_db)
     {
-#ifndef USE_LIBTHREAD_DB_DIRECTLY
       td_err_e (*td_ta_delete_p) (td_thragent_t *);
       td_err_e (*td_ta_clear_event_p) (const td_thragent_t *ta,
 				       td_thr_events_t *event);
 
+#ifndef USE_LIBTHREAD_DB_DIRECTLY
       td_ta_clear_event_p = dlsym (thread_db->handle, "td_ta_clear_event");
-      if (td_ta_clear_event_p != NULL)
+      td_ta_delete_p = dlsym (thread_db->handle, "td_ta_delete");
+#else
+      td_ta_delete_p = &td_ta_delete;
+      td_ta_clear_event_p = &td_ta_clear_event;
+#endif
+
+      if (detaching && td_ta_clear_event_p != NULL)
 	{
 	  td_thr_events_t events;
 
-	  /* Set the process wide mask saying we aren't interested in any
-	     events anymore.  */
+	  /* Set the process wide mask saying we aren't interested
+	     in any events anymore.  */
 	  td_event_fillset (&events);
 	  (*td_ta_clear_event_p) (thread_db->thread_agent, &events);
 	}
 
-      td_ta_delete_p = dlsym (thread_db->handle, "td_ta_delete");
       if (td_ta_delete_p != NULL)
 	(*td_ta_delete_p) (thread_db->thread_agent);
 
+#ifndef USE_LIBTHREAD_DB_DIRECTLY
       dlclose (thread_db->handle);
-#else
-      td_thr_events_t events;
-
-      td_event_fillset (&events);
-      td_ta_clear_event (thread_db->thread_agent, &events);
-      td_ta_delete (thread_db->thread_agent);
 #endif  /* USE_LIBTHREAD_DB_DIRECTLY  */
 
       free (thread_db);