This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdbserver: Reset current_thread when its process is removed.
- From: Pedro Alves <palves at redhat dot com>
- To: Aleksandar Ristovski <aristovski at qnx dot com>, gdb-patches at sourceware dot org
- Date: Thu, 15 Oct 2015 17:22:10 +0100
- Subject: Re: [PATCH] gdbserver: Reset current_thread when its process is removed.
- Authentication-results: sourceware.org; auth=none
- References: <1444919808-22088-1-git-send-email-aristovski at qnx dot com>
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.
- And then, if we remove the current thread, why not remove others?
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.
> remove_inferior (&all_processes, &process->entry);
> free (process);
> }
>
Thanks,
Pedro Alves