[binutils-gdb] gdbserver/x86: move no-xml code earlier in x86_linux_read_description

[binutils-gdb] gdbserver/x86: move no-xml code earlier in x86_linux_read_description

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0a7bb97ad2f2fe2d18a442dad265051e34eab13e

commit 0a7bb97ad2f2fe2d18a442dad265051e34eab13e
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Jan 27 09:33:14 2024 +0000

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

Diff:
---
 gdbserver/linux-x86-low.cc | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc
index 04202e355bb..872c3fc69c7 100644
--- a/gdbserver/linux-x86-low.cc
+++ b/gdbserver/linux-x86-low.cc
@@ -891,6 +891,22 @@ x86_linux_read_description (void)
 #endif
     }
 
+  /* If we are not allowed to send an XML target description then we need
+     to use the hard-wired target descriptions.  This corresponds to GDB's
+     default machine for x86.
+
+     This check needs to occur before any returns statements that might
+     generate some alternative target descriptions.  */
+  if (!use_xml)
+    {
+#ifdef __x86_64__
+      if (machine == EM_X86_64)
+	return tdesc_amd64_linux_no_xml.get ();
+      else
+#endif
+	return tdesc_i386_linux_no_xml.get ();
+    }
+
 #if !defined __x86_64__ && defined HAVE_PTRACE_GETFPXREGS
   if (machine == EM_386 && have_ptrace_getfpxregs == -1)
     {
@@ -907,17 +923,6 @@ x86_linux_read_description (void)
     }
 #endif
 
-  if (!use_xml)
-    {
-      /* Don't use XML.  */
-#ifdef __x86_64__
-      if (machine == EM_X86_64)
-	return tdesc_amd64_linux_no_xml.get ();
-      else
-#endif
-	return tdesc_i386_linux_no_xml.get ();
-    }
-
   if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
     {
       uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))];