refactor: Simplify SVE interface to read/write registers

This is a patch in preparation to upcoming patches enabling SME support.  It
attempts to simplify the gdb/gdbserver shared interface used to read/write
SVE registers.

Where the current code makes use of unique_ptr, allocating a new buffer by
hand and passing a buffer around, this patch makes that code use
gdb::byte_vector and passes a reference to this byte vector to the functions,
allowing the functions to have ready access to the size of the buffer.

It also shares a bit more code between gdb and gdbserver, in particular around
handling of ptrace get/set requests for SVE.

I think gdbserver could be refactored to handle register reads/writes more
like gdb's native layer as opposed to letting the generic linux-low layer do
the ptrace calls.  This is not very flexible and assumes one size for the
responses.  If you have something like NT_ARM_SVE, where you can have either
FPSIMD or SVE contents, it doesn't work that well.

I didn't want to change that interface right now as it is a bit too much work
and touches all the targets, some of which I can't easily test.

Hence the reason why the buffer the generic linux-now passes down to
linux-aarch64-low is unused or ignored.

No user-visible changes should happen as part of this refactor other than a
slightly reworded warning message.

While doing the refactor, I also noticed what seems to be a mistake in checking
if the register cache contains active (non-zero) SVE data.

For instance, the original code did something like this in
aarch64_sve_regs_copy_from_reg_buf:

has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_Z0_REGNUM + i
				       reg, sizeof (__int128_t));

"reg" is a zeroed-out buffer that we compare the Z register contents
past the first 128 bits.  The problem here is that raw_compare returns
1 if the contents compare the same, which means has_sve_state will be
true.  But if we compared the Z register contents to 0, it means we
*do not* have SVE state, and therefore has_sve_state should be false.

The consequence of this mistake is that we convert the initial
FPSIMD-formatted data we get from ptrace for the NT_ARM_SVE register
set to a SVE-formatted one.

In the end, this doesn't cause user-visible differences because the
values of both the Z and V registers will still be the same.  But the
logic is not correct.

I used the opportunity to fix this, and it gets tested later on by
the additional SME tests.

I do plan on submitting some SVE-specific tests to make sure we have
a bit more coverage in GDB's testsuite.

Regression-tested on aarch64-linux Ubuntu 22.04/20.04.

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

1 files changed, 21 insertions, 3 deletions

diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
index 8b22f19..7c633c2 100644
--- a/gdbserver/linux-aarch64-low.cc
+++ b/gdbserver/linux-aarch64-low.cc
@@ -719,9 +719,18 @@ aarch64_target::low_new_fork (process_info *parent,
 /* Wrapper for aarch64_sve_regs_copy_to_reg_buf.  */
 
 static void
-aarch64_sve_regs_copy_to_regcache (struct regcache *regcache, const void *buf)
+aarch64_sve_regs_copy_to_regcache (struct regcache *regcache,
+				   ATTRIBUTE_UNUSED const void *buf)
 {
-  return aarch64_sve_regs_copy_to_reg_buf (regcache, buf);
+  /* BUF is unused here since we collect the data straight from a ptrace
+     request in aarch64_sve_regs_copy_to_reg_buf, therefore bypassing
+     gdbserver's own call to ptrace.  */
+
+  int tid = lwpid_of (current_thread);
+
+  /* Update the register cache.  aarch64_sve_regs_copy_to_reg_buf handles
+     fetching the NT_ARM_SVE state from thread TID.  */
+  aarch64_sve_regs_copy_to_reg_buf (tid, regcache);
 }
 
 /* Wrapper for aarch64_sve_regs_copy_from_reg_buf.  */
@@ -729,7 +738,16 @@ aarch64_sve_regs_copy_to_regcache (struct regcache *regcache, const void *buf)
 static void
 aarch64_sve_regs_copy_from_regcache (struct regcache *regcache, void *buf)
 {
-  return aarch64_sve_regs_copy_from_reg_buf (regcache, buf);
+  int tid = lwpid_of (current_thread);
+
+  /* Update the thread SVE state.  aarch64_sve_regs_copy_from_reg_buf
+     handles writing the SVE/FPSIMD state back to thread TID.  */
+  aarch64_sve_regs_copy_from_reg_buf (tid, regcache);
+
+  /* We need to return the expected data in BUF, so copy whatever the kernel
+     already has to BUF.  */
+  gdb::byte_vector sve_state = aarch64_fetch_sve_regset (tid);
+  memcpy (buf, sve_state.data (), sve_state.size ());
 }
 
 /* Array containing all the possible register sets for AArch64/Linux.  During