Re: RFA: gdb linux nptl patch 1

["J. Johnston" <jjohnstn@redhat.com>]

Has anybody had a chance to look at this?

-- Jeff J.

J. Johnston wrote:
> The following is the first of a set of proposed patches for gdb to support
> the new nptl threading model for linux.
> 
> In the old linuxthreads model, the lwp was equal to the pid.  In the new
> nptl model, the lwp is unique for each thread and the pid value is common
> among sibling threads.  A side-effect of this difference is that with 
> linuxthreads,
> you could see individual threads externally on the ps list.  With the new
> nptl kernel, you will not be able to see child thread lwps on the list.
> 
> Another key difference is how a tid is represented.  In the linuxthreads 
> model,
> a tid was an index into a table and was restricted to 16-bits.  In the 
> new nptl
> model, there is no such restriction on the number of threads and the tid 
> is in
> fact an address (i.e. not 16 bits).  In the current linux-proc.c logic, 
> when
> performing a gcore command, pr_status note information is created by 
> taking the pid and
> concatenating this to the tid to form a 32-bit construct.  Obviously, in 
> the new
> nptl model this won't work as the tid itself is either 32-bits or 64-bits.
> It should also be mentioned that this logic does not match what the kernel
> spits out in a corefile.  For linuxthreads, the kernel is just putting the
> lwp = pid in the pr_status notes.
> 
> Logic that works for either model and matches what the kernel is doing 
> is to
> use the lwp and only the lwp in the pr_status notes.  This creates a slight
> problem because linux-proc.c uses the thread_info list to run through 
> all the
> threads.  Unfortunately, the thread_info list contains ptid_t structs that
> contain the pid and the tid, but no lwp.  In the old linuxthreads model, 
> this
> isn't a problem because the pid is in fact equal to the lwp.  In the nptl
> model, we can't simply convert a pid/tid combination into its associated 
> lwp
> because this logic is hidden in libthread_db.  At present, the libthread_db
> library is only loaded by the thread-db layer.  While there are routines to
> convert a tid to lwp and vice-versa, these routines are not externally 
> exposed
> in the target vector.  The only externalized method that could do such a 
> job
> is to_pid_to_str() which converts to character.  Usage of this interface 
> would
> require parsing the output for the characters "LWP".
> 
> This patch proposes adding a secondary list of thread_info structs that 
> keeps
> track of the lwps.  This list is kept distinct from the regular thread 
> list.
> Since manipulating either list involves common operations, some base 
> operations
> are added for generic thread_info list manipulation.  The lin-lwp.c 
> layer is
> changed to add and delete lwp thread_info structs to the lwp list when lwps
> are added or deleted.  The linux-proc.c code is changed to iterate 
> through the
> lwp thread_info list rather than the thread list when creating the 
> pr_status
> notes.
> 
> I had brought up this idea on the gdb mailing list and there was some 
> discussion
> about how the thread_info struct should be renamed to something more 
> generic.
> That issue was not resolved, but I feel that it can be performed 
> distinct from this
> patch as it will require a global change to many pieces of code.  To 
> keep some
> new names at a reasonable length, I chose to use the name tinfo_list 
> rather than
> thread_info_list.  This would obviously also be renamed.
> 
> Ok to commit?  I have reindented and checked in the three C files 
> involved prior to
> building this patch.
> 
> -- Jeff J.
> 
> 2003-03-28  Jeff Johnston  <jjohnstn@redhat.com>
> 
>     * gdbthread.h (tinfo_list): New struct to represent list of
>     thread_info structs.
>     (get_thread_tinfo_list, get_lwp_tinfo_list): New prototypes.
>     (add_thread_info,  delete_thread_info): Ditto.
>     (thread_info_id_to_pid, pid_to_thread_info_id): Ditto.
>     (in_tinfo_list, valid_thread_info_id, find_thread_info_pid): Ditto.
>     (iterate_over_tinfo_list): Ditto.   
>     * thread.c (init_thread_list): Clear lwp list as well as thread list.
>     (get_thread_tinfo_list, get_lwp_tinfo_list): New functions
>     to get either the thread or lwp lists respectively.
>     (add_thread_info,  delete_thread_info): New generic thread_info 
> functions.
>     (thread_info_id_to_pid, pid_to_thread_info_id): Ditto.
>     (in_tinfo_list, valid_thread_info_id, find_thread_info_pid): Ditto.
>     (iterate_over_tinfo_list): Ditto.
>     * linux-proc.c (linux_do_thread_registers): Store lwp for prstatus note
>     rather than a merge of pid and tid.
>     (linux_make_note_section): Iterate over list of lwps rather than 
> threads
>     to create note data.
>     * lin-lwp.c (add_lwp): Call add_thread_info() to add new lwp ptid to 
> lwp list.
>     (delete_lwp): Call delete_thread_info() to delete lwp ptid from lwp 
> list.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: gdbthread.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbthread.h,v
> retrieving revision 1.6
> diff -u -r1.6 gdbthread.h
> --- gdbthread.h	6 Dec 2002 07:35:55 -0000	1.6
> +++ gdbthread.h	29 Mar 2003 00:02:45 -0000
> @@ -73,6 +73,58 @@
>    struct private_thread_info *private;
>  };
>  
> +struct tinfo_list
> +{
> +  struct thread_info *start;
> +  int highest_num;
> +};
> +
> +/* Thread info list operations.  */
> +
> +/* Create an empty tinfo list, or empty an existing one.  */
> +extern void init_tinfo_list (struct tinfo_list *lp);
> +
> +/* Get the thread info list for threads.  */
> +extern struct tinfo_list *get_thread_tinfo_list (void);
> +
> +/* Get the thread info list for lwps.  */
> +extern struct tinfo_list *get_lwp_tinfo_list (void);
> +
> +/* Add a thread_info to a thread_info list.  */
> +extern struct thread_info *add_thread_info (struct tinfo_list *lp,
> +					    ptid_t ptid);
> +
> +/* Delete an existing thread_info list entry.  */
> +extern void delete_thread_info (struct tinfo_list *lp, ptid_t);
> +
> +/* Translate the integer thread_info id (GDB's homegrown id, not the system's)
> +   into a "pid" (which may be overloaded with extra thread information).  */
> +extern ptid_t thread_info_id_to_pid (struct tinfo_list *lp, int);
> +
> +/* Translate a 'pid' (which may be overloaded with extra thread information) 
> +   into the integer thread id (GDB's homegrown id, not the system's).  */
> +extern int pid_to_thread_info_id (struct tinfo_list *lp, ptid_t ptid);
> +
> +/* Boolean test for an already-known pid (which may be overloaded with
> +   extra thread information).  */
> +extern int in_tinfo_list (struct tinfo_list *lp, ptid_t ptid);
> +
> +/* Boolean test for an already-known thread id (GDB's homegrown id, 
> +   not the system's).  */
> +extern int valid_thread_info_id (struct tinfo_list *lp, int thread);
> +
> +/* Search function to lookup a thread_info by 'pid'.  */
> +extern struct thread_info *find_thread_info_pid (struct tinfo_list *lp,
> +						 ptid_t ptid);
> +
> +/* Iterator function to call a user-provided callback function
> +   once for each known thread_info in a list.  */
> +typedef int (*thread_callback_func) (struct thread_info *, void *);
> +extern struct thread_info *iterate_over_tinfo_list (
> +  struct tinfo_list *lp, thread_callback_func, void *);
> +
> +/* Thread operations.  */
> +
>  /* Create an empty thread list, or empty the existing one.  */
>  extern void init_thread_list (void);
>  
> @@ -108,7 +160,6 @@
>  
>  /* Iterator function to call a user-provided callback function
>     once for each known thread.  */
> -typedef int (*thread_callback_func) (struct thread_info *, void *);
>  extern struct thread_info *iterate_over_threads (thread_callback_func, void *);
>  
>  /* infrun context switch: save the debugger state for the given thread.  */
> Index: thread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread.c,v
> retrieving revision 1.29
> diff -u -r1.29 thread.c
> --- thread.c	28 Mar 2003 21:42:41 -0000	1.29
> +++ thread.c	29 Mar 2003 00:02:45 -0000
> @@ -49,11 +49,15 @@
>  
>  void _initialize_thread (void);
>  
> -/* Prototypes for local functions. */
> +/* Thread info lists.  */
> +
> +static struct tinfo_list thread_tinfo_list = { NULL, 0 };
> +static struct tinfo_list lwp_tinfo_list = { NULL, 0 };
>  
> -static struct thread_info *thread_list = NULL;
> -static int highest_thread_num;
> +/* Prototypes for local functions. */
>  
> +static struct thread_info *find_thread_info_id (struct tinfo_list *lp,
> +						int num);
>  static struct thread_info *find_thread_id (int num);
>  
>  static void thread_command (char *tidstr, int from_tty);
> @@ -65,25 +69,22 @@
>  static void switch_to_thread (ptid_t ptid);
>  static void prune_threads (void);
>  
> -void
> -delete_step_resume_breakpoint (void *arg)
> -{
> -  struct breakpoint **breakpointp = (struct breakpoint **) arg;
> -  struct thread_info *tp;
> +/* Thread info list functions.  */
>  
> -  if (*breakpointp != NULL)
> -    {
> -      delete_breakpoint (*breakpointp);
> -      for (tp = thread_list; tp; tp = tp->next)
> -	if (tp->step_resume_breakpoint == *breakpointp)
> -	  tp->step_resume_breakpoint = NULL;
> +struct tinfo_list *
> +get_thread_tinfo_list (void)
> +{
> +  return &thread_tinfo_list;
> +}
>  
> -      *breakpointp = NULL;
> -    }
> +struct tinfo_list *
> +get_lwp_tinfo_list (void)
> +{
> +  return &lwp_tinfo_list;
>  }
>  
>  static void
> -free_thread (struct thread_info *tp)
> +free_thread_info (struct thread_info *tp)
>  {
>    /* NOTE: this will take care of any left-over step_resume breakpoints,
>       but not any user-specified thread-specific breakpoints. */
> @@ -99,48 +100,46 @@
>  }
>  
>  void
> -init_thread_list (void)
> +init_tinfo_list (struct tinfo_list *lp)
>  {
> +  int i;
>    struct thread_info *tp, *tpnext;
>  
> -  highest_thread_num = 0;
> -  if (!thread_list)
> +  lp->highest_num = 0;
> +  if (lp->start == NULL)
>      return;
>  
> -  for (tp = thread_list; tp; tp = tpnext)
> +  for (tp = lp->start; tp; tp = tpnext)
>      {
>        tpnext = tp->next;
> -      free_thread (tp);
> +      free_thread_info (tp);
>      }
>  
> -  thread_list = NULL;
> +  lp->start = NULL;
>  }
>  
> -/* add_thread now returns a pointer to the new thread_info, 
> -   so that back_ends can initialize their private data.  */
> -
>  struct thread_info *
> -add_thread (ptid_t ptid)
> +add_thread_info (struct tinfo_list *lp, ptid_t ptid)
>  {
>    struct thread_info *tp;
>  
>    tp = (struct thread_info *) xmalloc (sizeof (*tp));
>    memset (tp, 0, sizeof (*tp));
>    tp->ptid = ptid;
> -  tp->num = ++highest_thread_num;
> -  tp->next = thread_list;
> -  thread_list = tp;
> +  tp->num = ++lp->highest_num;
> +  tp->next = lp->start;
> +  lp->start = tp;
>    return tp;
>  }
>  
>  void
> -delete_thread (ptid_t ptid)
> +delete_thread_info (struct tinfo_list *lp, ptid_t ptid)
>  {
>    struct thread_info *tp, *tpprev;
>  
>    tpprev = NULL;
>  
> -  for (tp = thread_list; tp; tpprev = tp, tp = tp->next)
> +  for (tp = lp->start; tp; tpprev = tp, tp = tp->next)
>      if (ptid_equal (tp->ptid, ptid))
>        break;
>  
> @@ -150,17 +149,17 @@
>    if (tpprev)
>      tpprev->next = tp->next;
>    else
> -    thread_list = tp->next;
> +    lp->start = tp->next;
>  
> -  free_thread (tp);
> +  free_thread_info (tp);
>  }
>  
>  static struct thread_info *
> -find_thread_id (int num)
> +find_thread_info_id (struct tinfo_list *lp, int num)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if (tp->num == num)
>        return tp;
>  
> @@ -169,11 +168,11 @@
>  
>  /* Find a thread_info by matching PTID.  */
>  struct thread_info *
> -find_thread_pid (ptid_t ptid)
> +find_thread_info_pid (struct tinfo_list *lp, ptid_t ptid)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if (ptid_equal (tp->ptid, ptid))
>        return tp;
>  
> @@ -181,7 +180,7 @@
>  }
>  
>  /*
> - * Thread iterator function.
> + * Thread info list iterator function.
>   *
>   * Calls a callback function once for each thread, so long as
>   * the callback function returns false.  If the callback function
> @@ -190,17 +189,16 @@
>   * search for a thread with arbitrary attributes, or for applying
>   * some operation to every thread.
>   *
> - * FIXME: some of the existing functionality, such as 
> - * "Thread apply all", might be rewritten using this functionality.
>   */
>  
>  struct thread_info *
> -iterate_over_threads (int (*callback) (struct thread_info *, void *),
> -		      void *data)
> +iterate_over_tinfo_list (struct tinfo_list *lp,
> +			 int (*callback) (struct thread_info *, void *),
> +			 void *data)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if ((*callback) (tp, data))
>        return tp;
>  
> @@ -208,11 +206,11 @@
>  }
>  
>  int
> -valid_thread_id (int num)
> +valid_thread_info_id (struct tinfo_list *lp, int num)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if (tp->num == num)
>        return 1;
>  
> @@ -220,11 +218,11 @@
>  }
>  
>  int
> -pid_to_thread_id (ptid_t ptid)
> +pid_to_thread_info_id (struct tinfo_list *lp, ptid_t ptid)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if (ptid_equal (tp->ptid, ptid))
>        return tp->num;
>  
> @@ -232,9 +230,9 @@
>  }
>  
>  ptid_t
> -thread_id_to_pid (int num)
> +thread_info_id_to_pid (struct tinfo_list *lp, int num)
>  {
> -  struct thread_info *thread = find_thread_id (num);
> +  struct thread_info *thread = find_thread_info_id (lp, num);
>    if (thread)
>      return thread->ptid;
>    else
> @@ -242,17 +240,117 @@
>  }
>  
>  int
> -in_thread_list (ptid_t ptid)
> +in_tinfo_list (struct tinfo_list *lp, ptid_t ptid)
>  {
>    struct thread_info *tp;
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = lp->start; tp; tp = tp->next)
>      if (ptid_equal (tp->ptid, ptid))
>        return 1;
>  
>    return 0;			/* Never heard of 'im */
>  }
>  
> +/* Thread list specific functions.  */
> +
> +void
> +init_thread_list (void)
> +{
> +  init_tinfo_list (&thread_tinfo_list);
> +  init_tinfo_list (&lwp_tinfo_list);
> +}
> +
> +/* add_thread now returns a pointer to the new thread_info, 
> +   so that back_ends can initialize their private data.  */
> +
> +struct thread_info *
> +add_thread (ptid_t ptid)
> +{
> +  return add_thread_info (&thread_tinfo_list, ptid);
> +}
> +
> +void
> +delete_thread (ptid_t ptid)
> +{
> +  delete_thread_info (&thread_tinfo_list, ptid);
> +}
> +
> +static struct thread_info *
> +find_thread_id (int num)
> +{
> +  return find_thread_info_id (&thread_tinfo_list, num);
> +}
> +
> +/* Find a thread_info by matching PTID.  */
> +struct thread_info *
> +find_thread_pid (ptid_t ptid)
> +{
> +  return find_thread_info_pid (&thread_tinfo_list, ptid);
> +}
> +
> +/*
> + * Thread iterator function.
> + *
> + * Calls a callback function once for each thread, so long as
> + * the callback function returns false.  If the callback function
> + * returns true, the iteration will end and the current thread
> + * will be returned.  This can be useful for implementing a 
> + * search for a thread with arbitrary attributes, or for applying
> + * some operation to every thread.
> + *
> + * FIXME: some of the existing functionality, such as 
> + * "Thread apply all", might be rewritten using this functionality.
> + */
> +
> +struct thread_info *
> +iterate_over_threads (int (*callback) (struct thread_info *, void *),
> +		      void *data)
> +{
> +  return iterate_over_tinfo_list (&thread_tinfo_list, callback, data);
> +}
> +
> +int
> +valid_thread_id (int num)
> +{
> +  return valid_thread_info_id (&thread_tinfo_list, num);
> +}
> +
> +int
> +pid_to_thread_id (ptid_t ptid)
> +{
> +  return pid_to_thread_info_id (&thread_tinfo_list, ptid);
> +}
> +
> +ptid_t
> +thread_id_to_pid (int num)
> +{
> +  return thread_info_id_to_pid (&thread_tinfo_list, num);
> +}
> +
> +int
> +in_thread_list (ptid_t ptid)
> +{
> +  return in_tinfo_list (&thread_tinfo_list, ptid);
> +}
> +
> +
> +void
> +delete_step_resume_breakpoint (void *arg)
> +{
> +  struct breakpoint **breakpointp = (struct breakpoint **) arg;
> +  struct thread_info *tp;
> +
> +  if (*breakpointp != NULL)
> +    {
> +      delete_breakpoint (*breakpointp);
> +      for (tp = thread_tinfo_list.start; tp; tp = tp->next)
> +	if (tp->step_resume_breakpoint == *breakpointp)
> +	  tp->step_resume_breakpoint = NULL;
> +
> +      *breakpointp = NULL;
> +    }
> +}
> +
>  /* Print a list of thread ids currently known, and the total number of
>     threads. To be used from within catch_errors. */
>  static int
> @@ -267,7 +365,7 @@
>  
>    cleanup_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "thread-ids");
>  
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = thread_tinfo_list.start; tp; tp = tp->next)
>      {
>        num++;
>        ui_out_field_int (uiout, "thread-id", tp->num);
> @@ -404,7 +502,7 @@
>  {
>    struct thread_info *tp, *next;
>  
> -  for (tp = thread_list; tp; tp = next)
> +  for (tp = thread_tinfo_list.start; tp; tp = next)
>      {
>        next = tp->next;
>        if (!thread_alive (tp))
> @@ -437,7 +535,7 @@
>    prune_threads ();
>    target_find_new_threads ();
>    current_ptid = inferior_ptid;
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = thread_tinfo_list.start; tp; tp = tp->next)
>      {
>        if (ptid_equal (tp->ptid, current_ptid))
>  	printf_filtered ("* ");
> @@ -564,7 +662,7 @@
>       execute_command */
>    saved_cmd = xstrdup (cmd);
>    saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> -  for (tp = thread_list; tp; tp = tp->next)
> +  for (tp = thread_tinfo_list.start; tp; tp = tp->next)
>      if (thread_alive (tp))
>        {
>  	switch_to_thread (tp->ptid);
> Index: linux-proc.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linux-proc.c,v
> retrieving revision 1.14
> diff -u -r1.14 linux-proc.c
> --- linux-proc.c	28 Mar 2003 21:42:41 -0000	1.14
> +++ linux-proc.c	29 Mar 2003 00:02:45 -0000
> @@ -171,14 +171,18 @@
>  #ifdef FILL_FPXREGSET
>    gdb_fpxregset_t fpxregs;
>  #endif
> -  unsigned long merged_pid = ptid_get_tid (ptid) << 16 | ptid_get_pid (ptid);
> +  unsigned long lwpid = 0;
> +  char *pid_str = NULL;
> +
> +  lwpid = ptid_get_lwp (ptid);
> +  if (lwpid == 0)
> +    lwpid = ptid_get_pid (ptid);
>  
>    fill_gregset (&gregs, -1);
>    note_data = (char *) elfcore_write_prstatus (obfd,
>  					       note_data,
>  					       note_size,
> -					       merged_pid,
> -					       stop_signal, &gregs);
> +					       lwpid, stop_signal, &gregs);
>  
>    fill_fpregset (&fpregs, -1);
>    note_data = (char *) elfcore_write_prfpreg (obfd,
> @@ -268,10 +272,11 @@
>    thread_args.note_data = note_data;
>    thread_args.note_size = note_size;
>    thread_args.num_notes = 0;
> -  iterate_over_threads (linux_corefile_thread_callback, &thread_args);
> +  iterate_over_tinfo_list (get_lwp_tinfo_list (),
> +			   linux_corefile_thread_callback, &thread_args);
>    if (thread_args.num_notes == 0)
>      {
> -      /* iterate_over_threads didn't come up with any threads;
> +      /* iterate_over_tinfo_list didn't come up with any lwps;
>           just use inferior_ptid.  */
>        note_data = linux_do_thread_registers (obfd, inferior_ptid,
>  					     note_data, note_size);
> Index: lin-lwp.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/lin-lwp.c,v
> retrieving revision 1.43
> diff -u -r1.43 lin-lwp.c
> --- lin-lwp.c	28 Mar 2003 21:42:41 -0000	1.43
> +++ lin-lwp.c	29 Mar 2003 00:02:45 -0000
> @@ -220,6 +220,8 @@
>    if (++num_lwps > 1)
>      threaded = 1;
>  
> +  add_thread_info (get_lwp_tinfo_list (), ptid);
> +
>    return lp;
>  }
>  
> @@ -249,6 +251,8 @@
>      lwp_list = lp->next;
>  
>    xfree (lp);
> +
> +  delete_thread_info (get_lwp_tinfo_list (), ptid);
>  }
>  
>  /* Return a pointer to the structure describing the LWP corresponding