[PATCH v2] [gdb/tdep] Fix calc_ldst_datasize for strb

[Tom de Vries]

Consider aarch64 instruction strb w1, [x0].

It stores the least significant byte of 32-bit register w1 to memory, at an
address indicated by x0.

Currently, function calc_ldst_datasize returns 4 for this insn.

The way it calculates load/store data size, is to accumulate the size of the
operands before the address operand, and indeed that's 4 in this case.

But obviously, for strb the data size is 1.

The function seems to have been written specifically for FEAT_LRCPC3
(Load-Acquire RCpc instructions version 3) instructions [1].

Fix this by:
- getting the access size from the address operand, if possible, and
- otherwise falling back to the current implementation.

Add unit tests for calling calc_ldst_datasize with:
- strb,
- ldp/stp, for which the access size from the address operand needs to be
  multiplied by two,
- ldap1, an insn from gas/testsuite/gas/aarch64/rcpc3-fp.s,
- stilp, an insn from gas/testsuite/gas/aarch64/rcpc3.s, and
- mov, for which 0 should be returned.

Tested gdb and gas testsuite on aarch64-linux.

Approved-By: Luis Machado <luis.machado.foss@gmail.com> (gdb)

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

[1] https://sourceware.org/pipermail/binutils/2024-January/131805.html
---
 gdb/aarch64-tdep.c    | 45 +++++++++++++++++++++++++++++++++++++++++++
 opcodes/aarch64-opc.c | 32 ++++++++++++++++++++----------
 2 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index f2e3ce2a36c..4dd3e25955c 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -4914,6 +4914,49 @@ static void aarch64_process_record_test (void);
 }
 #endif
 
+#if GDB_SELF_TEST
+namespace selftests {
+
+static void
+aarch64_insn_analysis (void)
+{
+  uint32_t insn;
+  aarch64_inst inst;
+
+  /* 0x39000001 == strb w1, [x0].  */
+  insn = 0x39000001;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 1);
+
+  /* 0xa8c17bfd == ldp x29, x30, [sp], #16.  */
+  insn = 0xa8c17bfd;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 16);
+
+  /* 0xa9bf7bf0 == stp x16, x30, [sp, #-16]!.  */
+  insn = 0xa9bf7bf0;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 16);
+
+  /* 0x0d4187e1 == ldap1 {v1.d}[0], [sp].  */
+  insn = 0x0d4187e1;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 8);
+
+  /* 0xd9011860 == stilp x0, x1, [x3].  */
+  insn = 0xd9011860;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 16);
+
+  /* 0x52800000 == mov w0, #0x0.  */
+  insn = 0x52800000;
+  SELF_CHECK (aarch64_decode_insn (insn, &inst, true, NULL) == 0);
+  SELF_CHECK (calc_ldst_datasize (inst.operands) == 0);
+}
+
+}
+#endif
+
 INIT_GDB_FILE (aarch64_tdep)
 {
   gdbarch_register (bfd_arch_aarch64, aarch64_gdbarch_init,
@@ -4933,6 +4976,8 @@ When on, AArch64 specific debugging is enabled."),
 			    selftests::aarch64_analyze_prologue_test);
   selftests::register_test ("aarch64-process-record",
 			    selftests::aarch64_process_record_test);
+  selftests::register_test ("aarch64-insn-analysis",
+			    selftests::aarch64_insn_analysis);
 #endif
 }
 
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index 5980841c394..8cdce6f82cc 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -1750,24 +1750,36 @@ check_za_access (const aarch64_opnd_info *opnd,
   return true;
 }
 
-/* Given a load/store operation, calculate the size of transferred data via a
-   cumulative sum of qualifier sizes preceding the address operand in the
-   OPNDS operand list argument.  */
+/* Given a load/store operation, calculate the size of transferred data.  */
+
 int
 calc_ldst_datasize (const aarch64_opnd_info *opnds)
 {
-  unsigned num_bytes = 0; /* total number of bytes transferred.  */
-  enum aarch64_operand_class opnd_class;
-  enum aarch64_opnd type;
+  int addr_opnd = -1;
 
   for (int i = 0; i < AARCH64_MAX_OPND_NUM; i++)
     {
-      type = opnds[i].type;
-      opnd_class = aarch64_operands[type].op_class;
+      enum aarch64_opnd opnd_type = opnds[i].type;
+      enum aarch64_operand_class opnd_class
+	= aarch64_operands[opnd_type].op_class;
       if (opnd_class == AARCH64_OPND_CLASS_ADDRESS)
-	break;
-      num_bytes += aarch64_get_qualifier_esize (opnds[i].qualifier);
+	{
+	  addr_opnd = i;
+	  break;
+	}
     }
+
+  if (addr_opnd == -1)
+    return 0;
+
+  enum aarch64_opnd_qualifier addr_opnd_qualifier
+    = opnds[addr_opnd].qualifier;
+  if (operand_variant_qualifier_p (addr_opnd_qualifier))
+    return addr_opnd * aarch64_get_qualifier_esize (addr_opnd_qualifier);
+
+  unsigned num_bytes = 0; /* total number of bytes transferred.  */
+  for (int i = 0; i < addr_opnd; i++)
+    num_bytes += aarch64_get_qualifier_esize (opnds[i].qualifier);
   return num_bytes;
 }
 

base-commit: 09292f4ae2ccb46130652f6b310ee7a5227326d3
-- 
2.43.0