Re: [PATCH v3 5/8] gdbserver: Transmit target description ID in thread list and stop reply

Thiago Jung Bauermann via Gdb-patches
2023-02-02T20:36:54Z

Simon Marchi <simark@simark.ca> writes:

> On 2/1/23 07:07, Andrew Burgess via Gdb-patches wrote:
>> Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org>
>> writes:
>>
>>> Now that an inferior thread can have a different target description than
>>> its process, there needs to be a way to communicate this target
>>> description to GDB. So add the concept of a target description ID to the
>>> remote protocol, which is used to reference them and allows them to be
>>> transferred only once over the wire.
>>>
>>> The ID is an unsigned integer, and is sent in the 'T AA n1:r1;n2:r2;...'
>>> stop reply packet as a new 'n:r' pair, where n is "tdesc" and "r" is an
>>> unsigned integer containing the ID.
>>>
>>> It is also sent in the threads list XML in the response of a
>>> qXfer:threads:read request. The ID is sent as a new "tdesc" attribute of
>>> the <thread> node.
>>>
>>> To request the target description XML of a given ID, GDB sends the
>>> qXfer:features:read request with "target-id-%u.xml" as the annex, where %u
>>> is the target description ID.
>>
>> Luis already commented that in some locations the ID is hex, while in
>> others it is not explicitly stated if the value is hex or decimal. I'd
>> like to second that feedback, and suggest that we pick one, and use that
>> consistently throughout.
>>
>> My thinking is that it will be easier to understand remote packet traces
>> if target descriptions are requested using an ID in the safe format as
>> was sent to GDB, and if IDs in different packets match up.
>>
>> Thus, I would suggest we switch to using 'target-id-%x.xml' here, and
>> send the ID as hex in the threads reply packet.
>
> Not that I disagree with you, but I just wanted to note that this
> decimal / hexadecimal mismatch already exists for the core field. It's
> hex in stop replies, decimal in XML. The thread id, however, is always
> hex (the special thread-id syntax).
>
> Regardless how this new field is formatted, it would be nice to be
> explicit about the core field too.

I agree it makes more sense to use hex everywhere.

In the case of XML attribute fields, enforcing hex would mean adding a
base parameter to xml_parse_unsigned_integer (which currently accepts
decimal, octal and hex). I'll do that.

In the case of the core field, to avoid breaking existing stubs it's
probably better to document that it can accept any of the three bases.

--
Thiago