From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1696 invoked by alias); 17 Jun 2003 15:28:02 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 1594 invoked from network); 17 Jun 2003 15:27:59 -0000 Received: from unknown (HELO crack.them.org) (146.82.138.56) by sources.redhat.com with SMTP; 17 Jun 2003 15:27:59 -0000 Received: from dsl093-172-017.pit1.dsl.speakeasy.net ([66.93.172.17] helo=nevyn.them.org ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 19SIO7-0005GW-00; Tue, 17 Jun 2003 10:28:47 -0500 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 19SINH-0004L1-00; Tue, 17 Jun 2003 11:27:55 -0400 Date: Tue, 17 Jun 2003 15:28:00 -0000 From: Daniel Jacobowitz To: "J. Johnston" Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: gdb linux nptl patch 1 Message-ID: <20030617152755.GA16545@nevyn.them.org> Mail-Followup-To: "J. Johnston" , gdb-patches@sources.redhat.com References: <3E84E672.3050506@redhat.com> <3EDF8D55.5000902@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3EDF8D55.5000902@redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-06/txt/msg00561.txt.bz2 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 > > > > * 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