Re: [PATCHv5 05/11] gdbserver/x86: move no-xml code earlier in x86_linux_read_description

[Luis Machado <luis.machado@arm.com>]

On 5/7/24 16:43, Andrew Burgess wrote:
> Luis Machado <luis.machado@arm.com> writes:
> 
>> On 4/29/24 15:34, Willgerodt, Felix wrote:
>>>> -----Original Message-----
>>>> From: Andrew Burgess <aburgess@redhat.com>
>>>> Sent: Freitag, 26. April 2024 17:02
>>>> To: gdb-patches@sourceware.org
>>>> Cc: Andrew Burgess <aburgess@redhat.com>; Willgerodt, Felix
>>>> <felix.willgerodt@intel.com>; John Baldwin <jhb@FreeBSD.org>
>>>> Subject: [PATCHv5 05/11] gdbserver/x86: move no-xml code earlier in
>>>> x86_linux_read_description
>>>>
>>>> This commit is part of a series that aims to share more of the x86
>>>> target description reading/generation code between GDB and gdbserver.
>>>>
>>>> There are a huge number of similarities between the code in
>>>> gdbserver's x86_linux_read_description function and GDB's
>>>> x86_linux_nat_target::read_description function, and it is this
>>>> similarity that I plan, in a later commit, to share between GDB and
>>>> gdbserver.
>>>>
>>>> However, one thing that is different in x86_linux_read_description is
>>>> the code inside the '!use_xml' block.  This is the code that handles
>>>> the case where gdbserver is not allowed to send an XML target
>>>> description back to GDB.  In this case gdbserver uses some predefined,
>>>> fixed, target descriptions.
>>>>
>>>> First, it's worth noting that I suspect this code is not tested any
>>>> more.  I couldn't find anything in the testsuite that tries to disable
>>>> XML target description support.  And the idea of having a single
>>>> "fixed" target description really doesn't work well when we think
>>>> about all the various x86 extensions that exist.  Part of me would
>>>> like to rip out the no-xml support in gdbserver (at least for x86),
>>>> and if a GDB connects that doesn't support XML target descriptions,
>>>> gdbserver can just give an error and drop the connection.  GDB has
>>>> supported XML target descriptions for 16 years now, I think it would
>>>> be reasonable for our shipped gdbserver to drop support for the old
>>>> way of doing things.
>>>
>>> Interesting. I for one would +1 this. Slightly related:
>>> I wonder if anyone really builds GDB without libexpat anymore and
>>> if we couldn't even think of making it mandatory. (Which doesn't
>>> mean dropping the ball on supporting stubs without XML.)
>>
>> I'd support making xml descriptions mandatory for some targets. AArch64 and
>> 32-bit Arm both rely heavily on xml description for feature discovery.
> 
> I might be wrong, but I don't think Felix was even suggesting we need to
> go that far.  I think the suggestion was just that GDB itself should
> require libexpat, which would mean we have the potential to read XML
> target descriptions.

Sorry, I might've read too much into it. But making libexpat a requirement for gdb
would be a good move in my opinion, nonetheless. Cross-building gdb sometimes yields a gdb
with no libexpat and that is not very useful for AArch64/Arm debugging for instance.

> 
> This wouldn't mean that targets would be required to send XML target
> descriptions.> 
> My comment was specifically that x86 gdbserver should drop its no XML
> support, and if a user tries to connect with a no XML GDB then gdbserver
> would reject the connection.

Understood.

> 
> Moving to make libexpat a requirement would help in this case as a user
> would no longer be able to build a GDB which couldn't connect to an x86
> gdbserver.
> 
>> Off the top of my head, I think very old probes and the Linux Kernel's kgdb don't
>> support xml. I'm not sure how often kgdb gets used though, but falling back to a
>> default description should work for AArch64 and 32-bit Arm.
> 
> I don't believe building GDB with libexpat support breaks the ability to
> connect to a target which doesn't support XML descriptions (if it does
> then that's a bug), so hopefully nobody else would be
> inconvenienced... except for those folk who are building GDB without
> libexpat support.

Makes sense. Thanks for clarifying.

> 
> I've added making these changes to my todo list, but I don't want to add
> them to this series.
> 
> Thanks,
> Andrew
>