Re: [PATCH] gdbserver: When attaching, add process before lwps

Pedro Alves
2019-01-25T18:23:08Z

On 01/25/2019 10:48 AM, Alan Hayward wrote:
> The recent BP/WP changes for AArch64 swapping the order in add_lwp()
> so that the process was added before the lwp. This was due to the lwp
> creation requiring the process data.
>
> This also needs changing in linux_attach().
>
> Fixes gdb.server/ext-attach.exp on Aarch64.
>
> (This regression was hidden due to the racy nature of the gdb.server
> tests - now they are no longer racy it'll be easier to spot. Also
> checked X86).
>
> gdb/gdbserver/ChangeLog:
>
> 2019-01-25 Alan Hayward <alan.hayward@arm.com>
>
> * linux-low.c (linux_attach): Add process before lwp.

> ---
> gdb/gdbserver/linux-low.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 17cce24d76..1ab2cfb1eb 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -1188,6 +1188,8 @@ linux_attach (unsigned long pid)
> ptid_t ptid = ptid_t (pid, pid, 0);
> int err;
>
> + proc = linux_add_process (pid, 1);
> +
> /* Attach to PID. We will check for other threads
> soon. */
> err = linux_attach_lwp (ptid);
> @@ -1198,8 +1200,6 @@ linux_attach (unsigned long pid)
> error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
> }
>
> - proc = linux_add_process (pid, 1);
> -

This fails to consider the error conditions. That error call
visible above throws an exception.

- If GDBserver is already attached to the process, this will
create another process_info object for the already-attached
process, and leave it behind.

For this case, it would seem better to check whether we're
already attached to a given PID in common code, around
server.c:attach_inferior.

- If GDBserver isn't already attached, but the attach fails,
this will likewise leave a stale process_info object behind.

Thanks,
Pedro Alves