[PATCH 1/2] Fix error when GDB connects to GDBserver with qC disabled
Pedro Alves
palves@redhat.com
Fri Jan 25 17:30:00 GMT 2013
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 ();
More information about the Gdb-patches
mailing list