From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10981 invoked by alias); 5 Jun 2003 18:35:04 -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 10905 invoked from network); 5 Jun 2003 18:35:02 -0000 Received: from unknown (HELO touchme.toronto.redhat.com) (207.219.125.105) by sources.redhat.com with SMTP; 5 Jun 2003 18:35:02 -0000 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id DB9F8800021 for ; Thu, 5 Jun 2003 14:35:01 -0400 (EDT) Message-ID: <3EDF8D55.5000902@redhat.com> Date: Thu, 05 Jun 2003 18:35:00 -0000 From: "J. Johnston" Organization: Red Hat Inc. User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0 X-Accept-Language: en-us, en MIME-Version: 1.0 To: gdb-patches@sources.redhat.com Subject: Re: RFA: gdb linux nptl patch 1 References: <3E84E672.3050506@redhat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-06/txt/msg00207.txt.bz2 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