[2/2] Mention gdb thread ID in new thread message

Message ID 20250112-announce-thread-pr-19584-v1-2-2260ceda33d4@tromey.com
State New
Headers
Series Show 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

Tom Tromey Jan. 12, 2025, 1:42 p.m. UTC
  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

Stephan Rohr Jan. 13, 2025, 8:44 a.m. UTC | #1
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
  

Patch

diff --git a/gdb/testsuite/gdb.threads/inf-thr-count.exp b/gdb/testsuite/gdb.threads/inf-thr-count.exp
index 1c85c95538443132a383617847a9041b1827efa2..389c7fc7000ecdefe2b55674b5fcbba006a6f349 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..8520f1edf60c68fc32f4ebd97b7238b384e14544 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..e89e31e149cf93cee8cfb25e8d38e6d2eb2d7f73 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..00871e3f51d0ee2357bf5a7827b836044e00a465 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..6fcb05a660f9a5fe4f63ff278aedb96280f3cc1c 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..a726934ee8ee2b5e5527ed0e3893c9801b51a704 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..785964967504b60af467abf8d2c533979e9f5436 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;