Re: [PATCH 2/2] gdb, remote: fix set_thread () in start_remote ()

[Simon Marchi <simark@simark.ca>]

On 2025-08-14 20:29, Thiago Jung Bauermann wrote:
> Simon Marchi <simark@simark.ca> writes:
> 
>> On 8/14/25 12:29 AM, Thiago Jung Bauermann wrote:
>>> Markus Metzger <markus.t.metzger@intel.com> writes:
>>>> -      set_continue_thread (minus_one_ptid);
>>>> +      set_general_thread (any_thread_ptid);
>>>>  
>>>>        if (thread_count (this) == 0)
>>>>  	{
>>>
>>> Also, your explanation implies that the doc comment in
>>> remote_target::set_thread isn't quite right. What about changing it to
>>> something along the following lines?
>>>
>>> -/* If PTID is MAGIC_NULL_PTID, don't set any thread.  If PTID is
>>> -   MINUS_ONE_PTID, set the thread to -1, so the stub returns the
>>> -   thread.  If GEN is set, set the general thread, if not, then set
>>> -   the step/continue thread.  */
>>> +/* If PTID is MAGIC_NULL_PTID or ANY_THREAD_PTID, set the thread to 0.
>>> +   If PTID is MINUS_ONE_PTID, set the thread to -1.  If GEN is set, set
>>> +   the general thread, if not, then set the step/continue thread.  If
>>> +   the thread is 0 or -1 and GEN is set, the stub returns the thread in
>>> +   the qC packet reply.  */
>>
>> The parts "set the thread to 0" and "set the thread to -1" seem not very
>> useful to me.  What does that mean concretely?  It would be more useful
>> to explain what that instruct the remote target to do.
>>
>> My understanding is that passing either MAGIC_NULL_PTID or
>> ANY_THREAD_PTID will result in sending `Hg0` or `Hc0`.  Passing
>> MINUS_ONE_PTID will result in sending `Hg-1` or `Hc-1`.
>>
>> On the other side, all of these will result in variable THREAD_ID being
>> set to null_ptid.  For Hg, we'll arbitrarily pick the first thread.  For
>> Hc, we'll set cs.cont_thread to null_ptid.
>>
>> Does that sound right?
> 
> For gdbserver, yes. My main doubt is whether we can assume other stubs
> "out there" behave the same way or do they treat Hg0 and Hg-1
> differently? The comment and code in remote_target::set_thread assume
> the latter, so is that a bug?

About the other stubs: I can't really help you there, I think the answer
is that we'll never really know.

Can you clarify what you think might be a bug?

Simon