Re: [5/7] procfs (solaris, iris, osf, unixware, aix5)

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

On Friday 15 August 2008 18:37:31, Daniel Jacobowitz wrote:
> On Fri, Aug 08, 2008 at 02:37:57AM +0100, Pedro Alves wrote:
> > This patch adjusts the procfs target, so that it registers the main
> > thread in GDB's thread table.
>
> This looks OK to me (took me a couple of tries to convince myself it
> was right, but I'm convinced now).

I'm sorry to had convinced you wrong.  :-(

I noticed that before, when we went multi-threaded, we would keep
the main process listed with proc->tid == 0, and we would just
create a new procinfo for the main lwp, which is a little
redundant, but indeed simplifies things.

procfs_wait:
		  /* In addition, it's possible that this is the first
		   * new thread we've seen, in which case we may not
		   * have created entries for inferior_ptid yet.
		   */
		  if (TIDGET (inferior_ptid) != 0)
		    {
		      if (!in_thread_list (inferior_ptid))
			add_thread (inferior_ptid);
		      if (find_procinfo (PIDGET (inferior_ptid),
					 TIDGET (inferior_ptid)) == NULL)
			create_procinfo (PIDGET (inferior_ptid),
					 TIDGET (inferior_ptid));
		    }

If I instead do that right on target_create_inferior and target_attach,
we're done -- there's always a thread, and there's always a procinfo for
it too.

You asked me on IRC if the main lwp is also available in
the /proc tree, and although I somehow missed it, indeed it is there.

Here, 5642 is a single-threaded app:

 pedro@opensolaris:~/orlando/gdb/baseline/build-solaris/gdb$ ls /proc/5642/lwp/
 1

Here, 5613 is a multi-threaded app:

 pedro@opensolaris:~/orlando/gdb/baseline/build-solaris/gdb$ ls /proc/5613/lwp/
 1  2  3

We still have to use the main process procinfo entry, to get at
/proc/<pid>/map /proc/<pid>/as, because as expected, those are
not found under the lwp/1 directory:

 pedro@opensolaris:~/orlando/gdb/baseline/build-solaris/gdb$  ls /proc/5613/lwp/1/
 lwpctl  lwpsinfo  lwpstatus  lwpusage  templates  xregs

My previous path had broken creating core dumps in multi-threaded apps,
as iterating over thread procinfo's would miss the main process.

Please find attached a new patch.  This one's much simpler.

Only procfs.c changed.

-- 
Pedro Alves

[-- Attachment #2: 005-procfs_always_a_thread.diff --]
[-- Type: text/x-diff, Size: 10159 bytes --]

2008-08-15  Pedro Alves  <pedro@codesourcery.com>

	* procfs.c (to_attach): Create a procinfo for the current lwp.
	Add it to gdb's thread list.
	(procfs_fetch_registers, procfs_store_registers): Assume there's
	always an lwp.
	(procfs_wait): And add the main thread here.
	(procfs_init_inferior): Create a procinfo for the main lwp here.
	Change main thread's ptid with thread_change_ptid.
	(procfs_notice_thread): Check for exited threads.
	(procfs_corefile_thread_callback): Remote check for the main
	process.
	(procfs_make_note_section): Assume there is always a thread.

	* sol-thread.c (sol_thread_attach): Clear sol_thread_active before
	attaching.  Change the main thread ptid with thread_change_ptid.
	(sol_thread_detach): Clear sol_thread_active.
	(sol_thread_wait): Check for exited threads.
	(sol_thread_create_inferior): Clear sol_thread_active before
	creating a new inferior.  Change the main thread ptid with
	thread_change_ptid.
	(sol_thread_mourn_inferior): Clear sol_thread_active.
	(sol_find_new_threads_callback): Check for exited threads.

---
 gdb/procfs.c     |   77 +++++++++++++++++++++----------------------------------
 gdb/sol-thread.c |   33 ++++++++++++-----------
 2 files changed, 48 insertions(+), 62 deletions(-)

Index: src/gdb/procfs.c
===================================================================
--- src.orig/gdb/procfs.c	2008-08-15 18:30:05.000000000 +0100
+++ src/gdb/procfs.c	2008-08-15 19:32:41.000000000 +0100
@@ -3664,6 +3664,7 @@ do_attach (ptid_t ptid)
 {
   procinfo *pi;
   int fail;
+  int lwpid;
 
   if ((pi = create_procinfo (PIDGET (ptid), 0)) == NULL)
     perror (_("procfs: out of memory in 'attach'"));
@@ -3713,7 +3714,16 @@ do_attach (ptid_t ptid)
 
   /* Let GDB know that the inferior was attached.  */
   attach_flag = 1;
-  return MERGEPID (pi->pid, proc_get_current_thread (pi));
+
+  /* Create a procinfo for the current lwp.  */
+  lwpid = proc_get_current_thread (pi);
+  create_procinfo (pi->pid, lwpid);
+
+  /* Add it to gdb's thread list.  */
+  ptid = MERGEPID (pi->pid, lwpid);
+  add_thread (ptid);
+
+  return ptid;
 }
 
 static void
@@ -3784,14 +3794,7 @@ procfs_fetch_registers (struct regcache 
   int tid = TIDGET (inferior_ptid);
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
-  /* First look up procinfo for the main process.  */
-  pi = find_procinfo_or_die (pid, 0);
-
-  /* If the event thread is not the same as GDB's requested thread
-     (ie. inferior_ptid), then look up procinfo for the requested
-     thread.  */
-  if (tid != 0 && tid != proc_get_current_thread (pi))
-    pi = find_procinfo_or_die (pid, tid);
+  pi = find_procinfo_or_die (pid, tid);
 
   if (pi == NULL)
     error (_("procfs: fetch_registers failed to find procinfo for %s"),
@@ -3850,14 +3853,7 @@ procfs_store_registers (struct regcache 
   int tid = TIDGET (inferior_ptid);
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
-  /* First find procinfo for main process.  */
-  pi = find_procinfo_or_die (pid, 0);
-
-  /* If the event thread is not the same as GDB's requested thread
-     (ie. inferior_ptid), then look up procinfo for the requested
-     thread.  */
-  if (tid != 0 && tid != proc_get_current_thread (pi))
-    pi = find_procinfo_or_die (pid, tid);
+  pi = find_procinfo_or_die (pid, tid);
 
   if (pi == NULL)
     error (_("procfs: store_registers: failed to find procinfo for %s"),
@@ -4350,20 +4346,6 @@ wait_again:
 		  add_thread (retval);
 		  if (find_procinfo (PIDGET (retval), TIDGET (retval)) == NULL)
 		    create_procinfo (PIDGET (retval), TIDGET (retval));
-
-		  /* In addition, it's possible that this is the first
-		   * new thread we've seen, in which case we may not
-		   * have created entries for inferior_ptid yet.
-		   */
-		  if (TIDGET (inferior_ptid) != 0)
-		    {
-		      if (!in_thread_list (inferior_ptid))
-			add_thread (inferior_ptid);
-		      if (find_procinfo (PIDGET (inferior_ptid),
-					 TIDGET (inferior_ptid)) == NULL)
-			create_procinfo (PIDGET (inferior_ptid),
-					 TIDGET (inferior_ptid));
-		    }
 		}
 	    }
 	  else	/* flags do not indicate STOPPED */
@@ -4891,6 +4873,7 @@ procfs_init_inferior (int pid)
   procinfo *pi;
   gdb_sigset_t signals;
   int fail;
+  int lwpid;
 
   /* This routine called on the parent side (GDB side)
      after GDB forks the inferior.  */
@@ -4951,9 +4934,17 @@ procfs_init_inferior (int pid)
   if (!proc_set_run_on_last_close (pi))
     proc_error (pi, "init_inferior, set_RLC", __LINE__);
 
-  /* The 'process ID' we return to GDB is composed of
-     the actual process ID plus the lwp ID. */
-  inferior_ptid = MERGEPID (pi->pid, proc_get_current_thread (pi));
+  /* We now have have access to the lwpid of the main thread/lwp.  */
+  lwpid = proc_get_current_thread (pi);
+
+  /* Create a procinfo for the main lwp.  */
+  create_procinfo (pid, lwpid);
+
+  /* We already have a main thread registered in the thread table at
+     this point, but it didn't have any lwp info yet.  Notify the core
+     about it.  This changes inferior_ptid as well.  */
+  thread_change_ptid (pid_to_ptid (pid),
+		      MERGEPID (pid, lwpid));
 
   /* Typically two, one trap to exec the shell, one to exec the
      program being debugged.  Defined by "inferior.h".  */
@@ -5212,7 +5203,7 @@ procfs_notice_thread (procinfo *pi, proc
 {
   ptid_t gdb_threadid = MERGEPID (pi->pid, thread->tid);
 
-  if (!in_thread_list (gdb_threadid))
+  if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
     add_thread (gdb_threadid);
 
   return 0;
@@ -6105,7 +6096,7 @@ procfs_corefile_thread_callback (procinf
 {
   struct procfs_corefile_thread_data *args = data;
 
-  if (pi != NULL && thread->tid != 0)
+  if (pi != NULL)
     {
       ptid_t saved_ptid = inferior_ptid;
       inferior_ptid = MERGEPID (pi->pid, thread->tid);
@@ -6167,17 +6158,9 @@ procfs_make_note_section (bfd *obfd, int
   thread_args.note_size = note_size;
   proc_iterate_over_threads (pi, procfs_corefile_thread_callback, &thread_args);
 
-  if (thread_args.note_data == note_data)
-    {
-      /* iterate_over_threads didn't come up with any threads;
-	 just use inferior_ptid. */
-      note_data = procfs_do_thread_registers (obfd, inferior_ptid,
-					      note_data, note_size);
-    }
-  else
-    {
-      note_data = thread_args.note_data;
-    }
+  /* There should be always at least one thread.  */
+  gdb_assert (thread_args.note_data != note_data);
+  note_data = thread_args.note_data;
 
   auxv_len = target_read_alloc (&current_target, TARGET_OBJECT_AUXV,
 				NULL, &auxv);
Index: src/gdb/sol-thread.c
===================================================================
--- src.orig/gdb/sol-thread.c	2008-08-15 18:30:05.000000000 +0100
+++ src/gdb/sol-thread.c	2008-08-15 19:53:22.000000000 +0100
@@ -253,7 +253,7 @@ td_state_string (td_thr_state_e statecod
 
 /* Convert a POSIX or Solaris thread ID into a LWP ID.  If THREAD_ID
    doesn't exist, that's an error.  If it's an inactive thread, return
-   DEFAULT_LPW.
+   DEFAULT_LWP.
 
    NOTE: This function probably shouldn't call error().  */
 
@@ -350,6 +350,7 @@ sol_thread_open (char *arg, int from_tty
 static void
 sol_thread_attach (char *args, int from_tty)
 {
+  sol_thread_active = 0;
   procfs_ops.to_attach (args, from_tty);
 
   /* Must get symbols from shared libraries before libthread_db can run!  */
@@ -357,14 +358,13 @@ sol_thread_attach (char *args, int from_
 
   if (sol_thread_active)
     {
+      ptid_t ptid;
       printf_filtered ("sol-thread active.\n");
       main_ph.ptid = inferior_ptid; /* Save for xfer_memory.  */
       push_target (&sol_thread_ops);
-      inferior_ptid = lwp_to_thread (inferior_ptid);
-      if (PIDGET (inferior_ptid) == -1)
-	inferior_ptid = main_ph.ptid;
-      else
-	add_thread (inferior_ptid);
+      ptid = lwp_to_thread (inferior_ptid);
+      if (PIDGET (ptid) != -1)
+	thread_change_ptid (inferior_ptid, ptid);
     }
 
   /* FIXME: Might want to iterate over all the threads and register
@@ -381,6 +381,7 @@ sol_thread_attach (char *args, int from_
 static void
 sol_thread_detach (char *args, int from_tty)
 {
+  sol_thread_active = 0;
   inferior_ptid = pid_to_ptid (PIDGET (main_ph.ptid));
   unpush_target (&sol_thread_ops);
   procfs_ops.to_detach (args, from_tty);
@@ -419,7 +420,7 @@ sol_thread_resume (ptid_t ptid, int step
   do_cleanups (old_chain);
 }
 
-/* Wait for any threads to stop.  We may have to convert PIID from a
+/* Wait for any threads to stop.  We may have to convert PTID from a
    thread ID to an LWP ID, and vice versa on the way out.  */
 
 static ptid_t
@@ -460,7 +461,8 @@ sol_thread_wait (ptid_t ptid, struct tar
       /* See if we have a new thread.  */
       if (is_thread (rtnval)
 	  && !ptid_equal (rtnval, save_ptid)
-	  && !in_thread_list (rtnval))
+	  && (!in_thread_list (rtnval)
+	      || is_exited (rtnval)))
 	add_thread (rtnval);
     }
 
@@ -754,21 +756,21 @@ static void
 sol_thread_create_inferior (char *exec_file, char *allargs, char **env,
 			    int from_tty)
 {
+  sol_thread_active = 0;
   procfs_ops.to_create_inferior (exec_file, allargs, env, from_tty);
 
   if (sol_thread_active && !ptid_equal (inferior_ptid, null_ptid))
     {
+      ptid_t ptid;
+
       /* Save for xfer_memory.  */
       main_ph.ptid = inferior_ptid;
 
       push_target (&sol_thread_ops);
 
-      inferior_ptid = lwp_to_thread (inferior_ptid);
-      if (PIDGET (inferior_ptid) == -1)
-	inferior_ptid = main_ph.ptid;
-
-      if (!in_thread_list (inferior_ptid))
-	add_thread (inferior_ptid);
+      ptid = lwp_to_thread (inferior_ptid);
+      if (PIDGET (ptid) != -1)
+	thread_change_ptid (inferior_ptid, ptid);
     }
 }
 
@@ -822,6 +824,7 @@ sol_thread_new_objfile (struct objfile *
 static void
 sol_thread_mourn_inferior (void)
 {
+  sol_thread_active = 0;
   unpush_target (&sol_thread_ops);
   procfs_ops.to_mourn_inferior ();
 }
@@ -1366,7 +1369,7 @@ sol_find_new_threads_callback (const td_
     return -1;
 
   ptid = BUILD_THREAD (ti.ti_tid, PIDGET (inferior_ptid));
-  if (!in_thread_list (ptid))
+  if (!in_thread_list (ptid) || is_exited (ptid))
     add_thread (ptid);
 
   return 0;