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

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Dienstag, 7. Mai 2024 17:44
> To: Luis Machado <luis.machado@arm.com>; Willgerodt, Felix
> <felix.willgerodt@intel.com>; gdb-patches@sourceware.org
> Cc: John Baldwin <jhb@FreeBSD.org>
> Subject: Re: [PATCHv5 05/11] gdbserver/x86: move no-xml code earlier in
> x86_linux_read_description
> 
> 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. 

You are correct.

> I think the suggestion was just that GDB itself should
> require libexpat, which would mean we have the potential to read XML
> target descriptions.
> 
> 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.
> 
> 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.

One immediate concern in my mind would be testability for no-xml stubs
(without having looked at the current testing situation). Anyway, this is not
really a topic for this series, just an interesting side discussion.

Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928