RFA: gdb linux nptl patch 1
Daniel Jacobowitz
drow@mvista.com
Tue Jun 17 15:28:00 GMT 2003
It looks reasonable to me. I don't really like the "tinfo" name but I
don't have a better idea to offer you. Gdbserver calls it an
inferior_list, but the concept is exactly the same.
On Thu, Jun 05, 2003 at 02:35:01PM -0400, J. Johnston wrote:
> 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
>
>
>
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
More information about the Gdb-patches
mailing list