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

[Pedro Alves <pedro at codesourcery dot com>]

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