diff mbox series

gdbserver: i387_cache_to_xsave: fix copy dest of zmm registers

Message ID 20230901181209.41391-1-simon.marchi@efficios.com
State New
Headers
Series gdbserver: i387_cache_to_xsave: fix copy dest of zmm registers |

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 warning Patch is already merged
linaro-tcwg-bot/tcwg_gdb_check--master-arm warning Patch is already merged

Commit Message

Simon Marchi Sept. 1, 2023, 6:12 p.m. UTC 
  On a machine with AVX512 support (AMD EPYC 9634), I see these failures:

    $ make check TESTS="gdb.arch/i386-avx512.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
    ...
    FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[16] after writing ZMM regs
    FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[17] after writing ZMM regs
    FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[18] after writing ZMM regs
    ...

The problem can be reduced to:

    (gdb) print $zmm16.v8_int64
    $1 = {0, 0, 0, 0, 0, 0, 0, 0}
    (gdb) print $zmm16.v8_int64 = {11,22,33,44,55,66,77,88}
    $2 = {11, 22, 33, 44, 55, 66, 77, 88}
    (gdb) print $zmm16.v8_int64
    $3 = {11, 22, 33, 44, 55, 66, 77, 88}
    (gdb) step
    5               ++x;
    (gdb) print $zmm16.v8_int64
    $4 = {11, 22, 77, 88, 0, 0, 0, 0}

Writing to the local regcache in GDB works fine, but the writeback to
gdbserver (which happens when resuming / stepping) doesn't work (the
code being stepped doesn't touch AVX registers, so we don't expect the
value of zmm16 to change when stepping).

The problem is on the gdbserver side, the zmmh and ymmh portions of the
zmm register are not memcpied at the right place in the xsave buffer.  Fix
that.  Note now how the two modified memcpy calls match the memcmp calls
just above them.

With this patch, gdb.arch/i386-avx512.exp passes completely for me.

Change-Id: I22c417e0f5e88d4bc635a0f08f8817a031c76433
---
 gdbserver/i387-fp.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 7fdd4fcee1c6698daa8eb5f52017e768b0771503

Comments

John Baldwin Sept. 1, 2023, 9:25 p.m. UTC | #1 
On 9/1/23 11:12 AM, Simon Marchi via Gdb-patches wrote:
> On a machine with AVX512 support (AMD EPYC 9634), I see these failures:
> 
>      $ make check TESTS="gdb.arch/i386-avx512.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
>      ...
>      FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[16] after writing ZMM regs
>      FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[17] after writing ZMM regs
>      FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[18] after writing ZMM regs
>      ...
> 
> The problem can be reduced to:
> 
>      (gdb) print $zmm16.v8_int64
>      $1 = {0, 0, 0, 0, 0, 0, 0, 0}
>      (gdb) print $zmm16.v8_int64 = {11,22,33,44,55,66,77,88}
>      $2 = {11, 22, 33, 44, 55, 66, 77, 88}
>      (gdb) print $zmm16.v8_int64
>      $3 = {11, 22, 33, 44, 55, 66, 77, 88}
>      (gdb) step
>      5               ++x;
>      (gdb) print $zmm16.v8_int64
>      $4 = {11, 22, 77, 88, 0, 0, 0, 0}
> 
> Writing to the local regcache in GDB works fine, but the writeback to
> gdbserver (which happens when resuming / stepping) doesn't work (the
> code being stepped doesn't touch AVX registers, so we don't expect the
> value of zmm16 to change when stepping).
> 
> The problem is on the gdbserver side, the zmmh and ymmh portions of the
> zmm register are not memcpied at the right place in the xsave buffer.  Fix
> that.  Note now how the two modified memcpy calls match the memcmp calls
> just above them.
> 
> With this patch, gdb.arch/i386-avx512.exp passes completely for me.
> 
> Change-Id: I22c417e0f5e88d4bc635a0f08f8817a031c76433
> ---
>   gdbserver/i387-fp.cc | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
> index 4229d60327e6..27f8e2dbb73f 100644
> --- a/gdbserver/i387-fp.cc
> +++ b/gdbserver/i387-fp.cc
> @@ -469,7 +469,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
>   	  if (memcmp (raw, p + 32, 32) != 0)
>   	    {
>   	      xstate_bv |= X86_XSTATE_ZMM;
> -	      memcpy (p, raw, 32);
> +	      memcpy (p + 32, raw, 32);
>   	    }
>   
>   	  /* YMMH sub-register.  */
> @@ -477,7 +477,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
>   	  if (memcmp (raw, p + 16, 16) != 0)
>   	    {
>   	      xstate_bv |= X86_XSTATE_ZMM;
> -	      memcpy (p, raw, 16);
> +	      memcpy (p + 16, raw, 16);
>   	    }
>   
>   	  /* XMM sub-register.  */
> 
> base-commit: 7fdd4fcee1c6698daa8eb5f52017e768b0771503

Nice find, looks good to me.

Reviewed-by: John Baldwin <jhb@FreeBSD.org>
Simon Marchi Sept. 2, 2023, 2:15 a.m. UTC | #2 
> Nice find, looks good to me.
> 
> Reviewed-by: John Baldwin <jhb@FreeBSD.org>

Thanks, pushed.

Simon
diff mbox series

Patch

diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index 4229d60327e6..27f8e2dbb73f 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -469,7 +469,7 @@  i387_cache_to_xsave (struct regcache *regcache, void *buf)
 	  if (memcmp (raw, p + 32, 32) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM;
-	      memcpy (p, raw, 32);
+	      memcpy (p + 32, raw, 32);
 	    }
 
 	  /* YMMH sub-register.  */
@@ -477,7 +477,7 @@  i387_cache_to_xsave (struct regcache *regcache, void *buf)
 	  if (memcmp (raw, p + 16, 16) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM;
-	      memcpy (p, raw, 16);
+	      memcpy (p + 16, raw, 16);
 	    }
 
 	  /* XMM sub-register.  */