Re: [PATCH v3 4/7] Per-inferior/Inferior-qualified thread IDs

Pedro Alves
2016-01-07T19:45:34Z

On 01/06/2016 06:51 PM, Simon Marchi wrote:
> A few comments:
>
> On 16-01-06 08:03 AM, Pedro Alves wrote:
>> This commit changes GDB to track thread numbers per-inferior. Then,
>> if you're debugging multiple inferiors, GDB displays
>> "inferior-num.thread-num" instead of just "thread-num" whenever it
>> needs to display a thread:
>>
>> (gdb) info inferiors
>> Num Description Executable
>> 1 process 6022 /home/pedro/gdb/tests/threads
>> * 2 process 6037 /home/pedro/gdb/tests/threads
>> (gdb) info threads
>> Id Target Id Frame
>> 1.1 Thread 0x7ffff7fc2740 (LWP 6022) "threads" (running)
>> 1.2 Thread 0x7ffff77c0700 (LWP 6028) "threads" (running)
>> 1.3 Thread 0x7ffff7fc2740 (LWP 6032) "threads" (running)
>> 2.1 Thread 0x7ffff7fc1700 (LWP 6037) "threads" (running)
>> 2.2 Thread 0x7ffff77c0700 (LWP 6038) "threads" (running)
>> * 2.3 Thread 0x7ffff7fc2740 (LWP 6039) "threads" (running)
>> (gdb)
>> ...
>> (gdb) thread 1.1
>> [Switching to thread 1.1 (Thread 0x7ffff7fc2740 (LWP 8155))]
>> (gdb)
>> ...
>
> It was already the case before your patch, but I just noticed that
> when switching to a running thread, there should probably be a
> space before (running):
>
> (gdb) thread 1.1
> [Switching to thread 1.1 (Thread 0x7ffff7fad9c0 (LWP 15027))](running)

Yeah.

>
>> diff --git a/gdb/testsuite/gdb.multi/tids.c b/gdb/testsuite/gdb.multi/tids.c
>> new file mode 100644
>> index 0000000..00a8298
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.multi/tids.c
>> @@ -0,0 +1,52 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> + Copyright 2015 Free Software Foundation, Inc.
>> +
>> + This program is free software; you can redistribute it and/or modify
>> + it under the terms of the GNU General Public License as published by
>> + the Free Software Foundation; either version 3 of the License, or
>> + (at your option) any later version.
>> +
>> + This program is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + GNU General Public License for more details.
>> +
>> + You should have received a copy of the GNU General Public License
>> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
>> +
>> +#include <unistd.h>
>> +#include <pthread.h>
>> +
>> +pthread_t child_thread[2];
>> +
>> +void *
>> +thread_function2 (void *arg)
>> +{
>> + while (1)
>> + sleep (1);
>
> Nit: this should probably return NULL (thread_function1 as well).

Guess it doesn't hurt.

>
>> diff --git a/gdb/testsuite/gdb.multi/tids.exp b/gdb/testsuite/gdb.multi/tids.exp
>> new file mode 100644
>> index 0000000..7b51c80
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.multi/tids.exp
>> @@ -0,0 +1,296 @@
>> +# Copyright 2015 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test thread ID parsing and display.
>> +
>> +load_lib gdb-python.exp
>> +
>> +standard_testfile
>> +
>> +# Multiple inferiors are needed, therefore both native and extended
>> +# gdbserver modes are supported. Only non-extended gdbserver is not
>> +# supported.
>> +if [target_info exists use_gdb_stub] {
>> + untested ${testfile}.exp
>> + return
>> +}
>> +
>> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {pthreads debug}] } {
>> + return -1
>> +}
>> +
>> +clean_restart ${testfile}
>> +
>> +if { ![runto_main] } then {
>> + return -1
>> +}
>> +
>> +# Issue "thread apply TID_LIST p 1" and expect EXP_TID_LIST (a list of
>
> Nit: in the code it's "p 1234".

Fixed.

>
>> +# thread ids) to be displayed.
>> +proc thread_apply {tid_list exp_tid_list {message ""}} {
>> + global decimal
>> + set any "\[^\r\n\]*"
>> + set expected [string_to_regexp $exp_tid_list]
>> +
>> + set r ""
>> + foreach tid $expected {
>> + append r "\[\r\n\]+"
>> + append r "Thread $tid $any:\r\n"
>> + append r "\\$$decimal = 1234"
>> + }
>> +
>> + set cmd "thread apply $tid_list"
>> + if {$message == ""} {
>> + set message $cmd
>> + }
>> + gdb_test "$cmd p 1234" $r $message
>> +}
>> +
>> +# Issue "info threads TID_LIST" and expect EXP_TID_LIST (a list of
>> +# thread ids) to be displayed.
>> +proc info_threads {tid_list exp_tid_list {message ""}} {
>> + set any "\[^\r\n\]*"
>> + set expected [string_to_regexp $exp_tid_list]
>> + set r [join $expected " ${any}\r\n${any} "]
>> + set r "${any} $r ${any}"
>> + set cmd "info threads $tid_list"
>> + if {$message == ""} {
>> + set message $cmd
>> + }
>> + gdb_test $cmd $r $message
>> +}
>> +
>> +# Issue both "info threads TID_LIST" and expect INFO_THR output. Then
>
> "Issue both" does not make sense in this sentence.

Whoops.

>
>> +# issue "thread apply TID_LIST" and expect THR_APPLY output. If
>> +# THR_APPLY is omitted, INFO_THR is expected instead.
>> +proc thr_apply_info_thr {tid_list info_thr {thr_apply ""}} {
>> + if {$thr_apply == ""} {
>> + set thr_apply $info_thr
>> + }
>> +
>> + info_threads $tid_list $info_thr
>> + thread_apply $tid_list $thr_apply
>> +}
>> +
>> +# Issue both "info threads TID_LIST" and "thread apply TID_LIST" and
>> +# expect both commands to error out with EXP_ERROR.
>> +proc thr_apply_info_thr_error {tid_list exp_error} {
>> + gdb_test "info threads $tid_list" \
>> + $exp_error
>> +
>> + gdb_test "thread apply $tid_list p 1234" \
>> + $exp_error \
>> + "thread apply $tid_list"
>> +}
>> +
>> +# Issue both "info threads TID_LIST" and "thread apply TID_LIST" and
>> +# expect the command to error out with "Invalid thread ID: $EXPECTED".
>> +# EXPECTED is a literal string, not a regexp.
>> +proc thr_apply_info_thr_invalid {tid_list expected} {
>> + set expected [string_to_regexp $expected]
>> + gdb_test "info threads $tid_list" \
>> + "Invalid thread ID: $expected"
>> +
>> + gdb_test "thread apply $tid_list p 1234" \
>> + "Invalid thread ID: $expected p 1234" \
>> + "thread apply $tid_list"
>> +}
>> +
>> +# "info threads" while there's only inferior 1 should show
>> +# single-number thread IDs.
>> +with_test_prefix "single inferior" {
>> + info_threads "" "1"
>> +
>> + gdb_test "thread" "Current thread is 1 .*"
>> +}
>> +
>> +# "info threads" while there are multiple inferiors should show
>> +# qualified thread IDs.
>> +with_test_prefix "two inferiors" {
>> + # Add another inferior.
>> + gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
>> +
>> + # Now that we'd added another inferior, thread IDs now show the
>
> "Now that we've added" ?

Yup, fixed.

>
>> + # inferior number.
>> + info_threads "" "1.1"
>> +
>> + gdb_test "thread" "Current thread is 1\.1 .*"
>> +
>> + gdb_test "inferior 2" "Switching to inferior 2 .*" "switch to inferior 2"
>> + gdb_test "file ${binfile}" ".*" "load file in inferior 2"
>> +
>> + runto_main
>> +
>> + # Now that we'd added another inferior, thread IDs now show the
>
> Again.

Fixed again.

>
>> diff --git a/gdb/tid-parse.h b/gdb/tid-parse.h
>> new file mode 100644
>> index 0000000..3c79c08
>> --- /dev/null
>> +++ b/gdb/tid-parse.h
>> @@ -0,0 +1,148 @@
>> +/* TID parsing for GDB, the GNU debugger.
>> + Copyright (C) 2015-2016 Free Software Foundation, Inc.
>> +
>> + This file is part of GDB.
>> +
>> + This program is free software; you can redistribute it and/or modify
>> + it under the terms of the GNU General Public License as published by
>> + the Free Software Foundation; either version 3 of the License, or
>> + (at your option) any later version.
>> +
>> + This program is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + GNU General Public License for more details.
>> +
>> + You should have received a copy of the GNU General Public License
>> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
>> +
>> +#ifndef TID_PARSE_H
>> +#define TID_PARSE_H
>> +
>> +#include "cli/cli-utils.h"
>> +
>> +struct thread_info;
>> +
>> +/* Issue an invalid thread ID error, pointing at STRING, the invalid
>> + ID. */
>> +extern void ATTRIBUTE_NORETURN invalid_thread_id_error (const char *string);
>> +
>> +/* Parse TIDSTR as a per-inferior thread ID, in either INF_NUM.THR_NUM
>> + or THR_NUM form. In the latter case, the missing INF_NUM is filled
>> + in from the current inferior. If ENDPTR is not NULL,
>> + parse_thread_id stores the address of the first character after the
>> + thread ID. Either a valid thread is returned, or an error is
>> + thrown. */
>> +struct thread_info *parse_thread_id (const char *tidstr, const char **end);
>> +
>> +/* The possible states of the tid range parser's state machine. */
>> +enum tid_range_state
>> +{
>> + /* Parsing the inferior number. */
>> + TID_RANGE_STATE_INFERIOR,
>> +
>> + /* Parsing the thread number or thread number range. */
>> + TID_RANGE_STATE_THREAD_RANGE,
>> +};
>> +
>> +/* An object of this type is passed to tid_range_parser_get_tid. It
>> + must be initialized by calling tid_range_parser_init. This type is
>> + defined here so that it can be stack-allocated, but all members
>> + should be treated as opaque. */
>> +struct tid_range_parser
>> +{
>> + /* What sub-component are we expecting. */
>> + enum tid_range_state state;
>> +
>> + /* The string being parsed. When parsing has finished, this points
>> + past the last parsed token. */
>> + const char *string;
>> +
>> + /* The range parser state when we're parsing the thread number
>> + sub-component. */
>> + struct get_number_or_range_state range_parser;
>> +
>> + /* Last inferior number returned. */
>> + int inf_num;
>> +
>> + /* True if the TID last parsed was explicitly inferior-qualified.
>> + IOW, whether the spec specified an inferior number
>> + explicitly. */
>> + int qualified;
>> +
>> + /* The inferior number to assume if the TID is not qualified. */
>> + int default_inferior;
>> +};
>> +
>> +/* Initialize a tid_range_parser for use with
>> + tid_range_parser_get_tid. TIDLIST is the string to be parsed.
>> + DEFAULT_INFERIOR is the inferior number to assume if a
>> + non-qualified thread ID is found. */
>> +extern void tid_range_parser_init (struct tid_range_parser *parser,
>> + const char *tidlist,
>> + int default_inferior);
>> +
>> +/* Parse a thread ID or a thread range list.
>> +
>> + A range will be of the form
>> + <inferior_num>.<thread_number1>-<thread_number1> and will represent
>
> - <inferior_num>.<thread_number1>-<thread_number1> and will represent
> + <inferior_num>.<thread_number1>-<thread_number2> and will represent

Bah. :-)

>
>> + all the threads of inferior inferior_num with number between
>> + thread_number1 and thread_number2, inclusive. <inferior_num> can
>> + also be omitted, as in <thread_number1>-<thread_number1>, in which
>
> Again.
>
>> + case GDB infers the inferior number from the current inferior.
>> +
>> + While processing a thread ID range list, this function is called
>> + iteratively; at each call it will return (in the INF_NUM and
>> + THR_NUM output parameters) the next thread in the range.
>
> Would it be good to precise that it's unrelated to the threads that
> exist in gdb? It iterates solely on the numbers and doesn't care
> if that thread really exists.

Can't hurt.

>
> Also, it might be good to mention what the stop condition for the iteration
> is. Maybe as a simple usage example?

I tried to do that now.

>
>> +
>> + At the beginning of parsing a thread range, the char pointer
>> + PARSER->string will be advanced past <thread_number1> and left
>> + pointing at the '-' token. Subsequent calls will not advance the
>> + pointer until the range is completed. The call that completes the
>> + range will advance the pointer past <thread_number2>. */
>
>
>> +extern void tid_range_parser_get_tid (struct tid_range_parser *parser,
>> + int *inf_num, int *thr_num);
>
> Is there a reason why _get_tid_range returns success/failure and _get_tid
> doesn't return anything? Can't it fail the same way, since the later is
> implemented with the former?

Nothing checks the result of _get_tid, so I didn't bother, but it can't
hurt to be consistent I guess. Did that now.

>
>> +
>> +/* Like tid_range_parser_get_tid, but return a thread range per call,
>> + rather then a single thread.
>> +
>> + If the next element in the list is a single thread ID, then
>> + *THR_START and *THR_END are set to the same value. E.g.:
>> +
>> + list: "1.2 3.4-6"
>> + 1st call: *INF_NUM=1; *THR_START=2; *THR_END=2
>> + 2nd call: *INF_NUM=3; *THR_START=4; *THR_END=4
>> +
>> +
>> + Returns true if parsed a thread/range successfully, false
>> + otherwise. */
>
> IIUC, the value of thr_end defines if you want the to get a single thread id,
> or a range. Should it be mentioned here?

I was leaving that as an internal implementation detail, didn't want to
have callers rely on that. I "factored out" a helper function instead
now, and added assertions tid_range_parser_get_tid_range.

>
>> +extern int tid_range_parser_get_tid_range (struct tid_range_parser *parser,
>> + int *inf_num,
>> + int *thr_start, int *thr_end);
>> +
>> +/* Returns non-zero if parsing has completed. */
>> +extern int tid_range_parser_finished (struct tid_range_parser *parser);
>> +
>> +/* Return the string being parsed. When parsing has finished, this
>> + points past the last parsed token. */
>> +const char *tid_range_parser_string (struct tid_range_parser *parser);
>> +
>> +/* When parsing a range, advance past the final token in the range. */
>> +extern void tid_range_parser_skip (struct tid_range_parser *parser);
>> +
>> +/* True if the TID last parsed was explicitly inferior-qualified.
>> + IOW, whether the spec specified an inferior number explicitly. */
>> +extern int tid_range_parser_qualified (struct tid_range_parser *parser);
>> +
>> +/* Accept a string-form list of thread IDs such as is accepted by
>> + tid_range_parser_get_tid. Return TRUE if the INF_NUM.THR.NUM
>
> TRUE -> true

Fixed.

>
>> + thread is in the list. DEFAULT_INFERIOR is the inferior number to
>> + assume if a non-qualified thread ID is found in the list.
>> +
>> + By definition, an empty list includes all threads. This is to be
>> + interpreted as typing a command such as "info threads" with no
>> + arguments. */
>> +extern int tid_is_in_list (const char *list, int default_inferior,
>> + int inf_num, int thr_num);
>> +
>> +#endif /* TID_PARSE_H */
>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>> index d47bdc2..8116a5b 100644
>> --- a/gdb/varobj.c
>> +++ b/gdb/varobj.c
>> @@ -78,7 +78,7 @@ struct varobj_root
>> not NULL. */
>> struct frame_id frame;
>>
>> - /* The thread ID that this varobj_root belong to. This field
>> + /* The global thread ID that this varobj_root belongs to. This field
>> is only valid if valid_block is not NULL.
>> When not 0, indicates which thread 'frame' belongs to.
>> When 0, indicates that the thread list was empty when the varobj_root
>> @@ -380,7 +380,7 @@ varobj_create (char *objname,
>> error (_("Failed to find the specified frame"));
>>
>> var->root->frame = get_frame_id (fi);
>> - var->root->thread_id = pid_to_thread_id (inferior_ptid);
>> + var->root->thread_id = ptid_to_global_thread_id (inferior_ptid);
>> old_id = get_frame_id (get_selected_frame (NULL));
>> select_frame (fi);
>> }
>> @@ -2363,8 +2363,9 @@ value_of_root_1 (struct varobj **var_handle)
>> }
>> else
>> {
>> - ptid_t ptid = thread_id_to_pid (var->root->thread_id);
>> - if (in_thread_list (ptid))
>> + ptid_t ptid = global_thread_id_to_ptid (var->root->thread_id);
>> +
>> + if (!ptid_equal (minus_one_ptid, ptid))
>> {
>> switch_to_thread (ptid);
>> within_scope = check_scope (var);
>>
>
> windows-tdep.c fails to build with this patch. There is still one usage of find_thread_id:
>
> /home/emaisin/src/binutils-gdb/gdb/windows-tdep.c: In function Â‘display_tibÂ’:
> /home/emaisin/src/binutils-gdb/gdb/windows-tdep.c:378:7: error: implicit declaration of function Â‘find_thread_idÂ’ [-Werror=implicit-function-declaration]
> tp = find_thread_id (gdb_id);
> ^
> /home/emaisin/src/binutils-gdb/gdb/windows-tdep.c:378:10: error: assignment makes pointer from integer without a cast [-Werror]
> tp = find_thread_id (gdb_id);
> ^
> cc1: all warnings being treated as errors
> make: *** [windows-tdep.o] Error 1

Whoops, forgot --enable-targets=all...

But, looking at the code, that's parsing a GDB thread ID, while the comment
says the argument is supposed to be a Windows thread ID, not a GDB ID...
This argument doesn't seem to be documented in either the manual or
gdb's online help. It's not hard to fix this, but maybe we can just
get rid of it? There's always "thread apply TID info w32 tib".
Pierre, what do you think?

Here are the changes addressing your review comments.

From d2f3387ebbed744da6d2e686ebc1f72f94a7e328 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 7 Jan 2016 19:26:07 +0000
Subject: [PATCH] review

---
gdb/testsuite/gdb.multi/tids.c | 4 +++
gdb/testsuite/gdb.multi/tids.exp | 10 ++++----
gdb/tid-parse.c | 30 +++++++++++++++++-----
gdb/tid-parse.h | 55 +++++++++++++++++++++++++++-------------
gdb/windows-tdep.c | 23 ++---------------
5 files changed, 72 insertions(+), 50 deletions(-)

diff --git a/gdb/testsuite/gdb.multi/tids.c b/gdb/testsuite/gdb.multi/tids.c
index 00a8298..84f0c56 100644
--- a/gdb/testsuite/gdb.multi/tids.c
+++ b/gdb/testsuite/gdb.multi/tids.c
@@ -25,6 +25,8 @@ thread_function2 (void *arg)
{
while (1)
sleep (1);
+
+ return NULL;
}

void *
@@ -34,6 +36,8 @@ thread_function1 (void *arg)

while (1)
sleep (1);
+
+ return NULL;
}

int
diff --git a/gdb/testsuite/gdb.multi/tids.exp b/gdb/testsuite/gdb.multi/tids.exp
index 7b51c80..33f8144 100644
--- a/gdb/testsuite/gdb.multi/tids.exp
+++ b/gdb/testsuite/gdb.multi/tids.exp
@@ -37,8 +37,8 @@ if { ![runto_main] } then {
return -1
}

-# Issue "thread apply TID_LIST p 1" and expect EXP_TID_LIST (a list of
-# thread ids) to be displayed.
+# Issue "thread apply TID_LIST p 1234" and expect EXP_TID_LIST (a list
+# of thread ids) to be displayed.
proc thread_apply {tid_list exp_tid_list {message ""}} {
global decimal
set any "\[^\r\n\]*"
@@ -72,7 +72,7 @@ proc info_threads {tid_list exp_tid_list {message ""}} {
gdb_test $cmd $r $message
}

-# Issue both "info threads TID_LIST" and expect INFO_THR output. Then
+# Issue "info threads TID_LIST" and expect INFO_THR output. Then
# issue "thread apply TID_LIST" and expect THR_APPLY output. If
# THR_APPLY is omitted, INFO_THR is expected instead.
proc thr_apply_info_thr {tid_list info_thr {thr_apply ""}} {
@@ -122,7 +122,7 @@ with_test_prefix "two inferiors" {
# Add another inferior.
gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"

- # Now that we'd added another inferior, thread IDs now show the
+ # Now that we've added another inferior, thread IDs now show the
# inferior number.
info_threads "" "1.1"

@@ -133,7 +133,7 @@ with_test_prefix "two inferiors" {

runto_main

- # Now that we'd added another inferior, thread IDs now show the
+ # Now that we've added another inferior, thread IDs now show the
# inferior number.
info_threads "" "1.1 2.1" \
"info threads show inferior numbers"
diff --git a/gdb/tid-parse.c b/gdb/tid-parse.c
index 7c30733..500c99d 100644
--- a/gdb/tid-parse.c
+++ b/gdb/tid-parse.c
@@ -159,11 +159,14 @@ tid_range_parser_qualified (struct tid_range_parser *parser)
return parser->qualified;
}

-/* See tid-parse.h. */
-
-int
-tid_range_parser_get_tid_range (struct tid_range_parser *parser, int *inf_num,
- int *thr_start, int *thr_end)
+/* Helper for tid_range_parser_get_tid and
+ tid_range_parser_get_tid_range. Basically like
+ tid_range_parser_get_tid_range, bue if THR_END is NULL returns a
+ single thread ID per call, rather than a range. */
+
+static int
+get_tid_or_range (struct tid_range_parser *parser, int *inf_num,
+ int *thr_start, int *thr_end)
{
if (parser->state == TID_RANGE_STATE_INFERIOR)
{
@@ -233,11 +236,24 @@ tid_range_parser_get_tid_range (struct tid_range_parser *parser, int *inf_num,

/* See tid-parse.h. */

-void
+int
+tid_range_parser_get_tid_range (struct tid_range_parser *parser, int *inf_num,
+ int *thr_start, int *thr_end)
+{
+ gdb_assert (inf_num != NULL && thr_start != NULL && thr_end != NULL);
+
+ get_tid_or_range (parser, inf_num, thr_start, thr_end);
+}
+
+/* See tid-parse.h. */
+
+int
tid_range_parser_get_tid (struct tid_range_parser *parser,
int *inf_num, int *thr_num)
{
- tid_range_parser_get_tid_range (parser, inf_num, thr_num, NULL);
+ gdb_assert (inf_num != NULL && thr_start != NULL);
+
+ return get_tid_range (parser, inf_num, thr_num, NULL);
}

/* See tid-parse.h. */
diff --git a/gdb/tid-parse.h b/gdb/tid-parse.h
index 3c79c08..8f963cd 100644
--- a/gdb/tid-parse.h
+++ b/gdb/tid-parse.h
@@ -85,34 +85,55 @@ extern void tid_range_parser_init (struct tid_range_parser *parser,
/* Parse a thread ID or a thread range list.

A range will be of the form
- <inferior_num>.<thread_number1>-<thread_number1> and will represent
- all the threads of inferior inferior_num with number between
- thread_number1 and thread_number2, inclusive. <inferior_num> can
- also be omitted, as in <thread_number1>-<thread_number1>, in which
- case GDB infers the inferior number from the current inferior.

- While processing a thread ID range list, this function is called
- iteratively; at each call it will return (in the INF_NUM and
- THR_NUM output parameters) the next thread in the range.
+ <inferior_num>.<thread_number1>-<thread_number2>
+
+ and will represent all the threads of inferior INFERIOR_NUM with
+ number between THREAD_NUMBER1 and THREAD_NUMBER2, inclusive.
+ <inferior_num> can also be omitted, as in
+
+ <thread_number1>-<thread_number2>
+
+ in which case GDB infers the inferior number from the current
+ inferior.
+
+ This function is designed to be called iteratively. While
+ processing a thread ID range list, at each call it will return (in
+ the INF_NUM and THR_NUM output parameters) the next thread ID in
+ the range (irrespective of whether the thread actually exists).

At the beginning of parsing a thread range, the char pointer
PARSER->string will be advanced past <thread_number1> and left
pointing at the '-' token. Subsequent calls will not advance the
pointer until the range is completed. The call that completes the
- range will advance the pointer past <thread_number2>. */
-extern void tid_range_parser_get_tid (struct tid_range_parser *parser,
+ range will advance the pointer past <thread_number2>.
+
+ This function advances through the input string for as long you
+ call it. Once the end of the input string is reached, a call to
+ tid_range_parser_finished returns false (see below).
+
+ E.g., with list: "1.2 3.4-6":
+
+ 1st call: *INF_NUM=1; *THR_NUM=2 (finished==0)
+ 2nd call: *INF_NUM=3; *THR_NUM=4 (finished==0)
+ 3rd call: *INF_NUM=3; *THR_NUM=5 (finished==0)
+ 4th call: *INF_NUM=3; *THR_NUM=6 (finished==1)
+
+ Returns true if parsed a thread/range successfully, false
+ otherwise. */
+extern int tid_range_parser_get_tid (struct tid_range_parser *parser,
int *inf_num, int *thr_num);

-/* Like tid_range_parser_get_tid, but return a thread range per call,
- rather then a single thread.
+/* Like tid_range_parser_get_tid, but return a thread ID range per
+ call, rather then a single thread ID.

If the next element in the list is a single thread ID, then
- *THR_START and *THR_END are set to the same value. E.g.:
+ *THR_START and *THR_END are set to the same value.

- list: "1.2 3.4-6"
- 1st call: *INF_NUM=1; *THR_START=2; *THR_END=2
- 2nd call: *INF_NUM=3; *THR_START=4; *THR_END=4
+ E.g.,. with list: "1.2 3.4-6"

+ 1st call: *INF_NUM=1; *THR_START=2; *THR_END=2 (finished==0)
+ 2nd call: *INF_NUM=3; *THR_START=4; *THR_END=6 (finished==1)

Returns true if parsed a thread/range successfully, false
otherwise. */
@@ -135,7 +156,7 @@ extern void tid_range_parser_skip (struct tid_range_parser *parser);
extern int tid_range_parser_qualified (struct tid_range_parser *parser);

/* Accept a string-form list of thread IDs such as is accepted by
- tid_range_parser_get_tid. Return TRUE if the INF_NUM.THR.NUM
+ tid_range_parser_get_tid. Return true if the INF_NUM.THR.NUM
thread is in the list. DEFAULT_INFERIOR is the inferior number to
assume if a non-qualified thread ID is found in the list.

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index dd8085f..cddbf23 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -361,31 +361,12 @@ display_one_tib (ptid_t ptid)
return 1;
}

-/* Display thread information block of a thread specified by ARGS.
- If ARGS is empty, display thread information block of current_thread
- if current_thread is non NULL.
- Otherwise ARGS is parsed and converted to a integer that should
- be the windows ThreadID (not the internal GDB thread ID). */
+/* Display thread information block of the current thread. */

static void
display_tib (char * args, int from_tty)
{
- if (args)
- {
- struct thread_info *tp;
- int gdb_id = value_as_long (parse_and_eval (args));
-
- tp = find_thread_id (gdb_id);
-
- if (!tp)
- error (_("Thread ID %d not known."), gdb_id);
-
- if (!target_thread_alive (tp->ptid))
- error (_("Thread ID %d has terminated."), gdb_id);
-
- display_one_tib (tp->ptid);
- }
- else if (!ptid_equal (inferior_ptid, null_ptid))
+ if (!ptid_equal (inferior_ptid, null_ptid))
display_one_tib (inferior_ptid);
}

--
1.9.3