[2/2] Mention gdb thread ID in new thread message
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
fail
|
Test failed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
fail
|
Test failed
|
Commit Message
Currently the "new thread" message that gdb might print does not
include gdb's own thread ID -- so, if you want to reference the
thread, you must first use "thread find" or "info threads".
This patch changes the announcement to mention the thread. It's
mildly ugly because the thread-to-string code adds a redundant
"Thread" string.
I wasn't sure whether this code should use print_full_thread_id or
just print_thread_id. I went with the former, on the theory that if
new inferior is started as well, it's handy to look back and see the
full ID; but this is easily changed if desirable.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19584
---
gdb/testsuite/gdb.threads/inf-thr-count.exp | 2 +-
gdb/testsuite/gdb.threads/infcall-thread-announce.exp | 4 ++--
gdb/testsuite/gdb.threads/multiple-successive-infcall.exp | 4 ++--
gdb/testsuite/gdb.threads/stepi-over-clone.exp | 2 +-
gdb/testsuite/gdb.threads/thread-specific.exp | 2 +-
gdb/testsuite/gdb.threads/thread_events.exp | 2 +-
gdb/thread.c | 4 +++-
7 files changed, 11 insertions(+), 9 deletions(-)
Comments
Hi Tom,
I like the idea of adding the additional thread-id to the 'New thread created'
message. I prefer 'print_full_thread_id', it is more helpful in case of multiple
inferiors.
I wonder if we should extend the 'thread exited' message, too? For example:
[New thread 1.2: Thread 0x155541e4d640 (LWP 1359403)]
[New thread 1.3: Thread 0x155541c4c640 (LWP 1359404)]
[Thread 0x155541e4d640 (LWP 1359403) exited]
[Thread 0x155541c4c640 (LWP 1359404) exited]
This would be more consistent in my point of view, e.g.,
[New thread 1.2: Thread 0x155541e4d640 (LWP 1359403)]
[Thread 1.2: Thread 0x155541e4d640 (LWP 1359403) exited]
I agree that printing the 'Thread' string twice is a bit ugly. I don't have an idea
how this can be improved, though.
Stephan
> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Sunday, 12 January 2025 14:43
> To: gdb-patches@sourceware.org
> Cc: Tom Tromey <tom@tromey.com>
> Subject: [PATCH 2/2] Mention gdb thread ID in new thread message
>
> Currently the "new thread" message that gdb might print does not
> include gdb's own thread ID -- so, if you want to reference the
> thread, you must first use "thread find" or "info threads".
>
> This patch changes the announcement to mention the thread. It's
> mildly ugly because the thread-to-string code adds a redundant
> "Thread" string.
>
> I wasn't sure whether this code should use print_full_thread_id or
> just print_thread_id. I went with the former, on the theory that if
> new inferior is started as well, it's handy to look back and see the
> full ID; but this is easily changed if desirable.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19584
> ---
> gdb/testsuite/gdb.threads/inf-thr-count.exp | 2 +-
> gdb/testsuite/gdb.threads/infcall-thread-announce.exp | 4 ++--
> gdb/testsuite/gdb.threads/multiple-successive-infcall.exp | 4 ++--
> gdb/testsuite/gdb.threads/stepi-over-clone.exp | 2 +-
> gdb/testsuite/gdb.threads/thread-specific.exp | 2 +-
> gdb/testsuite/gdb.threads/thread_events.exp | 2 +-
> gdb/thread.c | 4 +++-
> 7 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.threads/inf-thr-count.exp
> b/gdb/testsuite/gdb.threads/inf-thr-count.exp
> index
> 1c85c95538443132a383617847a9041b1827efa2..389c7fc7000ecdefe2b55
> 674b5fcbba006a6f349 100644
> --- a/gdb/testsuite/gdb.threads/inf-thr-count.exp
> +++ b/gdb/testsuite/gdb.threads/inf-thr-count.exp
> @@ -131,7 +131,7 @@ if {![wait_for_stage 1]} {
> if {[target_info exists gdb_protocol]
> && ([target_info gdb_protocol] == "remote"
> || [target_info gdb_protocol] == "extended-remote")} {
> - set new_thread_re "\\\[New Thread \[^\r\n\]+\\\]\r\n"
> + set new_thread_re "\\\[New thread $decimal\\\.$decimal: Thread
> \[^\r\n\]+\\\]\r\n"
> set exit_thread_re "\\\[Thread \[^\r\n\]+ exited\\\]\r\n"
> } else {
> set new_thread_re ""
> diff --git a/gdb/testsuite/gdb.threads/infcall-thread-announce.exp
> b/gdb/testsuite/gdb.threads/infcall-thread-announce.exp
> index
> e22245ea8e4cd17ffea99624231ff650f9fe5cbe..8520f1edf60c68fc32f4ebd9
> 7b7238b384e14544 100644
> --- a/gdb/testsuite/gdb.threads/infcall-thread-announce.exp
> +++ b/gdb/testsuite/gdb.threads/infcall-thread-announce.exp
> @@ -41,13 +41,13 @@ proc check_thread_count { adjustment } {
>
> with_test_prefix "starting threads" {
> gdb_test "call (void) start_thread()" \
> - "\\\[New Thread \[^\r\n\]+\\\]" \
> + "\\\[New thread $decimal\\\.$decimal: Thread \[^\r\n\]+\\\]" \
> "start a new thread, return value discarded"
> check_thread_count +1
>
> foreach_with_prefix call_type { print call } {
> gdb_test "$call_type start_thread()" \
> - "\\\[New Thread \[^\r\n\]+\\\]\r\n\\\$$decimal = 1" \
> + "\\\[New thread $decimal\\\.$decimal: Thread
> \[^\r\n\]+\\\]\r\n\\\$$decimal = 1" \
> "start another new thread"
> check_thread_count +1
> }
> diff --git a/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> b/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> index
> 4a97179a5b43429051da8957879ada90bd98986b..e89e31e149cf93cee8c
> fb25e8d38e6d2eb2d7f73 100644
> --- a/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> +++ b/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> @@ -37,10 +37,10 @@ gdb_continue_to_breakpoint
> "prethreadcreationmarker"
> set after_new_thread_message "created new thread"
> foreach_with_prefix thread {5 4 3} {
> gdb_test_multiple "continue" "${after_new_thread_message}" {
> - -re "\\\[New Thread ${hex} \\\(LWP \[0-9\]+\\\)\\\].*${gdb_prompt}" {
> + -re "\\\[New thread $decimal\\\.$decimal: Thread ${hex} \\\(LWP \[0-
> 9\]+\\\)\\\].*${gdb_prompt}" {
> pass "${after_new_thread_message}"
> }
> - -re -wrap "\\\[New Thread $decimal\\.$decimal\\\]\r\n.*" {
> + -re -wrap "\\\[New thread $decimal\\\.$decimal: Thread
> $decimal\\.$decimal\\\]\r\n.*" {
> pass $gdb_test_name
> }
> }
> diff --git a/gdb/testsuite/gdb.threads/stepi-over-clone.exp
> b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
> index
> b93cfe69c7f4c9c4426796c481c8d2ef65c8199c..00871e3f51d0ee2357bf5a
> 7827b836044e00a465 100644
> --- a/gdb/testsuite/gdb.threads/stepi-over-clone.exp
> +++ b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
> @@ -168,7 +168,7 @@ proc test {non_stop displaced third_thread} {
> verbose -log "XXX: Consume the initial command"
> exp_continue
> }
> - -re "^\\\[New Thread\[^\r\n\]+\\\]\r\n" {
> + -re "^\\\[New thread $decimal\\\.$decimal:
> Thread\[^\r\n\]+\\\]\r\n" {
> verbose -log "XXX: Consume new thread line"
> incr stepi_new_thread_count
> exp_continue
> diff --git a/gdb/testsuite/gdb.threads/thread-specific.exp
> b/gdb/testsuite/gdb.threads/thread-specific.exp
> index
> 26bba8b9961c672df0c180ae38d32bc61285c982..6fcb05a660f9a5fe4f63ff
> 278aedb96280f3cc1c 100644
> --- a/gdb/testsuite/gdb.threads/thread-specific.exp
> +++ b/gdb/testsuite/gdb.threads/thread-specific.exp
> @@ -36,7 +36,7 @@ proc get_thread_list { } {
> -re "info threads\r\n" {
> exp_continue
> }
> - -re "New Thread \[^\n\]*\n" {
> + -re "New thread \[^\n\]*\n" {
> exp_continue
> }
> -re "^ *Id *Target Id\[^\n\]*\n" {
> diff --git a/gdb/testsuite/gdb.threads/thread_events.exp
> b/gdb/testsuite/gdb.threads/thread_events.exp
> index
> ca51abeb545939ff4c17e9f4856f16c2a4d70a44..a726934ee8ee2b5e5527e
> d0e3893c9801b51a704 100644
> --- a/gdb/testsuite/gdb.threads/thread_events.exp
> +++ b/gdb/testsuite/gdb.threads/thread_events.exp
> @@ -50,7 +50,7 @@ proc gdb_test_thread_start {messages_enabled
> command pattern message} {
> set events_seen 0
>
> return [gdb_test_multiple $command $message {
> - -re "\\\[New Thread \[^\]\]*\\\]\r\n" {
> + -re "\\\[New thread $decimal\\\.$decimal: Thread \[^\]\]*\\\]\r\n" {
> incr events_seen
> exp_continue
> }
> diff --git a/gdb/thread.c b/gdb/thread.c
> index
> ef8a495a5a52418412fd0624f598c03d715afb5a..785964967504b60af467a
> bf8d2c533979e9f5436 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -330,7 +330,9 @@ add_thread_with_info (process_stratum_target
> *targ, ptid_t ptid,
> result->priv = std::move (priv);
>
> if (print_thread_events)
> - gdb_printf (_("[New %s]\n"), target_pid_to_str (ptid).c_str ());
> + gdb_printf (_("[New thread %s: %s]\n"),
> + print_full_thread_id (result),
> + target_pid_to_str (ptid).c_str ());
>
> annotate_new_thread ();
> return result;
>
> --
> 2.46.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
@@ -131,7 +131,7 @@ if {![wait_for_stage 1]} {
if {[target_info exists gdb_protocol]
&& ([target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote")} {
- set new_thread_re "\\\[New Thread \[^\r\n\]+\\\]\r\n"
+ set new_thread_re "\\\[New thread $decimal\\\.$decimal: Thread \[^\r\n\]+\\\]\r\n"
set exit_thread_re "\\\[Thread \[^\r\n\]+ exited\\\]\r\n"
} else {
set new_thread_re ""
@@ -41,13 +41,13 @@ proc check_thread_count { adjustment } {
with_test_prefix "starting threads" {
gdb_test "call (void) start_thread()" \
- "\\\[New Thread \[^\r\n\]+\\\]" \
+ "\\\[New thread $decimal\\\.$decimal: Thread \[^\r\n\]+\\\]" \
"start a new thread, return value discarded"
check_thread_count +1
foreach_with_prefix call_type { print call } {
gdb_test "$call_type start_thread()" \
- "\\\[New Thread \[^\r\n\]+\\\]\r\n\\\$$decimal = 1" \
+ "\\\[New thread $decimal\\\.$decimal: Thread \[^\r\n\]+\\\]\r\n\\\$$decimal = 1" \
"start another new thread"
check_thread_count +1
}
@@ -37,10 +37,10 @@ gdb_continue_to_breakpoint "prethreadcreationmarker"
set after_new_thread_message "created new thread"
foreach_with_prefix thread {5 4 3} {
gdb_test_multiple "continue" "${after_new_thread_message}" {
- -re "\\\[New Thread ${hex} \\\(LWP \[0-9\]+\\\)\\\].*${gdb_prompt}" {
+ -re "\\\[New thread $decimal\\\.$decimal: Thread ${hex} \\\(LWP \[0-9\]+\\\)\\\].*${gdb_prompt}" {
pass "${after_new_thread_message}"
}
- -re -wrap "\\\[New Thread $decimal\\.$decimal\\\]\r\n.*" {
+ -re -wrap "\\\[New thread $decimal\\\.$decimal: Thread $decimal\\.$decimal\\\]\r\n.*" {
pass $gdb_test_name
}
}
@@ -168,7 +168,7 @@ proc test {non_stop displaced third_thread} {
verbose -log "XXX: Consume the initial command"
exp_continue
}
- -re "^\\\[New Thread\[^\r\n\]+\\\]\r\n" {
+ -re "^\\\[New thread $decimal\\\.$decimal: Thread\[^\r\n\]+\\\]\r\n" {
verbose -log "XXX: Consume new thread line"
incr stepi_new_thread_count
exp_continue
@@ -36,7 +36,7 @@ proc get_thread_list { } {
-re "info threads\r\n" {
exp_continue
}
- -re "New Thread \[^\n\]*\n" {
+ -re "New thread \[^\n\]*\n" {
exp_continue
}
-re "^ *Id *Target Id\[^\n\]*\n" {
@@ -50,7 +50,7 @@ proc gdb_test_thread_start {messages_enabled command pattern message} {
set events_seen 0
return [gdb_test_multiple $command $message {
- -re "\\\[New Thread \[^\]\]*\\\]\r\n" {
+ -re "\\\[New thread $decimal\\\.$decimal: Thread \[^\]\]*\\\]\r\n" {
incr events_seen
exp_continue
}
@@ -330,7 +330,9 @@ add_thread_with_info (process_stratum_target *targ, ptid_t ptid,
result->priv = std::move (priv);
if (print_thread_events)
- gdb_printf (_("[New %s]\n"), target_pid_to_str (ptid).c_str ());
+ gdb_printf (_("[New thread %s: %s]\n"),
+ print_full_thread_id (result),
+ target_pid_to_str (ptid).c_str ());
annotate_new_thread ();
return result;