[PATCH v3 0/5] Add support for AArch64 MOPS instructions

[PATCH v3 0/5] Add support for AArch64 MOPS instructions

[Thiago Jung Bauermann]

Hello,

This version is to adapt to Luis' clarification that MOPS instructions
don't need to be treated as atomic sequences and can be single-stepped.
If the OS reschedules the inferior to a different CPU while a main or
epilogue instruction is executed, it will reset the sequence back to the
prologue instruction.

Therefore patch 1 is now much smaller and only disables displaced stepping
on MOPS instructions, since they do need to appear consecutively in
memory.  Luis suggested relocating the whole sequence as a block.  I will
implement that suggestion in the near future, but in the mean time I would
like to suggest this approach.

Patch 4 is the only other one that was changed. The
gdb.arch/aarch64-mops-atomic-inst.exp testcase was renamed to
gdb.arch/aarch64-mops-single-step.exp, and adjusted to expect the MOPS
sequence to reset back to the prologue instruction. Also, a small bug was
fixed in its corresponding C file (the bug didn't affect the effectiveness
of the test).

The other patches are unchanged from v2.

Here is the original cover letter for convenience:

This patch series implements GDB support for the new instructions in
AArch64's MOPS feature.  Patch 1 has a small overview.

What is needed from GDB is recognizing the MOPS sequences of instructions
as atomic so that they can be stepped over during instruction single
stepping, and also to avoid doing displaced stepping with them.  This is
done in patch 1.

Patch 2 adds support for the new instructions to the record an replay
target.

The other patches add testcases to test each of the aspects above, plus
one testcase to verify the interaction of the MOPS instructions with
watchpoints.

Tested on Ubuntu 23.10 aarch64-linux-gnu with no regressions, using the
Arm FVP emulator as well as QEMU v8.2.

Thiago Jung Bauermann (5):
  gdb/aarch64: Disable displaced single-step for MOPS instructions
  gdb/aarch64: Add record support for MOPS instructions.
  gdb/testsuite: Add gdb.arch/aarch64-mops-watchpoint.exp
  gdb/testsuite: Add gdb.arch/aarch64-mops-single-step.exp
  gdb/testsuite: Add gdb.reverse/aarch64-mops.exp

 gdb/aarch64-tdep.c                            |  92 +++++++++-
 .../gdb.arch/aarch64-mops-single-step.c       |  73 ++++++++
 .../gdb.arch/aarch64-mops-single-step.exp     | 132 ++++++++++++++
 .../gdb.arch/aarch64-mops-watchpoint.c        |  66 +++++++
 .../gdb.arch/aarch64-mops-watchpoint.exp      |  79 ++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.c      |  71 ++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.exp    | 171 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  61 +++++++
 8 files changed, 742 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp


base-commit: 5021daf303393722f58f4422d7ad53d526aa2d50

[PATCH v3 1/5] gdb/aarch64: Disable displaced single-step for MOPS instructions

[Thiago Jung Bauermann]

The AArch64 MOPS (Memory Operation) instructions provide a standardised
instruction sequence to perform a memset, memcpy or memmove.  A sequence is
always composed of three instructions: a prologue instruction, a main
instruction and an epilogue instruction.  As an illustration, here are the
implementations of these memory operations in glibc 2.39:

  (gdb) disassemble/r
  Dump of assembler code for function __memset_mops:
  => 0x0000fffff7e8d780 <+0>:     d503201f        nop
     0x0000fffff7e8d784 <+4>:     aa0003e3        mov     x3, x0
     0x0000fffff7e8d788 <+8>:     19c10443        setp    [x3]!, x2!, x1
     0x0000fffff7e8d78c <+12>:    19c14443        setm    [x3]!, x2!, x1
     0x0000fffff7e8d790 <+16>:    19c18443        sete    [x3]!, x2!, x1
     0x0000fffff7e8d794 <+20>:    d65f03c0        ret
  End of assembler dump.

  (gdb) disassemble/r
  Dump of assembler code for function __memcpy_mops:
  => 0x0000fffff7e8c580 <+0>:     d503201f        nop
     0x0000fffff7e8c584 <+4>:     aa0003e3        mov     x3, x0
     0x0000fffff7e8c588 <+8>:     19010443        cpyfp   [x3]!, [x1]!, x2!
     0x0000fffff7e8c58c <+12>:    19410443        cpyfm   [x3]!, [x1]!, x2!
     0x0000fffff7e8c590 <+16>:    19810443        cpyfe   [x3]!, [x1]!, x2!
     0x0000fffff7e8c594 <+20>:    d65f03c0        ret
  End of assembler dump.

  (gdb) disassemble/r
  Dump of assembler code for function __memmove_mops:
  => 0x0000fffff7e8d180 <+0>:     d503201f        nop
     0x0000fffff7e8d184 <+4>:     aa0003e3        mov     x3, x0
     0x0000fffff7e8d188 <+8>:     1d010443        cpyp    [x3]!, [x1]!, x2!
     0x0000fffff7e8d18c <+12>:    1d410443        cpym    [x3]!, [x1]!, x2!
     0x0000fffff7e8d190 <+16>:    1d810443        cpye    [x3]!, [x1]!, x2!
     0x0000fffff7e8d194 <+20>:    d65f03c0        ret
  End of assembler dump.

The Arm Architecture Reference Manual says that "the prologue, main, and
epilogue instructions are expected to be run in succession and to appear
consecutively in memory".  Therefore this patch disables displaced stepping
on them.

PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
---
 gdb/aarch64-tdep.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Change in v3:
- Remove aarch64_software_single_step_mops function and the change to call
  it from aarch64_software_single_step, since Luis clarified that it is in
  fact possible to single step through MOPS sequences.

No change in v2.

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 8d0553f3d7cd..05ecd421cd0e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3808,10 +3808,12 @@ aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch,
   if (aarch64_decode_insn (insn, &inst, 1, NULL) != 0)
     return NULL;
 
-  /* Look for a Load Exclusive instruction which begins the sequence.  */
-  if (inst.opcode->iclass == ldstexcl && bit (insn, 22))
+  /* Look for a Load Exclusive instruction which begins the sequence,
+     or for a MOPS instruction.  */
+  if ((inst.opcode->iclass == ldstexcl && bit (insn, 22))
+      || AARCH64_CPU_HAS_FEATURE (*inst.opcode->avariant, MOPS))
     {
-      /* We can't displaced step atomic sequences.  */
+      /* We can't displaced step atomic sequences nor MOPS instructions.  */
       return NULL;
     }
 

[PATCH v3 2/5] gdb/aarch64: Add record support for MOPS instructions.

[Thiago Jung Bauermann]

There are two kinds of MOPS instructions: set instructions and copy
instructions.  Within each group there are variants with minor
differences in how they read or write to memory — e.g., non-temporal
read and/or write, unprivileged read and/or write and permutations of
those — but they work in the same way in terms of the registers and
regions of memory that they modify.

PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
---
 gdb/aarch64-tdep.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

No change since v1.

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 05ecd421cd0e..a9a67107675c 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -5188,6 +5188,86 @@ aarch64_record_asimd_load_store (aarch64_insn_decode_record *aarch64_insn_r)
   return AARCH64_RECORD_SUCCESS;
 }
 
+/* Record handler for Memory Copy and Memory Set instructions.  */
+
+static unsigned int
+aarch64_record_memcopy_memset (aarch64_insn_decode_record *aarch64_insn_r)
+{
+  if (record_debug)
+    debug_printf ("Process record: memory copy and memory set\n");
+
+  uint8_t op1 = bits (aarch64_insn_r->aarch64_insn, 22, 23);
+  uint8_t op2 = bits (aarch64_insn_r->aarch64_insn, 12, 15);
+  uint32_t reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
+  uint32_t reg_rn = bits (aarch64_insn_r->aarch64_insn, 5, 9);
+  uint32_t record_buf[3];
+  uint64_t record_buf_mem[4];
+
+  if (op1 != 3)
+    {
+      /* Copy instructions.  */
+      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
+
+      record_buf[0] = reg_rd;
+      record_buf[1] = reg_rn;
+      record_buf[2] = reg_rs;
+      aarch64_insn_r->reg_rec_count = 3;
+
+      ULONGEST dest_addr;
+      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
+				  &dest_addr);
+      ULONGEST source_addr;
+      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
+				  &source_addr);
+      LONGEST length;
+      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
+				&length);
+
+      /* In a processor using algorithm option A, the length in Rn has an
+	 inverted sign.  */
+      if (length < 0)
+	length *= -1;
+
+      record_buf_mem[0] = length;
+      record_buf_mem[1] = dest_addr;
+      record_buf_mem[2] = length;
+      record_buf_mem[3] = source_addr;
+      aarch64_insn_r->mem_rec_count = 2;
+    }
+  else if ((op1 == 3 && op2 < 12) || (op1 == 3 && op2 < 12))
+    {
+      /* Set instructions.  */
+      record_buf[0] = reg_rd;
+      record_buf[1] = reg_rn;
+      aarch64_insn_r->reg_rec_count = 2;
+
+      ULONGEST address;
+      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
+				  &address);
+
+      LONGEST length;
+      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
+				&length);
+
+      /* In a processor using algorithm option B, the length in Rn has an
+	 inverted sign.  */
+      if (length < 0)
+	length *= -1;
+
+      record_buf_mem[0] = length;
+      record_buf_mem[1] = address;
+      aarch64_insn_r->mem_rec_count = 1;
+    }
+  else
+    return AARCH64_RECORD_UNKNOWN;
+
+  MEM_ALLOC (aarch64_insn_r->aarch64_mems, aarch64_insn_r->mem_rec_count,
+	     record_buf_mem);
+  REG_ALLOC (aarch64_insn_r->aarch64_regs, aarch64_insn_r->reg_rec_count,
+	     record_buf);
+  return AARCH64_RECORD_SUCCESS;
+}
+
 /* Record handler for load and store instructions.  */
 
 static unsigned int
@@ -5465,6 +5545,10 @@ aarch64_record_load_store (aarch64_insn_decode_record *aarch64_insn_r)
       if (insn_bits10_11 == 0x01 || insn_bits10_11 == 0x03)
 	record_buf[aarch64_insn_r->reg_rec_count++] = reg_rn;
     }
+  /* Memory Copy and Memory Set instructions.  */
+  else if ((insn_bits24_27 & 1) == 1 && insn_bits28_29 == 1
+	   && insn_bits10_11 == 1 && !insn_bit21)
+    return aarch64_record_memcopy_memset (aarch64_insn_r);
   /* Advanced SIMD load/store instructions.  */
   else
     return aarch64_record_asimd_load_store (aarch64_insn_r);

[PATCH v3 3/5] gdb/testsuite: Add gdb.arch/aarch64-mops-watchpoint.exp

[Thiago Jung Bauermann]

Test behaviour of watchpoints triggered by MOPS instructions.  This test
is similar to gdb.base/memops-watchpoint.exp, but specifically for MOPS
instructions rather than whatever instructions are used in the libc's
implementation of memset/memcpy/memmove.

There's a separate watched variable for each set of instructions so that
the testcase can test whether GDB correctly identified the watchpoint
that triggered in each case.
---
 .../gdb.arch/aarch64-mops-watchpoint.c        | 66 ++++++++++++++++
 .../gdb.arch/aarch64-mops-watchpoint.exp      | 79 +++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     | 61 ++++++++++++++
 3 files changed, 206 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp

No change since v1.

diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
new file mode 100644
index 000000000000..b981f033d210
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
@@ -0,0 +1,66 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main (void)
+{
+  char source[40] __attribute__ ((aligned (8)))
+    = "This is a relatively long string...";
+  char a[40] __attribute__ ((aligned (8)))
+    = "String to be overwritten with zeroes";
+  char b[40] __attribute__ ((aligned (8)))
+    = "Another string to be memcopied...";
+  char c[40] __attribute__ ((aligned (8)))
+    = "Another string to be memmoved...";
+  char *p, *q;
+  long size, zero;
+
+  /* Break here.  */
+  p = a;
+  size = sizeof (a);
+  zero = 0;
+  /* memset implemented in MOPS instructions.  */
+  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
+		    "setm [%0]!, %1!, %2\n\t"
+		    "sete [%0]!, %1!, %2\n\t"
+		    : "+&r"(p), "+&r"(size)
+		    : "r"(zero)
+		    : "memory");
+
+  p = b;
+  q = source;
+  size = sizeof (b);
+  /* memmove implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyp   [%0]!, [%1]!, %2!\n\t"
+		    "cpym   [%0]!, [%1]!, %2!\n\t"
+		    "cpye   [%0]!, [%1]!, %2!\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+  p = c;
+  q = source;
+  size = sizeof (c);
+  /* memcpy implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyfp   [%0]!, [%1]!, %2!\n\t"
+		    "cpyfm   [%0]!, [%1]!, %2!\n\t"
+		    "cpyfe   [%0]!, [%1]!, %2!\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
new file mode 100644
index 000000000000..9e210602d800
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
@@ -0,0 +1,79 @@
+# Copyright 2024 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/>.
+
+# Test a binary that uses MOPS (Memory Operations) instructions.
+# This test is similar to gdb.base/memops-watchpoint.exp, but specifically
+# tests MOPS instructions rather than whatever instructions are used in the
+# system libc's implementation of memset/memcpy/memmove.
+
+require allow_hw_watchpoint_tests allow_aarch64_mops_tests
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  [list debug additional_flags=-march=armv9.3-a]] } {
+    return -1
+}
+
+set linespec ${srcfile}:[gdb_get_line_number "Break here"]
+if ![runto ${linespec}] {
+    return -1
+}
+
+gdb_test "watch -location a\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
+    "set watch on a"
+gdb_test "watch -location b\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
+    "set watchpoint on b"
+gdb_test "watch -location c\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
+    "set watchpoint on c"
+
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "Hardware watchpoint ${decimal}: -location a\\\[28\\\]" \
+	 "" \
+	 "Old value = 104 'h'" \
+	 "New value = 0 '\\\\000'" \
+	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
+	 "${decimal}\\s+__asm__ volatile \\(\"setp.*\\\\n\\\\t\""] \
+    "continue until set watchpoint hits"
+
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "Hardware watchpoint ${decimal}: -location b\\\[28\\\]" \
+	 "" \
+	 "Old value = 101 'e'" \
+	 "New value = 114 'r'" \
+	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
+	 "${decimal}\\s+__asm__ volatile \\(\"cpyp.*\\\\n\\\\t\""] \
+    "continue until cpy watchpoint hits"
+
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "Hardware watchpoint ${decimal}: -location c\\\[28\\\]" \
+	 "" \
+	 "Old value = 100 'd'" \
+	 "New value = 114 'r'" \
+	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
+	 "${decimal}\\s+__asm__ volatile \\(\"cpyfp.*\\\\n\\\\t\""] \
+    "continue until cpyf watchpoint hits"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0d78691c381b..25b272fdaabc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4497,6 +4497,67 @@ proc aarch64_supports_sme_svl { length } {
     return 1
 }
 
+# Run a test on the target to see if it supports Aarch64 MOPS (Memory
+# Operations) extensions.  Return 0 if so, 1 if it does not.  Note this causes
+# a restart of GDB.
+
+gdb_caching_proc allow_aarch64_mops_tests {} {
+    global srcdir subdir gdb_prompt inferior_exited_re
+
+    set me "allow_aarch64_mops_tests"
+
+    if { ![is_aarch64_target]} {
+	return 0
+    }
+
+    # ARMv9.3-A contains the MOPS extension.  The test program doesn't use it,
+    # but take the opportunity to check whether the toolchain knows about MOPS.
+    set compile_flags "{additional_flags=-march=armv9.3-a}"
+
+    # Compile a program that tests the MOPS feature.
+    set src {
+	#include <stdbool.h>
+	#include <sys/auxv.h>
+
+	#ifndef HWCAP2_MOPS
+	#define HWCAP2_MOPS (1UL << 43)
+	#endif
+
+	int main() {
+	    bool mops_supported = getauxval (AT_HWCAP2) & HWCAP2_MOPS;
+
+	    return !mops_supported;
+	}
+    }
+
+    if {![gdb_simple_compile $me $src executable $compile_flags]} {
+	return 0
+    }
+
+    # Compilation succeeded so now run it via gdb.
+    clean_restart $obj
+    gdb_run_cmd
+    gdb_expect {
+	-re ".*$inferior_exited_re with code 01.*${gdb_prompt} $" {
+	    verbose -log "\n$me mops support not detected"
+	    set allow_mops_tests 0
+	}
+	-re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
+	    verbose -log "\n$me: mops support detected"
+	    set allow_mops_tests 1
+	}
+	default {
+	  warning "\n$me: default case taken"
+	    set allow_mops_tests 0
+	}
+    }
+    gdb_exit
+    remote_file build delete $obj
+
+    verbose "$me:  returning $allow_mops_tests" 2
+    return $allow_mops_tests
+}
+
 # A helper that compiles a test case to see if __int128 is supported.
 proc gdb_int128_helper {lang} {
     return [gdb_can_simple_compile "i128-for-$lang" {

[PATCH v3 4/5] gdb/testsuite: Add gdb.arch/aarch64-mops-single-step.exp

[Thiago Jung Bauermann]

The testcase verifies that MOPS sequences are correctly single-stepped.
---
 .../gdb.arch/aarch64-mops-single-step.c       |  73 ++++++++++
 .../gdb.arch/aarch64-mops-single-step.exp     | 132 ++++++++++++++++++
 2 files changed, 205 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp

Changes in v3:
- Renamed gdb.arch/aarch64-mops-atomic-inst.exp to
  gdb.arch/aarch64-mops-single-step.exp.
- Adjusted test to expect the MOPS sequence to reset back to the prologue
  instruction.
- Set size variable before the cpyf and cpy sequences, because after each
  sequence the variable is set to zero.  This bug didn't affect the
  effectiveness of the test.

Changes in v2:
- Add prfm instruction after each MOPS sequence and look for it in the
  testcase to verify that the sequence was stepped through (Suggested by
  Christophe).

diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
new file mode 100644
index 000000000000..4a27867d4b57
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
@@ -0,0 +1,73 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+#define TEST_STRING "Just a test string."
+#define BUF_SIZE sizeof(TEST_STRING)
+
+int
+main (void)
+{
+  char source[BUF_SIZE] = TEST_STRING;
+  char dest[BUF_SIZE];
+  char *p, *q;
+  long size, zero;
+
+  /* Note: The prfm instruction in the asm statements below is there just
+     to allow the testcase to recognize when the PC is at the instruction
+     right after the MOPS sequence.  */
+
+  p = dest;
+  size = sizeof (dest);
+  zero = 0;
+  /* Break memset.  */
+  /* memset implemented in MOPS instructions.  */
+  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
+		    "setm [%0]!, %1!, %2\n\t"
+		    "sete [%0]!, %1!, %2\n\t"
+		    "prfm pldl3keep, [%0, #0]\n\t"
+		    : "+&r"(p), "+&r"(size)
+		    : "r"(zero)
+		    : "memory");
+
+  p = dest;
+  q = source;
+  size = sizeof (dest);
+  /* Break memcpy.  */
+  /* memcpy implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyfp [%0]!, [%1]!, %2!\n\t"
+		    "cpyfm [%0]!, [%1]!, %2!\n\t"
+		    "cpyfe [%0]!, [%1]!, %2!\n\t"
+		    "prfm pldl3keep, [%0, #0]\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+
+  p = dest;
+  q = source;
+  size = sizeof (dest);
+  /* Break memmove.  */
+  /* memmove implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyp [%0]!, [%1]!, %2!\n\t"
+		    "cpym [%0]!, [%1]!, %2!\n\t"
+		    "cpye [%0]!, [%1]!, %2!\n\t"
+		    "prfm pldl3keep, [%0, #0]\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
new file mode 100644
index 000000000000..a6390d4bff7e
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
@@ -0,0 +1,132 @@
+# Copyright 2024 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.
+
+# Test single stepping through MOPS (memory operations) instruction sequences.
+
+require allow_aarch64_mops_tests
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  [list debug additional_flags=-march=armv9.3-a]] } {
+    return -1
+}
+
+# INSTRUCTION should be just its mnemonic, without any arguments.
+proc is_at_instruction { instruction } {
+    global gdb_prompt hex
+
+    set test "pc points to $instruction"
+    gdb_test_multiple {x/i $pc} $test {
+	-re -wrap "=> $hex \[^\r\n\]+:\t$instruction\t\[^\r\n\]+" {
+	    return 1
+	}
+	-re "\r\n$gdb_prompt $" {
+	    return 0
+	}
+    }
+
+    return 0
+}
+
+proc arrive_at_instruction { instruction } {
+    set count 0
+
+    while { [is_at_instruction $instruction] != 1 } {
+	gdb_test -nopass "stepi" ".*__asm__ volatile.*" \
+	    "stepi #$count to reach $instruction"
+	incr count
+
+	if { $count > 50 } {
+	    fail "didn't reach $instruction"
+	    return 0
+	}
+    }
+
+    return 1
+}
+
+# If the inferior is reschedule to another CPU while a main or epilogue
+# instruction is executed, the OS resets the inferior back to the prologue
+# instruction, so we need to allow for that possibility.
+proc step_through_sequence { prefix } {
+    set count 0
+
+    while { [is_at_instruction ${prefix}p] == 1 && $count < 50 } {
+	incr count
+
+	# The stepi output isn't useful to detect whether we stepped over
+	# the instruction.
+	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}p"
+	if { [is_at_instruction ${prefix}m] == 1 } {
+	    pass "stepped over ${prefix}p"
+	} else {
+	    fail "stepped over ${prefix}e"
+	    return 0
+	}
+
+	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}m"
+	if { [is_at_instruction ${prefix}e] == 1 } {
+	    pass "stepped over ${prefix}m"
+	} elseif { [is_at_instruction ${prefix}p] == 1 } {
+	    # The inferior was rescheduled to another CPU.
+	    pass "${prefix}m: reset back to prologue"
+	    continue
+	} else {
+	    fail "stepped over ${prefix}m"
+	    return 0
+	}
+
+	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}e"
+	if { [is_at_instruction prfm] == 1 } {
+	    pass "stepped over ${prefix}e"
+	    return 1
+	} elseif { [is_at_instruction ${prefix}p] == 1 } {
+	    # The inferior was rescheduled to another CPU.
+	    pass "${prefix}e: reset back to prologue"
+	    continue
+	}
+    }
+
+    fail "step through $prefix sequence"
+    return 0
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memset"]
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memcpy"]
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memmove"]
+
+gdb_continue_to_breakpoint "memset breakpoint"
+
+if { [arrive_at_instruction setp] } {
+    step_through_sequence set
+}
+
+gdb_continue_to_breakpoint "memcpy breakpoint"
+
+if { [arrive_at_instruction cpyfp] } {
+    step_through_sequence cpyf
+}
+
+gdb_continue_to_breakpoint "memmove breakpoint"
+
+if { [arrive_at_instruction cpyp] } {
+    step_through_sequence cpy
+}

[PATCH v3 5/5] gdb/testsuite: Add gdb.reverse/aarch64-mops.exp

[Thiago Jung Bauermann]

The testcase verifies that MOPS instructions are recorded and correctly
reversed.  Not all variants of the copy and set instructions are tested,
since there are many and the record and replay target processes them in
the same way.
---
 gdb/testsuite/gdb.reverse/aarch64-mops.c   |  71 +++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.exp | 171 +++++++++++++++++++++
 2 files changed, 242 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp

No change since v1.

diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.c b/gdb/testsuite/gdb.reverse/aarch64-mops.c
new file mode 100644
index 000000000000..513f324b9dd6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/aarch64-mops.c
@@ -0,0 +1,71 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+#define TEST_STRING "Just a test string."
+#define BUF_SIZE sizeof(TEST_STRING)
+
+int
+main (void)
+{
+  char dest[BUF_SIZE];
+  char source[BUF_SIZE] = TEST_STRING;
+  register char *p asm ("x19");
+  register char *q asm ("x20");
+  register long size asm ("x21");
+  register long zero asm ("x22");
+
+  p = dest;
+  size = BUF_SIZE;
+  zero = 0;
+  /* Before setp.  */
+  /* memset implemented in MOPS instructions.  */
+  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
+                    "setm [%0]!, %1!, %2\n\t"
+                    "sete [%0]!, %1!, %2\n\t"
+                    : "+&r"(p), "+&r"(size)
+                    : "r"(zero)
+                    : "memory");
+
+  /* After sete.  */
+  p = dest;
+  q = source;
+  size = BUF_SIZE;
+  /* Before cpyp.  */
+  /* memmove implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyp   [%0]!, [%1]!, %2!\n\t"
+		    "cpym   [%0]!, [%1]!, %2!\n\t"
+		    "cpye   [%0]!, [%1]!, %2!\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+  /* After cpye.  */
+  p = dest;
+  q = source;
+  size = BUF_SIZE;
+  /* Before cpyfp.  */
+  /* memcpy implemented in MOPS instructions.  */
+  __asm__ volatile ("cpyfp   [%0]!, [%1]!, %2!\n\t"
+		    "cpyfm   [%0]!, [%1]!, %2!\n\t"
+		    "cpyfe   [%0]!, [%1]!, %2!\n\t"
+		    : "+&r" (p), "+&r" (q), "+&r" (size)
+		    :
+		    : "memory");
+  /* After cpyfe.  */
+  p = dest;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.exp b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
new file mode 100644
index 000000000000..f9c1257e0b11
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
@@ -0,0 +1,171 @@
+# Copyright 2024 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/>.
+
+# Test instruction record for AArch64 FEAT_MOPS instructions.
+# Based on gdb.reverse/ppc_record_test_isa_3_1.exp
+#
+# The basic flow of the record tests are:
+#    1) Stop before executing the instructions of interest.  Record
+#       the initial value of the registers that the instruction will
+#       change, i.e. the destination register.
+#    2) Execute the instructions.  Record the new value of the
+#       registers that changed.
+#    3) Reverse the direction of the execution and execute back to
+#       just before the instructions of interest.  Record the final
+#       value of the registers of interest.
+#    4) Check that the initial and new values of the registers are
+#       different, i.e. the instruction changed the registers as expected.
+#    5) Check that the initial and final values of the registers are
+#       the same, i.e. GDB record restored the registers to their
+#       original values.
+
+require allow_aarch64_mops_tests
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  [list debug additional_flags=-march=armv9.3-a]] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test_no_output "record full"
+
+proc do_test { insn_prefix } {
+    global decimal hex
+
+    set before_seq [gdb_get_line_number "Before ${insn_prefix}p"]
+    set after_seq [gdb_get_line_number "After ${insn_prefix}e"]
+
+    with_test_prefix $insn_prefix {
+	gdb_test "break $before_seq" \
+	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
+	    "break before instruction sequence"
+	gdb_test "continue" \
+	    [multi_line \
+		 "Continuing\\." \
+		 "" \
+		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
+		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
+	    "about to execute instruction sequence"
+
+	# Record the initial register values.
+	set x19_initial [capture_command_output "info register x19" ""]
+	set x21_initial [capture_command_output "info register x21" ""]
+
+	# The set instructions use the ZERO variable, but not Q, and the other
+	# instructions are the opposite.
+	if {[string compare $insn_prefix "set"] == 0} {
+	    set x22_initial [capture_command_output "info register x22" ""]
+	} else {
+	    set x20_initial [capture_command_output "info register x20" ""]
+	}
+
+	gdb_test "break $after_seq" \
+	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
+	    "break after instruction sequence"
+	gdb_test "continue" \
+	    [multi_line \
+		 "Continuing\\." \
+		 "" \
+		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
+		 "$decimal\[ \t\]+p = dest;"] \
+	    "executed instruction sequence"
+
+	# Record the new register values.
+	set x19_new [capture_command_output "info register x19" ""]
+	set x21_new [capture_command_output "info register x21" ""]
+
+	if {[string compare $insn_prefix "set"] == 0} {
+	    set x22_new [capture_command_output "info register x22" ""]
+	} else {
+	    set x20_new [capture_command_output "info register x20" ""]
+	}
+
+	# Execute in reverse to before the instruction sequence.
+	gdb_test_no_output "set exec-direction reverse"
+
+	gdb_test "continue" \
+	    [multi_line \
+		 "Continuing\\." \
+		 "" \
+		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
+		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
+	    "reversed execution of instruction sequence"
+
+	# Record the final register values.
+	set x19_final [capture_command_output "info register x19" ""]
+	set x21_final [capture_command_output "info register x21" ""]
+
+	if {[string compare $insn_prefix "set"] == 0} {
+	    set x22_final [capture_command_output "info register x22" ""]
+	} else {
+	    set x20_final [capture_command_output "info register x20" ""]
+	}
+
+	# Check initial and new values of x19 are different.
+	gdb_assert [string compare $x19_initial $x19_new] \
+	    "check x19 initial value versus x19 new value"
+
+	# Check initial and new values of x21 are different.
+	gdb_assert [string compare $x21_initial $x21_new] \
+	    "check x21 initial value versus x21 new value"
+
+	if {[string compare $insn_prefix "set"] == 0} {
+	    # Check initial and new values of x22 are the same.
+	    # The register with the value to set shouldn't change.
+	    gdb_assert ![string compare $x22_initial $x22_new] \
+		"check x22 initial value versus x22 new value"
+	} else {
+	    # Check initial and new values of x20 are different.
+	    gdb_assert [string compare $x20_initial $x20_new] \
+		"check x20 initial value versus x20 new value"
+	}
+
+	# Check initial and final values of x19 are the same.
+	gdb_assert ![string compare $x19_initial $x19_final] \
+	    "check x19 initial value versus x19 final value"
+
+	# Check initial and final values of x21 are the same.
+	gdb_assert ![string compare $x21_initial $x21_final] \
+	    "check x21 initial value versus x21 final value"
+
+	if {[string compare $insn_prefix "set"] == 0} {
+	    # Check initial and final values of x22 are the same.
+	    # The register with the value to set shouldn't change.
+	    gdb_assert ![string compare $x22_initial $x22_final] \
+		"check x22 initial value versus x22 final value"
+	} else {
+	    # Check initial and final values of x20 are the same.
+	    gdb_assert ![string compare $x20_initial $x20_final] \
+		"check x20 initial value versus x20 final value"
+	}
+
+	# Restore forward execution and go to end of recording.
+	gdb_test_no_output "set exec-direction forward"
+	gdb_test "record goto end" \
+	    [multi_line \
+		"Go forward to insn number $decimal" \
+		"#0  main \\(\\) at .*/aarch64-mops.c:$decimal" \
+		"$decimal\[ \t\]+p = dest;"]
+    }
+}
+
+do_test "set"
+do_test "cpy"
+do_test "cpyf"

Re: [PATCH v3 2/5] gdb/aarch64: Add record support for MOPS instructions.

[Luis Machado]

Hi,

Just a minor suggestion. Otherwise the patch looks OK.

On 5/10/24 06:24, Thiago Jung Bauermann wrote:
> There are two kinds of MOPS instructions: set instructions and copy
> instructions.  Within each group there are variants with minor
> differences in how they read or write to memory — e.g., non-temporal
> read and/or write, unprivileged read and/or write and permutations of
> those — but they work in the same way in terms of the registers and
> regions of memory that they modify.
> 
> PR tdep/31666
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
> ---
>  gdb/aarch64-tdep.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> No change since v1.
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 05ecd421cd0e..a9a67107675c 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -5188,6 +5188,86 @@ aarch64_record_asimd_load_store (aarch64_insn_decode_record *aarch64_insn_r)
>    return AARCH64_RECORD_SUCCESS;
>  }
>  
> +/* Record handler for Memory Copy and Memory Set instructions.  */
> +
> +static unsigned int
> +aarch64_record_memcopy_memset (aarch64_insn_decode_record *aarch64_insn_r)
> +{
> +  if (record_debug)
> +    debug_printf ("Process record: memory copy and memory set\n");
> +
> +  uint8_t op1 = bits (aarch64_insn_r->aarch64_insn, 22, 23);
> +  uint8_t op2 = bits (aarch64_insn_r->aarch64_insn, 12, 15);
> +  uint32_t reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
> +  uint32_t reg_rn = bits (aarch64_insn_r->aarch64_insn, 5, 9);
> +  uint32_t record_buf[3];
> +  uint64_t record_buf_mem[4];
> +
> +  if (op1 != 3)
> +    {
> +      /* Copy instructions.  */
> +      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
> +
> +      record_buf[0] = reg_rd;
> +      record_buf[1] = reg_rn;
> +      record_buf[2] = reg_rs;
> +      aarch64_insn_r->reg_rec_count = 3;
> +
> +      ULONGEST dest_addr;
> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
> +				  &dest_addr);
> +      ULONGEST source_addr;
> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
> +				  &source_addr);
> +      LONGEST length;
> +      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
> +				&length);
> +
> +      /* In a processor using algorithm option A, the length in Rn has an
> +	 inverted sign.  */
> +      if (length < 0)
> +	length *= -1;
> +
> +      record_buf_mem[0] = length;
> +      record_buf_mem[1] = dest_addr;
> +      record_buf_mem[2] = length;
> +      record_buf_mem[3] = source_addr;
> +      aarch64_insn_r->mem_rec_count = 2;
> +    }
> +  else if ((op1 == 3 && op2 < 12) || (op1 == 3 && op2 < 12))
> +    {
> +      /* Set instructions.  */
> +      record_buf[0] = reg_rd;
> +      record_buf[1] = reg_rn;
> +      aarch64_insn_r->reg_rec_count = 2;
> +
> +      ULONGEST address;
> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
> +				  &address);
> +
> +      LONGEST length;
> +      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
> +				&length);
> +
> +      /* In a processor using algorithm option B, the length in Rn has an
> +	 inverted sign.  */
> +      if (length < 0)
> +	length *= -1;
> +
> +      record_buf_mem[0] = length;
> +      record_buf_mem[1] = address;
> +      aarch64_insn_r->mem_rec_count = 1;
> +    }

Looks like both cases above could potentially share some code in a common path. Would be nice
to do so if it doesn't make the code all awkward. If it does, then this is fine.

> +  else
> +    return AARCH64_RECORD_UNKNOWN;
> +
> +  MEM_ALLOC (aarch64_insn_r->aarch64_mems, aarch64_insn_r->mem_rec_count,
> +	     record_buf_mem);
> +  REG_ALLOC (aarch64_insn_r->aarch64_regs, aarch64_insn_r->reg_rec_count,
> +	     record_buf);
> +  return AARCH64_RECORD_SUCCESS;
> +}
> +
>  /* Record handler for load and store instructions.  */
>  
>  static unsigned int
> @@ -5465,6 +5545,10 @@ aarch64_record_load_store (aarch64_insn_decode_record *aarch64_insn_r)
>        if (insn_bits10_11 == 0x01 || insn_bits10_11 == 0x03)
>  	record_buf[aarch64_insn_r->reg_rec_count++] = reg_rn;
>      }
> +  /* Memory Copy and Memory Set instructions.  */
> +  else if ((insn_bits24_27 & 1) == 1 && insn_bits28_29 == 1
> +	   && insn_bits10_11 == 1 && !insn_bit21)
> +    return aarch64_record_memcopy_memset (aarch64_insn_r);
>    /* Advanced SIMD load/store instructions.  */
>    else
>      return aarch64_record_asimd_load_store (aarch64_insn_r);

Re: [PATCH v3 3/5] gdb/testsuite: Add gdb.arch/aarch64-mops-watchpoint.exp

[Luis Machado]

Thanks.

Just a nit below. I'll set something up on my end to give these tests a try.

On 5/10/24 06:24, Thiago Jung Bauermann wrote:
> Test behaviour of watchpoints triggered by MOPS instructions.  This test
> is similar to gdb.base/memops-watchpoint.exp, but specifically for MOPS
> instructions rather than whatever instructions are used in the libc's
> implementation of memset/memcpy/memmove.
> 
> There's a separate watched variable for each set of instructions so that
> the testcase can test whether GDB correctly identified the watchpoint
> that triggered in each case.
> ---
>  .../gdb.arch/aarch64-mops-watchpoint.c        | 66 ++++++++++++++++
>  .../gdb.arch/aarch64-mops-watchpoint.exp      | 79 +++++++++++++++++++
>  gdb/testsuite/lib/gdb.exp                     | 61 ++++++++++++++
>  3 files changed, 206 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
> 
> No change since v1.
> 
> diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
> new file mode 100644
> index 000000000000..b981f033d210
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
> @@ -0,0 +1,66 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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/>.  */
> +
> +int
> +main (void)
> +{
> +  char source[40] __attribute__ ((aligned (8)))
> +    = "This is a relatively long string...";
> +  char a[40] __attribute__ ((aligned (8)))
> +    = "String to be overwritten with zeroes";
> +  char b[40] __attribute__ ((aligned (8)))
> +    = "Another string to be memcopied...";
> +  char c[40] __attribute__ ((aligned (8)))
> +    = "Another string to be memmoved...";
> +  char *p, *q;
> +  long size, zero;
> +
> +  /* Break here.  */
> +  p = a;
> +  size = sizeof (a);
> +  zero = 0;
> +  /* memset implemented in MOPS instructions.  */
> +  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
> +		    "setm [%0]!, %1!, %2\n\t"
> +		    "sete [%0]!, %1!, %2\n\t"
> +		    : "+&r"(p), "+&r"(size)
> +		    : "r"(zero)
> +		    : "memory");
> +
> +  p = b;
> +  q = source;
> +  size = sizeof (b);
> +  /* memmove implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpym   [%0]!, [%1]!, %2!\n\t"
> +		    "cpye   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +  p = c;
> +  q = source;
> +  size = sizeof (c);
> +  /* memcpy implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyfp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfm   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfe   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
> new file mode 100644
> index 000000000000..9e210602d800
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp
> @@ -0,0 +1,79 @@
> +# Copyright 2024 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/>.
> +
> +# Test a binary that uses MOPS (Memory Operations) instructions.
> +# This test is similar to gdb.base/memops-watchpoint.exp, but specifically
> +# tests MOPS instructions rather than whatever instructions are used in the
> +# system libc's implementation of memset/memcpy/memmove.
> +
> +require allow_hw_watchpoint_tests allow_aarch64_mops_tests
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  [list debug additional_flags=-march=armv9.3-a]] } {
> +    return -1
> +}
> +
> +set linespec ${srcfile}:[gdb_get_line_number "Break here"]
> +if ![runto ${linespec}] {
> +    return -1
> +}
> +
> +gdb_test "watch -location a\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
> +    "set watch on a"
> +gdb_test "watch -location b\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
> +    "set watchpoint on b"
> +gdb_test "watch -location c\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
> +    "set watchpoint on c"
> +
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "Hardware watchpoint ${decimal}: -location a\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 104 'h'" \
> +	 "New value = 0 '\\\\000'" \
> +	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
> +	 "${decimal}\\s+__asm__ volatile \\(\"setp.*\\\\n\\\\t\""] \
> +    "continue until set watchpoint hits"
> +
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "Hardware watchpoint ${decimal}: -location b\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 101 'e'" \
> +	 "New value = 114 'r'" \
> +	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
> +	 "${decimal}\\s+__asm__ volatile \\(\"cpyp.*\\\\n\\\\t\""] \
> +    "continue until cpy watchpoint hits"
> +
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "Hardware watchpoint ${decimal}: -location c\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 100 'd'" \
> +	 "New value = 114 'r'" \
> +	 "$hex in main \\(\\) at .*aarch64-mops-watchpoint.c:$decimal" \
> +	 "${decimal}\\s+__asm__ volatile \\(\"cpyfp.*\\\\n\\\\t\""] \
> +    "continue until cpyf watchpoint hits"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0d78691c381b..25b272fdaabc 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4497,6 +4497,67 @@ proc aarch64_supports_sme_svl { length } {
>      return 1
>  }
>  
> +# Run a test on the target to see if it supports Aarch64 MOPS (Memory

s/Aarch64/AArch64

> +# Operations) extensions.  Return 0 if so, 1 if it does not.  Note this causes
> +# a restart of GDB.
> +
> +gdb_caching_proc allow_aarch64_mops_tests {} {
> +    global srcdir subdir gdb_prompt inferior_exited_re
> +
> +    set me "allow_aarch64_mops_tests"
> +
> +    if { ![is_aarch64_target]} {
> +	return 0
> +    }
> +
> +    # ARMv9.3-A contains the MOPS extension.  The test program doesn't use it,
> +    # but take the opportunity to check whether the toolchain knows about MOPS.
> +    set compile_flags "{additional_flags=-march=armv9.3-a}"
> +
> +    # Compile a program that tests the MOPS feature.
> +    set src {
> +	#include <stdbool.h>
> +	#include <sys/auxv.h>
> +
> +	#ifndef HWCAP2_MOPS
> +	#define HWCAP2_MOPS (1UL << 43)
> +	#endif
> +
> +	int main() {
> +	    bool mops_supported = getauxval (AT_HWCAP2) & HWCAP2_MOPS;
> +
> +	    return !mops_supported;
> +	}
> +    }
> +
> +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
> +	return 0
> +    }
> +
> +    # Compilation succeeded so now run it via gdb.
> +    clean_restart $obj
> +    gdb_run_cmd
> +    gdb_expect {
> +	-re ".*$inferior_exited_re with code 01.*${gdb_prompt} $" {
> +	    verbose -log "\n$me mops support not detected"
> +	    set allow_mops_tests 0
> +	}
> +	-re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> +	    verbose -log "\n$me: mops support detected"
> +	    set allow_mops_tests 1
> +	}
> +	default {
> +	  warning "\n$me: default case taken"
> +	    set allow_mops_tests 0
> +	}
> +    }
> +    gdb_exit
> +    remote_file build delete $obj
> +
> +    verbose "$me:  returning $allow_mops_tests" 2
> +    return $allow_mops_tests
> +}
> +
>  # A helper that compiles a test case to see if __int128 is supported.
>  proc gdb_int128_helper {lang} {
>      return [gdb_can_simple_compile "i128-for-$lang" {

Re: [PATCH v3 4/5] gdb/testsuite: Add gdb.arch/aarch64-mops-single-step.exp

[Luis Machado]

Thanks. Just one nit.

On 5/10/24 06:24, Thiago Jung Bauermann wrote:
> The testcase verifies that MOPS sequences are correctly single-stepped.
> ---
>  .../gdb.arch/aarch64-mops-single-step.c       |  73 ++++++++++
>  .../gdb.arch/aarch64-mops-single-step.exp     | 132 ++++++++++++++++++
>  2 files changed, 205 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
> 
> Changes in v3:
> - Renamed gdb.arch/aarch64-mops-atomic-inst.exp to
>   gdb.arch/aarch64-mops-single-step.exp.
> - Adjusted test to expect the MOPS sequence to reset back to the prologue
>   instruction.
> - Set size variable before the cpyf and cpy sequences, because after each
>   sequence the variable is set to zero.  This bug didn't affect the
>   effectiveness of the test.
> 
> Changes in v2:
> - Add prfm instruction after each MOPS sequence and look for it in the
>   testcase to verify that the sequence was stepped through (Suggested by
>   Christophe).
> 
> diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
> new file mode 100644
> index 000000000000..4a27867d4b57
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.c
> @@ -0,0 +1,73 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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/>.  */
> +
> +#define TEST_STRING "Just a test string."
> +#define BUF_SIZE sizeof(TEST_STRING)
> +
> +int
> +main (void)
> +{
> +  char source[BUF_SIZE] = TEST_STRING;
> +  char dest[BUF_SIZE];
> +  char *p, *q;
> +  long size, zero;
> +
> +  /* Note: The prfm instruction in the asm statements below is there just
> +     to allow the testcase to recognize when the PC is at the instruction
> +     right after the MOPS sequence.  */
> +
> +  p = dest;
> +  size = sizeof (dest);
> +  zero = 0;
> +  /* Break memset.  */
> +  /* memset implemented in MOPS instructions.  */
> +  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
> +		    "setm [%0]!, %1!, %2\n\t"
> +		    "sete [%0]!, %1!, %2\n\t"
> +		    "prfm pldl3keep, [%0, #0]\n\t"
> +		    : "+&r"(p), "+&r"(size)
> +		    : "r"(zero)
> +		    : "memory");
> +
> +  p = dest;
> +  q = source;
> +  size = sizeof (dest);
> +  /* Break memcpy.  */
> +  /* memcpy implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyfp [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfm [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfe [%0]!, [%1]!, %2!\n\t"
> +		    "prfm pldl3keep, [%0, #0]\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +
> +  p = dest;
> +  q = source;
> +  size = sizeof (dest);
> +  /* Break memmove.  */
> +  /* memmove implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyp [%0]!, [%1]!, %2!\n\t"
> +		    "cpym [%0]!, [%1]!, %2!\n\t"
> +		    "cpye [%0]!, [%1]!, %2!\n\t"
> +		    "prfm pldl3keep, [%0, #0]\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
> new file mode 100644
> index 000000000000..a6390d4bff7e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
> @@ -0,0 +1,132 @@
> +# Copyright 2024 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.
> +
> +# Test single stepping through MOPS (memory operations) instruction sequences.
> +
> +require allow_aarch64_mops_tests
> +
> +standard_testfile
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  [list debug additional_flags=-march=armv9.3-a]] } {
> +    return -1
> +}
> +
> +# INSTRUCTION should be just its mnemonic, without any arguments.
> +proc is_at_instruction { instruction } {
> +    global gdb_prompt hex
> +
> +    set test "pc points to $instruction"
> +    gdb_test_multiple {x/i $pc} $test {
> +	-re -wrap "=> $hex \[^\r\n\]+:\t$instruction\t\[^\r\n\]+" {
> +	    return 1
> +	}
> +	-re "\r\n$gdb_prompt $" {
> +	    return 0
> +	}
> +    }
> +
> +    return 0
> +}
> +
> +proc arrive_at_instruction { instruction } {
> +    set count 0
> +
> +    while { [is_at_instruction $instruction] != 1 } {
> +	gdb_test -nopass "stepi" ".*__asm__ volatile.*" \
> +	    "stepi #$count to reach $instruction"
> +	incr count
> +
> +	if { $count > 50 } {
> +	    fail "didn't reach $instruction"
> +	    return 0
> +	}
> +    }
> +
> +    return 1
> +}
> +
> +# If the inferior is reschedule to another CPU while a main or epilogue

s/reschedule/rescheduled

> +# instruction is executed, the OS resets the inferior back to the prologue
> +# instruction, so we need to allow for that possibility.
> +proc step_through_sequence { prefix } {
> +    set count 0
> +
> +    while { [is_at_instruction ${prefix}p] == 1 && $count < 50 } {
> +	incr count
> +
> +	# The stepi output isn't useful to detect whether we stepped over
> +	# the instruction.
> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}p"
> +	if { [is_at_instruction ${prefix}m] == 1 } {
> +	    pass "stepped over ${prefix}p"
> +	} else {
> +	    fail "stepped over ${prefix}e"
> +	    return 0
> +	}
> +
> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}m"
> +	if { [is_at_instruction ${prefix}e] == 1 } {
> +	    pass "stepped over ${prefix}m"
> +	} elseif { [is_at_instruction ${prefix}p] == 1 } {
> +	    # The inferior was rescheduled to another CPU.
> +	    pass "${prefix}m: reset back to prologue"
> +	    continue
> +	} else {
> +	    fail "stepped over ${prefix}m"
> +	    return 0
> +	}
> +
> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}e"
> +	if { [is_at_instruction prfm] == 1 } {
> +	    pass "stepped over ${prefix}e"
> +	    return 1
> +	} elseif { [is_at_instruction ${prefix}p] == 1 } {
> +	    # The inferior was rescheduled to another CPU.
> +	    pass "${prefix}e: reset back to prologue"
> +	    continue
> +	}
> +    }
> +
> +    fail "step through $prefix sequence"
> +    return 0
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memset"]
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memcpy"]
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "Break memmove"]
> +
> +gdb_continue_to_breakpoint "memset breakpoint"
> +
> +if { [arrive_at_instruction setp] } {
> +    step_through_sequence set
> +}
> +
> +gdb_continue_to_breakpoint "memcpy breakpoint"
> +
> +if { [arrive_at_instruction cpyfp] } {
> +    step_through_sequence cpyf
> +}
> +
> +gdb_continue_to_breakpoint "memmove breakpoint"
> +
> +if { [arrive_at_instruction cpyp] } {
> +    step_through_sequence cpy
> +}

Re: [PATCH v3 5/5] gdb/testsuite: Add gdb.reverse/aarch64-mops.exp

[Luis Machado]

Thanks. Looks OK to me.

On 5/10/24 06:24, Thiago Jung Bauermann wrote:
> The testcase verifies that MOPS instructions are recorded and correctly
> reversed.  Not all variants of the copy and set instructions are tested,
> since there are many and the record and replay target processes them in
> the same way.
> ---
>  gdb/testsuite/gdb.reverse/aarch64-mops.c   |  71 +++++++++
>  gdb/testsuite/gdb.reverse/aarch64-mops.exp | 171 +++++++++++++++++++++
>  2 files changed, 242 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
>  create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp
> 
> No change since v1.
> 
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.c b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> new file mode 100644
> index 000000000000..513f324b9dd6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> @@ -0,0 +1,71 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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/>.  */
> +
> +#define TEST_STRING "Just a test string."
> +#define BUF_SIZE sizeof(TEST_STRING)
> +
> +int
> +main (void)
> +{
> +  char dest[BUF_SIZE];
> +  char source[BUF_SIZE] = TEST_STRING;
> +  register char *p asm ("x19");
> +  register char *q asm ("x20");
> +  register long size asm ("x21");
> +  register long zero asm ("x22");
> +
> +  p = dest;
> +  size = BUF_SIZE;
> +  zero = 0;
> +  /* Before setp.  */
> +  /* memset implemented in MOPS instructions.  */
> +  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
> +                    "setm [%0]!, %1!, %2\n\t"
> +                    "sete [%0]!, %1!, %2\n\t"
> +                    : "+&r"(p), "+&r"(size)
> +                    : "r"(zero)
> +                    : "memory");
> +
> +  /* After sete.  */
> +  p = dest;
> +  q = source;
> +  size = BUF_SIZE;
> +  /* Before cpyp.  */
> +  /* memmove implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpym   [%0]!, [%1]!, %2!\n\t"
> +		    "cpye   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +  /* After cpye.  */
> +  p = dest;
> +  q = source;
> +  size = BUF_SIZE;
> +  /* Before cpyfp.  */
> +  /* memcpy implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyfp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfm   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfe   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +  /* After cpyfe.  */
> +  p = dest;
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.exp b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
> new file mode 100644
> index 000000000000..f9c1257e0b11
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
> @@ -0,0 +1,171 @@
> +# Copyright 2024 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/>.
> +
> +# Test instruction record for AArch64 FEAT_MOPS instructions.
> +# Based on gdb.reverse/ppc_record_test_isa_3_1.exp
> +#
> +# The basic flow of the record tests are:
> +#    1) Stop before executing the instructions of interest.  Record
> +#       the initial value of the registers that the instruction will
> +#       change, i.e. the destination register.
> +#    2) Execute the instructions.  Record the new value of the
> +#       registers that changed.
> +#    3) Reverse the direction of the execution and execute back to
> +#       just before the instructions of interest.  Record the final
> +#       value of the registers of interest.
> +#    4) Check that the initial and new values of the registers are
> +#       different, i.e. the instruction changed the registers as expected.
> +#    5) Check that the initial and final values of the registers are
> +#       the same, i.e. GDB record restored the registers to their
> +#       original values.
> +
> +require allow_aarch64_mops_tests
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  [list debug additional_flags=-march=armv9.3-a]] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test_no_output "record full"
> +
> +proc do_test { insn_prefix } {
> +    global decimal hex
> +
> +    set before_seq [gdb_get_line_number "Before ${insn_prefix}p"]
> +    set after_seq [gdb_get_line_number "After ${insn_prefix}e"]
> +
> +    with_test_prefix $insn_prefix {
> +	gdb_test "break $before_seq" \
> +	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	    "break before instruction sequence"
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
> +	    "about to execute instruction sequence"
> +
> +	# Record the initial register values.
> +	set x19_initial [capture_command_output "info register x19" ""]
> +	set x21_initial [capture_command_output "info register x21" ""]
> +
> +	# The set instructions use the ZERO variable, but not Q, and the other
> +	# instructions are the opposite.
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_initial [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_initial [capture_command_output "info register x20" ""]
> +	}
> +
> +	gdb_test "break $after_seq" \
> +	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	    "break after instruction sequence"
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+p = dest;"] \
> +	    "executed instruction sequence"
> +
> +	# Record the new register values.
> +	set x19_new [capture_command_output "info register x19" ""]
> +	set x21_new [capture_command_output "info register x21" ""]
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_new [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_new [capture_command_output "info register x20" ""]
> +	}
> +
> +	# Execute in reverse to before the instruction sequence.
> +	gdb_test_no_output "set exec-direction reverse"
> +
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
> +	    "reversed execution of instruction sequence"
> +
> +	# Record the final register values.
> +	set x19_final [capture_command_output "info register x19" ""]
> +	set x21_final [capture_command_output "info register x21" ""]
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_final [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_final [capture_command_output "info register x20" ""]
> +	}
> +
> +	# Check initial and new values of x19 are different.
> +	gdb_assert [string compare $x19_initial $x19_new] \
> +	    "check x19 initial value versus x19 new value"
> +
> +	# Check initial and new values of x21 are different.
> +	gdb_assert [string compare $x21_initial $x21_new] \
> +	    "check x21 initial value versus x21 new value"
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    # Check initial and new values of x22 are the same.
> +	    # The register with the value to set shouldn't change.
> +	    gdb_assert ![string compare $x22_initial $x22_new] \
> +		"check x22 initial value versus x22 new value"
> +	} else {
> +	    # Check initial and new values of x20 are different.
> +	    gdb_assert [string compare $x20_initial $x20_new] \
> +		"check x20 initial value versus x20 new value"
> +	}
> +
> +	# Check initial and final values of x19 are the same.
> +	gdb_assert ![string compare $x19_initial $x19_final] \
> +	    "check x19 initial value versus x19 final value"
> +
> +	# Check initial and final values of x21 are the same.
> +	gdb_assert ![string compare $x21_initial $x21_final] \
> +	    "check x21 initial value versus x21 final value"
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    # Check initial and final values of x22 are the same.
> +	    # The register with the value to set shouldn't change.
> +	    gdb_assert ![string compare $x22_initial $x22_final] \
> +		"check x22 initial value versus x22 final value"
> +	} else {
> +	    # Check initial and final values of x20 are the same.
> +	    gdb_assert ![string compare $x20_initial $x20_final] \
> +		"check x20 initial value versus x20 final value"
> +	}
> +
> +	# Restore forward execution and go to end of recording.
> +	gdb_test_no_output "set exec-direction forward"
> +	gdb_test "record goto end" \
> +	    [multi_line \
> +		"Go forward to insn number $decimal" \
> +		"#0  main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		"$decimal\[ \t\]+p = dest;"]
> +    }
> +}
> +
> +do_test "set"
> +do_test "cpy"
> +do_test "cpyf"

Re: [PATCH v3 0/5] Add support for AArch64 MOPS instructions

[Pedro Alves]

On 2024-05-10 06:24, Thiago Jung Bauermann wrote:
> Hello,
> 
> This version is to adapt to Luis' clarification that MOPS instructions
> don't need to be treated as atomic sequences and can be single-stepped.
> If the OS reschedules the inferior to a different CPU while a main or
> epilogue instruction is executed, it will reset the sequence back to the
> prologue instruction.

Curious -- if you single step each of the instructions, then there will
be kernel code executed on the CPU in between each of the instructions
in the sequence, and other userspace code (of other tasks too, like the
debugger itself, potentially).  So the kernel is free to context switch
in between the instructions in the sequence, and _only_ restarts the sequence
when the task is moved to another CPU?  Weird that it can context switch
without losing state on the same CPU but not to a different CPU.

But then again, I have no idea what the instructions themselves do.  :-)

Re: [PATCH v3 5/5] gdb/testsuite: Add gdb.reverse/aarch64-mops.exp

[Guinevere Larsen]

On 5/10/24 02:24, Thiago Jung Bauermann wrote:
> The testcase verifies that MOPS instructions are recorded and correctly
> reversed.  Not all variants of the copy and set instructions are tested,
> since there are many and the record and replay target processes them in
> the same way.

Hi!

Thanks for adding this test! I agree that, if the record backend treats 
all instructions the same, there's no need to test them all.

As I mentioned in my reply to v2, I'd prefer if this was squashed to 
commit 4, but if you don't end up sending a new series, or feel strongly 
that they shouldn't be together, I'm ok with separating the commits.

I have a couple questions inlined.

> ---
>   gdb/testsuite/gdb.reverse/aarch64-mops.c   |  71 +++++++++
>   gdb/testsuite/gdb.reverse/aarch64-mops.exp | 171 +++++++++++++++++++++
>   2 files changed, 242 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
>   create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp
>
> No change since v1.
>
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.c b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> new file mode 100644
> index 000000000000..513f324b9dd6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> @@ -0,0 +1,71 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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/>.  */
> +
> +#define TEST_STRING "Just a test string."
> +#define BUF_SIZE sizeof(TEST_STRING)
> +
> +int
> +main (void)
> +{
> +  char dest[BUF_SIZE];
> +  char source[BUF_SIZE] = TEST_STRING;
> +  register char *p asm ("x19");
> +  register char *q asm ("x20");
> +  register long size asm ("x21");
> +  register long zero asm ("x22");

I remember that its not always easy to guarantee that a variable will 
end up in a register, so this has me slightly confused. Is this 
portable? Have you/could you double check that clang also does what you 
expect?

If it isn't portable, please add the gcc requirement, so we don't get 
new failures in clang testing :)

> +
> +  p = dest;
> +  size = BUF_SIZE;
> +  zero = 0;
> +  /* Before setp.  */
> +  /* memset implemented in MOPS instructions.  */
> +  __asm__ volatile ("setp [%0]!, %1!, %2\n\t"
> +                    "setm [%0]!, %1!, %2\n\t"
> +                    "sete [%0]!, %1!, %2\n\t"
> +                    : "+&r"(p), "+&r"(size)
> +                    : "r"(zero)
> +                    : "memory");
> +
> +  /* After sete.  */
> +  p = dest;
> +  q = source;
> +  size = BUF_SIZE;
> +  /* Before cpyp.  */
> +  /* memmove implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpym   [%0]!, [%1]!, %2!\n\t"
> +		    "cpye   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +  /* After cpye.  */
> +  p = dest;
> +  q = source;
> +  size = BUF_SIZE;
> +  /* Before cpyfp.  */
> +  /* memcpy implemented in MOPS instructions.  */
> +  __asm__ volatile ("cpyfp   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfm   [%0]!, [%1]!, %2!\n\t"
> +		    "cpyfe   [%0]!, [%1]!, %2!\n\t"
> +		    : "+&r" (p), "+&r" (q), "+&r" (size)
> +		    :
> +		    : "memory");
> +  /* After cpyfe.  */
> +  p = dest;
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.exp b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
> new file mode 100644
> index 000000000000..f9c1257e0b11
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
> @@ -0,0 +1,171 @@
> +# Copyright 2024 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/>.
> +
> +# Test instruction record for AArch64 FEAT_MOPS instructions.
> +# Based on gdb.reverse/ppc_record_test_isa_3_1.exp
> +#
> +# The basic flow of the record tests are:
> +#    1) Stop before executing the instructions of interest.  Record
> +#       the initial value of the registers that the instruction will
> +#       change, i.e. the destination register.
> +#    2) Execute the instructions.  Record the new value of the
> +#       registers that changed.
> +#    3) Reverse the direction of the execution and execute back to
> +#       just before the instructions of interest.  Record the final
> +#       value of the registers of interest.
> +#    4) Check that the initial and new values of the registers are
> +#       different, i.e. the instruction changed the registers as expected.
> +#    5) Check that the initial and final values of the registers are
> +#       the same, i.e. GDB record restored the registers to their
> +#       original values.
> +
> +require allow_aarch64_mops_tests
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  [list debug additional_flags=-march=armv9.3-a]] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test_no_output "record full"
> +
> +proc do_test { insn_prefix } {

more of a personal preference, maybe, but you could use:

     foreach_with_prefix insn_prefix {"set" "cpy" "cpyf"}

> +    global decimal hex
> +
> +    set before_seq [gdb_get_line_number "Before ${insn_prefix}p"]
> +    set after_seq [gdb_get_line_number "After ${insn_prefix}e"]
> +
> +    with_test_prefix $insn_prefix {
> +	gdb_test "break $before_seq" \
> +	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	    "break before instruction sequence"
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
> +	    "about to execute instruction sequence"

Why did you choose to not use gdb_continue_to_breakpoint in here?

Similar question to all other uses of "continue"

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> +
> +	# Record the initial register values.
> +	set x19_initial [capture_command_output "info register x19" ""]
> +	set x21_initial [capture_command_output "info register x21" ""]
> +
> +	# The set instructions use the ZERO variable, but not Q, and the other
> +	# instructions are the opposite.
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_initial [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_initial [capture_command_output "info register x20" ""]
> +	}
> +
> +	gdb_test "break $after_seq" \
> +	    "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	    "break after instruction sequence"
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+p = dest;"] \
> +	    "executed instruction sequence"
> +
> +	# Record the new register values.
> +	set x19_new [capture_command_output "info register x19" ""]
> +	set x21_new [capture_command_output "info register x21" ""]
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_new [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_new [capture_command_output "info register x20" ""]
> +	}
> +
> +	# Execute in reverse to before the instruction sequence.
> +	gdb_test_no_output "set exec-direction reverse"
> +
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Continuing\\." \
> +		 "" \
> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		 "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
> +	    "reversed execution of instruction sequence"
> +
> +	# Record the final register values.
> +	set x19_final [capture_command_output "info register x19" ""]
> +	set x21_final [capture_command_output "info register x21" ""]
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    set x22_final [capture_command_output "info register x22" ""]
> +	} else {
> +	    set x20_final [capture_command_output "info register x20" ""]
> +	}
> +
> +	# Check initial and new values of x19 are different.
> +	gdb_assert [string compare $x19_initial $x19_new] \
> +	    "check x19 initial value versus x19 new value"
> +
> +	# Check initial and new values of x21 are different.
> +	gdb_assert [string compare $x21_initial $x21_new] \
> +	    "check x21 initial value versus x21 new value"
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    # Check initial and new values of x22 are the same.
> +	    # The register with the value to set shouldn't change.
> +	    gdb_assert ![string compare $x22_initial $x22_new] \
> +		"check x22 initial value versus x22 new value"
> +	} else {
> +	    # Check initial and new values of x20 are different.
> +	    gdb_assert [string compare $x20_initial $x20_new] \
> +		"check x20 initial value versus x20 new value"
> +	}
> +
> +	# Check initial and final values of x19 are the same.
> +	gdb_assert ![string compare $x19_initial $x19_final] \
> +	    "check x19 initial value versus x19 final value"
> +
> +	# Check initial and final values of x21 are the same.
> +	gdb_assert ![string compare $x21_initial $x21_final] \
> +	    "check x21 initial value versus x21 final value"
> +
> +	if {[string compare $insn_prefix "set"] == 0} {
> +	    # Check initial and final values of x22 are the same.
> +	    # The register with the value to set shouldn't change.
> +	    gdb_assert ![string compare $x22_initial $x22_final] \
> +		"check x22 initial value versus x22 final value"
> +	} else {
> +	    # Check initial and final values of x20 are the same.
> +	    gdb_assert ![string compare $x20_initial $x20_final] \
> +		"check x20 initial value versus x20 final value"
> +	}
> +
> +	# Restore forward execution and go to end of recording.
> +	gdb_test_no_output "set exec-direction forward"
> +	gdb_test "record goto end" \
> +	    [multi_line \
> +		"Go forward to insn number $decimal" \
> +		"#0  main \\(\\) at .*/aarch64-mops.c:$decimal" \
> +		"$decimal\[ \t\]+p = dest;"]
> +    }
> +}
> +
> +do_test "set"
> +do_test "cpy"
> +do_test "cpyf"
>

Re: [PATCH v3 0/5] Add support for AArch64 MOPS instructions

[Thiago Jung Bauermann]

Pedro Alves <pedro@palves.net> writes:

> On 2024-05-10 06:24, Thiago Jung Bauermann wrote:
>> Hello,
>> 
>> This version is to adapt to Luis' clarification that MOPS instructions
>> don't need to be treated as atomic sequences and can be single-stepped.
>> If the OS reschedules the inferior to a different CPU while a main or
>> epilogue instruction is executed, it will reset the sequence back to the
>> prologue instruction.
>
> Curious -- if you single step each of the instructions, then there will
> be kernel code executed on the CPU in between each of the instructions
> in the sequence, and other userspace code (of other tasks too, like the
> debugger itself, potentially).  So the kernel is free to context switch
> in between the instructions in the sequence, and _only_ restarts the sequence
> when the task is moved to another CPU?  Weird that it can context switch
> without losing state on the same CPU but not to a different CPU.

The kernel commits implementing this behaviour actually have a good
explanation on this:

$ git log --reverse -n2 8cd076a67dc8 
commit 8536ceaa747174ded7983f13906b225e0c33ac51
Author:     Kristina Martsenko <kristina.martsenko@arm.com>
AuthorDate: Tue May 9 15:22:31 2023 +0100
Commit:     Catalin Marinas <catalin.marinas@arm.com>
CommitDate: Mon Jun 5 17:05:41 2023 +0100

    arm64: mops: handle MOPS exceptions
    
    The memory copy/set instructions added as part of FEAT_MOPS can take an
    exception (e.g. page fault) part-way through their execution and resume
    execution afterwards.
    
    If however the task is re-scheduled and execution resumes on a different
    CPU, then the CPU may take a new type of exception to indicate this.
    This is because the architecture allows two options (Option A and Option
    B) to implement the instructions and a heterogeneous system can have
    different implementations between CPUs.
    
    In this case the OS has to reset the registers and restart execution
    from the prologue instruction. The algorithm for doing this is provided
    as part of the Arm ARM.
    
    Add an exception handler for the new exception and wire it up for
    userspace tasks.
    
    Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
    Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
    Link: https://lore.kernel.org/r/20230509142235.3284028-8-kristina.martsenko@arm.com
    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

commit 8cd076a67dc8eac5d613b3258f656efa7a54412e
Author:     Kristina Martsenko <kristina.martsenko@arm.com>
AuthorDate: Tue May 9 15:22:32 2023 +0100
Commit:     Catalin Marinas <catalin.marinas@arm.com>
CommitDate: Mon Jun 5 17:05:41 2023 +0100

    arm64: mops: handle single stepping after MOPS exception
    
    When a MOPS main or epilogue instruction is being executed, the task may
    get scheduled on a different CPU and restart execution from the prologue
    instruction. If the main or epilogue instruction is being single stepped
    then it makes sense to finish the step and take the step exception
    before starting to execute the next (prologue) instruction. So
    fast-forward the single step state machine when taking a MOPS exception.
    
    This means that if a main or epilogue instruction is single stepped with
    ptrace, the debugger will sometimes observe the PC moving back to the
    prologue instruction. (As already mentioned, this should be rare as it
    only happens when the task is scheduled to another CPU during the step.)
    
    This also ensures that perf breakpoints count prologue instructions
    consistently (i.e. every time they are executed), rather than skipping
    them when there also happens to be a breakpoint on a main or epilogue
    instruction.
    
    Acked-by: Catalin Marinas <catalin.marinas@arm.com>
    Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
    Link: https://lore.kernel.org/r/20230509142235.3284028-9-kristina.martsenko@arm.com
    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>


> But then again, I have no idea what the instructions themselves do.  :-)

From the Arm ARM:

"CPYP performs some preconditioning of the arguments suitable for using
the CPYM instruction, and performs an IMPLEMENTATION DEFINED amount of
the memory copy. CPYM performs an IMPLEMENTATION DEFINED amount of the
memory copy. CPYE performs the last part of the memory copy."

Ditto for other kinds of prologue, main and epilogue instructions.
I would say that the prologue instruction is copies some poorly aligned
bytes at the beginning of the memory region, then the main instruction
copies the memory in chunks that are convenient for the processor
implementation, then the epilogue instruction copies the remaining
poorly aligned bytes at the end.

-- 
Thiago

Re: [PATCH v3 5/5] gdb/testsuite: Add gdb.reverse/aarch64-mops.exp

[Thiago Jung Bauermann]

Hello Guinevere,

Thank you for your review!

Guinevere Larsen <blarsen@redhat.com> writes:

> On 5/10/24 02:24, Thiago Jung Bauermann wrote:
>> The testcase verifies that MOPS instructions are recorded and correctly
>> reversed.  Not all variants of the copy and set instructions are tested,
>> since there are many and the record and replay target processes them in
>> the same way.
>
> Hi!
>
> Thanks for adding this test! I agree that, if the record backend treats all instructions
> the same, there's no need to test them all.

Nice, thanks for confirming.

> As I mentioned in my reply to v2, I'd prefer if this was squashed to commit 4, but if you
> don't end up sending a new series, or feel strongly that they shouldn't be together, I'm
> ok with separating the commits.

No problem, makes sense to me. Though I assume you mean commit 2. Done
for v4.

>> +int
>> +main (void)
>> +{
>> +  char dest[BUF_SIZE];
>> +  char source[BUF_SIZE] = TEST_STRING;
>> +  register char *p asm ("x19");
>> +  register char *q asm ("x20");
>> +  register long size asm ("x21");
>> +  register long zero asm ("x22");
>
> I remember that its not always easy to guarantee that a variable will end up in a
> register, so this has me slightly confused. Is this portable? Have you/could you double
> check that clang also does what you expect?
>
> If it isn't portable, please add the gcc requirement, so we don't get new failures in
> clang testing :)

That's a good point, thanks for bringing it up. This is a GNU C
extension, documented here:

https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html

And it's only supported for one specific case:

 "The only supported use for this feature is to specify registers for
  input and output operands when calling Extended asm (see Extended Asm
  - Assembler Instructions with C Expression Operands)."

which is how I'm using it in the testcase.

This extension isn't listed in Clang's list of supported extensions
here:

https://clang.llvm.org/docs/LanguageExtensions.html

Though that page says:

 "This document describes the language extensions provided by Clang. In
  addition to the language extensions listed here, Clang aims to support
  a broad range of GCC extensions. Please see the GCC manual for more
  information on these extensions."

I checked and Clang does what I expect and doesn't warn about it, so it
is indeed supported.

That said, the line numbers it generates for the asm statement are a bit
strange and the testcase ends up setting a breakpoint a few instructions
before the prologue instruction it is aiming for. To make the testcase
support Clang, I had to make it check which instruction GDB is stopped
at and stepi until it reaches the intended instruction.

>> +require allow_aarch64_mops_tests
>> +
>> +standard_testfile
>> +
>> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
>> +	  [list debug additional_flags=-march=armv9.3-a]] } {
>> +    return -1
>> +}
>> +
>> +if ![runto_main] {
>> +    return -1
>> +}
>> +
>> +gdb_test_no_output "record full"
>> +
>> +proc do_test { insn_prefix } {
>
> more of a personal preference, maybe, but you could use:
>
>     foreach_with_prefix insn_prefix {"set" "cpy" "cpyf"}

I agree it is better, thanks for the suggestion. Done in v4.

>> +    global decimal hex
>> +
>> +    set before_seq [gdb_get_line_number "Before ${insn_prefix}p"]
>> +    set after_seq [gdb_get_line_number "After ${insn_prefix}e"]
>> +
>> +    with_test_prefix $insn_prefix {
>> +	gdb_test "break $before_seq" \
>> + "Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
>> +	    "break before instruction sequence"
>> +	gdb_test "continue" \
>> +	    [multi_line \
>> +		 "Continuing\\." \
>> +		 "" \
>> +		 "Breakpoint $decimal, main \\(\\) at .*/aarch64-mops.c:$decimal" \
>> + "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""] \
>> +	    "about to execute instruction sequence"
>
> Why did you choose to not use gdb_continue_to_breakpoint in here?
>
> Similar question to all other uses of "continue"

I wasn't aware that with gdb_continue_to_breakpoint I was able to
specify the location pattern that the test should match. I'm using it
now in v4, thanks for the suggestion.

-- 
Thiago

Re: [PATCH v3 2/5] gdb/aarch64: Add record support for MOPS instructions.

[Thiago Jung Bauermann]

Hello Luis,

Luis Machado <luis.machado@arm.com> writes:

> Just a minor suggestion. Otherwise the patch looks OK.

Great, thanks!

> On 5/10/24 06:24, Thiago Jung Bauermann wrote:
>> +static unsigned int
>> +aarch64_record_memcopy_memset (aarch64_insn_decode_record *aarch64_insn_r)
>> +{
>> +  if (record_debug)
>> +    debug_printf ("Process record: memory copy and memory set\n");
>> +
>> +  uint8_t op1 = bits (aarch64_insn_r->aarch64_insn, 22, 23);
>> +  uint8_t op2 = bits (aarch64_insn_r->aarch64_insn, 12, 15);
>> +  uint32_t reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
>> +  uint32_t reg_rn = bits (aarch64_insn_r->aarch64_insn, 5, 9);
>> +  uint32_t record_buf[3];
>> +  uint64_t record_buf_mem[4];
>> +
>> +  if (op1 != 3)
>> +    {
>> +      /* Copy instructions.  */
>> +      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
>> +
>> +      record_buf[0] = reg_rd;
>> +      record_buf[1] = reg_rn;
>> +      record_buf[2] = reg_rs;
>> +      aarch64_insn_r->reg_rec_count = 3;
>> +
>> +      ULONGEST dest_addr;
>> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
>> +				  &dest_addr);
>> +      ULONGEST source_addr;
>> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
>> +				  &source_addr);
>> +      LONGEST length;
>> +      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
>> +				&length);
>> +
>> +      /* In a processor using algorithm option A, the length in Rn has an
>> +	 inverted sign.  */
>> +      if (length < 0)
>> +	length *= -1;
>> +
>> +      record_buf_mem[0] = length;
>> +      record_buf_mem[1] = dest_addr;
>> +      record_buf_mem[2] = length;
>> +      record_buf_mem[3] = source_addr;
>> +      aarch64_insn_r->mem_rec_count = 2;
>> +    }
>> +  else if ((op1 == 3 && op2 < 12) || (op1 == 3 && op2 < 12))

The or operands are identical. Fixed in v4.

>> +    {
>> +      /* Set instructions.  */
>> +      record_buf[0] = reg_rd;
>> +      record_buf[1] = reg_rn;
>> +      aarch64_insn_r->reg_rec_count = 2;
>> +
>> +      ULONGEST address;
>> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd,
>> +				  &address);
>> +
>> +      LONGEST length;
>> +      regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn,
>> +				&length);
>> +
>> +      /* In a processor using algorithm option B, the length in Rn has an
>> +	 inverted sign.  */
>> +      if (length < 0)
>> +	length *= -1;
>> +
>> +      record_buf_mem[0] = length;
>> +      record_buf_mem[1] = address;
>> +      aarch64_insn_r->mem_rec_count = 1;
>> +    }
>
> Looks like both cases above could potentially share some code in a common path. Would be
> nice
> to do so if it doesn't make the code all awkward. If it does, then this is fine.

Well spotted. It turns out that for the copy instructions we need to
record the same information as for the set instructions, plus an
additional register and memory region. Using a common path improves the
function. Thanks for the suggestion. Fixed in v4.

>> +  else
>> +    return AARCH64_RECORD_UNKNOWN;
>> +
>> +  MEM_ALLOC (aarch64_insn_r->aarch64_mems, aarch64_insn_r->mem_rec_count,
>> +	     record_buf_mem);
>> +  REG_ALLOC (aarch64_insn_r->aarch64_regs, aarch64_insn_r->reg_rec_count,
>> +	     record_buf);
>> +  return AARCH64_RECORD_SUCCESS;
>> +}

-- 
Thiago

Re: [PATCH v3 3/5] gdb/testsuite: Add gdb.arch/aarch64-mops-watchpoint.exp

[Thiago Jung Bauermann]

Luis Machado <luis.machado@arm.com> writes:

> Thanks.
>
> Just a nit below. I'll set something up on my end to give these tests a try.

Thanks!

> On 5/10/24 06:24, Thiago Jung Bauermann wrote:
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 0d78691c381b..25b272fdaabc 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -4497,6 +4497,67 @@ proc aarch64_supports_sme_svl { length } {
>>      return 1
>>  }
>>  
>> +# Run a test on the target to see if it supports Aarch64 MOPS (Memory
>
> s/Aarch64/AArch64

Thanks. Fixed in v4.

>> +# Operations) extensions.  Return 0 if so, 1 if it does not.  Note this causes
>> +# a restart of GDB.
>> +
>> +gdb_caching_proc allow_aarch64_mops_tests {} {
>> +    global srcdir subdir gdb_prompt inferior_exited_re
>> +
>> +    set me "allow_aarch64_mops_tests"
>> +
>> +    if { ![is_aarch64_target]} {
>> +	return 0
>> +    }

-- 
Thiago

Re: [PATCH v3 4/5] gdb/testsuite: Add gdb.arch/aarch64-mops-single-step.exp

[Thiago Jung Bauermann]

Luis Machado <luis.machado@arm.com> writes:

> Thanks. Just one nit.

Thanks!

> On 5/10/24 06:24, Thiago Jung Bauermann wrote:
>> +# INSTRUCTION should be just its mnemonic, without any arguments.
>> +proc is_at_instruction { instruction } {
>> +    global gdb_prompt hex
>> +
>> +    set test "pc points to $instruction"
>> +    gdb_test_multiple {x/i $pc} $test {
>> +	-re -wrap "=> $hex \[^\r\n\]+:\t$instruction\t\[^\r\n\]+" {
>> +	    return 1
>> +	}
>> +	-re "\r\n$gdb_prompt $" {
>> +	    return 0
>> +	}
>> +    }
>> +
>> +    return 0
>> +}
>> +
>> +proc arrive_at_instruction { instruction } {
>> +    set count 0
>> +
>> +    while { [is_at_instruction $instruction] != 1 } {
>> +	gdb_test -nopass "stepi" ".*__asm__ volatile.*" \
>> +	    "stepi #$count to reach $instruction"
>> +	incr count
>> +
>> +	if { $count > 50 } {
>> +	    fail "didn't reach $instruction"
>> +	    return 0
>> +	}
>> +    }
>> +
>> +    return 1
>> +}
>> +
>> +# If the inferior is reschedule to another CPU while a main or epilogue
>
> s/reschedule/rescheduled

Thanks. Fixed in v4.

>> +# instruction is executed, the OS resets the inferior back to the prologue
>> +# instruction, so we need to allow for that possibility.
>> +proc step_through_sequence { prefix } {
>> +    set count 0
>> +
>> +    while { [is_at_instruction ${prefix}p] == 1 && $count < 50 } {
>> +	incr count
>> +
>> +	# The stepi output isn't useful to detect whether we stepped over
>> +	# the instruction.
>> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}p"
>> +	if { [is_at_instruction ${prefix}m] == 1 } {
>> +	    pass "stepped over ${prefix}p"
>> +	} else {
>> +	    fail "stepped over ${prefix}e"
>> +	    return 0
>> +	}
>> +
>> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}m"
>> +	if { [is_at_instruction ${prefix}e] == 1 } {
>> +	    pass "stepped over ${prefix}m"
>> +	} elseif { [is_at_instruction ${prefix}p] == 1 } {
>> +	    # The inferior was rescheduled to another CPU.
>> +	    pass "${prefix}m: reset back to prologue"
>> +	    continue
>> +	} else {
>> +	    fail "stepped over ${prefix}m"
>> +	    return 0
>> +	}
>> +
>> +	gdb_test "stepi" "\[^\r\n\]+" "step over ${prefix}e"
>> +	if { [is_at_instruction prfm] == 1 } {
>> +	    pass "stepped over ${prefix}e"
>> +	    return 1
>> +	} elseif { [is_at_instruction ${prefix}p] == 1 } {
>> +	    # The inferior was rescheduled to another CPU.
>> +	    pass "${prefix}e: reset back to prologue"
>> +	    continue
>> +	}
>> +    }
>> +
>> +    fail "step through $prefix sequence"
>> +    return 0
>> +}

-- 
Thiago