On 15-10-15 12:22 PM, Pedro Alves wrote:
> On 10/15/2015 03:36 PM, Aleksandar Ristovski wrote:
>> In case of running gdbserver with --multi, if a nat file
>> removes a process, current_thread may remain set to now
>> freed 'process' entry.
>>
>> This may lead to wrong operation or a crash.
>>
>> gdb/gdbserver/ChangeLog:
>> 	* inferiors.c (remove_process): Reset current_thread if its
>> 	associated process gets removed.
>> ---
>>  gdb/gdbserver/inferiors.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
>> index 21f45fa..4688a44 100644
>> --- a/gdb/gdbserver/inferiors.c
>> +++ b/gdb/gdbserver/inferiors.c
>> @@ -291,6 +291,11 @@ remove_process (struct process_info *process)
>>  {
>>    clear_symbol_cache (&process->symbol_cache);
>>    free_all_breakpoints (process);
>> +  if (current_thread && get_thread_process (current_thread) == process)
>> +    {
>> +      remove_thread (current_thread);
>> +      current_thread = NULL;
>> +    }
> 
> This seems very papering-over-something-else.
> 
> - I could argue that current_thread = NULL would be better done
> inside remove_thread than here, since what you say would happen
> as well if the current thread is removed, even without removing
> the process.

Agree.

> 
> - And then, if we remove the current thread, why not remove others?
> 

True again.

> AFAICS, other targets remove threads from within target_mourn, as that
> way they have a chance of clearing auxiliary info associated with
> the threads (the inferior_target_data()), and then some call
> clear_inferiors, which also clears current_thread.

clear_inferiors wouldn't work if we want to keep other inferiors.

> 
>>    remove_inferior (&all_processes, &process->entry);
>>    free (process);
>>  }
>>

Ok, if this is intended use, to first remove threads then it will work.
I still think resetting current_thread in remove_thread is in order.
Assert in remove_process would help people like me.

Attached is another version as a proposal, though now not strictly
necessary. The patch did not show any regressions testing on a
native-gdbserver target board. (GNU/Linux x86_64)


Thanks,

Aleksandar