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

[Willgerodt, Felix]

> -----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.)

 
> Anyway.... this commit doesn't do that.
>
> What I did notice was that, over time, the '!use_xml' block appears to
> have "drifted" within the x86_linux_read_description function; it's
> now not the first check we do.  Instead we make some ptrace calls and
> return a target description generated based on the result of these
> ptrace calls.  Surely it only makes sense to generate variable target
> descriptions if we can send these back to GDB?
> 
> So in this commit I propose to move the '!use_xml' block earlier in
> the x86_linux_read_description function.
> 
> The benefit of this is that this leaves the later half of
> x86_linux_read_description much more similar to the GDB function
> x86_linux_nat_target::read_description and sets us up for potentially
> sharing code between GDB and gdbserver in a later commit.
> 
> Approved-By: John Baldwin <jhb@FreeBSD.org>
> ---
>  gdbserver/linux-x86-low.cc | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 

Looks fine to me.

Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>

Thanks,
Felix

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928