[remote/rfc] Let GDB know if a remote server originally attached to a process.

[remote/rfc] Let GDB know if a remote server originally attached to a process.

[Pedro Alves]

Hi folks,

When gdbserver is used to attach to a running
process ('gdbserver --attach PID'), exiting GDB kills the inferior.  By
contrast, when GDB is attached to a running process, exiting GDB detaches.

There's an obvious workaround for this (detach explicitly, then quit GDB),
but this ought to be changed for consistency (*).

The issue here, is that GDB has no idea that gdbserver originaly
attached to the remote process, instead of having created
it (vs gdbserver :9999 foo).  This suggests that we need a new remote
protocol query.

I've came up with the attached patch as a sort of proof-of-concept.  This
is what happens with the patch installed:

 $ gdbserver :9999 --attach 10211

 (gdb) tar remote :9999
 Remote debugging using :9999
 (...)
 0x00007fc77a1b6796 in pthread_join () from /lib/libpthread.so.0
 (gdb) q
 The program is running.  Quit anyway (and detach it)? (y or n)    
                                       ^^^^^^^^^^^^^

Whereas with current GDB one gets:

 (gdb) q
 The program is running.  Quit anyway (and kill it)? (y or n)  
                                       ^^^^^^^^^^^

(another example, is, connecting to gdbserver with 'target extended-remote',
then issue an "attach" command.  In this case GDB *does* know that the
inferior was attached to.  Now, issue a "disconnect", and reconnect again.
This time, although gdbserver is still controlling the same inferior, GDB
doesn't know anymore that gdbserver originally attached to the inferior,
and will now offer to "kill" it.  )

This works by simply defining a new "qAttached:PID"/"qAttached" query to
GDB, to which the stub returns "1" --- attached, "0" --- not
attached, or "" --- not supported, as for all other optional packets.

This is simple enough --- the patch is smaller than this
whole description of it :-)  I was considering if we want to
generalize a mechanism for this or not, say similar to target
descriptions, but I don't have any other use case for such a thing.
The target description mechanism as is currently isn't exactly right
for this case.  E.g., there's only a single description for a
target interface, not one per inferior.  That *may* need to change
in the future, but I'm not certain yet.

What do you think?  Should I just stick with the below and go
write documentation for it?  Or...?


(*) - That was the simple version, which actually did come through a customer
request.  Here's another, albeit more complicated use case, which ends up
needing the same solution:  I have a multi-process aware remote stub, for
this OS where code sections are shared between processes.  One of the features
this enabled was support for so called "global breakpoints".  That is,
we insert a breakpoint at a given address, as usual, but, what can happen
is that a process that GDB and the stub wasn't attached to yet, hits
this breakpoint.  When this happens, the stub "auto-attaches" to the process
that hit the breakpoint, and reports the SIGTRAP as usual to GDB, which on
its end adds this process to its internal inferior table.  So far, so good,
all is simple in this regard.  Thing is, GDB has no way to know that we
had "attached" to these processes to begin with, and that we want
"quit" to *not* kill these processes, but to instead "detach" from them.  This
is seriously annoying when we add non-stop mode to the mix, since in
that case, we have to carefully and manually remove all breakpoints (in
case a process auto attaches just while we were exiting), then detach from
all inferiors manually, and only can we safelly "quit" from GDB.

-- 
Pedro Alves

2009-03-04  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* remote.c (PACKET_qAttached): New.
	(remote_query_attached): New.
	(remote_add_inferior): New.
	(notice_new_inferiors): Use it.
	(remote_start_remote): Use it.
	(_initialize_remote): Add "set remote query-attached-packet"
	command.

2009-03-04  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* server.c (handle_query): Handle "qAttached".

---
 gdb/gdbserver/server.c |    7 ++++++
 gdb/remote.c           |   57 ++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 61 insertions(+), 3 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2009-03-04 00:38:28.000000000 +0000
+++ src/gdb/remote.c	2009-03-04 00:41:14.000000000 +0000
@@ -992,6 +992,7 @@ enum {
   PACKET_vKill,
   PACKET_qXfer_siginfo_read,
   PACKET_qXfer_siginfo_write,
+  PACKET_qAttached,
   PACKET_MAX
 };
 
@@ -1118,6 +1119,53 @@ static ptid_t any_thread_ptid;
 static ptid_t general_thread;
 static ptid_t continue_thread;
 
+/* Try to find out if the stub attached to PID (and hence GDB should
+   detach instead of killing it when bailing out).  */
+
+static int
+remote_query_attached (int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (remote_protocol_packets[PACKET_qAttached].support == PACKET_DISABLE)
+    return 0;
+
+  if (remote_multi_process_p (rs))
+    sprintf (rs->buf, "qAttached:%x", pid);
+  else
+    sprintf (rs->buf, "qAttached");
+
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+
+  if (packet_ok (rs->buf,
+		 &remote_protocol_packets[PACKET_qAttached]) == PACKET_OK)
+    {
+      if (strcmp (rs->buf, "1") == 0)
+	return 1;
+    }
+
+  return 0;
+}
+
+static void
+remote_add_inferior (int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct inferior *inf;
+  int attached;
+
+  /* Check whether this process we're learning about is to be
+     considered attached, or if is to be considered to have been
+     spawned by the stub.  Do this before adding the inferior to the
+     tables, in case we throw an unexpected error, to avoid leaving
+     things inconsistent.  */
+  attached = remote_query_attached (pid);
+
+  inf = add_inferior (pid);
+  inf->attach_flag = attached;
+}
+
 static void
 notice_new_inferiors (ptid_t currthread)
 {
@@ -1161,7 +1209,7 @@ notice_new_inferiors (ptid_t currthread)
 	 may not know about it yet.  Add it before adding its child
 	 thread, so notifications are emitted in a sensible order.  */
       if (!in_inferior_list (ptid_get_pid (currthread)))
-	add_inferior (ptid_get_pid (currthread));
+	remote_add_inferior (ptid_get_pid (currthread));
 
       /* This is really a new thread.  Add it.  */
       add_thread (currthread);
@@ -2157,7 +2205,7 @@ remote_threads_info (struct target_ops *
 			 threads, so notifications are emitted in a
 			 sensible order.  */
 		      if (!in_inferior_list (ptid_get_pid (new_thread)))
-			add_inferior (ptid_get_pid (new_thread));
+			remote_add_inferior (ptid_get_pid (new_thread));
 
 		      add_thread (new_thread);
 
@@ -2628,7 +2676,7 @@ remote_start_remote (struct ui_out *uiou
       /* Now, if we have thread information, update inferior_ptid.  */
       inferior_ptid = remote_current_thread (inferior_ptid);
 
-      add_inferior (ptid_get_pid (inferior_ptid));
+      remote_add_inferior (ptid_get_pid (inferior_ptid));
 
       /* Always add the main thread.  */
       add_thread_silent (inferior_ptid);
@@ -9127,6 +9175,9 @@ Show the maximum size of the address (in
   add_packet_config_cmd (&remote_protocol_packets[PACKET_vKill],
 			 "vKill", "kill", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qAttached],
+			 "qAttached", "query-attached", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2009-03-04 00:38:28.000000000 +0000
+++ src/gdb/gdbserver/server.c	2009-03-04 00:41:14.000000000 +0000
@@ -1040,6 +1040,13 @@ handle_query (char *own_buf, int packet_
       return;
     }
 
+  if (strcmp (own_buf, "qAttached") == 0)
+    {
+      require_running (own_buf);
+      strcpy (own_buf, attached ? "1" : "0");
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Joel Brobecker]

> There's an obvious workaround for this (detach explicitly, then quit GDB),
> but this ought to be changed for consistency (*).

I agree. Not only that, but differences like these can be very annoying
when setting up automated testing. You write a test script that expects
one thing because that's what happens in the native case, then all of
a sudden, you have an exception when using gdbserver...

> This is simple enough --- the patch is smaller than this
> whole description of it :-)  I was considering if we want to
> generalize a mechanism for this or not, say similar to target
> descriptions, but I don't have any other use case for such a thing.

I'm not very familiar with the remote protocol, but I don't see much
benefit in generalizing this approach. Perhaps you'd like to expand,
but I'm happy with the simple approach you took.

The patch looks OK to me too. There are one or two new functions
that are undocumented, and we've been bugging contributors about this,
so it'd be nice to have a short description preceding them. Other than
that, no comment.

-- 
Joel

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Pedro Alves]

On Wednesday 04 March 2009 19:18:00, Joel Brobecker wrote:
> > There's an obvious workaround for this (detach explicitly, then quit GDB),
> > but this ought to be changed for consistency (*).
> 
> I agree. Not only that, but differences like these can be very annoying
> when setting up automated testing. You write a test script that expects
> one thing because that's what happens in the native case, then all of
> a sudden, you have an exception when using gdbserver...

We're all singing the same song then.  Great!

> > This is simple enough --- the patch is smaller than this
> > whole description of it :-)  I was considering if we want to
> > generalize a mechanism for this or not, say similar to target
> > descriptions, but I don't have any other use case for such a thing.
> 
> I'm not very familiar with the remote protocol, but I don't see much
> benefit in generalizing this approach. Perhaps you'd like to expand,
> but I'm happy with the simple approach you took.

I don't know, something like a generic process properties
mechanism, xml based, probably.  I'm happy with this approach, but I
though I'd ask if others were of a different opinion.

> The patch looks OK to me too. There are one or two new functions
> that are undocumented, and we've been bugging contributors about this,
> so it'd be nice to have a short description preceding them. Other than
> that, no comment.

I've checked in a patch, that ended up adding a remote_add_inferior
function for a different purpose (*) --- I documented things better
in that patch.

* http://sourceware.org/ml/gdb-patches/2009-03/msg00058.html

This updated code patch is a bit different due to that, but
it ends up adding to mostly the same.  I've also added documentation
bits.

Daniel, are you OK with this and the gdbserver bit?

Eli, is the documentation addition OK?

-- 
Pedro Alves
2009-03-06  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	
	* remote.c (PACKET_qAttached): New.
	(remote_query_attached): New.
	(remote_add_inferior): Add new `attached' argument.  Handle it.
	(remote_notice_new_inferior, remote_start_remote): Adjust to pass
	-1 to remote_add_inferior in new parameter.
	(extended_remote_attach_1): Adjust to pass 1 to
	remote_add_inferior in the new parameter.
	(extended_remote_create_inferior_1): Adjust to pass 0 to
	remote_add_inferior in the new parameter.
	(_initialize_remote): Add "set/show remote query-attached-packet"
	commands.

2009-03-06  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* server.c (handle_query): Handle "qAttached".

2009-03-06  Pedro Alves  <pedro@codesourcery.com>

	gdb/doc/
	* gdb.texinfo (Remote Configuration): Document query-attached'
        (General Query Packets): Document qAttached.

---
 gdb/doc/gdb.texinfo    |   29 +++++++++++++++++++++
 gdb/gdbserver/server.c |    7 +++++
 gdb/remote.c           |   66 +++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 94 insertions(+), 8 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/remote.c	2009-03-06 21:08:25.000000000 +0000
@@ -992,6 +992,7 @@ enum {
   PACKET_vKill,
   PACKET_qXfer_siginfo_read,
   PACKET_qXfer_siginfo_write,
+  PACKET_qAttached,
   PACKET_MAX
 };
 
@@ -1118,18 +1119,66 @@ static ptid_t any_thread_ptid;
 static ptid_t general_thread;
 static ptid_t continue_thread;
 
+/* Find out if the stub attached to PID (and hence GDB should offer to
+   detach instead of killing it when bailing out).  */
+
+static int
+remote_query_attached (int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (remote_protocol_packets[PACKET_qAttached].support == PACKET_DISABLE)
+    return 0;
+
+  if (remote_multi_process_p (rs))
+    sprintf (rs->buf, "qAttached:%x", pid);
+  else
+    sprintf (rs->buf, "qAttached");
+
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+
+  switch (packet_ok (rs->buf,
+		     &remote_protocol_packets[PACKET_qAttached]) == PACKET_OK)
+    {
+    case PACKET_OK:
+      if (strcmp (rs->buf, "1") == 0)
+	return 1;
+      break;
+    case PACKET_ERROR:
+      warning (_("Remote failure reply: %s"), rs->buf);
+      break;
+    case PACKET_UNKNOWN:
+      break;
+    }
+
+  return 0;
+}
+
 /* Add PID to GDB's inferior table.  Since we can be connected to a
    remote system before before knowing about any inferior, mark the
-   target with execution when we find the first inferior.  */
+   target with execution when we find the first inferior.  If ATTACHED
+   is 1, then we had just attached to this inferior.  If it is 0, then
+   we just created this inferior.  If it is -1, then try querying the
+   remote stub to find out if it had attached to the inferior or
+   not.  */
 
 static struct inferior *
-remote_add_inferior (int pid)
+remote_add_inferior (int pid, int attached)
 {
   struct remote_state *rs = get_remote_state ();
   struct inferior *inf;
 
+  /* Check whether this process we're learning about is to be
+     considered attached, or if is to be considered to have been
+     spawned by the stub.  */
+  if (attached == -1)
+    attached = remote_query_attached (pid);
+
   inf = add_inferior (pid);
 
+  inf->attach_flag = attached;
+
   /* This may be the first inferior we hear about.  */
   if (!target_has_execution)
     {
@@ -1207,7 +1256,7 @@ remote_notice_new_inferior (ptid_t currt
 	 may not know about it yet.  Add it before adding its child
 	 thread, so notifications are emitted in a sensible order.  */
       if (!in_inferior_list (ptid_get_pid (currthread)))
-	inf = remote_add_inferior (ptid_get_pid (currthread));
+	inf = remote_add_inferior (ptid_get_pid (currthread), -1);
 
       /* This is really a new thread.  Add it.  */
       remote_add_thread (currthread, running);
@@ -2665,7 +2714,7 @@ remote_start_remote (struct ui_out *uiou
       /* Now, if we have thread information, update inferior_ptid.  */
       inferior_ptid = remote_current_thread (inferior_ptid);
 
-      remote_add_inferior (ptid_get_pid (inferior_ptid));
+      remote_add_inferior (ptid_get_pid (inferior_ptid), -1);
 
       /* Always add the main thread.  */
       add_thread_silent (inferior_ptid);
@@ -3390,7 +3439,6 @@ extended_remote_attach_1 (struct target_
   int pid;
   char *dummy;
   char *wait_status = NULL;
-  struct inferior *inf;
 
   if (!args)
     error_no_arg (_("process-id to attach"));
@@ -3436,8 +3484,7 @@ extended_remote_attach_1 (struct target_
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
-  inf = remote_add_inferior (pid);
-  inf->attach_flag = 1;
+  remote_add_inferior (pid, 1);
 
   if (non_stop)
     /* Get list of threads.  */
@@ -6761,7 +6808,7 @@ extended_remote_create_inferior_1 (char 
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
-  remote_add_inferior (ptid_get_pid (inferior_ptid));
+  remote_add_inferior (ptid_get_pid (inferior_ptid), 0);
   add_thread_silent (inferior_ptid);
 
   /* Get updated offsets, if the stub uses qOffsets.  */
@@ -9161,6 +9208,9 @@ Show the maximum size of the address (in
   add_packet_config_cmd (&remote_protocol_packets[PACKET_vKill],
 			 "vKill", "kill", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qAttached],
+			 "qAttached", "query-attached", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/gdbserver/server.c	2009-03-06 21:08:25.000000000 +0000
@@ -1040,6 +1040,13 @@ handle_query (char *own_buf, int packet_
       return;
     }
 
+  if (strcmp (own_buf, "qAttached") == 0)
+    {
+      require_running (own_buf);
+      strcpy (own_buf, attached ? "1" : "0");
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2009-03-06 21:08:25.000000000 +0000
@@ -14425,6 +14425,10 @@ are:
 @item @code{osdata}
 @tab @code{qXfer:osdata:read}
 @tab @code{info os}
+
+@item @code{query-attached}
+@tab @code{qAttached}
+@tab Querying remote process attach state.
 @end multitable
 
 @node Remote Stub
@@ -27306,6 +27310,31 @@ not recognize the @var{object} keyword, 
 @var{object} does not recognize the @var{operation} keyword, the stub
 must respond with an empty packet.
 
+@item qAttached:@var{pid}
+@cindex query attached, remote request
+@cindex @samp{qAttached} packet
+Return if the remote target is attached to the process with the
+specified process ID, instead of having created it.  @var{pid} is a
+hexadecimal integer identifying the target process.  When the
+multiprocess protocol extensions are not supported
+(@pxref{multiprocess extensions}), @value{GDBN} will omit the
+@var{pid} field, thus the query packet will be simplified as
+@samp{qAttached}.
+
+This query is used to, for example, know whether the remote process
+should be detached or killed when a @value{GDBN} session is ended with
+the @code{quit} command.
+
+Reply:
+@table @samp
+@item 1
+The target process was attached to.
+@item 0
+The target process was not attached to.
+@item E @var{NN}
+A badly formed request or an error was encountered.
+@end table
+
 @end table
 
 @node Register Packet Format

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 6 Mar 2009 21:24:01 +0000
> Cc: Joel Brobecker <brobecker@adacore.com>,
>  Daniel Jacobowitz <drow@false.org>,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Eli, is the documentation addition OK?

I have a few comments.

> +@item qAttached:@var{pid}
> +@cindex query attached, remote request
> +@cindex @samp{qAttached} packet
> +Return if the remote target is attached to the process with the
> +specified process ID, instead of having created it.

This sentence probably needs to be rephrased, but I don't understand
what it means, and so cannot suggest how to rephrase it.  It sounds
like you meant to say "Return an indication of whether the remote
target is attached ...", but then what is the purpose of adding
"instead of having created it"?  Could you please explain?

> @var{pid} is a
> +hexadecimal integer identifying the target process.

There's no such thing as "hexadecimal integer".  I think you mean
"an integer in hexadecimal format".

> +This query is used to, for example, know whether the remote process

Don't separate "to" from its verb:

  This query is used, for example, to know whether ...

Thanks.

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Pedro Alves]

On Saturday 07 March 2009 10:29:42, Eli Zaretskii wrote:

> > +@item qAttached:@var{pid}
> > +@cindex query attached, remote request
> > +@cindex @samp{qAttached} packet
> > +Return if the remote target is attached to the process with the
> > +specified process ID, instead of having created it.
> 
> This sentence probably needs to be rephrased, but I don't understand
> what it means, and so cannot suggest how to rephrase it.  It sounds
> like you meant to say "Return an indication of whether the remote
> target is attached ...", but then what is the purpose of adding
> "instead of having created it"?  Could you please explain?

Let me try.  This is about returning an indication of how did the
process that is now under the stub's control originaly get under
the stub's control.  Was it due to an "attach"-like operation?, or, was
it due to a "run"-like operation?  The latter was the "created it"
version.

> 
> > @var{pid} is a
> > +hexadecimal integer identifying the target process.
> 
> There's no such thing as "hexadecimal integer".  I think you mean
> "an integer in hexadecimal format".

Ok.

> 
> > +This query is used to, for example, know whether the remote process
> 
> Don't separate "to" from its verb:
> 
>   This query is used, for example, to know whether ...

Ok.

> 
> Thanks.
> 

Thank you.

-- 
Pedro Alves

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sat, 7 Mar 2009 12:27:36 +0000
> Cc: brobecker@adacore.com,
>  drow@false.org
> 
> On Saturday 07 March 2009 10:29:42, Eli Zaretskii wrote:
> 
> > > +@item qAttached:@var{pid}
> > > +@cindex query attached, remote request
> > > +@cindex @samp{qAttached} packet
> > > +Return if the remote target is attached to the process with the
> > > +specified process ID, instead of having created it.
> > 
> > This sentence probably needs to be rephrased, but I don't understand
> > what it means, and so cannot suggest how to rephrase it.  It sounds
> > like you meant to say "Return an indication of whether the remote
> > target is attached ...", but then what is the purpose of adding
> > "instead of having created it"?  Could you please explain?
> 
> Let me try.  This is about returning an indication of how did the
> process that is now under the stub's control originaly get under
> the stub's control.  Was it due to an "attach"-like operation?, or, was
> it due to a "run"-like operation?  The latter was the "created it"
> version.

In that case, I suggest this text:

  Return an indication of whether the remote stub attached to an
  existing process or created a new process.

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Pedro Alves]

On Saturday 07 March 2009 14:20:00, Eli Zaretskii wrote:
> In that case, I suggest this text:
> 
>   Return an indication of whether the remote stub attached to an
>   existing process or created a new process.

Sounds very good, thank you.  Here's how it looks now:

@item qAttached:@var{pid}
@cindex query attached, remote request
@cindex @samp{qAttached} packet
Return an indication of whether the remote server attached to an
existing process or created a new process.  When the multiprocess
protocol extensions are supported (@pxref{multiprocess extensions}),
@var{pid} is an integer in hexadecimal format identifying the target
process.  Otherwise, @value{GDBN} will omit the @var{pid} field and
the query packet will be simplified as @samp{qAttached}.

This query is used, for example, to know whether the remote process
should be detached or killed when a @value{GDBN} session is ended with
the @code{quit} command.

Reply:
@table @samp
@item 1
The remote server attached to an existing process.
@item 0
The remote server created a new process.
@item E @var{NN}
A badly formed request or an error was encountered.
@end table

-- 
Pedro Alves

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sat, 7 Mar 2009 14:53:37 +0000
> Cc: brobecker@adacore.com,
>  drow@false.org
> 
> On Saturday 07 March 2009 14:20:00, Eli Zaretskii wrote:
> > In that case, I suggest this text:
> > 
> >   Return an indication of whether the remote stub attached to an
> >   existing process or created a new process.
> 
> Sounds very good, thank you.  Here's how it looks now:

Thanks, this version is good to go.

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Daniel Jacobowitz]

On Fri, Mar 06, 2009 at 09:24:01PM +0000, Pedro Alves wrote:
> Daniel, are you OK with this and the gdbserver bit?

Yes, I am.  Thanks!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [remote/rfc] Let GDB know if a remote server originally attached to a process.

[Pedro Alves]

On Friday 13 March 2009 16:00:20, Daniel Jacobowitz wrote:
> On Fri, Mar 06, 2009 at 09:24:01PM +0000, Pedro Alves wrote:
> > Daniel, are you OK with this and the gdbserver bit?
> 
> Yes, I am.  Thanks!

Thanks everyone!  I've checked it in.

-- 
Pedro Alves

2009-03-14  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* remote.c (PACKET_qAttached): New.
	(remote_query_attached): New.
	(remote_add_inferior): Add new `attached' argument.  Handle it.
	(remote_notice_new_inferior, remote_start_remote): Adjust to pass
	-1 to remote_add_inferior in new parameter.
	(extended_remote_attach_1): Adjust to pass 1 to
	remote_add_inferior in the new parameter.
	(extended_remote_create_inferior_1): Adjust to pass 0 to
	remote_add_inferior in the new parameter.
	(_initialize_remote): Add "set/show remote query-attached-packet"
	commands.

2009-03-14  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* server.c (handle_query): Handle "qAttached".

2009-03-14  Pedro Alves  <pedro@codesourcery.com>

	gdb/doc/
	* gdb.texinfo (Remote Configuration): Document query-attached.
        (General Query Packets): Document qAttached.

---
 gdb/doc/gdb.texinfo    |   28 ++++++++++++++++++++
 gdb/gdbserver/server.c |    7 +++++
 gdb/remote.c           |   66 +++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 93 insertions(+), 8 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/remote.c	2009-03-06 21:08:25.000000000 +0000
@@ -992,6 +992,7 @@ enum {
   PACKET_vKill,
   PACKET_qXfer_siginfo_read,
   PACKET_qXfer_siginfo_write,
+  PACKET_qAttached,
   PACKET_MAX
 };
 
@@ -1118,18 +1119,66 @@ static ptid_t any_thread_ptid;
 static ptid_t general_thread;
 static ptid_t continue_thread;
 
+/* Find out if the stub attached to PID (and hence GDB should offer to
+   detach instead of killing it when bailing out).  */
+
+static int
+remote_query_attached (int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (remote_protocol_packets[PACKET_qAttached].support == PACKET_DISABLE)
+    return 0;
+
+  if (remote_multi_process_p (rs))
+    sprintf (rs->buf, "qAttached:%x", pid);
+  else
+    sprintf (rs->buf, "qAttached");
+
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+
+  switch (packet_ok (rs->buf,
+		     &remote_protocol_packets[PACKET_qAttached]) == PACKET_OK)
+    {
+    case PACKET_OK:
+      if (strcmp (rs->buf, "1") == 0)
+	return 1;
+      break;
+    case PACKET_ERROR:
+      warning (_("Remote failure reply: %s"), rs->buf);
+      break;
+    case PACKET_UNKNOWN:
+      break;
+    }
+
+  return 0;
+}
+
 /* Add PID to GDB's inferior table.  Since we can be connected to a
    remote system before before knowing about any inferior, mark the
-   target with execution when we find the first inferior.  */
+   target with execution when we find the first inferior.  If ATTACHED
+   is 1, then we had just attached to this inferior.  If it is 0, then
+   we just created this inferior.  If it is -1, then try querying the
+   remote stub to find out if it had attached to the inferior or
+   not.  */
 
 static struct inferior *
-remote_add_inferior (int pid)
+remote_add_inferior (int pid, int attached)
 {
   struct remote_state *rs = get_remote_state ();
   struct inferior *inf;
 
+  /* Check whether this process we're learning about is to be
+     considered attached, or if is to be considered to have been
+     spawned by the stub.  */
+  if (attached == -1)
+    attached = remote_query_attached (pid);
+
   inf = add_inferior (pid);
 
+  inf->attach_flag = attached;
+
   /* This may be the first inferior we hear about.  */
   if (!target_has_execution)
     {
@@ -1207,7 +1256,7 @@ remote_notice_new_inferior (ptid_t currt
 	 may not know about it yet.  Add it before adding its child
 	 thread, so notifications are emitted in a sensible order.  */
       if (!in_inferior_list (ptid_get_pid (currthread)))
-	inf = remote_add_inferior (ptid_get_pid (currthread));
+	inf = remote_add_inferior (ptid_get_pid (currthread), -1);
 
       /* This is really a new thread.  Add it.  */
       remote_add_thread (currthread, running);
@@ -2665,7 +2714,7 @@ remote_start_remote (struct ui_out *uiou
       /* Now, if we have thread information, update inferior_ptid.  */
       inferior_ptid = remote_current_thread (inferior_ptid);
 
-      remote_add_inferior (ptid_get_pid (inferior_ptid));
+      remote_add_inferior (ptid_get_pid (inferior_ptid), -1);
 
       /* Always add the main thread.  */
       add_thread_silent (inferior_ptid);
@@ -3390,7 +3439,6 @@ extended_remote_attach_1 (struct target_
   int pid;
   char *dummy;
   char *wait_status = NULL;
-  struct inferior *inf;
 
   if (!args)
     error_no_arg (_("process-id to attach"));
@@ -3436,8 +3484,7 @@ extended_remote_attach_1 (struct target_
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
-  inf = remote_add_inferior (pid);
-  inf->attach_flag = 1;
+  remote_add_inferior (pid, 1);
 
   if (non_stop)
     /* Get list of threads.  */
@@ -6761,7 +6808,7 @@ extended_remote_create_inferior_1 (char 
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
-  remote_add_inferior (ptid_get_pid (inferior_ptid));
+  remote_add_inferior (ptid_get_pid (inferior_ptid), 0);
   add_thread_silent (inferior_ptid);
 
   /* Get updated offsets, if the stub uses qOffsets.  */
@@ -9161,6 +9208,9 @@ Show the maximum size of the address (in
   add_packet_config_cmd (&remote_protocol_packets[PACKET_vKill],
 			 "vKill", "kill", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qAttached],
+			 "qAttached", "query-attached", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/gdbserver/server.c	2009-03-06 21:08:25.000000000 +0000
@@ -1040,6 +1040,13 @@ handle_query (char *own_buf, int packet_
       return;
     }
 
+  if (strcmp (own_buf, "qAttached") == 0)
+    {
+      require_running (own_buf);
+      strcpy (own_buf, attached ? "1" : "0");
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2009-03-06 21:05:26.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2009-03-07 14:49:42.000000000 +0000
@@ -14425,6 +14425,10 @@ are:
 @item @code{osdata}
 @tab @code{qXfer:osdata:read}
 @tab @code{info os}
+
+@item @code{query-attached}
+@tab @code{qAttached}
+@tab Querying remote process attach state.
 @end multitable
 
 @node Remote Stub
@@ -27306,6 +27310,30 @@ not recognize the @var{object} keyword, 
 @var{object} does not recognize the @var{operation} keyword, the stub
 must respond with an empty packet.
 
+@item qAttached:@var{pid}
+@cindex query attached, remote request
+@cindex @samp{qAttached} packet
+Return an indication of whether the remote server attached to an
+existing process or created a new process.  When the multiprocess
+protocol extensions are supported (@pxref{multiprocess extensions}),
+@var{pid} is an integer in hexadecimal format identifying the target
+process.  Otherwise, @value{GDBN} will omit the @var{pid} field and
+the query packet will be simplified as @samp{qAttached}.
+
+This query is used, for example, to know whether the remote process
+should be detached or killed when a @value{GDBN} session is ended with
+the @code{quit} command.
+
+Reply:
+@table @samp
+@item 1
+The remote server attached to an existing process.
+@item 0
+The remote server created a new process.
+@item E @var{NN}
+A badly formed request or an error was encountered.
+@end table
+
 @end table
 
 @node Register Packet Format