[binutils-gdb] gdb/x86: use size of XSAVE area of enabled features

[Simon Marchi]

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

commit 054f25955c2b77f6e21073bfdd70a60e9df1ffe7
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Wed Sep 27 12:15:22 2023 -0400

    gdb/x86: use size of XSAVE area of enabled features
    
    Since commit b42405a1594 ("gdb: Update x86 Linux architectures to
    support XSAVE layouts."), the test gdb.base/gcore.exp fails on my AMD
    Ryzen 3700X machine:
    
        FAIL: gdb.base/gcore.exp: corefile restored all registers
    
    The test gets the register state (saves the output of "info
    all-registers"), saves a core with the "gcore" command, loads the core,
    and checks the register state against the one previously saved.  The
    problem is that when reading registers from the core file, the last half
    of ymm registers is unavailable:
    
        (gdb) print $ymm0.v32_int8
        $1 = {0, -77, -23, -9, -1, 127, 0, 0, 0, -77, -23, -9, -1, 127, 0, 0, <unavailable> <repeats 16 times>}
    
    One strange thing with this machine is that the bitset of state
    components supported by XCR0 is 0x207, meaning "x87 | SSE | AVX | PKRU",
    but XCR0 at runtime is 0x7, meaning "x87 | SSE | AVX".  So, PKRU appears
    to be supported by the processor, but disabled by the kernel.  I didn't
    find why yet.
    
    From CPUID leaf EAX=0Dh, ECX=00h, GDB can get:
    
     - from EBX: max size of the XSAVE area required by features currently
       enabled in XCR0.  On my machine, it's 0x340 (832).
     - from ECX: max size of the XSAVE area required by all features
       supported by XCR0.  On my machine, it's 0x380 (896).
    
    At runtime, GDB uses ECX (max size required by all supported features)
    to fill the x86_xsave_layout::sizeof_xsave.  So, when writing the core
    file note for the XSAVE state, it writes a note of size 896, even though
    it doesn't write the PKRU state.  When loading back the core, GDB tries
    to figure out the layout of the XSAVE area based on what features are
    enabled in XCR0 and the size of the note (the size of the XSAVE area).
    Since my combination of XCR0 and size of XSAVE area doesn't match any
    combination known by GDB, GDB falls back to a gdbarch supporting only
    x87 and SSE.
    
    This patch changes GDB to populate the x86_xsave_layout::sizeof_xsave
    field (and consequently the size of the XSAVE state note in core files)
    using EBX, the size of the XSAVE area required by currently enabled
    features in XCR0.  This makes i387_guess_xsave_layout recognize my case
    with this condition:
    
      else if (HAS_AVX (xcr0) && xsave_size == 832)
        {
          /* Intel and AMD CPUs supporting AVX.  */
          layout.avx_offset = 576;
        }
    
    In other words, just as if my machine didn't support PKRU at all.
    
    Another reason why I think this change makes sense is that XSAVE state
    notes in kernel-generated cores on this machine have size 832.  So this
    change makes GDB-generated cores more similar to kernel-generated ones,
    reducing the diversity of XSAVE state notes that GDB needs to be able to
    figure out.
    
    Note that if PKRU was enabled on my machine, then the effective XSAVE
    area size would be 896 bytes.  We would need to add a case in
    i387_guess_xsave_layout for that combination, since there is no
    currently.  But I don't have a way to test that right now, since I don't
    know why PKRU is disabled.
    
    Relevant review note from John Baldwin:
    
      One further note is that the Linux x86 arches use x86_xsave_length()
      to infer ("guess") the size of the XSAVE register set that the Linux
      kernel writes out in core dumps.  On FreeBSD x86 arches, GDB is able
      to query this size directly from the kernel via ptrace.  My use of ECX
      for this guess earlier was just not the best guess.  In the case that
      the kernel enables all of the available features, then ECX and EBX
      have the same values, so this only matters for a system where the
      kernel has enabled a subset of available XSAVE extensions.
    
    Change-Id: If64f30307f3a2e5ca3e1fd1cb7379ea840805a85
    Reviewed-By: John Baldwin <jhb@FreeBSD.org>

Diff:
---
 gdb/nat/x86-xstate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/nat/x86-xstate.c b/gdb/nat/x86-xstate.c
index 9fdc572356a..5ae014af4f4 100644
--- a/gdb/nat/x86-xstate.c
+++ b/gdb/nat/x86-xstate.c
@@ -42,11 +42,11 @@ xsave_feature_offset (uint64_t xcr0, int feature)
 int
 x86_xsave_length ()
 {
-  uint32_t ecx;
+  uint32_t ebx;
 
-  if (!x86_cpuid_count (0xd, 0, nullptr, nullptr, &ecx, nullptr))
+  if (!x86_cpuid_count (0xd, 0, nullptr, &ebx, nullptr, nullptr))
     return 0;
-  return ecx;
+  return ebx;
 }
 
 /* See x86-xstate.h.  */