[PATCH 0/3] Small step in supporting AVX instructions

[PATCH 0/3] Small step in supporting AVX instructions

[Guinevere Larsen]

This patch series is the first, very small, step in supporting AVX and
AVX2 instructions for the record-full target. It is important that we
support it since glibc has been using avx instructions for a long time
(at least fedora 21), so depending on which functions an inferior uses,
they might be very inconvenienced.

Patch 1 adds capability to identify the VEX prefix, but no instruction
support. Patches 2 and 3 add support for a total of 10 instructions,
which covers around 5% of all AVX instructions used by system libraries
in my fedora 39 box.

While this support is extremely minimal, I figured I could propose the
patch series early so it was open for others with more free time could
help contributing to this :)

As for filed bugs, there are 2 that I could find on bugzilla, but both
name a specific instruction (vmovdqa and vmovdqu) which were not added
by this series, so I figured I should only mention it once those are
added.

Guinevere Larsen (3):
  gdb: Start supporting AVX instruction
  gdb/record: add support to vmovd and vmovq instructions
  gdb/record: add support to AVX unpack instructions

 gdb/amd64-tdep.c                              |   3 +-
 gdb/i386-tdep.c                               | 170 ++++++++++++++-
 gdb/i386-tdep.h                               |   2 +
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 127 +++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 197 ++++++++++++++++++
 5 files changed, 496 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp

-- 
2.45.1

[PATCH 1/3] gdb: Start supporting AVX instruction

[Guinevere Larsen]

This patch introduces the information needed to properly identify the
VEX prefix, used to signal an AVX and AVX2 instruction, and introduces
a helper function to handle all AVX instruction, instead of adding to
the 3000 line long recording function.

The new helper also handles unsupported instructions so that the largest
part of the i386_process_record doesn't have to be shifted by 2 spaces,
which made an unreadably big patch file.

The only expected difference added by this patch is a small change to
the unsupported message.

As a note for the future, we don't handle xmm16-31 and ymm16-31 because
those require the EVEX prefix, meaning avx512 support.
---
 gdb/amd64-tdep.c |  3 +-
 gdb/i386-tdep.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
 gdb/i386-tdep.h  |  2 ++
 3 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index df6b882a3fb..27d6e20f90c 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3134,7 +3134,8 @@ static const int amd64_record_regmap[] =
   AMD64_R8_REGNUM, AMD64_R9_REGNUM, AMD64_R10_REGNUM, AMD64_R11_REGNUM,
   AMD64_R12_REGNUM, AMD64_R13_REGNUM, AMD64_R14_REGNUM, AMD64_R15_REGNUM,
   AMD64_RIP_REGNUM, AMD64_EFLAGS_REGNUM, AMD64_CS_REGNUM, AMD64_SS_REGNUM,
-  AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM
+  AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM,
+  AMD64_XMM0_REGNUM, AMD64_YMM0H_REGNUM
 };
 
 /* Implement the "in_indirect_branch_thunk" gdbarch function.  */
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index f1f909e1616..93a0926c4bc 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4637,6 +4637,12 @@ struct i386_record_s
   int rip_offset;
   int popl_esp_hack;
   const int *regmap;
+
+  /* These are used by VEX and XOP prefixes.  */
+  uint8_t map_select;
+  uint8_t vvvv;
+  uint8_t pp;
+  uint8_t l;
 };
 
 /* Parse the "modrm" part of the memory address irp->addr points at.
@@ -4975,6 +4981,31 @@ static int i386_record_floats (struct gdbarch *gdbarch,
   return 0;
 }
 
+/* i386_process_record helper to deal with instructions that start
+   with VEX prefix.  */
+
+static bool
+i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
+		 int opcode, struct gdbarch *gdbarch)
+{
+  switch (opcode)
+    {
+    default:
+      gdb_printf (gdb_stderr,
+		  _("Process record does not support VEX instruction 0x%02x "
+		    "at address %s.\n"),
+		  (unsigned int) (opcode),
+		  paddress (gdbarch, ir->orig_addr));
+      return -1;
+    }
+
+  record_full_arch_list_add_reg (ir->regcache, ir->regmap[X86_RECORD_REIP_REGNUM]);
+  if (record_full_arch_list_add_end ())
+    return -1;
+
+  return 0;
+}
+
 /* Parse the current instruction, and record the values of the
    registers and memory that will be changed by the current
    instruction.  Returns -1 if something goes wrong, 0 otherwise.  */
@@ -4997,6 +5028,7 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
   uint8_t rex_w = -1;
   uint8_t rex_r = 0;
+  bool vex_prefix = false;
 
   memset (&ir, 0, sizeof (struct i386_record_s));
   ir.regcache = regcache;
@@ -5082,6 +5114,53 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
 	  else					/* 32 bit target */
 	    goto out_prefixes;
 	  break;
+	case 0xc4:	/* 3-byte VEX prefixes (for AVX/AVX2 instructions).  */
+	  {
+	    /* The first byte just identifies the VEX prefix.  Data is stored
+	       on the following 2 bytes.  */
+	    uint8_t byte;
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+
+	    rex_r = !((byte >> 7) & 0x1);
+	    ir.rex_x = !((byte >> 6) & 0x1);
+	    ir.rex_b = !((byte >> 5) & 0x1);
+	    ir.map_select = byte & 0x1f;
+	    /* Collect the last byte of the prefix.  */
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+	    rex_w = (byte >> 7) & 0x1;
+	    ir.vvvv = (~(byte >> 3) & 0xf);
+	    ir.l = (byte >> 2) & 0x1;
+	    ir.pp = byte & 0x3;
+	    vex_prefix = true;
+
+	    break;
+	  }
+	case 0xc5:	/* 2-byte VEX prefix for AVX/AVX2 instructions.  */
+	  {
+	    /* The first byte just identifies the VEX prefix.  Data is stored
+	       on the following 2 bytes.  */
+	    uint8_t byte;
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+
+	    /* On the 2-byte versions, these are pre-defined.  */
+	    ir.rex_x = 0;
+	    ir.rex_b = 0;
+	    rex_w = 0;
+	    ir.map_select = 1;
+
+	    rex_r = !((byte >> 7) & 0x1);
+	    ir.vvvv = (~(byte >> 3) & 0xf);
+	    ir.l = (byte >> 2) & 0x1;
+	    ir.pp = byte & 0x3;
+	    vex_prefix = true;
+	    break;
+	  }
 	default:
 	  goto out_prefixes;
 	  break;
@@ -5104,6 +5183,12 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
 
   /* Now check op code.  */
   opcode = (uint32_t) opcode8;
+  if (vex_prefix)
+    {
+      /* If we found the VEX prefix, i386 will either record or warn that
+	 the instruction isn't supported, so we can return the VEX result.  */
+      return i386_record_vex (&ir, rex_w, rex_r, opcode, gdbarch);
+    }
  reswitch:
   switch (opcode)
     {
@@ -8088,9 +8173,11 @@ static const int i386_record_regmap[] =
 {
   I386_EAX_REGNUM, I386_ECX_REGNUM, I386_EDX_REGNUM, I386_EBX_REGNUM,
   I386_ESP_REGNUM, I386_EBP_REGNUM, I386_ESI_REGNUM, I386_EDI_REGNUM,
-  0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0,
+  0, 0, 0, 0,
   I386_EIP_REGNUM, I386_EFLAGS_REGNUM, I386_CS_REGNUM, I386_SS_REGNUM,
-  I386_DS_REGNUM, I386_ES_REGNUM, I386_FS_REGNUM, I386_GS_REGNUM
+  I386_DS_REGNUM, I386_ES_REGNUM, I386_FS_REGNUM, I386_GS_REGNUM,
+  /* xmm0_regnum */ 0, I386_YMM0H_REGNUM
 };
 
 /* Check that the given address appears suitable for a fast
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index a85e0a984a0..1f21f8de06c 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -339,6 +339,8 @@ enum record_i386_regnum
   X86_RECORD_ES_REGNUM,
   X86_RECORD_FS_REGNUM,
   X86_RECORD_GS_REGNUM,
+  X86_RECORD_XMM0_REGNUM,
+  X86_RECORD_YMM0H_REGNUM,
 };
 
 #define I386_NUM_GREGS	16
-- 
2.45.1

[PATCH 2/3] gdb/record: add support to vmovd and vmovq instructions

[Guinevere Larsen]

This commit adds support to the x86_64 AVX instructions vmovd and vmovq.
The programmers manuals for Intel and AMD describe these 2 instructions
as being almost the same, but my local testing, using gcc 13.2 on Fedora
39, showed several differences and inconsistencies.

The instruction is supposed to always use the 3-byte VEX prefix, but I
could only find 2-byte versions. The instructions aren't differentiated
by the VEX.w bit, but by opcodes and VEX.pp.

This patch adds a test with many different uses for both vmovd and
vmovq.
---
 gdb/i386-tdep.c                               |  69 ++++++-
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  |  90 +++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 171 ++++++++++++++++++
 3 files changed, 329 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 93a0926c4bc..d2848970ec4 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4985,11 +4985,78 @@ static int i386_record_floats (struct gdbarch *gdbarch,
    with VEX prefix.  */
 
 static bool
-i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
+i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
 		 int opcode, struct gdbarch *gdbarch)
 {
   switch (opcode)
     {
+    case 0x6e:	/* VMOVD XMM, reg/mem  */
+      /* This is moving from a regular register or memory region into an
+	 XMM register. */
+      i386_record_modrm (ir);
+      /* ModR/M only has the 3 least significant bits of the destination
+	 register, the last one is indicated by VEX.R (stored inverted).  */
+      record_full_arch_list_add_reg (ir->regcache,
+				     ir->regmap[X86_RECORD_XMM0_REGNUM]
+				       + ir->reg + vex_r * 8);
+      break;
+    case 0x7e:	/* VMOV(D/Q)  */
+      i386_record_modrm (ir);
+      /* Both the intel and AMD manual are wrong about this.  According to
+	 it, the only difference between vmovq and vmovd should be the rex_w
+	 bit, but in empirical testing, it seems that they share this opcode,
+	 and the way to differentiate it here is looking at VEX.PP.  */
+      if (ir->pp == 2)
+	{
+	  /* This is vmovq moving from a regular register or memory
+	     into an XMM register.  As above, VEX.R is the final bit for
+	     destination register.  */
+	  record_full_arch_list_add_reg (ir->regcache,
+					 ir->regmap[X86_RECORD_XMM0_REGNUM]
+					 + ir->reg + vex_r * 8);
+	}
+      else if (ir->pp == 1)
+	{
+	  /* This is the vmovd version that stores into a regular register
+	     or memory region.  */
+	  /* If ModRM.mod is 11 we are saving into a register.  */
+	  if (ir->mod == 3)
+	    record_full_arch_list_add_reg (ir->regcache, ir->regmap[ir->rm]);
+	  else
+	    {
+	      /* Calculate the size of memory that will be modified
+		 and store it in the form of 1 << ir->ot, since that
+		 is how the function uses it.  In theory, VEX.W is supposed
+		 to indicate the size of the memory. In practice, I only
+		 ever seen it set to 0, and for 16 bytes, 0xD6 opcode
+		 is used.  */
+	      if (vex_w)
+		ir->ot = 4;
+	      else
+		ir->ot = 3;
+
+	      i386_record_lea_modrm (ir);
+	    }
+	}
+      else
+	error ("Unrecognized VEX.PP value %d at address %s.",
+	       ir->pp, paddress(gdbarch, ir->orig_addr));
+      break;
+    case 0xd6: /* VMOVQ reg/mem XMM  */
+      i386_record_modrm (ir);
+      /* This is the vmovq version that stores into a regular register
+	 or memory region.  */
+      /* If ModRM.mod is 11 we are saving into a register.  */
+      if (ir->mod == 3)
+	record_full_arch_list_add_reg (ir->regcache, ir->regmap[ir->rm]);
+      else
+	{
+	  /* We know that this operation is always 64 bits.  */
+	  ir->ot = 4;
+	  i386_record_lea_modrm (ir);
+	}
+      break;
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
new file mode 100644
index 00000000000..216b593736b
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -0,0 +1,90 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+#include <stdlib.h>
+
+char global_buf0[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+		      0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
+char global_buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		      0, 0, 0, 0, 0, 0, 0, 0};
+char *dyn_buf0;
+char *dyn_buf1;
+
+void
+vmov_test ()
+{
+  char buf0[] = {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+		 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f};
+  char buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, 0, 0};
+
+  /* Operations on registers.  */
+  asm volatile ("mov $0, %rcx");
+  asm volatile ("mov $0xbeef, %rax");
+  asm volatile ("vmovd %rax, %xmm0");
+  asm volatile ("vmovd %xmm0, %rcx");
+
+  /* Operations based on local buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(buf1));
+
+  /* Operations based on global buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(global_buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(global_buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(global_buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(global_buf1));
+
+  /* Operations based on dynamic buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(*dyn_buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(*dyn_buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(*dyn_buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(*dyn_buf1));
+
+  /* Reset all relevant buffers. */
+  asm volatile ("vmovq %%xmm15, %0": "=m" (buf1));
+  asm volatile ("vmovq %%xmm15, %0": "=m" (global_buf1));
+  asm volatile ("vmovq %%xmm15, %0": "=m" (*dyn_buf1));
+
+  /* Quick test for a different xmm register.  */
+  asm volatile ("vmovd %0, %%xmm15": "=m" (buf0));
+  asm volatile ("vmovd %0, %%xmm15": "=m" (buf1));
+  asm volatile ("vmovq %0, %%xmm15": "=m" (buf0));
+  asm volatile ("vmovq %0, %%xmm15": "=m" (buf1));
+} /* end vmov_test */
+
+int
+main ()
+{
+  dyn_buf0 = (char *) malloc(sizeof(char) * 16);
+  dyn_buf1 = (char *) malloc(sizeof(char) * 16);
+  for (int i =0; i < 16; i++)
+    {
+      dyn_buf0[i] = 0x20 + i;
+      dyn_buf1[i] = 0;
+    }
+  /* Zero relevant xmm registers, se we know what to look for.  */
+  asm volatile ("vmovq %0, %%xmm0": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm15": : "m" (global_buf1));
+
+  /* Start recording. */
+  vmov_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
new file mode 100644
index 00000000000..42ddc3a6526
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -0,0 +1,171 @@
+# Copyright 2009-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+require supports_reverse
+
+if {![istarget "*86*-*linux*"]} {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 [list debug $additional_flags]]} {
+    return -1
+}
+
+# Shorthand to test reversing through one instruction and
+# testing if a register has the expected value.
+# Prefix, if included, should end with a colon and space.
+
+proc test_one_register {insn register value {prefix ""}} {
+    gdb_test "reverse-step" "$insn.*" \
+	"${prefix}reverse-step from $insn to test register $register"
+
+    gdb_test "info register $register" \
+	"$register.*uint128 = $value.*" \
+	"${prefix}verify $register before $insn"
+}
+
+# Shorthand to test reversing through one instruction and
+# testing if a variable has the expected value.
+# Prefix, if used, should end with a colon and space.
+
+proc test_one_memory {insn mem value {dynamic false} {prefix ""}} {
+    gdb_test "reverse-step" "$insn.*" \
+	"${prefix}reverse-step from $insn to test memory $mem"
+
+    # For the dynamic buffer, we have to cast and dereference the pointer
+    set cast ""
+    if {$dynamic == true} {
+	set cast {(char [16]) *}
+    }
+
+    gdb_test "p/x $cast$mem" \
+	".*$value.*" \
+	"${prefix}verify $mem before $insn"
+
+}
+
+# Record the execution for the whole function, and stop at its end
+# to check if we can correctly reconstruct the state.
+# In the source code, the function must be FUNCTION_test, and
+# at the end, it must have a comment in the form:
+# /* end FUNCTION_test */
+# Returns true if the full function could be recorded, false otherwise.
+proc record_full_function {function} {
+    set end [gdb_get_line_number "end ${function}_test "]
+    gdb_breakpoint $end temporary
+
+    if [supports_process_record] {
+	# Activate process record/replay.
+	gdb_test_no_output "record" "${function}: turn on process record"
+    }
+
+    gdb_test_multiple "continue" "continue to end of ${function}_test" {
+	-re " end ${function}_test .*\r\n$::gdb_prompt $" {
+	    pass $gdb_test_name
+	}
+	-re " Illegal instruction.*\r\n$::gdb_prompt $" {
+	    fail $gdb_test_name
+	    return false
+	}
+	-re "Process record does not support VEX instruction.*" {
+	    fail $gdb_test_name
+	    return false
+	}
+    }
+    return true
+}
+
+set end_of_main          [gdb_get_line_number " end of main "]
+set rec_start            [gdb_get_line_number " Start recording"]
+
+runto_main
+
+gdb_breakpoint $rec_start
+gdb_continue_to_breakpoint "vmov_test" ".*vmov_test.*"
+
+global hex
+global decimal
+
+# Record all the execution for vmov tests first.
+
+if {[record_full_function "vmov"] == true} {
+    # Now execute backwards, checking all instructions.
+    # First we test all instructions handling global buffers.
+
+    test_one_register "vmovq" "xmm15" "0x3736353433323130" "reg_reset: "
+    test_one_register "vmovq" "xmm15" "0x0"
+    test_one_register "vmovd" "xmm15" "0x33323130" "reg_reset: "
+    test_one_register "vmovd" "xmm15" "0x0"
+
+    with_test_prefix buffer_reset {
+	test_one_memory "vmovq" "dyn_buf1" \
+	    "0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x0" true
+	test_one_memory "vmovq" "global_buf1" \
+	    "0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x0"
+	test_one_memory "vmovq" "buf1" \
+	    "0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x0"
+    }
+
+    with_test_prefix dynamic_buffers {
+	test_one_memory "vmovq" "dyn_buf1" "0x20, 0x21, 0x22, 0x23, 0x0" true
+	test_one_register "vmovq" "xmm0" "0x23222120"
+	test_one_memory "vmovd" "dyn_buf1" "0x0 .repeats 16 times" true
+	test_one_register "vmovd" "xmm0" "0x1716151413121110"
+    }
+
+    with_test_prefix global_buffers {
+	test_one_memory "vmovq" "global_buf1" "0x10, 0x11, 0x12, 0x13, 0x0"
+	test_one_register "vmovq" "xmm0" "0x13121110"
+	test_one_memory "vmovd" "global_buf1" "0x0 .repeats 16 times"
+	test_one_register "vmovd" "xmm0" "0x3736353433323130"
+    }
+
+    with_test_prefix local_buffers {
+	test_one_memory "vmovq" "buf1" "0x30, 0x31, 0x32, 0x33, 0x0"
+	test_one_register "vmovq" "xmm0" "0x33323130"
+	test_one_memory "vmovd" "buf1" "0x0 .repeats 16 times"
+	test_one_register "vmovd" "xmm0" "0xbeef"
+    }
+
+    # regular registers don't have uint128 members, so do it manually
+    with_test_prefix registers {
+	gdb_test "reverse-step" "vmovd %xmm0, %rcx.*" \
+	    "reverse step to check rcx recording"
+	gdb_test "print/x \$rcx" "= 0x0" "rcx was recorded"
+
+	test_one_register "vmovd" "xmm0" "0x0"
+    }
+} else {
+    untested "could not record vmov_test"
+}
+
+# Move to the end of vmov_test to set up next.
+# Stop recording in case of recording errors.
+gdb_test "record stop" "Process record is stopped.*" \
+    "delete history for vmov_test"
+gdb_test "finish" "Run till exit from.*vmov_test.*" "leaving vmov_test"
-- 
2.45.1

[PATCH 3/3] gdb/record: add support to AVX unpack instructions

[Guinevere Larsen]

This commit adds support to recording instructions to unpack high
or low data from XMM registers, identified by the mnemonics in the
form: VPUNPCK [L|H] [BW|WD|DQ|QDQ].
All these instructions are encoded the exact same way, and only affect
the destination register, making them trivial to implement together.

It also updates the test gdb.reverse/i386-avx-reverse.exp to test these
new instructions.
---
 gdb/i386-tdep.c                               | 12 ++++++
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 37 +++++++++++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 26 +++++++++++++
 3 files changed, 75 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index d2848970ec4..250aff73389 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5057,6 +5057,18 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
 	}
       break;
 
+    case 0x60:	/* VPUNPCKLBW  */
+    case 0x61:	/* VPUNPCKLWD  */
+    case 0x62:	/* VPUNPCKLDQ  */
+    case 0x6c:	/* VPUNPCKLQDQ */
+    case 0x68:	/* VPUNPCKHBW  */
+    case 0x69:	/* VPUNPCKHWD  */
+    case 0x6a:	/* VPUNPCKHDQ  */
+    case 0x6d:	/* VPUNPCKHQDQ */
+      i386_record_modrm (ir);
+      record_full_arch_list_add_reg (ir->regcache, ir->regmap[X86_RECORD_XMM0_REGNUM] + ir->reg + vex_r * 8);
+      break;
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index 216b593736b..4a4785ee99e 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -70,6 +70,40 @@ vmov_test ()
   asm volatile ("vmovq %0, %%xmm15": "=m" (buf1));
 } /* end vmov_test */
 
+/* Test if we can properly record (and undo) vpunpck style instructions.
+   Most tests will use xmm0 and xmm1 as sources. The registers xmm15 and xmm2
+   are used as destination to ensure we're reading the VEX.R bit correctly.  */
+void
+vpunpck_test  ()
+{
+  /* Using GDB, load these values onto registers, for ease of testing.
+     xmm0.uint128 = 0x1f1e1d1c1b1a19181716151413121110
+     xmm1.uint128 = 0x2f2e2d2c2b2a29282726252423222120
+     so that's easy to confirm that the unpacking went as expected.  */
+
+  /* 17 27 16 26 15 25 14 24 ...*/
+  asm volatile ("vpunpcklbw  %xmm0, %xmm1, %xmm15");
+  /* 17 16 27 26 15 14 25 24 ...*/
+  asm volatile ("vpunpcklwd  %0, %%xmm1, %%xmm15"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 27 26 25 24 ...*/
+  asm volatile ("vpunpckldq  %0, %%xmm1,  %%xmm2"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 13 12 11 10 ...*/
+  asm volatile ("vpunpcklqdq %xmm0, %xmm1, %xmm2");
+
+  /* 17 27 16 26 15 25 14 24 ...*/
+  asm volatile ("vpunpckhbw  %xmm0, %xmm1, %xmm15");
+  /* 17 16 27 26 15 14 25 24 ...*/
+  asm volatile ("vpunpckhwd  %0, %%xmm1, %%xmm15"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 27 26 25 24 ...*/
+  asm volatile ("vpunpckhdq  %0, %%xmm1,  %%xmm2"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 13 12 11 10 ...*/
+  asm volatile ("vpunpckhqdq %xmm0, %xmm1, %xmm2");
+} /* end vpunpck_test */
+
 int
 main ()
 {
@@ -82,9 +116,12 @@ main ()
     }
   /* Zero relevant xmm registers, se we know what to look for.  */
   asm volatile ("vmovq %0, %%xmm0": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm1": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm2": : "m" (global_buf1));
   asm volatile ("vmovq %0, %%xmm15": : "m" (global_buf1));
 
   /* Start recording. */
   vmov_test ();
+  vpunpck_test ();
   return 0;	/* end of main */
 }
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index 42ddc3a6526..c5e82eaeca6 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -169,3 +169,29 @@ if {[record_full_function "vmov"] == true} {
 gdb_test "record stop" "Process record is stopped.*" \
     "delete history for vmov_test"
 gdb_test "finish" "Run till exit from.*vmov_test.*" "leaving vmov_test"
+
+# Starting vpunpck tests.
+gdb_test "step" "vpunpck_test \\\(\\\) at .*" "entering vpunpck_test function"
+gdb_test_no_output "set \$xmm0.v2_int64 = {0x1716151413121110, 0x1f1e1d1c1b1a1918}"
+gdb_test_no_output "set \$xmm1.v2_int64 = {0x2726252423222120, 0x2f2e2d2c2b2a2928}"
+gdb_test_no_output "set \$xmm2.uint128 = 0x0"
+gdb_test_no_output "set \$xmm15.uint128 = 0x0"
+if {[record_full_function "vpunpck"] == true} {
+    test_one_register "vpunpckhqdq" "xmm2" "0x"
+    test_one_register "vpunpckhdq" "xmm2" "0x"
+    test_one_register "vpunpckhwd" "xmm15" "0x"
+    test_one_register "vpunpckhbw" "xmm15" "0x"
+
+    test_one_register "vpunpcklqdq" "xmm2" "0x17161514272625241312111023222120"
+    test_one_register "vpunpckldq" "xmm2" "0x0"
+    test_one_register "vpunpcklwd" "xmm15" "0x17271626152514241323122211211020"
+    test_one_register "vpunpcklbw" "xmm15" "0x0"
+} else {
+    untested "couldn't test vpunpck tests"
+}
+
+# Move to the end of vmov_test to set up next.
+# Stop recording in case of recording errors.
+gdb_test "record stop" "Process record is stopped.*" \
+    "delete history for vpunpck_test"
+gdb_test "finish" "Run till exit from.*vpunpck_test.*" "leaving vpunpck_test"
-- 
2.45.1

[PING][PATCH 0/3] Small step in supporting AVX instructions

[Guinevere Larsen]

Ping!

Since this patch is only x86 record full, if there are no comments until 
next week, I will push this change.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

On 5/21/24 17:27, Guinevere Larsen wrote:
> This patch series is the first, very small, step in supporting AVX and
> AVX2 instructions for the record-full target. It is important that we
> support it since glibc has been using avx instructions for a long time
> (at least fedora 21), so depending on which functions an inferior uses,
> they might be very inconvenienced.
>
> Patch 1 adds capability to identify the VEX prefix, but no instruction
> support. Patches 2 and 3 add support for a total of 10 instructions,
> which covers around 5% of all AVX instructions used by system libraries
> in my fedora 39 box.
>
> While this support is extremely minimal, I figured I could propose the
> patch series early so it was open for others with more free time could
> help contributing to this :)
>
> As for filed bugs, there are 2 that I could find on bugzilla, but both
> name a specific instruction (vmovdqa and vmovdqu) which were not added
> by this series, so I figured I should only mention it once those are
> added.
>
> Guinevere Larsen (3):
>    gdb: Start supporting AVX instruction
>    gdb/record: add support to vmovd and vmovq instructions
>    gdb/record: add support to AVX unpack instructions
>
>   gdb/amd64-tdep.c                              |   3 +-
>   gdb/i386-tdep.c                               | 170 ++++++++++++++-
>   gdb/i386-tdep.h                               |   2 +
>   gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 127 +++++++++++
>   .../gdb.reverse/i386-avx-reverse.exp          | 197 ++++++++++++++++++
>   5 files changed, 496 insertions(+), 3 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>   create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>

Re: [PATCH 1/3] gdb: Start supporting AVX instruction

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> +  /* These are used by VEX and XOP prefixes.  */
Guinevere> +  uint8_t map_select;
Guinevere> +  uint8_t vvvv;
Guinevere> +  uint8_t pp;
Guinevere> +  uint8_t l;

Are these the names used by the ISA?  Since if not they are pretty ugly.

Guinevere> +static bool
Guinevere> +i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
Guinevere> +		 int opcode, struct gdbarch *gdbarch)
Guinevere> +{
Guinevere> +  switch (opcode)
Guinevere> +    {
Guinevere> +    default:
Guinevere> +      gdb_printf (gdb_stderr,
Guinevere> +		  _("Process record does not support VEX instruction 0x%02x "
Guinevere> +		    "at address %s.\n"),
Guinevere> +		  (unsigned int) (opcode),
Guinevere> +		  paddress (gdbarch, ir->orig_addr));
Guinevere> +      return -1;

This should use true or false, not -1, because the function returns
bool.

I wouldn't be surprised to get some kind of dead code warning here,
since the switch only has a single branch.

Tom

Re: [PATCH 2/3] gdb/record: add support to vmovd and vmovq instructions

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> This commit adds support to the x86_64 AVX instructions vmovd and vmovq.
Guinevere> The programmers manuals for Intel and AMD describe these 2 instructions
Guinevere> as being almost the same, but my local testing, using gcc 13.2 on Fedora
Guinevere> 39, showed several differences and inconsistencies.

Guinevere>  static bool
Guinevere> -i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
Guinevere> +i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
Guinevere>  		 int opcode, struct gdbarch *gdbarch)
Guinevere>  {
...
Guinevere> +	error ("Unrecognized VEX.PP value %d at address %s.",
Guinevere> +	       ir->pp, paddress(gdbarch, ir->orig_addr));

I don't know anything about record, really, but it seems weird to have a
function that has two error return mechanisms -- either calling error or
printing a message and returning some value:

Guinevere>      default:
Guinevere>        gdb_printf (gdb_stderr,
Guinevere>  		  _("Process record does not support VEX instruction 0x%02x "

What's the deal with that?

Tom

Re: [PING][PATCH 0/3] Small step in supporting AVX instructions

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> Since this patch is only x86 record full, if there are no comments
Guinevere> until next week, I will push this change.

I normally don't really look at record patches but I took a quick look
and saw a couple of minor oddities.

Tom

Re: [PATCH 1/3] gdb: Start supporting AVX instruction

[Guinevere Larsen]

On 6/5/24 13:11, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> +  /* These are used by VEX and XOP prefixes.  */
> Guinevere> +  uint8_t map_select;
> Guinevere> +  uint8_t vvvv;
> Guinevere> +  uint8_t pp;
> Guinevere> +  uint8_t l;
>
> Are these the names used by the ISA?  Since if not they are pretty ugly.
Yes, these are the ISA names for those bits. I would use more 
descriptive names instead of ISA ones, but I have no idea what they are 
supposed to mean.
>
> Guinevere> +static bool
> Guinevere> +i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
> Guinevere> +		 int opcode, struct gdbarch *gdbarch)
> Guinevere> +{
> Guinevere> +  switch (opcode)
> Guinevere> +    {
> Guinevere> +    default:
> Guinevere> +      gdb_printf (gdb_stderr,
> Guinevere> +		  _("Process record does not support VEX instruction 0x%02x "
> Guinevere> +		    "at address %s.\n"),
> Guinevere> +		  (unsigned int) (opcode),
> Guinevere> +		  paddress (gdbarch, ir->orig_addr));
> Guinevere> +      return -1;
>
> This should use true or false, not -1, because the function returns
> bool.
nice catch! the function should be returning int instead.
>
> I wouldn't be surprised to get some kind of dead code warning here,
> since the switch only has a single branch.
no warnings in my system... but that is possible, I guess. Do you think 
it would be better to merge this commit with the following one?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Tom
>

Re: [PATCH 2/3] gdb/record: add support to vmovd and vmovq instructions

[Guinevere Larsen]

On 6/5/24 13:13, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> This commit adds support to the x86_64 AVX instructions vmovd and vmovq.
> Guinevere> The programmers manuals for Intel and AMD describe these 2 instructions
> Guinevere> as being almost the same, but my local testing, using gcc 13.2 on Fedora
> Guinevere> 39, showed several differences and inconsistencies.
>
> Guinevere>  static bool
> Guinevere> -i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, uint8_t rex_r,
> Guinevere> +i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
> Guinevere>  		 int opcode, struct gdbarch *gdbarch)
> Guinevere>  {
> ...
> Guinevere> +	error ("Unrecognized VEX.PP value %d at address %s.",
> Guinevere> +	       ir->pp, paddress(gdbarch, ir->orig_addr));
>
> I don't know anything about record, really, but it seems weird to have a
> function that has two error return mechanisms -- either calling error or
> printing a message and returning some value:
>
> Guinevere>      default:
> Guinevere>        gdb_printf (gdb_stderr,
> Guinevere>  		  _("Process record does not support VEX instruction 0x%02x "
>
> What's the deal with that?

Christmas vacation in between, mostly.

gdb_printf is what i386_process_record already uses, so felt reasonable, 
then I chose "error" int the other situation because it felt more 
appropriate for me to have a strong error when we find something that, 
to the best of my knowledge, breaks the ISA. To me it marks a difference 
between "we're not equipped to handle" versus "up to most recent ISA, 
this is straight up incorrect", but if you think it's better to only use 
one error mechanism, I can switch to gdb_printf everywhere.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Tom
>

Re: [PATCH 2/3] gdb/record: add support to vmovd and vmovq instructions

[Guinevere Larsen]

On 6/5/24 15:24, Guinevere Larsen wrote:
> On 6/5/24 13:13, Tom Tromey wrote:
>>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
>> Guinevere> This commit adds support to the x86_64 AVX instructions 
>> vmovd and vmovq.
>> Guinevere> The programmers manuals for Intel and AMD describe these 2 
>> instructions
>> Guinevere> as being almost the same, but my local testing, using gcc 
>> 13.2 on Fedora
>> Guinevere> 39, showed several differences and inconsistencies.
>>
>> Guinevere>  static bool
>> Guinevere> -i386_record_vex (struct i386_record_s *ir, uint8_t rex_w, 
>> uint8_t rex_r,
>> Guinevere> +i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, 
>> uint8_t vex_r,
>> Guinevere>           int opcode, struct gdbarch *gdbarch)
>> Guinevere>  {
>> ...
>> Guinevere> +    error ("Unrecognized VEX.PP value %d at address %s.",
>> Guinevere> +           ir->pp, paddress(gdbarch, ir->orig_addr));
>>
>> I don't know anything about record, really, but it seems weird to have a
>> function that has two error return mechanisms -- either calling error or
>> printing a message and returning some value:
>>
>> Guinevere>      default:
>> Guinevere>        gdb_printf (gdb_stderr,
>> Guinevere>            _("Process record does not support VEX 
>> instruction 0x%02x "
>>
>> What's the deal with that?
>
> Christmas vacation in between, mostly.
>
> gdb_printf is what i386_process_record already uses, so felt 
> reasonable, then I chose "error" int the other situation because it 
> felt more appropriate for me to have a strong error when we find 
> something that, to the best of my knowledge, breaks the ISA. To me it 
> marks a difference between "we're not equipped to handle" versus "up 
> to most recent ISA, this is straight up incorrect", but if you think 
> it's better to only use one error mechanism, I can switch to 
> gdb_printf everywhere.
>
Interestingly enough, I just double checked what happens, and for the 
end user, the gdb_printf option looks stronger because gdbarch will add

 > Process record: failed to record execution log.

and error will not print anything but the message given. I'll 
standardize on the gdb_printf version instead.  Thanks for the pointer!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

RE: [PING][PATCH 0/3] Small step in supporting AVX instructions

[Willgerodt, Felix]

> -----Original Message-----
> From: Guinevere Larsen <blarsen@redhat.com>
> Sent: Dienstag, 4. Juni 2024 21:10
> To: Guinevere Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
> Subject: [PING][PATCH 0/3] Small step in supporting AVX instructions
> 
> Ping!
> 
> Since this patch is only x86 record full, if there are no comments until
> next week, I will push this change.
> 
> --
> Cheers,
> Guinevere Larsen
> She/Her/Hers
> 

I didn't yet find the time to review this, nor do I really have much expertise
in VEX nor record full. I also don't really know if/when I will find the time.
But if you are confident enough I would be ok to merging this, after skimming
over it once, I didn't see much beyond what Tom already pointed out.


What I did is run some testing on your patches. I found this failure on two nodes
(Fedora 39 and Ubuntu 22.04, just using gcc and unix.exp and parallel testing):

(gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main            
continue^M                                                                      
Continuing.^M                                                                   
Process record does not support VEX instruction 0x78 at address 0x7ffff7e53315.^M
Process record: inferior program stopped.^M                                     
^M                                                                              
Program stopped.^M                                                              
0x00007ffff7e53315 in __memset_avx2_unaligned_erms () from /lib64/libc.so.6^M   
(gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main


Though I didn't see this when I ran it manually on a different Ubuntu 22.04 node.
There I saw this:

(gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main            
continue^M                                                                      
Continuing.^M                                                                   
Process record does not support instruction bound.^M                            
Process record does not support instruction 0x62 at address 0x7ffff7f289c7.^M   
Process record: failed to record execution log.^M                               
^M                                                                              
Program stopped.^M                                                              
0x00007ffff7f289c7 in __memset_evex_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S: No such file or directory^M
(gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS: record/30807)

I didn't have the time to investigate further. Not sure if the error messages tell you enough.
I can help if you want to know more about the CPUs or libraries.
Note that the two machines where it failed, it didn't fail for master.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PING][PATCH 0/3] Small step in supporting AVX instructions

[Guinevere Larsen]

On 6/6/24 05:16, Willgerodt, Felix wrote:
>> -----Original Message-----
>> From: Guinevere Larsen <blarsen@redhat.com>
>> Sent: Dienstag, 4. Juni 2024 21:10
>> To: Guinevere Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
>> Subject: [PING][PATCH 0/3] Small step in supporting AVX instructions
>>
>> Ping!
>>
>> Since this patch is only x86 record full, if there are no comments until
>> next week, I will push this change.
>>
>> --
>> Cheers,
>> Guinevere Larsen
>> She/Her/Hers
>>
> I didn't yet find the time to review this, nor do I really have much expertise
> in VEX nor record full. I also don't really know if/when I will find the time.
> But if you are confident enough I would be ok to merging this, after skimming
> over it once, I didn't see much beyond what Tom already pointed out.
>
>
> What I did is run some testing on your patches. I found this failure on two nodes
> (Fedora 39 and Ubuntu 22.04, just using gcc and unix.exp and parallel testing):
>
> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> continue^M
> Continuing.^M
> Process record does not support VEX instruction 0x78 at address 0x7ffff7e53315.^M
> Process record: inferior program stopped.^M
> ^M
> Program stopped.^M
> 0x00007ffff7e53315 in __memset_avx2_unaligned_erms () from /lib64/libc.so.6^M
> (gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main
>
>
> Though I didn't see this when I ran it manually on a different Ubuntu 22.04 node.
> There I saw this:
>
> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> continue^M
> Continuing.^M
> Process record does not support instruction bound.^M
> Process record does not support instruction 0x62 at address 0x7ffff7f289c7.^M
> Process record: failed to record execution log.^M
> ^M
> Program stopped.^M
> 0x00007ffff7f289c7 in __memset_evex_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
> warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S: No such file or directory^M
> (gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS: record/30807)
>
> I didn't have the time to investigate further. Not sure if the error messages tell you enough.

Yeah, that's enough to give me a sense of what's up. These are the 
motivators for this series (though the second one is further in the 
future). GDB doesn't know how to disassemble AVX, AVX2 and AVX512 
instructions, which is what leads to "process record does not support 
(...)".

The KFAIL (second error) is with regard to AVX-512, identified by the 
prefix 0x62. The first one used to have a KFAIL too, but I changed the 
output message so now it just shows up as FAIL (used to point to 
https://sourceware.org/bugzilla/show_bug.cgi?id=23188). I will update 
the step-precsave regexp to identify the current issue and emit a KFAIL 
again.

> I can help if you want to know more about the CPUs or libraries.
> Note that the two machines where it failed, it didn't fail for master.
I don't understand how it could have passed on the same machines. I'm 
more curious about the output when it did pass, if you still have it. 
Did it use a different path for memset?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Regards,
> Felix
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PING][PATCH 0/3] Small step in supporting AVX instructions

[Willgerodt, Felix]

> > What I did is run some testing on your patches. I found this failure on two
> nodes
> > (Fedora 39 and Ubuntu 22.04, just using gcc and unix.exp and parallel
> testing):
> >
> > (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> > continue^M
> > Continuing.^M
> > Process record does not support VEX instruction 0x78 at address
> 0x7ffff7e53315.^M
> > Process record: inferior program stopped.^M
> > ^M
> > Program stopped.^M
> > 0x00007ffff7e53315 in __memset_avx2_unaligned_erms () from
> /lib64/libc.so.6^M
> > (gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main
> >
> >
> > Though I didn't see this when I ran it manually on a different Ubuntu 22.04
> node.
> > There I saw this:
> >
> > (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> > continue^M
> > Continuing.^M
> > Process record does not support instruction bound.^M
> > Process record does not support instruction 0x62 at address
> 0x7ffff7f289c7.^M
> > Process record: failed to record execution log.^M
> > ^M
> > Program stopped.^M
> > 0x00007ffff7f289c7 in __memset_evex_unaligned_erms () at
> ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
> > warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-
> erms.S: No such file or directory^M
> > (gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS:
> record/30807)
> >
> > I didn't have the time to investigate further. Not sure if the error messages
> tell you enough.
> 
> Yeah, that's enough to give me a sense of what's up. These are the
> motivators for this series (though the second one is further in the
> future). GDB doesn't know how to disassemble AVX, AVX2 and AVX512
> instructions, which is what leads to "process record does not support
> (...)".
> 
> The KFAIL (second error) is with regard to AVX-512, identified by the
> prefix 0x62. The first one used to have a KFAIL too, but I changed the
> output message so now it just shows up as FAIL (used to point to
> https://sourceware.org/bugzilla/show_bug.cgi?id=23188). I will update
> the step-precsave regexp to identify the current issue and emit a KFAIL
> again.
> 
> > I can help if you want to know more about the CPUs or libraries.
> > Note that the two machines where it failed, it didn't fail for master.
> I don't understand how it could have passed on the same machines. I'm
> more curious about the output when it did pass, if you still have it.
> Did it use a different path for memset?
> 

I think the PATH difference is just a Fedora/Ubuntu difference.
I copied the Fedora 39 one above for the FAIL case.
And I think I might have confused you with "it didn't fail for master."
Sorry if that was the case. I meant that it KFAIL'ed on master.
I didn't mean to say it produced a PASS.

This is Fedora 39 with our master:

(gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main            
continue^M                                                                      
Continuing.^M                                                                   
Process record does not support instruction 0xc5 at address 0x7ffff7e53304.^M   
Process record: failed to record execution log.^M                               
^M                                                                              
Program stopped.^M                                                              
0x00007ffff7e53304 in __memset_avx2_unaligned_erms () from /lib64/libc.so.6^M   
(gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS: record/23188)

So 3 different hex values seem to be the difference.


For completeness sake, here is the failing Ubuntu 22.04 log (your patches):

(gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main            
continue^M                                                                      
Continuing.^M                                                                   
Process record does not support VEX instruction 0x78 at address 0x7ffff7f25f8b.^M
Process record: inferior program stopped.^M                                     
^M                                                                              
Program stopped.^M                                                              
0x00007ffff7f25f8b in __memset_avx2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S: No such file or directory^M
(gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PING][PATCH 0/3] Small step in supporting AVX instructions

[Guinevere Larsen]

On 6/6/24 10:36, Willgerodt, Felix wrote:
>>> What I did is run some testing on your patches. I found this failure on two
>> nodes
>>> (Fedora 39 and Ubuntu 22.04, just using gcc and unix.exp and parallel
>> testing):
>>> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
>>> continue^M
>>> Continuing.^M
>>> Process record does not support VEX instruction 0x78 at address
>> 0x7ffff7e53315.^M
>>> Process record: inferior program stopped.^M
>>> ^M
>>> Program stopped.^M
>>> 0x00007ffff7e53315 in __memset_avx2_unaligned_erms () from
>> /lib64/libc.so.6^M
>>> (gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main
>>>
>>>
>>> Though I didn't see this when I ran it manually on a different Ubuntu 22.04
>> node.
>>> There I saw this:
>>>
>>> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
>>> continue^M
>>> Continuing.^M
>>> Process record does not support instruction bound.^M
>>> Process record does not support instruction 0x62 at address
>> 0x7ffff7f289c7.^M
>>> Process record: failed to record execution log.^M
>>> ^M
>>> Program stopped.^M
>>> 0x00007ffff7f289c7 in __memset_evex_unaligned_erms () at
>> ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
>>> warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-
>> erms.S: No such file or directory^M
>>> (gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS:
>> record/30807)
>>> I didn't have the time to investigate further. Not sure if the error messages
>> tell you enough.
>>
>> Yeah, that's enough to give me a sense of what's up. These are the
>> motivators for this series (though the second one is further in the
>> future). GDB doesn't know how to disassemble AVX, AVX2 and AVX512
>> instructions, which is what leads to "process record does not support
>> (...)".
>>
>> The KFAIL (second error) is with regard to AVX-512, identified by the
>> prefix 0x62. The first one used to have a KFAIL too, but I changed the
>> output message so now it just shows up as FAIL (used to point to
>> https://sourceware.org/bugzilla/show_bug.cgi?id=23188). I will update
>> the step-precsave regexp to identify the current issue and emit a KFAIL
>> again.
>>
>>> I can help if you want to know more about the CPUs or libraries.
>>> Note that the two machines where it failed, it didn't fail for master.
>> I don't understand how it could have passed on the same machines. I'm
>> more curious about the output when it did pass, if you still have it.
>> Did it use a different path for memset?
>>
> I think the PATH difference is just a Fedora/Ubuntu difference.
> I copied the Fedora 39 one above for the FAIL case.
> And I think I might have confused you with "it didn't fail for master."
> Sorry if that was the case. I meant that it KFAIL'ed on master.
> I didn't mean to say it produced a PASS.
Oh ok, yes I was confused by it not failing on master. I will fix the 
precsave test case and do another pass on the gdb.reverse subdirectory 
to see if there are any other tests I missed. Thanks for checking!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> This is Fedora 39 with our master:
>
> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> continue^M
> Continuing.^M
> Process record does not support instruction 0xc5 at address 0x7ffff7e53304.^M
> Process record: failed to record execution log.^M
> ^M
> Program stopped.^M
> 0x00007ffff7e53304 in __memset_avx2_unaligned_erms () from /lib64/libc.so.6^M
> (gdb) KFAIL: gdb.reverse/step-precsave.exp: run to end of main (PRMS: record/23188)
>
> So 3 different hex values seem to be the difference.
>
>
> For completeness sake, here is the failing Ubuntu 22.04 log (your patches):
>
> (gdb) PASS: gdb.reverse/step-precsave.exp: breakpoint at end of main
> continue^M
> Continuing.^M
> Process record does not support VEX instruction 0x78 at address 0x7ffff7f25f8b.^M
> Process record: inferior program stopped.^M
> ^M
> Program stopped.^M
> 0x00007ffff7f25f8b in __memset_avx2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:182^M
> warning: 182    ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S: No such file or directory^M
> (gdb) FAIL: gdb.reverse/step-precsave.exp: run to end of main
>
> Regards,
> Felix
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/3] gdb: Start supporting AVX instruction

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

>> I wouldn't be surprised to get some kind of dead code warning here,
>> since the switch only has a single branch.

Guinevere> no warnings in my system... but that is possible, I guess. Do you
Guinevere> think it would be better to merge this commit with the following one?

I wouldn't worry about it.

Tom