[PATCHv6,0/9] x86/Linux Target Description Changes

Message ID cover.1715185683.git.aburgess@redhat.com
Headers
Series x86/Linux Target Description Changes |

Message

Andrew Burgess May 8, 2024, 4:46 p.m. UTC
  In v6:

  - I merged some of the smaller patches which had been approved, and
    which were only loosely connected to this series, i.e. they were
    refactoring, or trivial cleanup patches.

  - I've reordered some of the remaining patches, moving the smaller
    patches towards the start of the series, these are largely
    unchanged and have mostly already been reviewed and/or approved.
    If there's no negative feedback for these smaller patches in this
    new order then I'll likely merge some more of these smaller ones
    to try and get the size of this series down.

  - Patches #1, #2, #3 are pretty much unchaged, and are either
    already approved or have at least been reviewed.

  - Patch #4 is new, though this is pretty similar to patch #3, I
    don't expect any problems from this patch.

  - Patch #5 has an error restored that Felix pointed out I'd deleted
    by mistake.  The error was, as Felix commented, restored (in a
    different way) in a later patch anyway, but dropping it in this
    patch was a mistake.

  - Patch #6 has already been approved.

  - Patch #7 has had some changes after feedback from Felix, the
    x86_linux_tdesc_for_tid function has dropped its callback
    argument, and we now take two pointers in which the function
    caches two different pieces of state.  I think this is actually a
    huge improvement, thanks Felix!

    I've also removed passing have_ptrace_getfpxregs by pointer to
    this function.  Instead, after patch #4, we can now access
    have_ptrace_getfpxregs as a global, just like we do for
    have_ptrace_getregset, this is another great improvement I think.

    I've also fixed a bug in x86_linux_tdesc_for_tid which was present
    in the original code, if we're NOT on x86-64, and we do have
    HAVE_PTRACE_GETFPXREGS defined then we will now always call
    i386_linux_read_description.  Previously, we'd only call
    i386_linux_read_description when have_ptrace_getfpxregs was
    TRIBOOL_UNKNOWN, which was the first time x86_linux_tdesc_for_tid
    was called.

  - Patch #8, I've renamed some of the data structures, and reworded
    some of the comments, to try and make it clearer that the code
    added in this commit is all about checking xstate (xcr0) feature
    bits, and is not claiming to represent all possible state which
    might ever need to be checked when creating a tdesc.  Felix
    correctly pointed out that the previous naming / commenting was
    rather misleading, and though I believe the code was (and is)
    doing the right thing, the comments gave the impression that
    things had been overlooked.

  - Patch #9, the core ideas of this patch are unchanged.  I've
    reworked the commit message to try and explain myself better.

In v5:

  - Felix pointed out that building gdbserver with the '-m32' flag on
    an x86-64 host would fail.  This is fixed in V5 with the addition
    of patch #4.  This patch moves the have_ptrace_getfpxregs global
    into the gdb/nat/ directory and fixes the includes so that the
    the declaration is seen where needed,

  - I've rebased onto a slightly later commit.

In v4:

  - I tried merging V3, but it turned out I broke pretty much
    everything that wasn't x86 based when configured with
    --enable-targets=all,

  - The problem was a failure to correctly split the shared code
    between the gdb/arch/ and gdb/nat/ directories, as a consequence,
    code which is needed on a non x86 based host to support x86 based
    targets wasn't available to the compilation, and the build failed,

  - In V4 I've gone through every patch and resplit the code in a way
    which I now believe is correct, I've done the following tests:

    + On a non x86 host I've built GDB to support only the current
    host as a target, to support all targets, and to support x86-64
    and i386 linux targets,

    + On an i386 virtual machine I built GDB only for the host as a
    target, and for all targets.  I regression tested the all targets
    build for unix, native-gdbserver, and native-extended-gdbserver,

    + On an x86-64 machine I've built GDB for only the current host as
    a target, and for all targets.  I regression tested the all targets
    build for unix, native-gdbserver, and native-extended-gdbserver.

  - Only patches 6, 8, and 10 require significant review.  All of the
    other patches are pretty trivial (though reviews always welcome).

  - I think there's more improvements that can be made to the x86
    target description creation/lookup/caching.  This series only
    changes the Linux lookup, and we still cache i386/amd64/x32
    separately.

    In the future I think we can merge all x86 target description
    caching into a single data structure, this would be for all OS
    variants and all ABI variants.

    Though making that "grand unification" will certainly require some
    of the code in this series to change, I think the bulk of it will
    remain, and trying to do everything in one series is just going to
    result in an even larger series.  I'd prefer to get these first
    patches merged, then come back to build on this work once this is
    merged and we know there's no problems with it.

In v3:

  - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
    I've resolved, but am unable to test.  Reposting so the author of
    that other commit can validate.

  - Initial testing looks good.  Full tests are still running.

In v2:

  - Rebase to current upstream/master, no merge conflicts,

  - Retested.

---

Andrew Burgess (9):
  gdb/gdbserver: share I386_LINUX_XSAVE_XCR0_OFFSET definition
  gdbserver/x86: move no-xml code earlier in x86_linux_read_description
  gdb/x86: move have_ptrace_getfpxregs global into gdb/nat directory
  gdb/x86: move have_ptrace_getregset global into gdb/nat directory
  gdb/x86: move reading of cs and ds state into gdb/nat directory
  gdb: move xcr0 == 0 check into i386_linux_core_read_description
  gdb/gdbserver: share some code relating to target description creation
  gdbserver: update target description creation for x86/linux
  gdb/gdbserver: share x86/linux tdesc caching

 gdb/Makefile.in                               |   9 +-
 gdb/amd64-linux-tdep.c                        |  32 +--
 gdb/amd64-linux-tdep.h                        |   6 -
 gdb/arch/amd64-linux-tdesc.c                  |  61 ++++
 gdb/arch/amd64-linux-tdesc.h                  |  30 ++
 gdb/arch/i386-linux-tdesc.c                   |  51 ++++
 .../i386-linux-tdesc.h}                       |  17 +-
 gdb/arch/x86-linux-tdesc-features.c           | 267 ++++++++++++++++++
 gdb/arch/x86-linux-tdesc-features.h           |  62 ++++
 gdb/arch/x86-linux-tdesc.h                    |  37 +++
 gdb/configure.nat                             |   8 +-
 gdb/configure.tgt                             |  11 +-
 gdb/i386-linux-nat.c                          |  26 +-
 gdb/i386-linux-tdep.c                         |  42 +--
 gdb/i386-linux-tdep.h                         |  23 --
 gdb/linux-nat.c                               |   3 -
 gdb/linux-nat.h                               |   3 -
 gdb/nat/x86-linux-tdesc.c                     | 128 +++++++++
 gdb/nat/x86-linux-tdesc.h                     |  54 ++++
 gdb/nat/x86-linux.c                           |  59 ++++
 gdb/nat/x86-linux.h                           |  45 +++
 gdb/x86-linux-nat.c                           | 120 +-------
 gdbserver/configure.srv                       |  11 +
 gdbserver/i387-fp.cc                          |   9 +-
 gdbserver/i387-fp.h                           |   4 +-
 gdbserver/linux-amd64-ipa.cc                  |  46 +--
 gdbserver/linux-i386-ipa.cc                   |  26 +-
 gdbserver/linux-low.cc                        |   3 -
 gdbserver/linux-low.h                         |   2 -
 gdbserver/linux-x86-low.cc                    | 208 +++++---------
 gdbserver/linux-x86-tdesc.cc                  | 142 +---------
 gdbserver/linux-x86-tdesc.h                   |  56 ----
 gdbsupport/x86-xstate.h                       |  20 ++
 33 files changed, 980 insertions(+), 641 deletions(-)
 create mode 100644 gdb/arch/amd64-linux-tdesc.c
 create mode 100644 gdb/arch/amd64-linux-tdesc.h
 create mode 100644 gdb/arch/i386-linux-tdesc.c
 rename gdb/{i386-linux-nat.h => arch/i386-linux-tdesc.h} (64%)
 create mode 100644 gdb/arch/x86-linux-tdesc-features.c
 create mode 100644 gdb/arch/x86-linux-tdesc-features.h
 create mode 100644 gdb/arch/x86-linux-tdesc.h
 create mode 100644 gdb/nat/x86-linux-tdesc.c
 create mode 100644 gdb/nat/x86-linux-tdesc.h
 delete mode 100644 gdbserver/linux-x86-tdesc.h


base-commit: 868883583e7520ff1bd99fcb224d2b33a990edff