[PATCH v4 0/3] Add support for AArch64 MOPS instructions

[PATCH v4 0/3] Add support for AArch64 MOPS instructions

[Thiago Jung Bauermann]

Hello,

Almost all of the changes in this version are in patch 2. Its changelog
has the details.

One of them is a simplification of the code in
aarch64_record_memcopy_memset because Luis noticed that both code paths in
it can share code.  Also, the gdb.reverse/aarch64-mops.exp was moved from
patch 5 (which doesn't exist anymore) to patch 2, and code cleanups
suggested by Guinevere were implemented. In addition, it was adapted to
work with Clang's line number information, which considers some register
preparation instructions as part of the line with the asm statement.

Also, the testcase gdb.arch/aarch64-mops-single-step.exp was moved to
patch 1 so patch 4 doesn't exist anymore either.

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 (3):
  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/aarch64-tdep.c                            |  77 +++++++-
 .../gdb.arch/aarch64-mops-single-step.c       |  73 +++++++
 .../gdb.arch/aarch64-mops-single-step.exp     |  98 +++++++++
 .../gdb.arch/aarch64-mops-watchpoint.c        |  66 +++++++
 .../gdb.arch/aarch64-mops-watchpoint.exp      |  79 ++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.c      |  78 ++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.exp    | 186 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  99 ++++++++++
 8 files changed, 753 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: 002ccda0ef390fc2f02c0a27f01993bd5009f03d

[PATCH v4 1/3] 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.

The testcase verifies that MOPS sequences are correctly single-stepped.

PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
---

Changes in v4:
- Fix typo in comment (Spotted by Luis).
- Moved procedures is_at_instruction and arrive_at_instruction to
  lib/gdb.exp so that they can be used by gdb.reverse/aarch64-mops.exp.
- Moved procedure allow_aarch64_mops_tests to this patch.

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.

 gdb/aarch64-tdep.c                            |  8 +-
 .../gdb.arch/aarch64-mops-single-step.c       | 73 ++++++++++++++
 .../gdb.arch/aarch64-mops-single-step.exp     | 98 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     | 99 +++++++++++++++++++
 4 files changed, 275 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

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;
     }
 
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..2cb5fdcca36b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-mops-single-step.exp
@@ -0,0 +1,98 @@
+# 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
+}
+
+# If the inferior is rescheduled 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
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 55f6ab1fd8b9..762cf1cd5cf0 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -848,6 +848,44 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
     }]
 }
 
+# Check whether GDB is stopped at the given instruction.
+# 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
+}
+
+# Single-steps GDB until it arrives at the given instruction.
+# INSTRUCTION should be just its mnemonic, without any arguments.
+
+proc arrive_at_instruction { instruction } {
+    set count 0
+
+    while { [is_at_instruction $instruction] != 1 } {
+	gdb_test -nopass "stepi" "\[^\r\n\]+" \
+	    "stepi #$count to reach $instruction"
+	incr count
+
+	if { $count > 50 } {
+	    fail "didn't reach $instruction"
+	    return 0
+	}
+    }
+
+    return 1
+}
 
 # gdb_internal_error_resync:
 #
@@ -4497,6 +4535,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 1 if so, 0 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 v4 2/3] 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.

The new gdb.reverse/aarch64-mops.exp 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.

PR tdep/31666
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
---

Changes in v4:
- Implemented common path for set and copy instructions in
  aarch64_record_memcopy_memset (Suggested by Luis).
- Removed identical or operands in aarch64_record_memcopy_memset.
- Moved gdb.reverse/aarch64-mops.exp testcase to this patch (Suggested by
  Guinevere).
- Use foreach_with_prefix instead of test procedure in testcase (Suggested
  by Guinevere).
- Use gdb_continue_to_breakpoint instead of gdb_test "continue" in
  testcase (Suggested by Guinevere).
- Test that source and dest variables are correctly recorded.
- Reset dest variable after each memops sequence.
- Step to prologue instruction after hitting "before" breakpoint and put a
  breakpoint there, to cope with Clang's line number info including some
  register preparation instructions as part of the line with the asm
  statement.

No change in v1, v2 or v3.

 gdb/aarch64-tdep.c                         |  69 ++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.c   |  78 +++++++++
 gdb/testsuite/gdb.reverse/aarch64-mops.exp | 186 +++++++++++++++++++++
 3 files changed, 333 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
 create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 05ecd421cd0e..8b7582d12be9 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -5188,6 +5188,71 @@ 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 && op2 > 11)
+    /* Unallocated instructions.  */
+    return AARCH64_RECORD_UNKNOWN;
+
+  /* Set instructions have two registers and one memory region to be
+     recorded.  */
+  record_buf[0] = reg_rd;
+  record_buf[1] = reg_rn;
+  aarch64_insn_r->reg_rec_count = 2;
+
+  ULONGEST dest_addr;
+  regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd, &dest_addr);
+
+  LONGEST length;
+  regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn, &length);
+
+  /* In one ofthe algorithm options a processor can implement, the length
+     in Rn has an inverted sign.  */
+  if (length < 0)
+    length *= -1;
+
+  record_buf_mem[0] = length;
+  record_buf_mem[1] = dest_addr;
+  aarch64_insn_r->mem_rec_count = 1;
+
+  if (op1 != 3)
+    {
+      /* Copy instructions have an additional register and an additional
+	 memory region to be recorded.  */
+      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
+
+      record_buf[2] = reg_rs;
+      aarch64_insn_r->reg_rec_count++;
+
+      ULONGEST source_addr;
+      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
+				  &source_addr);
+
+      record_buf_mem[2] = length;
+      record_buf_mem[3] = source_addr;
+      aarch64_insn_r->mem_rec_count++;
+    }
+
+  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 +5530,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);
diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.c b/gdb/testsuite/gdb.reverse/aarch64-mops.c
new file mode 100644
index 000000000000..95318fed9d93
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/aarch64-mops.c
@@ -0,0 +1,78 @@
+/* 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/>.  */
+
+#include <string.h>
+
+#define INITIAL_STRING "Initial fill value."
+#define NEW_STRING "Just a test string."
+#define BUF_SIZE sizeof(NEW_STRING)
+
+int
+main (void)
+{
+  char dest[BUF_SIZE] = INITIAL_STRING;
+  char source[BUF_SIZE] = NEW_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;
+  memcpy (dest, INITIAL_STRING, sizeof (dest));
+  /* 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;
+  memcpy (dest, INITIAL_STRING, sizeof (dest));
+  /* 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..05a991d4bfb9
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
@@ -0,0 +1,186 @@
+# 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"
+
+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"]
+
+    gdb_test "break $before_seq" \
+	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
+	"break before instruction sequence"
+    gdb_continue_to_breakpoint "about to execute instruction sequence" \
+	[multi_line ".*/aarch64-mops.c:$decimal" \
+	     "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""]
+
+    # Depending on the compiler, the line number information may put GDB a few
+    # instructions before the beginning of the asm statement.
+    arrive_at_instruction "${insn_prefix}p"
+    # Add a breakpoint that we're sure is at the prologue instruction.
+    gdb_test "break *\$pc" \
+	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
+	"break at prologue instruction"
+
+    # Record the initial memory and register values.
+    set dest_initial [get_valueof "/x" "dest" "unable to read initial" \
+			  "get dest initial value"]
+    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 nor SOURCE,
+    # 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" ""]
+	set source_initial [get_valueof "/x" "source" "unable to read initial" \
+				"get source initial value"]
+    }
+
+    gdb_test "break $after_seq" \
+	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
+	"break after instruction sequence"
+    gdb_continue_to_breakpoint "executed instruction sequence" \
+	[multi_line ".*/aarch64-mops.c:$decimal" "$decimal\[ \t\]+p = dest;"]
+
+    # Record the new memory and register values.
+    set dest_new [get_valueof "/x" "dest" "unable to read new" \
+			  "get dest new value"]
+    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" ""]
+	set source_new [get_valueof "/x" "source" "unable to read new" \
+			    "get source new value"]
+    }
+
+    # Execute in reverse to before the instruction sequence.
+    gdb_test_no_output "set exec-direction reverse"
+
+    gdb_continue_to_breakpoint "reversed execution of instruction sequence" \
+	[multi_line ".*/aarch64-mops.c:$decimal" \
+	     "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""]
+
+    # Record the final memory and register values.
+    set dest_final [get_valueof "/x" "dest" "unable to read final" \
+			"get dest final value"]
+    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" ""]
+	set source_final [get_valueof "/x" "source" "unable to read final" \
+			      "get source final value"]
+    }
+
+    # Check initial and new values of dest are different.
+    gdb_assert [string compare $dest_initial $dest_new] \
+	"check dest initial value versus dest new value"
+
+    # 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 new values of source are the same.
+	gdb_assert ![string compare $source_initial $source_new] \
+	    "check source initial value versus source new value"
+    }
+
+    # Check initial and final values of dest are the same.
+    gdb_assert ![string compare $dest_initial $dest_final] \
+	"check dest initial value versus dest final 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.
+	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"
+
+	# Check initial and final values of source are the same.
+	gdb_assert ![string compare $source_initial $source_final] \
+	    "check source initial value versus source 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;"]
+}

[PATCH v4 3/3] 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.
---

Changes in v4:
- Moved procedure allow_aarch64_mops_tests to patch 1.
- Fix typo in comment (Spotted by Luis).

No change in v1, v2 or v3.

 .../gdb.arch/aarch64-mops-watchpoint.c        | 66 ++++++++++++++++
 .../gdb.arch/aarch64-mops-watchpoint.exp      | 79 +++++++++++++++++++
 2 files changed, 145 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-mops-watchpoint.exp

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"

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

[Luis Machado]

On 5/23/24 04:51, 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.
> 
> The new gdb.reverse/aarch64-mops.exp 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.
> 
> PR tdep/31666
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31666
> ---
> 
> Changes in v4:
> - Implemented common path for set and copy instructions in
>   aarch64_record_memcopy_memset (Suggested by Luis).
> - Removed identical or operands in aarch64_record_memcopy_memset.
> - Moved gdb.reverse/aarch64-mops.exp testcase to this patch (Suggested by
>   Guinevere).
> - Use foreach_with_prefix instead of test procedure in testcase (Suggested
>   by Guinevere).
> - Use gdb_continue_to_breakpoint instead of gdb_test "continue" in
>   testcase (Suggested by Guinevere).
> - Test that source and dest variables are correctly recorded.
> - Reset dest variable after each memops sequence.
> - Step to prologue instruction after hitting "before" breakpoint and put a
>   breakpoint there, to cope with Clang's line number info including some
>   register preparation instructions as part of the line with the asm
>   statement.
> 
> No change in v1, v2 or v3.
> 
>  gdb/aarch64-tdep.c                         |  69 ++++++++
>  gdb/testsuite/gdb.reverse/aarch64-mops.c   |  78 +++++++++
>  gdb/testsuite/gdb.reverse/aarch64-mops.exp | 186 +++++++++++++++++++++
>  3 files changed, 333 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.c
>  create mode 100644 gdb/testsuite/gdb.reverse/aarch64-mops.exp
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 05ecd421cd0e..8b7582d12be9 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -5188,6 +5188,71 @@ 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 && op2 > 11)
> +    /* Unallocated instructions.  */
> +    return AARCH64_RECORD_UNKNOWN;
> +
> +  /* Set instructions have two registers and one memory region to be
> +     recorded.  */
> +  record_buf[0] = reg_rd;
> +  record_buf[1] = reg_rn;
> +  aarch64_insn_r->reg_rec_count = 2;
> +
> +  ULONGEST dest_addr;
> +  regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd, &dest_addr);
> +
> +  LONGEST length;
> +  regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn, &length);
> +
> +  /* In one ofthe algorithm options a processor can implement, the length

s/ofthe/of the

> +     in Rn has an inverted sign.  */
> +  if (length < 0)
> +    length *= -1;
> +
> +  record_buf_mem[0] = length;
> +  record_buf_mem[1] = dest_addr;
> +  aarch64_insn_r->mem_rec_count = 1;
> +
> +  if (op1 != 3)
> +    {
> +      /* Copy instructions have an additional register and an additional
> +	 memory region to be recorded.  */
> +      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
> +
> +      record_buf[2] = reg_rs;
> +      aarch64_insn_r->reg_rec_count++;
> +
> +      ULONGEST source_addr;
> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
> +				  &source_addr);
> +
> +      record_buf_mem[2] = length;
> +      record_buf_mem[3] = source_addr;
> +      aarch64_insn_r->mem_rec_count++;
> +    }
> +
> +  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 +5530,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);
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-mops.c b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> new file mode 100644
> index 000000000000..95318fed9d93
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.c
> @@ -0,0 +1,78 @@
> +/* 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/>.  */
> +
> +#include <string.h>
> +
> +#define INITIAL_STRING "Initial fill value."
> +#define NEW_STRING "Just a test string."
> +#define BUF_SIZE sizeof(NEW_STRING)
> +
> +int
> +main (void)
> +{
> +  char dest[BUF_SIZE] = INITIAL_STRING;
> +  char source[BUF_SIZE] = NEW_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;
> +  memcpy (dest, INITIAL_STRING, sizeof (dest));
> +  /* 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;
> +  memcpy (dest, INITIAL_STRING, sizeof (dest));
> +  /* 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..05a991d4bfb9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-mops.exp
> @@ -0,0 +1,186 @@
> +# 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"
> +
> +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"]
> +
> +    gdb_test "break $before_seq" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	"break before instruction sequence"
> +    gdb_continue_to_breakpoint "about to execute instruction sequence" \
> +	[multi_line ".*/aarch64-mops.c:$decimal" \
> +	     "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""]
> +
> +    # Depending on the compiler, the line number information may put GDB a few
> +    # instructions before the beginning of the asm statement.
> +    arrive_at_instruction "${insn_prefix}p"
> +    # Add a breakpoint that we're sure is at the prologue instruction.
> +    gdb_test "break *\$pc" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	"break at prologue instruction"
> +
> +    # Record the initial memory and register values.
> +    set dest_initial [get_valueof "/x" "dest" "unable to read initial" \
> +			  "get dest initial value"]
> +    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 nor SOURCE,
> +    # 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" ""]
> +	set source_initial [get_valueof "/x" "source" "unable to read initial" \
> +				"get source initial value"]
> +    }
> +
> +    gdb_test "break $after_seq" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-mops.c, line $decimal\\." \
> +	"break after instruction sequence"
> +    gdb_continue_to_breakpoint "executed instruction sequence" \
> +	[multi_line ".*/aarch64-mops.c:$decimal" "$decimal\[ \t\]+p = dest;"]
> +
> +    # Record the new memory and register values.
> +    set dest_new [get_valueof "/x" "dest" "unable to read new" \
> +			  "get dest new value"]
> +    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" ""]
> +	set source_new [get_valueof "/x" "source" "unable to read new" \
> +			    "get source new value"]
> +    }
> +
> +    # Execute in reverse to before the instruction sequence.
> +    gdb_test_no_output "set exec-direction reverse"
> +
> +    gdb_continue_to_breakpoint "reversed execution of instruction sequence" \
> +	[multi_line ".*/aarch64-mops.c:$decimal" \
> +	     "$decimal\[ \t\]+__asm__ volatile \\(\"${insn_prefix}p \[^\r\n\]+\""]
> +
> +    # Record the final memory and register values.
> +    set dest_final [get_valueof "/x" "dest" "unable to read final" \
> +			"get dest final value"]
> +    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" ""]
> +	set source_final [get_valueof "/x" "source" "unable to read final" \
> +			      "get source final value"]
> +    }
> +
> +    # Check initial and new values of dest are different.
> +    gdb_assert [string compare $dest_initial $dest_new] \
> +	"check dest initial value versus dest new value"
> +
> +    # 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 new values of source are the same.
> +	gdb_assert ![string compare $source_initial $source_new] \
> +	    "check source initial value versus source new value"
> +    }
> +
> +    # Check initial and final values of dest are the same.
> +    gdb_assert ![string compare $dest_initial $dest_final] \
> +	"check dest initial value versus dest final 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.
> +	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"
> +
> +	# Check initial and final values of source are the same.
> +	gdb_assert ![string compare $source_initial $source_final] \
> +	    "check source initial value versus source 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;"]
> +}

Overall looks OK code-wise. I need to put together a newer VM with a compiler that supports
MOPS instructions. Bear with me while I set that up and give these tests a go.

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

[Thiago Jung Bauermann]

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

> On 5/23/24 04:51, Thiago Jung Bauermann wrote:
>> +/* 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 && op2 > 11)
>> +    /* Unallocated instructions.  */
>> +    return AARCH64_RECORD_UNKNOWN;
>> +
>> +  /* Set instructions have two registers and one memory region to be
>> +     recorded.  */
>> +  record_buf[0] = reg_rd;
>> +  record_buf[1] = reg_rn;
>> +  aarch64_insn_r->reg_rec_count = 2;
>> +
>> +  ULONGEST dest_addr;
>> +  regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rd, &dest_addr);
>> +
>> +  LONGEST length;
>> +  regcache_raw_read_signed (aarch64_insn_r->regcache, reg_rn, &length);
>> +
>> +  /* In one ofthe algorithm options a processor can implement, the length
>
> s/ofthe/of the

Thanks, fixed locally.

>> +     in Rn has an inverted sign.  */
>> +  if (length < 0)
>> +    length *= -1;
>> +
>> +  record_buf_mem[0] = length;
>> +  record_buf_mem[1] = dest_addr;
>> +  aarch64_insn_r->mem_rec_count = 1;
>> +
>> +  if (op1 != 3)
>> +    {
>> +      /* Copy instructions have an additional register and an additional
>> +	 memory region to be recorded.  */
>> +      uint32_t reg_rs = bits (aarch64_insn_r->aarch64_insn, 16, 20);
>> +
>> +      record_buf[2] = reg_rs;
>> +      aarch64_insn_r->reg_rec_count++;
>> +
>> +      ULONGEST source_addr;
>> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rs,
>> +				  &source_addr);
>> +
>> +      record_buf_mem[2] = length;
>> +      record_buf_mem[3] = source_addr;
>> +      aarch64_insn_r->mem_rec_count++;
>> +    }
>> +
>> +  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;
>> +}
>> +
>
> Overall looks OK code-wise. I need to put together a newer VM with a compiler that
> supports
> MOPS instructions. Bear with me while I set that up and give these tests a go.

Of course. Thanks for testing!

-- 
Thiago

Re: [PATCH v4 0/3] Add support for AArch64 MOPS instructions

[Luis Machado]

On 5/23/24 04:51, Thiago Jung Bauermann wrote:
> Hello,
> 
> Almost all of the changes in this version are in patch 2. Its changelog
> has the details.
> 
> One of them is a simplification of the code in
> aarch64_record_memcopy_memset because Luis noticed that both code paths in
> it can share code.  Also, the gdb.reverse/aarch64-mops.exp was moved from
> patch 5 (which doesn't exist anymore) to patch 2, and code cleanups
> suggested by Guinevere were implemented. In addition, it was adapted to
> work with Clang's line number information, which considers some register
> preparation instructions as part of the line with the asm statement.
> 
> Also, the testcase gdb.arch/aarch64-mops-single-step.exp was moved to
> patch 1 so patch 4 doesn't exist anymore either.
> 
> 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 (3):
>   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/aarch64-tdep.c                            |  77 +++++++-
>  .../gdb.arch/aarch64-mops-single-step.c       |  73 +++++++
>  .../gdb.arch/aarch64-mops-single-step.exp     |  98 +++++++++
>  .../gdb.arch/aarch64-mops-watchpoint.c        |  66 +++++++
>  .../gdb.arch/aarch64-mops-watchpoint.exp      |  79 ++++++++
>  gdb/testsuite/gdb.reverse/aarch64-mops.c      |  78 ++++++++
>  gdb/testsuite/gdb.reverse/aarch64-mops.exp    | 186 ++++++++++++++++++
>  gdb/testsuite/lib/gdb.exp                     |  99 ++++++++++
>  8 files changed, 753 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: 002ccda0ef390fc2f02c0a27f01993bd5009f03d

I went through the tests and validated they work fine on an emulator.

Unless there are other specific objections (from Guinevere on the record/replay side or Pedro),
this looks good to me.

Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>

Re: [PATCH v4 0/3] Add support for AArch64 MOPS instructions

[Thiago Jung Bauermann]

Hello Luis,

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

> On 5/23/24 04:51, Thiago Jung Bauermann wrote:
>> Hello,
>> 
>> Almost all of the changes in this version are in patch 2. Its changelog
>> has the details.
>> 
>> One of them is a simplification of the code in
>> aarch64_record_memcopy_memset because Luis noticed that both code paths in
>> it can share code.  Also, the gdb.reverse/aarch64-mops.exp was moved from
>> patch 5 (which doesn't exist anymore) to patch 2, and code cleanups
>> suggested by Guinevere were implemented. In addition, it was adapted to
>> work with Clang's line number information, which considers some register
>> preparation instructions as part of the line with the asm statement.
>> 
>> Also, the testcase gdb.arch/aarch64-mops-single-step.exp was moved to
>> patch 1 so patch 4 doesn't exist anymore either.
>> 
>> 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 (3):
>>   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/aarch64-tdep.c                            |  77 +++++++-
>>  .../gdb.arch/aarch64-mops-single-step.c       |  73 +++++++
>>  .../gdb.arch/aarch64-mops-single-step.exp     |  98 +++++++++
>>  .../gdb.arch/aarch64-mops-watchpoint.c        |  66 +++++++
>>  .../gdb.arch/aarch64-mops-watchpoint.exp      |  79 ++++++++
>>  gdb/testsuite/gdb.reverse/aarch64-mops.c      |  78 ++++++++
>>  gdb/testsuite/gdb.reverse/aarch64-mops.exp    | 186 ++++++++++++++++++
>>  gdb/testsuite/lib/gdb.exp                     |  99 ++++++++++
>>  8 files changed, 753 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: 002ccda0ef390fc2f02c0a27f01993bd5009f03d
>
> I went through the tests and validated they work fine on an emulator.
>
> Unless there are other specific objections (from Guinevere on the record/replay side or
> Pedro),
> this looks good to me.
>
> Approved-By: Luis Machado <luis.machado@arm.com>
> Tested-By: Luis Machado <luis.machado@arm.com>

Thank you! I'll wait for additional feedback.

I would also like to know whether it's fine to backport this series to
the release branch.

-- 
Thiago

Re: [PATCH v4 0/3] Add support for AArch64 MOPS instructions

[Luis Machado]

On 6/3/24 18:07, Thiago Jung Bauermann wrote:
> Hello Luis,
> 
> Luis Machado <luis.machado@arm.com> writes:
> 
>> On 5/23/24 04:51, Thiago Jung Bauermann wrote:
>>> Hello,
>>>
>>> Almost all of the changes in this version are in patch 2. Its changelog
>>> has the details.
>>>
>>> One of them is a simplification of the code in
>>> aarch64_record_memcopy_memset because Luis noticed that both code paths in
>>> it can share code.  Also, the gdb.reverse/aarch64-mops.exp was moved from
>>> patch 5 (which doesn't exist anymore) to patch 2, and code cleanups
>>> suggested by Guinevere were implemented. In addition, it was adapted to
>>> work with Clang's line number information, which considers some register
>>> preparation instructions as part of the line with the asm statement.
>>>
>>> Also, the testcase gdb.arch/aarch64-mops-single-step.exp was moved to
>>> patch 1 so patch 4 doesn't exist anymore either.
>>>
>>> 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 (3):
>>>   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/aarch64-tdep.c                            |  77 +++++++-
>>>  .../gdb.arch/aarch64-mops-single-step.c       |  73 +++++++
>>>  .../gdb.arch/aarch64-mops-single-step.exp     |  98 +++++++++
>>>  .../gdb.arch/aarch64-mops-watchpoint.c        |  66 +++++++
>>>  .../gdb.arch/aarch64-mops-watchpoint.exp      |  79 ++++++++
>>>  gdb/testsuite/gdb.reverse/aarch64-mops.c      |  78 ++++++++
>>>  gdb/testsuite/gdb.reverse/aarch64-mops.exp    | 186 ++++++++++++++++++
>>>  gdb/testsuite/lib/gdb.exp                     |  99 ++++++++++
>>>  8 files changed, 753 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: 002ccda0ef390fc2f02c0a27f01993bd5009f03d
>>
>> I went through the tests and validated they work fine on an emulator.
>>
>> Unless there are other specific objections (from Guinevere on the record/replay side or
>> Pedro),
>> this looks good to me.
>>
>> Approved-By: Luis Machado <luis.machado@arm.com>
>> Tested-By: Luis Machado <luis.machado@arm.com>
> 
> Thank you! I'll wait for additional feedback.
> 
> I would also like to know whether it's fine to backport this series to
> the release branch.
> 

Given it is a smaller scope of change, I don't see a problem with that.

Re: [PATCH v4 0/3] Add support for AArch64 MOPS instructions

[Thiago Jung Bauermann]

Hello,

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

> On 6/3/24 18:07, Thiago Jung Bauermann wrote:
>> Hello Luis,
>> 
>> Luis Machado <luis.machado@arm.com> writes:
>> 
>>> I went through the tests and validated they work fine on an emulator.
>>>
>>> Unless there are other specific objections (from Guinevere on the record/replay side or
>>> Pedro),
>>> this looks good to me.
>>>
>>> Approved-By: Luis Machado <luis.machado@arm.com>
>>> Tested-By: Luis Machado <luis.machado@arm.com>
>> 
>> Thank you! I'll wait for additional feedback.
>> 
>> I would also like to know whether it's fine to backport this series to
>> the release branch.
>
> Given it is a smaller scope of change, I don't see a problem with that.

Thanks! I just pushed to the main branch ending at commit 55e3fcf5e523
and to gdb-15-branch ending at commit 3504ea29464f.
-- 
Thiago