[PATCHv2,1/6] gdb/remote: announce thread exit events for remote targets

Message ID d6d49338119d2a32e060f95219aacac31bdf4886.1674207665.git.aburgess@redhat.com
State New
Headers
Series Inferior specific breakpoints |

Commit Message

Andrew Burgess Jan. 20, 2023, 9:46 a.m. UTC
  For some reason the "[Thread XXXX exited]" messages are not printed
inside thread.c from functions like delete_thread, etc as might be
expected.  Instead, each target seems to print the message before
calling delete_thread.

This doesn't seem ideal, and I can't help but feel that the printing
should be moved into thread.c, however, I have not tried to do that in
this commit, as I suspect there will be lots of fallout that needs
fixing up.

Instead, in this commit, I have added the printing code into remote.c,
so that the remote target will now correctly tell the user when a
thread has exited.

This fixes some test failures in gdb.threads/thread-specific-bp.exp
when run with the native-gdbserver and native-extended-gdbserver
board.

When using the native-extended-gdbserver board I still see 1 test
failure, but I think this is not related to the issue fixed in this
commit, so I'm ignoring that for now.
---
 gdb/remote.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Pedro Alves Feb. 2, 2023, 5:50 p.m. UTC | #1
On 2023-01-20 9:46 a.m., Andrew Burgess via Gdb-patches wrote:
> For some reason the "[Thread XXXX exited]" messages are not printed
> inside thread.c from functions like delete_thread, etc as might be
> expected.  Instead, each target seems to print the message before
> calling delete_thread.
> 
> This doesn't seem ideal, and I can't help but feel that the printing
> should be moved into thread.c, however, I have not tried to do that in
> this commit, as I suspect there will be lots of fallout that needs
> fixing up.

I had done exactly that in the step over clone/exit series:

  Centralize "[Thread ...exited]" notifications
  https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/T/#m9f9858077e6b2b817148bfb9c32ec15bec724931

Pedro Alves

> 
> Instead, in this commit, I have added the printing code into remote.c,
> so that the remote target will now correctly tell the user when a
> thread has exited.
> 
> This fixes some test failures in gdb.threads/thread-specific-bp.exp
> when run with the native-gdbserver and native-extended-gdbserver
> board.
> 
> When using the native-extended-gdbserver board I still see 1 test
> failure, but I think this is not related to the issue fixed in this
> commit, so I'm ignoring that for now.
> ---
>  gdb/remote.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 0a6e293c095..4a508981a96 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -3977,6 +3977,10 @@ remote_target::update_thread_list ()
>  	      if (has_single_non_exited_thread (tp->inf))
>  		continue;
>  
> +	      if (print_thread_events)
> +		gdb_printf (_("[%s exited]\n"),
> +			    target_pid_to_str (tp->ptid).c_str ());
> +
>  	      /* Not found.  */
>  	      delete_thread (tp);
>  	    }
>
  
Andrew Burgess Feb. 4, 2023, 3:46 p.m. UTC | #2
Pedro Alves <pedro@palves.net> writes:

> On 2023-01-20 9:46 a.m., Andrew Burgess via Gdb-patches wrote:
>> For some reason the "[Thread XXXX exited]" messages are not printed
>> inside thread.c from functions like delete_thread, etc as might be
>> expected.  Instead, each target seems to print the message before
>> calling delete_thread.
>> 
>> This doesn't seem ideal, and I can't help but feel that the printing
>> should be moved into thread.c, however, I have not tried to do that in
>> this commit, as I suspect there will be lots of fallout that needs
>> fixing up.
>
> I had done exactly that in the step over clone/exit series:
>
>   Centralize "[Thread ...exited]" notifications
>   https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/T/#m9f9858077e6b2b817148bfb9c32ec15bec724931
>

OK.  I guess I'll try to move this series forward without this patch for
now, and wait for your work to land, hopefully we can get your series in
soon.

Thanks,
Andrew


> Pedro Alves
>
>> 
>> Instead, in this commit, I have added the printing code into remote.c,
>> so that the remote target will now correctly tell the user when a
>> thread has exited.
>> 
>> This fixes some test failures in gdb.threads/thread-specific-bp.exp
>> when run with the native-gdbserver and native-extended-gdbserver
>> board.
>> 
>> When using the native-extended-gdbserver board I still see 1 test
>> failure, but I think this is not related to the issue fixed in this
>> commit, so I'm ignoring that for now.
>> ---
>>  gdb/remote.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index 0a6e293c095..4a508981a96 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -3977,6 +3977,10 @@ remote_target::update_thread_list ()
>>  	      if (has_single_non_exited_thread (tp->inf))
>>  		continue;
>>  
>> +	      if (print_thread_events)
>> +		gdb_printf (_("[%s exited]\n"),
>> +			    target_pid_to_str (tp->ptid).c_str ());
>> +
>>  	      /* Not found.  */
>>  	      delete_thread (tp);
>>  	    }
>>
  

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 0a6e293c095..4a508981a96 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3977,6 +3977,10 @@  remote_target::update_thread_list ()
 	      if (has_single_non_exited_thread (tp->inf))
 		continue;
 
+	      if (print_thread_events)
+		gdb_printf (_("[%s exited]\n"),
+			    target_pid_to_str (tp->ptid).c_str ());
+
 	      /* Not found.  */
 	      delete_thread (tp);
 	    }