Fix kill of processes created by win32_create_inferior

Message ID 20200208182539.5775-1-ssbssa@yahoo.de
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Feb. 8, 2020, 6:25 p.m. UTC
  handle_v_kill uses signal_pid because win32 doesn't support multi-process.

gdbserver/ChangeLog:

2020-02-08  Hannes Domani  <ssbssa@yahoo.de>

	* win32-low.c (win32_create_inferior): Set signal_pid.
---
 gdbserver/win32-low.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Luis Machado Feb. 10, 2020, 10:33 p.m. UTC | #1
On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
> 
> gdbserver/ChangeLog:
> 
> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* win32-low.c (win32_create_inferior): Set signal_pid.
> ---
>   gdbserver/win32-low.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
> index 9d0343788f..557c90d97c 100644
> --- a/gdbserver/win32-low.c
> +++ b/gdbserver/win32-low.c
> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>        (assuming success).  */
>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>   
> +  /* Necessary for handle_v_kill.  */
> +  signal_pid = current_process_id;
> +
>     return current_process_id;
>   }
>   
> 

Thanks. This one looks OK to me.

It would be nice to augment the description with what situation this 
fixes. I suppose it doesn't kill the process properly?
  
Simon Marchi Feb. 12, 2020, 4:18 a.m. UTC | #2
On 2020-02-10 5:33 p.m., Luis Machado wrote:
> On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
>> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
>>
>> gdbserver/ChangeLog:
>>
>> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
>>
>> 	* win32-low.c (win32_create_inferior): Set signal_pid.
>> ---
>>   gdbserver/win32-low.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
>> index 9d0343788f..557c90d97c 100644
>> --- a/gdbserver/win32-low.c
>> +++ b/gdbserver/win32-low.c
>> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>>        (assuming success).  */
>>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>>   
>> +  /* Necessary for handle_v_kill.  */
>> +  signal_pid = current_process_id;
>> +
>>     return current_process_id;
>>   }
>>   
>>
> 
> Thanks. This one looks OK to me.
> 
> It would be nice to augment the description with what situation this 
> fixes. I suppose it doesn't kill the process properly?

Agreed.  The patch looks ok, as far as I understand, but it would be good to
explain in the commit message what's the bug, how you provoke it, how the new
behavior differs from the previous behavior.

Simon
  
Simon Marchi Feb. 12, 2020, 4:57 a.m. UTC | #3
On 2020-02-11 11:18 p.m., Simon Marchi wrote:
> On 2020-02-10 5:33 p.m., Luis Machado wrote:
>> On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
>>> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
>>>
>>> gdbserver/ChangeLog:
>>>
>>> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
>>>
>>> 	* win32-low.c (win32_create_inferior): Set signal_pid.
>>> ---
>>>   gdbserver/win32-low.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
>>> index 9d0343788f..557c90d97c 100644
>>> --- a/gdbserver/win32-low.c
>>> +++ b/gdbserver/win32-low.c
>>> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>>>        (assuming success).  */
>>>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>>>   
>>> +  /* Necessary for handle_v_kill.  */
>>> +  signal_pid = current_process_id;
>>> +
>>>     return current_process_id;
>>>   }
>>>   
>>>
>>
>> Thanks. This one looks OK to me.
>>
>> It would be nice to augment the description with what situation this 
>> fixes. I suppose it doesn't kill the process properly?
> 
> Agreed.  The patch looks ok, as far as I understand, but it would be good to
> explain in the commit message what's the bug, how you provoke it, how the new
> behavior differs from the previous behavior.
> 
> Simon
> 

Sorry, I just saw that you have already posted a v2, I'll at it now.

Simon
  

Patch

diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
index 9d0343788f..557c90d97c 100644
--- a/gdbserver/win32-low.c
+++ b/gdbserver/win32-low.c
@@ -709,6 +709,9 @@  win32_create_inferior (const char *program,
      (assuming success).  */
   cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
 
+  /* Necessary for handle_v_kill.  */
+  signal_pid = current_process_id;
+
   return current_process_id;
 }