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

Yao Qi
2013-01-25T10:44:22Z

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 (é½å°§)