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.

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


base-commit: 4befded43f524d0840bb88fff7b77415b73a3851
  

On 9/27/23 5:15 PM, Simon Marchi wrote:
> 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.
> 
> Change-Id: If64f30307f3a2e5ca3e1fd1cb7379ea840805a85
> ---
>   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 9fdc572356ab..5ae014af4f49 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.  */
> 
> base-commit: 4befded43f524d0840bb88fff7b77415b73a3851

Reviewed-By: John Baldwin <jhb@FreeBSD.org>

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.
  

On 9/27/23 16:20, John Baldwin wrote:
> On 9/27/23 5:15 PM, Simon Marchi wrote:
>> 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.
>>
>> Change-Id: If64f30307f3a2e5ca3e1fd1cb7379ea840805a85
>> ---
>>   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 9fdc572356ab..5ae014af4f49 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.  */
>>
>> base-commit: 4befded43f524d0840bb88fff7b77415b73a3851
> 
> Reviewed-By: John Baldwin <jhb@FreeBSD.org>
> 
> 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.

Thanks, will push.  I'll copy paste that note in the commit message for
posterity, it might help people (including me) in the future.

Simon