[PATCH 2/2] Don't query stub if the pid is faked

[PATCH 2/2] Don't query stub if the pid is faked

[Yao Qi]

Even with the previous patch applied, we still see an error from GDBserver,
in the reply to 'qAttached'.

Sending packet: $QNonStop:0#8c...Packet received: OK
Sending packet: $?#3f...Packet received: T0505:00000000;04:00f0ffbf;08:b0c2e44c;thread:p35eb.35eb;core:0;
Sending packet: $Hc-1#09...Packet received: E01
Sending packet: $qC#b4...Packet received:
Sending packet: $qAttached:a410#bf...Packet received: E01
Packet qAttached (query-attached) is supported
warning: Remote failure reply: E01

If the stub doesn't support "qC", the 'inferior_ptid' in GDB is
'magic_null_ptid'.  Then GDB will query the stub of pid of
'magic_null_ptid', so the stub doesn't know it and reports error.

This patch fixes this problem by skipping query to the stub is the
pid is faked.

P.S. even with this patch, function inferior_appeared is still called
in remote_add_inferior with the faked PID.  So if we start GDBserver
with --disable-qC and start GDB in MI mode, we can see that the wrong
"pid" is sent to MI front-end, as shown below.

target remote :1234
&"target remote :1234\n"
~"Remote debugging using :1234\n"
=thread-group-started,id="i1",pid="42000" <----
=thread-created,id="1",group-id="i1"
=library-loaded,id="/lib/ld-linux.so.2",target-name="/lib/ld-linux.so.2",host-name="/lib/ld-linux.so.2",symbols-loaded="0",thread-group="i1"
~"Reading symbols from /lib/ld-linux.so.2..."
~"(no debugging symbols found)...done.\n"
~"Loaded symbols for /lib/ld-linux.so.2\n"
~"[Switching to Thread 1445]\n"
~"0x4ce4c2b0 in _start () from /lib/ld-linux.so.2\n"
*stopped,frame={addr="0x4ce4c2b0",func="_start",args=[],from="/lib/ld-linux.so.2"},thread-id="1",stopped-threads="all",core="2"

I tried to fix it, however looks GDB is unable to get the real pid
until the stop reply arrives and my change reorders the MI
notification "=thread-group-started" and "=thread-created".  So I give
up on it.

gdb:

2013-01-22  Yao Qi  <yao@codesourcery.com>

	* remote.c (remote_add_inferior): Don't call remote_query_attached
	if 'fake_pid_p' is true.
---
 gdb/remote.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 3271ca0..07410a3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1463,8 +1463,9 @@ remote_add_inferior (int fake_pid_p, int pid, 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.  */
-  if (attached == -1)
+     spawned by the stub.  Don't bother the stub if the PID is
+     faked.  */
+  if (attached == -1 && !fake_pid_p)
     attached = remote_query_attached (pid);
 
   if (gdbarch_has_global_solist (target_gdbarch ()))
-- 
1.7.7.6

[PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled

[Yao Qi]

GDB gets an internal error when it connects to GDBserver started with
'--disable-packet=qC'.

Sending packet: $QNonStop:0#8c...Packet received: OK
Sending packet: $?#3f...Packet received: T0505:00000000;04:00f0ffbf;08:b0c2e44c;thread:p4255.4255;core:1;
Sending packet: $Hc-1#09...Packet received: E01
Sending packet: $qC#b4...Packet received:
Sending packet: $qAttached:a410#bf...Packet received: E01
Packet qAttached (query-attached) is supported
warning: Remote failure reply: E01
Sending packet: $qOffsets#4b...Packet received:
../../../git/gdb/target.c:3248: internal-error: Can't determine the current address space of thread Thread 16981

When start remote, the call chain is as follows,

remote_start_remote
  add_current_inferior_and_thread <--[1]
  ...
  start_remote
    wait_for_inferior
      remote_wait_as
        process_stop_reply
          get_thread_arch_regcache   <--[2]
          remote_notice_new_inferior <--[3]

GDB sends packet "qC" in [1] and adds the thread/inferior if the remote
stubs understands "qC".  In [2], GDB looks for the inferior to build a
regcache, and notices a new inferior in [3].  As we can see, GDB assumes
that the inferior can be found in [2].  Once the remote stub doesn't
support "qC", GDB can't look for the inferior in [2], and emits an
internal error.

This patch fix this internal error by exchanging the order of [2] and
[3].

gdb:

2013-01-22  Yao Qi  <yao@codesourcery.com>

	* remote.c (process_stop_reply): Call remote_notice_new_inferior
	earlier.
---
 gdb/remote.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 7ea9597..3271ca0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5690,11 +5690,18 @@ process_stop_reply (struct stop_reply *stop_reply,
       /* Expedited registers.  */
       if (stop_reply->regcache)
 	{
-	  struct regcache *regcache
-	    = get_thread_arch_regcache (ptid, target_gdbarch ());
+	  struct regcache *regcache;
 	  cached_reg_t *reg;
 	  int ix;
 
+	  /* Add the inferior earlier, because the following
+	     'get_thread_arch_regcache' calls
+	     'target_thread_address_space', which requires the
+	     inferior should be found in the inferior list.  */
+	  remote_notice_new_inferior (ptid, 0);
+
+	  regcache = get_thread_arch_regcache (ptid,
+					       target_gdbarch ());
 	  for (ix = 0;
 	       VEC_iterate(cached_reg_t, stop_reply->regcache, ix, reg);
 	       ix++)
@@ -5705,7 +5712,6 @@ process_stop_reply (struct stop_reply *stop_reply,
       remote_stopped_by_watchpoint_p = stop_reply->stopped_by_watchpoint_p;
       remote_watch_data_address = stop_reply->watch_data_address;
 
-      remote_notice_new_inferior (ptid, 0);
       demand_private_info (ptid)->core = stop_reply->core;
     }
 
-- 
1.7.7.6

Re: [PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled

[Pedro Alves]

On 01/23/2013 08:35 AM, Yao Qi wrote:
> GDB gets an internal error when it connects to GDBserver started with
> '--disable-packet=qC'.
> 
> Sending packet: $QNonStop:0#8c...Packet received: OK
> Sending packet: $?#3f...Packet received: T0505:00000000;04:00f0ffbf;08:b0c2e44c;thread:p4255.4255;core:1;
> Sending packet: $Hc-1#09...Packet received: E01
> Sending packet: $qC#b4...Packet received:
> Sending packet: $qAttached:a410#bf...Packet received: E01
> Packet qAttached (query-attached) is supported
> warning: Remote failure reply: E01
> Sending packet: $qOffsets#4b...Packet received:
> ../../../git/gdb/target.c:3248: internal-error: Can't determine the current address space of thread Thread 16981
> 
> When start remote, the call chain is as follows,
> 
> remote_start_remote
>   add_current_inferior_and_thread <--[1]
>   ...
>   start_remote
>     wait_for_inferior
>       remote_wait_as
>         process_stop_reply
>           get_thread_arch_regcache   <--[2]
>           remote_notice_new_inferior <--[3]
> 
> GDB sends packet "qC" in [1] and adds the thread/inferior if the remote
> stubs understands "qC".  In [2], GDB looks for the inferior to build a
> regcache, and notices a new inferior in [3].  As we can see, GDB assumes
> that the inferior can be found in [2].  Once the remote stub doesn't
> support "qC", GDB can't look for the inferior in [2], and emits an
> internal error.
> 
> This patch fix this internal error by exchanging the order of [2] and
> [3].
> 

Thanks.

> gdb:
> 
> 2013-01-22  Yao Qi  <yao@codesourcery.com>
> 
> 	* remote.c (process_stop_reply): Call remote_notice_new_inferior
> 	earlier.
> ---
>  gdb/remote.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 7ea9597..3271ca0 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -5690,11 +5690,18 @@ process_stop_reply (struct stop_reply *stop_reply,
>        /* Expedited registers.  */
>        if (stop_reply->regcache)
>  	{
> -	  struct regcache *regcache
> -	    = get_thread_arch_regcache (ptid, target_gdbarch ());
> +	  struct regcache *regcache;
>  	  cached_reg_t *reg;
>  	  int ix;
>  
> +	  /* Add the inferior earlier, because the following
> +	     'get_thread_arch_regcache' calls
> +	     'target_thread_address_space', which requires the
> +	     inferior should be found in the inferior list.  */
> +	  remote_notice_new_inferior (ptid, 0);
> +
> +	  regcache = get_thread_arch_regcache (ptid,
> +					       target_gdbarch ());
>  	  for (ix = 0;
>  	       VEC_iterate(cached_reg_t, stop_reply->regcache, ix, reg);
>  	       ix++)
> @@ -5705,7 +5712,6 @@ process_stop_reply (struct stop_reply *stop_reply,
>        remote_stopped_by_watchpoint_p = stop_reply->stopped_by_watchpoint_p;
>        remote_watch_data_address = stop_reply->watch_data_address;
>  
> -      remote_notice_new_inferior (ptid, 0);
>        demand_private_info (ptid)->core = stop_reply->core;
>      }

Two things don't look right here.

First, this removes the remote_notice_new_inferior call in
the case the target doesn't send any expedited registers.
That is, the call is conditional on "if (stop_reply->regcache)".

Second, the point of the get_thread_arch_regcache call is to
pre-fill the regcache with the expedite register values, before
any other code needs one of the likely registers in the expedited
set (usually PC, SP, FP), thus saving a roundtrip.
I haven't checked if notice_new_inferior (the core function) ends
needing to fetch up registers; it's possible it ends up fetching
the whole g set anyway, but still, it doesn't feel right.

I had another idea.  Right after the initial connection, we
query the target for its state, with the ? packet.  We
store the resulting wait status / stop reply aside, and
query the target for the current thread, using qC, which
fails, so we fake a ptid for the target's thread.
We then later, after the initial setup, end up consuming that
set-aside wait status, parsing the T stop reply, which contains
a "thread" "register" (which was the thread the target would
have replied to qC).  We get into trouble because the ptid
in that stop reply doesn't match our faked up ptid in the initial
setup, although the target threads are the same...

The idea is that, we had the T stop reply handy all along.
We might as well extract the thread's ptid from it, and
avoid all the resulting issues.

qC is also used after vRun, in order to discover the
new process'es main thread.  But, vRun's reply is
also a wait status, just like '?''s, which is quite
convenient.

This means that if we have a "Txx thread: ptid" reply, then we
don't really need qC...  That's what the patch below does.
The patch still tries qC first, but I'm thinking we can flip that
around, and only try qC if the stop reply didn't include a thread.
I looked back in history trying to find the rationale for qC, but
I couldn't find it.  The packet handling seems to have been added
in gdb-4.18 (1999), and I see that in that same release,
"Txx thread: ptid" didn't exist yet, which explains why nobody
though of doing this then.

I ran the testsuite with it against a gdbserver with qC disabled,
and saw no regressions.

2013-01-24  Pedro Alves  <palves@redhat.com>

	* remote.c (stop_reply_extract_thread): New.
	(add_current_inferior_and_thread): New parameter 'wait_status'.
	Handle it.
	(remote_start_remote): Pass wait status to
	add_current_inferior_and_thread.
	(extended_remote_run): Update comment.
	(extended_remote_create_inferior_1): Pass wait status to
	add_current_inferior_and_thread.
---

diff --git a/gdb/remote.c b/gdb/remote.c
index 7ea9597..56f1a49 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3224,13 +3224,57 @@ send_interrupt_sequence (void)
 		    interrupt_sequence_mode);
 }
 
+
+/* If STOP_REPLY is a T stop reply, look for the "thread" register,
+   and extract the PTID.  Returns NULL_PTID if not found.  */
+
+static ptid_t
+stop_reply_extract_thread (char *stop_reply)
+{
+  if (stop_reply[0] == 'T' && strlen (stop_reply) > 3)
+    {
+      char *p;
+
+      /* Txx r:val ; r:val (...)  */
+      p = &stop_reply[3];
+
+      /* Look for "register" named "thread".  */
+      while (*p != '\0')
+	{
+	  char *p1;
+
+	  p1 = strchr (p, ':');
+	  if (p1 == NULL)
+	    return null_ptid;
+
+	  if (strncmp (p, "thread", p1 - p) == 0)
+	    return read_ptid (++p1, &p);
+
+	  p1 = strchr (p, ';');
+	  if (p1 == NULL)
+	    return null_ptid;
+	  p1++;
+
+	  p = p1;
+	}
+    }
+
+  return null_ptid;
+}
+
 /* Query the remote target for which is the current thread/process,
    add it to our tables, and update INFERIOR_PTID.  The caller is
    responsible for setting the state such that the remote end is ready
-   to return the current thread.  */
+   to return the current thread.
+
+   This function is called after handling the '?' or 'vRun' packets,
+   whose response is a stop reply from which we can also try
+   extracting the thread.  If the target doesn't support the explicit
+   qC query, we infer the current thread from that stop reply, passed
+   in in WAIT_STATUS, which may be NULL.  */
 
 static void
-add_current_inferior_and_thread (void)
+add_current_inferior_and_thread (char *wait_status)
 {
   struct remote_state *rs = get_remote_state ();
   int fake_pid_p = 0;
@@ -3238,8 +3282,14 @@ add_current_inferior_and_thread (void)
 
   inferior_ptid = null_ptid;
 
-  /* Now, if we have thread information, update inferior_ptid.  */
+  /* Now, if we have thread information, update inferior_ptid.  First
+     ask the remote which is the current thread, with qC.  If that's
+     not supported, and we have a stop reply handy, maybe it's a T
+     stop reply with a "thread" register.  */
   ptid = remote_current_thread (inferior_ptid);
+  if (ptid_equal (ptid, null_ptid) && wait_status != NULL)
+    ptid = stop_reply_extract_thread (wait_status);
+
   if (!ptid_equal (ptid, null_ptid))
     {
       if (!remote_multi_process_p (rs))
@@ -3400,7 +3450,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
       /* Let the stub know that we want it to return the thread.  */
       set_continue_thread (minus_one_ptid);
 
-      add_current_inferior_and_thread ();
+      add_current_inferior_and_thread (wait_status);
 
       /* init_wait_for_inferior should be called before get_offsets in order
 	 to manage `inserted' flag in bp loc in a correct state.
@@ -7836,7 +7886,7 @@ extended_remote_run (char *args)
 
   if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vRun]) == PACKET_OK)
     {
-      /* We have a wait response; we don't need it, though.  All is well.  */
+      /* We have a wait response.  All is well.  */
       return 0;
     }
   else if (remote_protocol_packets[PACKET_vRun].support == PACKET_DISABLE)
@@ -7863,6 +7913,10 @@ static void
 extended_remote_create_inferior_1 (char *exec_file, char *args,
 				   char **env, int from_tty)
 {
+  int run_worked;
+  char *stop_reply;
+  struct remote_state *rs = get_remote_state ();
+
   /* If running asynchronously, register the target file descriptor
      with the event loop.  */
   if (target_can_async_p ())
@@ -7873,7 +7927,8 @@ extended_remote_create_inferior_1 (char *exec_file, char *args,
     extended_remote_disable_randomization (disable_randomization);
 
   /* Now restart the remote server.  */
-  if (extended_remote_run (args) == -1)
+  run_worked = extended_remote_run (args) != -1;
+  if (!run_worked)
     {
       /* vRun was not supported.  Fail if we need it to do what the
 	 user requested.  */
@@ -7895,7 +7950,9 @@ extended_remote_create_inferior_1 (char *exec_file, char *args,
       init_wait_for_inferior ();
     }
 
-  add_current_inferior_and_thread ();
+  /* vRun's success return is a stop reply.  */
+  stop_reply = run_worked ? rs->buf : NULL;
+  add_current_inferior_and_thread (stop_reply);
 
   /* Get updated offsets, if the stub uses qOffsets.  */
   get_offsets ();

Re: [PATCH 2/2] Don't query stub if the pid is faked

[Pedro Alves]

On 01/23/2013 08:35 AM, Yao Qi wrote:

> This patch fixes this problem by skipping query to the stub is the pid is faked.

Sorry, that's not correct.

`qAttached:pid'
    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 (...), pid is an integer in hexadecimal format identifying the
    target process. Otherwise, gdb will omit the pid field and the query
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    packet will be simplified as `qAttached'.
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

And in that case, the pid will be faked.
If the target supports the multiprocess extensions, then the T stop
reply will include the "thread:ptid" bit, so my alternative patch
to 1/2 also fixes this.  We could perhaps add some warning, but
I'm thinking it's not worth the bother.

-- 
Pedro Alves

-- 
Pedro Alves

Re: [PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled

[Yao Qi]

On 01/25/2013 01:04 AM, Pedro Alves wrote:
> First, this removes the remote_notice_new_inferior call in
> the case the target doesn't send any expedited registers.
> That is, the call is conditional on "if (stop_reply->regcache)".
>

My intention was to move "remote_notice_new_inferior" before the block 
"if (stop_reply->regcache)".  Don't know why moved it into the block.

> Second, the point of the get_thread_arch_regcache call is to
> pre-fill the regcache with the expedite register values, before
> any other code needs one of the likely registers in the expedited
> set (usually PC, SP, FP), thus saving a roundtrip.
> I haven't checked if notice_new_inferior (the core function) ends
> needing to fetch up registers; it's possible it ends up fetching
> the whole g set anyway, but still, it doesn't feel right.

So the rationale here is to prefer to use expedite registers in the stop 
reply, and postpone or even avoid fetching the whole registers by 'g' 
packet, right?

>
> This means that if we have a "Txx thread: ptid" reply, then we
> don't really need qC...  That's what the patch below does.
> The patch still tries qC first, but I'm thinking we can flip that
> around, and only try qC if the stop reply didn't include a thread.

Why don't we do in this way?

>   /* Query the remote target for which is the current thread/process,
>      add it to our tables, and update INFERIOR_PTID.  The caller is
>      responsible for setting the state such that the remote end is ready
> -   to return the current thread.  */
> +   to return the current thread.
> +
> +   This function is called after handling the '?' or 'vRun' packets,
> +   whose response is a stop reply from which we can also try
> +   extracting the thread.  If the target doesn't support the explicit
> +   qC query, we infer the current thread from that stop reply, passed
> +   in in WAIT_STATUS, which may be NULL.  */

redundant "in"?

The patch looks right to me.  Thanks.

-- 
Yao (齐尧)

Re: [PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled

[Pedro Alves]

On 01/25/2013 10:42 AM, Yao Qi wrote:

> So the rationale here is to prefer to use expedite registers in the stop reply, and postpone or even avoid fetching the whole registers by 'g' packet, right?

Right.

>> The patch still tries qC first, but I'm thinking we can flip that
>> around, and only try qC if the stop reply didn't include a thread.
> 
> Why don't we do in this way?

I've made it do that now.

>> +
>> +   This function is called after handling the '?' or 'vRun' packets,
>> +   whose response is a stop reply from which we can also try
>> +   extracting the thread.  If the target doesn't support the explicit
>> +   qC query, we infer the current thread from that stop reply, passed
>> +   in in WAIT_STATUS, which may be NULL.  */
> 
> redundant "in"?

Sounds right to me as is.  Read as, the stop reply is "passed in".
But where?, "in WAITSTATUS".  I'll change it to something else if
a native speaker prefers it.


> The patch looks right to me.  Thanks.

Okay great, below's what I checked in.

-- 
Pedro Alves

2013-01-25  Pedro Alves  <palves@redhat.com>

	* remote.c (stop_reply_extract_thread): New.
	(add_current_inferior_and_thread): New parameter 'wait_status'.
	Handle it.
	(remote_start_remote): Pass wait status to
	add_current_inferior_and_thread.
	(extended_remote_run): Update comment.
	(extended_remote_create_inferior_1): Pass wait status to
	add_current_inferior_and_thread.
---
 gdb/remote.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 7ea9597..18fe61d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3224,22 +3224,77 @@ send_interrupt_sequence (void)
 		    interrupt_sequence_mode);
 }
 
+
+/* If STOP_REPLY is a T stop reply, look for the "thread" register,
+   and extract the PTID.  Returns NULL_PTID if not found.  */
+
+static ptid_t
+stop_reply_extract_thread (char *stop_reply)
+{
+  if (stop_reply[0] == 'T' && strlen (stop_reply) > 3)
+    {
+      char *p;
+
+      /* Txx r:val ; r:val (...)  */
+      p = &stop_reply[3];
+
+      /* Look for "register" named "thread".  */
+      while (*p != '\0')
+	{
+	  char *p1;
+
+	  p1 = strchr (p, ':');
+	  if (p1 == NULL)
+	    return null_ptid;
+
+	  if (strncmp (p, "thread", p1 - p) == 0)
+	    return read_ptid (++p1, &p);
+
+	  p1 = strchr (p, ';');
+	  if (p1 == NULL)
+	    return null_ptid;
+	  p1++;
+
+	  p = p1;
+	}
+    }
+
+  return null_ptid;
+}
+
 /* Query the remote target for which is the current thread/process,
    add it to our tables, and update INFERIOR_PTID.  The caller is
    responsible for setting the state such that the remote end is ready
-   to return the current thread.  */
+   to return the current thread.
+
+   This function is called after handling the '?' or 'vRun' packets,
+   whose response is a stop reply from which we can also try
+   extracting the thread.  If the target doesn't support the explicit
+   qC query, we infer the current thread from that stop reply, passed
+   in in WAIT_STATUS, which may be NULL.  */
 
 static void
-add_current_inferior_and_thread (void)
+add_current_inferior_and_thread (char *wait_status)
 {
   struct remote_state *rs = get_remote_state ();
   int fake_pid_p = 0;
-  ptid_t ptid;
+  ptid_t ptid = null_ptid;
 
   inferior_ptid = null_ptid;
 
-  /* Now, if we have thread information, update inferior_ptid.  */
-  ptid = remote_current_thread (inferior_ptid);
+  /* Now, if we have thread information, update inferior_ptid.  First
+     if we have a stop reply handy, maybe it's a T stop reply with a
+     "thread" register we can extract the current thread from.  If
+     not, ask the remote which is the current thread, with qC.  The
+     former method avoids a roundtrip.  Note we don't use
+     remote_parse_stop_reply as that makes use of the target
+     architecture, which we haven't yet fully determined at this
+     point.  */
+  if (wait_status != NULL)
+    ptid = stop_reply_extract_thread (wait_status);
+  if (ptid_equal (ptid, null_ptid))
+    ptid = remote_current_thread (inferior_ptid);
+
   if (!ptid_equal (ptid, null_ptid))
     {
       if (!remote_multi_process_p (rs))
@@ -3400,7 +3455,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
       /* Let the stub know that we want it to return the thread.  */
       set_continue_thread (minus_one_ptid);
 
-      add_current_inferior_and_thread ();
+      add_current_inferior_and_thread (wait_status);
 
       /* init_wait_for_inferior should be called before get_offsets in order
 	 to manage `inserted' flag in bp loc in a correct state.
@@ -7836,7 +7891,7 @@ extended_remote_run (char *args)
 
   if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vRun]) == PACKET_OK)
     {
-      /* We have a wait response; we don't need it, though.  All is well.  */
+      /* We have a wait response.  All is well.  */
       return 0;
     }
   else if (remote_protocol_packets[PACKET_vRun].support == PACKET_DISABLE)
@@ -7863,6 +7918,10 @@ static void
 extended_remote_create_inferior_1 (char *exec_file, char *args,
 				   char **env, int from_tty)
 {
+  int run_worked;
+  char *stop_reply;
+  struct remote_state *rs = get_remote_state ();
+
   /* If running asynchronously, register the target file descriptor
      with the event loop.  */
   if (target_can_async_p ())
@@ -7873,7 +7932,8 @@ extended_remote_create_inferior_1 (char *exec_file, char *args,
     extended_remote_disable_randomization (disable_randomization);
 
   /* Now restart the remote server.  */
-  if (extended_remote_run (args) == -1)
+  run_worked = extended_remote_run (args) != -1;
+  if (!run_worked)
     {
       /* vRun was not supported.  Fail if we need it to do what the
 	 user requested.  */
@@ -7895,7 +7955,9 @@ extended_remote_create_inferior_1 (char *exec_file, char *args,
       init_wait_for_inferior ();
     }
 
-  add_current_inferior_and_thread ();
+  /* vRun's success return is a stop reply.  */
+  stop_reply = run_worked ? rs->buf : NULL;
+  add_current_inferior_and_thread (stop_reply);
 
   /* Get updated offsets, if the stub uses qOffsets.  */
   get_offsets ();

Re: [PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled

[Pedro Alves]

On 01/25/2013 05:29 PM, Pedro Alves wrote:
> On 01/25/2013 10:42 AM, Yao Qi wrote:


>>> +
>>> +   This function is called after handling the '?' or 'vRun' packets,
>>> +   whose response is a stop reply from which we can also try
>>> +   extracting the thread.  If the target doesn't support the explicit
>>> +   qC query, we infer the current thread from that stop reply, passed
>>> +   in in WAIT_STATUS, which may be NULL.  */
>>
>> redundant "in"?
> 
> Sounds right to me as is.  Read as, the stop reply is "passed in".
> But where?, "in WAITSTATUS".  I'll change it to something else if
> a native speaker prefers it.

Actually, never mind that... I've updated the other comment to
mention we now try T first, but forgot to update that comment
above.  Applied.

-- 
Pedro Alves

Fix add_current_inferior_and_thread's describing comment.

add_current_inferior_and_thread tries the stop reply first.  Tweak its
comment to reflect reality.

2013-01-25  Pedro Alves  <palves@redhat.com>

	* remote.c (add_current_inferior_and_thread): Tweak comment.

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c
+++ src/gdb/remote.c
@@ -3268,10 +3268,10 @@ stop_reply_extract_thread (char *stop_re
    to return the current thread.

    This function is called after handling the '?' or 'vRun' packets,
-   whose response is a stop reply from which we can also try
-   extracting the thread.  If the target doesn't support the explicit
-   qC query, we infer the current thread from that stop reply, passed
-   in in WAIT_STATUS, which may be NULL.  */
+   whose response is a stop reply from which we can try extracting the
+   current thread, so we try that first.  WAIT_STATUS is that stop
+   reply.  It may be NULL if a stop reply is not
+   available/applicable.  */

 static void
 add_current_inferior_and_thread (char *wait_status)