gdb/testsuite: fix gdb.arch/amd64-init-x87-values.exp on AMD CPUs

Message ID 20230908022722.430741-1-simon.marchi@efficios.com
State New
Headers
Series gdb/testsuite: fix gdb.arch/amd64-init-x87-values.exp on AMD CPUs |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Simon Marchi Sept. 8, 2023, 2:26 a.m. UTC
  I see the following failure when running this test on an AMD machine:

    p/x $fioff^M
    $24 = 0x0^M
    (gdb) FAIL: gdb.arch/amd64-init-x87-values.exp: check_x87_regs_around_init: check post FLD1 value of $fioff

The register that GDB calls fioff normally contains the address of the
last instruction executed by the x87 unit.  It is available through the
FSAVE/FXSAVE/XSAVE instructions, at offset 0x8 of the FSAVE/FXSAVE/XSAVE
area.  You can read about it in the Intel manual [1] at section "10.5.1
FXSAVE Area" (and equivalent sections for FSAVE and XSAVE) or in the AMD
manual [2] at section "11.4.4 Saving Media and x87 Execution Unit
State".

The test therefore expects that after executing the FLD1 instruction,
the fioff register contains the address of the FLD1 instruction.

However, the FXSAVE and XSAVE instructions (which the kernel uses to
dump x87 register state which it provides GDB through ptrace) behave
differently on AMD CPUs.  In section "11.4.4.3 FXSAVE and FXRSTOR
Instructions" of the AMD manual, we read:

    The FXSAVE and FXRSTOR instructions save and restore the entire
    128-bit media, 64-bit media, and x87 state. These instructions
    usually execute faster than FSAVE/FNSAVE and FRSTOR because they do
    not normally save and restore the x87 exception pointers
    (last-instruction pointer, last data-operand pointer, and last
    opcode). The only case in which they do save the exception pointers
    is the relatively rare case in which the exception-summary bit in
    the x87 status word (FSW.ES) is set to 1, indicating that an
    unmasked exception has occurred.

So, unless a floating point exception happened and that exception is
unmasked in the x87 FPU control register (which isn't by default on
Linux, from what I saw), the "last instruction address" register (or
fioff as GDB calls it) will always be 0 on an AMD CPU.

For this reason, I think it's fine to change the test to accept the
value 0 - that's just how the processor works.

I toyed with the idea of changing the test program to make it so the CPU
would generate a non-zero fioff.  That is by unmasking an FPU exception
and executing an instruction to raise that kind exception.  It worked,
but then I would have to change the test more extensively, and it didn't
seem to be worth it.

[1] https://cdrdv2.intel.com/v1/dl/getContent/671200
[2] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf

Change-Id: If2e1d932f600ca01b15f30b14b8d38bf08a3e00b
---
 gdb/testsuite/gdb.arch/amd64-init-x87-values.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 3c11aea0297a3f038e552eee424c214dc5a2c4bc
  

Comments

John Baldwin Sept. 8, 2023, 8:38 p.m. UTC | #1
On 9/7/23 7:26 PM, Simon Marchi via Gdb-patches wrote:
> I see the following failure when running this test on an AMD machine:
> 
>      p/x $fioff^M
>      $24 = 0x0^M
>      (gdb) FAIL: gdb.arch/amd64-init-x87-values.exp: check_x87_regs_around_init: check post FLD1 value of $fioff
> 
> The register that GDB calls fioff normally contains the address of the
> last instruction executed by the x87 unit.  It is available through the
> FSAVE/FXSAVE/XSAVE instructions, at offset 0x8 of the FSAVE/FXSAVE/XSAVE
> area.  You can read about it in the Intel manual [1] at section "10.5.1
> FXSAVE Area" (and equivalent sections for FSAVE and XSAVE) or in the AMD
> manual [2] at section "11.4.4 Saving Media and x87 Execution Unit
> State".
> 
> The test therefore expects that after executing the FLD1 instruction,
> the fioff register contains the address of the FLD1 instruction.
> 
> However, the FXSAVE and XSAVE instructions (which the kernel uses to
> dump x87 register state which it provides GDB through ptrace) behave
> differently on AMD CPUs.  In section "11.4.4.3 FXSAVE and FXRSTOR
> Instructions" of the AMD manual, we read:
> 
>      The FXSAVE and FXRSTOR instructions save and restore the entire
>      128-bit media, 64-bit media, and x87 state. These instructions
>      usually execute faster than FSAVE/FNSAVE and FRSTOR because they do
>      not normally save and restore the x87 exception pointers
>      (last-instruction pointer, last data-operand pointer, and last
>      opcode). The only case in which they do save the exception pointers
>      is the relatively rare case in which the exception-summary bit in
>      the x87 status word (FSW.ES) is set to 1, indicating that an
>      unmasked exception has occurred.
> 
> So, unless a floating point exception happened and that exception is
> unmasked in the x87 FPU control register (which isn't by default on
> Linux, from what I saw), the "last instruction address" register (or
> fioff as GDB calls it) will always be 0 on an AMD CPU.
> 
> For this reason, I think it's fine to change the test to accept the
> value 0 - that's just how the processor works.
> 
> I toyed with the idea of changing the test program to make it so the CPU
> would generate a non-zero fioff.  That is by unmasking an FPU exception
> and executing an instruction to raise that kind exception.  It worked,
> but then I would have to change the test more extensively, and it didn't
> seem to be worth it.
> 
> [1] https://cdrdv2.intel.com/v1/dl/getContent/671200
> [2] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
> 
> Change-Id: If2e1d932f600ca01b15f30b14b8d38bf08a3e00b
> ---
>   gdb/testsuite/gdb.arch/amd64-init-x87-values.exp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp b/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
> index eb0e35ff7f7a..0738fc4e4745 100644
> --- a/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
> @@ -99,7 +99,7 @@ proc_with_prefix check_x87_regs_around_init {} {
>   				     "fstat" "0x3800" \
>   				     "ftag" "0x3fff" \
>   				     "fiseg" "0x0" \
> -				     "fioff" $addr \
> +				     "fioff" "($addr|0x0)" \
>   				     "foseg" "0x0" \
>   				     "fooff" "0x0" \
>   				     "fop" "0x0" \
> 
> base-commit: 3c11aea0297a3f038e552eee424c214dc5a2c4bc

This seems fine to me.  It might be dubious to be testing anything about the segment
and offset registers at this point anyway.  For amd64 you get a slightly different
layout where the segment registers don't exist and the offsets are 64-bits which
I don't think we currently handle correctly btw (we still read the segments from
bits 32:47 of the offsets and only read the low 32-bits of the offsets).

Reviewed-by: John Baldwin <jhb@FreeBSD.org>
  
Simon Marchi Sept. 11, 2023, 1:56 a.m. UTC | #2
> This seems fine to me.  It might be dubious to be testing anything about the segment
> and offset registers at this point anyway.  For amd64 you get a slightly different
> layout where the segment registers don't exist and the offsets are 64-bits which
> I don't think we currently handle correctly btw (we still read the segments from
> bits 32:47 of the offsets and only read the low 32-bits of the offsets).
Indeed, that's what I saw when fiddling with this, fioff is just the
lower 32 bits.  When testing on Intel, the test happens to pass because
it's compiled with -static, so the PC is in the 0x400000 range.  I don't
plan on working on this further, I just wanted to fix the failure I was
witnessing.

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

Thanks, will push.

Simon
  

Patch

diff --git a/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp b/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
index eb0e35ff7f7a..0738fc4e4745 100644
--- a/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
+++ b/gdb/testsuite/gdb.arch/amd64-init-x87-values.exp
@@ -99,7 +99,7 @@  proc_with_prefix check_x87_regs_around_init {} {
 				     "fstat" "0x3800" \
 				     "ftag" "0x3fff" \
 				     "fiseg" "0x0" \
-				     "fioff" $addr \
+				     "fioff" "($addr|0x0)" \
 				     "foseg" "0x0" \
 				     "fooff" "0x0" \
 				     "fop" "0x0" \