Patchwork gdbserver: When attaching, add process before lwps

login
register
mail settings
Submitter Alan Hayward
Date Jan. 25, 2019, 10:48 a.m.
Message ID <20190125104834.2595-1-alan.hayward@arm.com>
Download mbox | patch
Permalink /patch/31209/
State New
Headers show

Comments

Alan Hayward - Jan. 25, 2019, 10:48 a.m.
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(-)
Pedro Alves - Jan. 25, 2019, 6:23 p.m.
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

Patch

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);
-
   /* Don't ignore the initial SIGSTOP if we just attached to this
      process.  It will be collected by wait shortly.  */
   initial_thread = find_thread_ptid (ptid_t (pid, pid, 0));