Re: [RFA] Thread exit messages on MS-Windows

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Fri, 26 Apr 2013 12:46:56 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> This is from the node "Threads" of the manual:
> 
>   `set print thread-events'
>   `set print thread-events on'
>   `set print thread-events off'
>        The `set print thread-events' command allows you to enable or
>        disable printing of messages when GDB notices that new threads have
>        started or that threads have exited.  By default, these messages
>        will be printed if detection of these events is supported by the
>        target.  Note that these messages cannot be disabled on all
>        targets.
> 
> However, debugging MinGW programs on MS-Windows, I see only messages
> about new threads, like this:
> 
>   [New Thread 6184.0x1bbc]
>   [New Thread 6184.0x13c8]
>   [New Thread 6184.0x1a3c]
> 
> I never see any messages about threads that exited, although examining
> the details of the program being debugged, I clearly see that most of
> them did.
> 
> Does that mean that GDB doesn't support thread exit messages on
> Windows?  What feature(s) are missing for this support to be
> available?
> 
> I can get thread exit messages from windows-nat.c such as
> 
>   [Deleting Thread 8112.0x1494]
>   [Deleting Thread 8112.0x11d0]
> 
> if I "set verbose on", but that mode causes GDB to become much more
> talkative than I'd like.
> 
> In thread.c, I see that add_thread_with_info will announce new threads
> if print_thread_events is non-zero, but I see no similar announcement
> in delete_thread or its subroutines.  Is this supposed to be handled
> by target-specific back ends?  I see something like that in, e.g.,
> linux-nat.c and in inf-ttrace.c, but I'm unsure whether that is a
> conclusive evidence.
> 
> If indeed thread deletion should be announced by the target, why this
> asymmetry with thread creation?
> 
> TIA for any help or info.

No one replied, so I'm now converting this into an RFA.  The patch
below causes GDB on Windows to display thread exit messages like this:

  [Thread 5920.0x13e4 exited with code 0]
  [Thread 5920.0x12d0 exited with code 0]
  [Thread 5920.0x1cbc exited with code 0]

OK to commit this (on the trunk)?

2013-04-27  Eli Zaretskii  <eliz@gnu.org>

	* windows-nat.c (windows_delete_thread): Accept an additional
	argument, the thread's exit code, and announce thread death when
	print_thread_events is non-zero and we are deleting a thread that
	is not the main thread.
	(get_windows_debug_event): Pass thread exit code to
	windows_delete_thread.

--- gdb/windows-nat.c~0	2013-02-27 21:42:26.000000000 +0200
+++ gdb/windows-nat.c	2013-04-27 10:46:32.937875000 +0300
@@ -386,7 +386,7 @@ windows_init_thread_list (void)
 
 /* Delete a thread from the list of threads.  */
 static void
-windows_delete_thread (ptid_t ptid)
+windows_delete_thread (ptid_t ptid, DWORD exit_code)
 {
   thread_info *th;
   DWORD id;
@@ -397,6 +397,9 @@ windows_delete_thread (ptid_t ptid)
 
   if (info_verbose)
     printf_unfiltered ("[Deleting %s]\n", target_pid_to_str (ptid));
+  else if (print_thread_events && id != main_thread_id)
+    printf_unfiltered (_("[%s exited with code %d]\n"),
+		       target_pid_to_str (ptid), exit_code);
   delete_thread (ptid);
 
   for (th = &thread_head;
@@ -1496,7 +1499,8 @@ get_windows_debug_event (struct target_o
       if (current_event.dwThreadId != main_thread_id)
 	{
 	  windows_delete_thread (ptid_build (current_event.dwProcessId, 0,
-					   current_event.dwThreadId));
+					     current_event.dwThreadId),
+				 current_event.u.ExitThread.dwExitCode);
 	  th = &dummy_thread_info;
 	}
       break;
@@ -1513,7 +1517,7 @@ get_windows_debug_event (struct target_o
       current_process_handle = current_event.u.CreateProcessInfo.hProcess;
       if (main_thread_id)
 	windows_delete_thread (ptid_build (current_event.dwProcessId, 0,
-					   main_thread_id));
+					   main_thread_id), 0);
       main_thread_id = current_event.dwThreadId;
       /* Add the main thread.  */
       th = windows_add_thread (ptid_build (current_event.dwProcessId, 0,

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

One minor correction:

> +  else if (print_thread_events && id != main_thread_id)
> +    printf_unfiltered (_("[%s exited with code %d]\n"),
                                                  ^^
This should be %lu, not %d.

Re: [RFA] Thread exit messages on MS-Windows

[Corinna Vinschen]

On Apr 27 12:00, Eli Zaretskii wrote:
> One minor correction:
> 
> > +  else if (print_thread_events && id != main_thread_id)
> > +    printf_unfiltered (_("[%s exited with code %d]\n"),
>                                                   ^^
> This should be %lu, not %d.

If this code is generic and runs under Cygwin as well, then please make
that %u, not %lu.  DWORD is a 4 byte value and under 64 bit Cygwin
(LP64), it's not the same as long, but as int.  Alternatively, add an
explicit cast to unsigned long to the argument.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Sat, 27 Apr 2013 20:10:40 +0200
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> On Apr 27 12:00, Eli Zaretskii wrote:
> > One minor correction:
> > 
> > > +  else if (print_thread_events && id != main_thread_id)
> > > +    printf_unfiltered (_("[%s exited with code %d]\n"),
> >                                                   ^^
> > This should be %lu, not %d.
> 
> If this code is generic and runs under Cygwin as well, then please make
> that %u, not %lu.  DWORD is a 4 byte value and under 64 bit Cygwin
> (LP64), it's not the same as long, but as int.  Alternatively, add an
> explicit cast to unsigned long to the argument.

Thanks, will do (the former, probably).

Re: [RFA] Thread exit messages on MS-Windows

[asmwarrior]

On 2013-4-27 15:58, Eli Zaretskii wrote:
>> Date: Fri, 26 Apr 2013 12:46:56 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>> This is from the node "Threads" of the manual:
>>
>>   `set print thread-events'
>>   `set print thread-events on'
>>   `set print thread-events off'
>>        The `set print thread-events' command allows you to enable or
>>        disable printing of messages when GDB notices that new threads have
>>        started or that threads have exited.  By default, these messages
>>        will be printed if detection of these events is supported by the
>>        target.  Note that these messages cannot be disabled on all
>>        targets.
>>
>> However, debugging MinGW programs on MS-Windows, I see only messages
>> about new threads, like this:
>>
>>   [New Thread 6184.0x1bbc]
>>   [New Thread 6184.0x13c8]
>>   [New Thread 6184.0x1a3c]
>>
>> I never see any messages about threads that exited, although examining
>> the details of the program being debugged, I clearly see that most of
>> them did.
>>
>> Does that mean that GDB doesn't support thread exit messages on
>> Windows?  What feature(s) are missing for this support to be
>> available?
>>
>> I can get thread exit messages from windows-nat.c such as
>>
>>   [Deleting Thread 8112.0x1494]
>>   [Deleting Thread 8112.0x11d0]
>>
>> if I "set verbose on", but that mode causes GDB to become much more
>> talkative than I'd like.
>>
>> In thread.c, I see that add_thread_with_info will announce new threads
>> if print_thread_events is non-zero, but I see no similar announcement
>> in delete_thread or its subroutines.  Is this supposed to be handled
>> by target-specific back ends?  I see something like that in, e.g.,
>> linux-nat.c and in inf-ttrace.c, but I'm unsure whether that is a
>> conclusive evidence.
>>
>> If indeed thread deletion should be announced by the target, why this
>> asymmetry with thread creation?
>>
>> TIA for any help or info.
> 
> No one replied, so I'm now converting this into an RFA.  The patch
> below causes GDB on Windows to display thread exit messages like this:
> 
>   [Thread 5920.0x13e4 exited with code 0]
>   [Thread 5920.0x12d0 exited with code 0]
>   [Thread 5920.0x1cbc exited with code 0]
> 
> OK to commit this (on the trunk)?
> 
I just applied your patch, and the result GDB works fine. Great work!

Yuanhui Zhang

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Sat, 27 Apr 2013 17:58:36 +0800
> From: asmwarrior <asmwarrior@gmail.com>
> CC: gdb-patches@sourceware.org
> 
> > No one replied, so I'm now converting this into an RFA.  The patch
> > below causes GDB on Windows to display thread exit messages like this:
> > 
> >   [Thread 5920.0x13e4 exited with code 0]
> >   [Thread 5920.0x12d0 exited with code 0]
> >   [Thread 5920.0x1cbc exited with code 0]
> > 
> > OK to commit this (on the trunk)?
> > 
> I just applied your patch, and the result GDB works fine. Great work!

Thanks for testing.

Re: [RFA] Thread exit messages on MS-Windows

[asmwarrior]

On 2013-4-27 15:58, Eli Zaretskii wrote:
>> Date: Fri, 26 Apr 2013 12:46:56 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>> This is from the node "Threads" of the manual:
>>
>>   `set print thread-events'
>>   `set print thread-events on'
>>   `set print thread-events off'
>>        The `set print thread-events' command allows you to enable or
>>        disable printing of messages when GDB notices that new threads have
>>        started or that threads have exited.  By default, these messages
>>        will be printed if detection of these events is supported by the
>>        target.  Note that these messages cannot be disabled on all
>>        targets.
>>
>> However, debugging MinGW programs on MS-Windows, I see only messages
>> about new threads, like this:
I'm not sure it is the correct behavior under Windows.
When debugging a single thread app, I do receive one New Thread message (for the main thread of the inferior), but there is not corresponding exit thread message report for this main thread. For multiply thread apps, the first New Thread messages has no corresponding exit messages, but other threads have both new and their corresponding exit messages.

Yuanhui Zhang

BTW: 
Corresponding messages have the same thread ID like:
[New Thread 2628.0xa4c]
[Thread 2628.0xa4c exited with code 0]

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Sat, 27 Apr 2013 21:56:06 +0800
> From: asmwarrior <asmwarrior@gmail.com>
> CC: gdb-patches@sourceware.org
> 
> I'm not sure it is the correct behavior under Windows.
> When debugging a single thread app, I do receive one New Thread message (for the main thread of the inferior), but there is not corresponding exit thread message report for this main thread. For multiply thread apps, the first New Thread messages has no corresponding exit messages, but other threads have both new and their corresponding exit messages.

This is on purpose: I think announcing the death of the main thread is
redundant, because it is always followed by this:

  [Inferior 1 (process 4216) exited normally]

Re: [RFA] Thread exit messages on MS-Windows

[Joel Brobecker]

> No one replied, so I'm now converting this into an RFA.  The patch
> below causes GDB on Windows to display thread exit messages like this:
> 
>   [Thread 5920.0x13e4 exited with code 0]
>   [Thread 5920.0x12d0 exited with code 0]
>   [Thread 5920.0x1cbc exited with code 0]

> 2013-04-27  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* windows-nat.c (windows_delete_thread): Accept an additional
> 	argument, the thread's exit code, and announce thread death when
> 	print_thread_events is non-zero and we are deleting a thread that
> 	is not the main thread.
> 	(get_windows_debug_event): Pass thread exit code to
> 	windows_delete_thread.

Looks good to me, modulo the comments already made. It's a little
unusual to see an exit code for a thread, but it could be useful
information, and it does not unnecessarily clutter the output.

> @@ -1513,7 +1517,7 @@ get_windows_debug_event (struct target_o
>        current_process_handle = current_event.u.CreateProcessInfo.hProcess;
>        if (main_thread_id)
>  	windows_delete_thread (ptid_build (current_event.dwProcessId, 0,
> -					   main_thread_id));
> +					   main_thread_id), 0);

One tiny nitpick, very possibly influenced by personal preferences,
so feel free to ignore...  I think that the code would be faster
to read if the added parameter was moved to the next line. That way,
all parameters in call to windows_delete_thread would have the same
indentation level.

-- 
Joel

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Mon, 29 Apr 2013 14:21:00 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > No one replied, so I'm now converting this into an RFA.  The patch
> > below causes GDB on Windows to display thread exit messages like this:
> > 
> >   [Thread 5920.0x13e4 exited with code 0]
> >   [Thread 5920.0x12d0 exited with code 0]
> >   [Thread 5920.0x1cbc exited with code 0]
> 
> > 2013-04-27  Eli Zaretskii  <eliz@gnu.org>
> > 
> > 	* windows-nat.c (windows_delete_thread): Accept an additional
> > 	argument, the thread's exit code, and announce thread death when
> > 	print_thread_events is non-zero and we are deleting a thread that
> > 	is not the main thread.
> > 	(get_windows_debug_event): Pass thread exit code to
> > 	windows_delete_thread.
> 
> Looks good to me, modulo the comments already made.

I know what to do with Corinna's comment, but not what to decide about
announcing the death of the main thread.  Do you have an opinion?

> It's a little unusual to see an exit code for a thread, but it could
> be useful information, and it does not unnecessarily clutter the
> output.

My reading of the code is that you already see that in linux-native
debugging, see linux-nat.c.

> > @@ -1513,7 +1517,7 @@ get_windows_debug_event (struct target_o
> >        current_process_handle = current_event.u.CreateProcessInfo.hProcess;
> >        if (main_thread_id)
> >  	windows_delete_thread (ptid_build (current_event.dwProcessId, 0,
> > -					   main_thread_id));
> > +					   main_thread_id), 0);
> 
> One tiny nitpick, very possibly influenced by personal preferences,
> so feel free to ignore...  I think that the code would be faster
> to read if the added parameter was moved to the next line. That way,
> all parameters in call to windows_delete_thread would have the same
> indentation level.

I'm surprised, but I don't mind, and will do that, too.

Re: [RFA] Thread exit messages on MS-Windows

[Joel Brobecker]

> I know what to do with Corinna's comment, but not what to decide about
> announcing the death of the main thread.  Do you have an opinion?

No strong opinion, but I share your preference.

> My reading of the code is that you already see that in linux-native
> debugging, see linux-nat.c.

'must have missed it... sorry :).

> > One tiny nitpick, very possibly influenced by personal preferences,
> > so feel free to ignore...  I think that the code would be faster
> > to read if the added parameter was moved to the next line. That way,
> > all parameters in call to windows_delete_thread would have the same
> > indentation level.
> 
> I'm surprised, but I don't mind, and will do that, too.

Then don't. It's such a small comment that it's not worth changing.

-- 
Joel

Re: [RFA] Thread exit messages on MS-Windows

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> In thread.c, I see that add_thread_with_info will announce new threads
>> if print_thread_events is non-zero, but I see no similar announcement
>> in delete_thread or its subroutines.  Is this supposed to be handled
>> by target-specific back ends?  I see something like that in, e.g.,
>> linux-nat.c and in inf-ttrace.c, but I'm unsure whether that is a
>> conclusive evidence.
>> 
>> If indeed thread deletion should be announced by the target, why this
>> asymmetry with thread creation?

Eli> No one replied, so I'm now converting this into an RFA.  The patch
Eli> below causes GDB on Windows to display thread exit messages like this:

I didn't answer, but I also wonder why it is not done by delete_thread.

Tom

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 29 Apr 2013 08:37:06 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> In thread.c, I see that add_thread_with_info will announce new threads
> >> if print_thread_events is non-zero, but I see no similar announcement
> >> in delete_thread or its subroutines.  Is this supposed to be handled
> >> by target-specific back ends?  I see something like that in, e.g.,
> >> linux-nat.c and in inf-ttrace.c, but I'm unsure whether that is a
> >> conclusive evidence.
> >> 
> >> If indeed thread deletion should be announced by the target, why this
> >> asymmetry with thread creation?
> 
> Eli> No one replied, so I'm now converting this into an RFA.  The patch
> Eli> below causes GDB on Windows to display thread exit messages like this:
> 
> I didn't answer, but I also wonder why it is not done by delete_thread.

If we decide to do this in delete_thread, then we will have to remove
the announcements in several target-specific files which do that, like
linux-nat.c.  Is that OK?

Re: [RFA] Thread exit messages on MS-Windows

[Joel Brobecker]

> If we decide to do this in delete_thread, then we will have to remove
> the announcements in several target-specific files which do that, like
> linux-nat.c.  Is that OK?

I would like us to go that route also, but let's do that separately?
When I looked at it, the announcement was almost always printed right
before calling delete_thread, but not always (on some platforms, the
announcement is sent while the thread is marked as "dying", and the
deletion is done later on). Not sure if moving the annoucement later
would make any difference in practice.

-- 
Joel

Re: [RFA] Thread exit messages on MS-Windows

[Joel Brobecker]

> > > If we decide to do this in delete_thread, then we will have to remove
> > > the announcements in several target-specific files which do that, like
> > > linux-nat.c.  Is that OK?
> > 
> > I would like us to go that route also, but let's do that separately?
> 
> So you are saying I should commit my proposed patch now, and deal with
> the platform-agnostic change later?  I'm OK with that.

That's correct. Your change is already a worthwhile improvement,
and won't impair the more general adjustment for later.

-- 
Joel

Re: [RFA] Thread exit messages on MS-Windows

[Eli Zaretskii]

> Date: Wed, 1 May 2013 09:14:32 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> > > > If we decide to do this in delete_thread, then we will have to remove
> > > > the announcements in several target-specific files which do that, like
> > > > linux-nat.c.  Is that OK?
> > > 
> > > I would like us to go that route also, but let's do that separately?
> > 
> > So you are saying I should commit my proposed patch now, and deal with
> > the platform-agnostic change later?  I'm OK with that.
> 
> That's correct. Your change is already a worthwhile improvement,
> and won't impair the more general adjustment for later.

Done.