Re: [RFA/commit] GDB crash re-running program on Windows (native)

Simon Marchi
2019-01-03T03:48:37Z

On 2019-01-02 22:33, Joel Brobecker wrote:
> Hi Simon,
>
> Thanks for the review!
>
>> > With that in mind, this commit fixes the issue by deleting the thread
>> > when the inferior sends the exit-process event as opposed to deleting it
>> > later, while starting a new inferior.
>> >
>> > This restores also restores the printing of the thread-exit notification
>>
>> "This restores also restores"
>>
>> > for the main thread, which was missing before. Looking at the transcript
>> > of the example show above, we can see 4 thread creation notifications,
>> > and only 3 notifications for thread exits. Now creation and exit
>> > notifications are balanced.
>>
>> Another choice is to not show the main thread's creation and exit (as
>> is
>> done on Linux), since it's kind of redundant with the process creation
>> and
>> exit.
>
> Indeed, that's a good idea. I propose to work on that as a followup
> patch, as I am a little short on time these next couple of weeks
> (broken hand :-().
>
>> > In the handling of EXIT_THREAD_DEBUG_EVENT, the main_thread_id
>> > check is removed because deemed unnecessary: The main thread was
>> > introduced by a CREATE_THREAD_DEBUG_EVENT, and thus the kernel
>> > is expected to report its death via EXIT_PROCESS_DEBUG_EVENT.
>>
>> Should that last EXIT_PROCESS_DEBUG_EVENT actually be
>> EXIT_THREAD_DEBUG_EVENT?
>
> Not quite, but actually a good question nonetheless. There was
> an auto-completion error in the text above. New text:
>
> | In the handling of EXIT_THREAD_DEBUG_EVENT, the main_thread_id
> | check is removed because deemed unnecessary: The main thread was
> | introduced by a CREATE_PROCESS_DEBUG_EVENT, and thus the kernel
> | is expected to report its death via EXIT_PROCESS_DEBUG_EVENT.
>
> In other words, we don't expect to receive an EXIT_THREAD_DEBUG_EVENT
> for the main thread, because, at the Windows level, that thread really
> isn't a thread, but really a process.
>
>
>> > @@ -1607,6 +1599,9 @@ get_windows_debug_event (struct target_ops *ops,
>> > }
>> > else if (saw_create == 1)
>> > {
>> > + windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
>> > + main_thread_id),
>> > + 0);
>> > ourstatus->kind = TARGET_WAITKIND_EXITED;
>> > ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
>> > thread_id = main_thread_id;
>>
>> If what you said above is right (that the kernel reports the main
>> thread's
>> death through an EXIT_THREAD_DEBUG_EVENT), why is this new call to
>> windows_delete_thread needed? Shouldn't it already be deleted at this
>> point?
>
> Does the answer above allow us to make sense of this hunk?

Yes, thanks for the explanation, everything LGTM.

Simon