[rfc][3/3] gdbserver bi-arch support: fix s390x partial register access

["Ulrich Weigand" <uweigand@de.ibm.com>]

Hello,

this is the final patch required to enable bi-arch support for gdbserver on
s390x-linux.  It fixes problems caused by accessing 32-bit register values
from a 64-bit program using the 64-bit ptrace interface and regsets.

When dealing with 32-bit partial register values, for some registers these
need to be placed in the high 32-bit of a 64-bit register, while for others
they need to be placed in the low 32-bit.  There already is a field
left_pad_xfer in the low target structure that allows the low target to
select this -- but this field currently has only a single boolean setting
which is applied to all registers.

This patch changes that field to a function depending on the register
number, and adapts the sole current user (linux-ppc64-low.c).

The second problem is that when *setting* a partial register value, current
code always pads the remainder of the register with zero.  This is a problem
with the PSW mask register on s390x, where one of the bits in the second
half needs to be and remain set; otherwise the PSW is invalid and the
inferior crashes.

The patch fixes this by changing usr_store_inferior_registers by using a
read-modify-write cycle when partially updating a register so that the
bits in the part of the register that is not changed keep their old
values.

Finally, the patch fixes s390_fill_gregset to also handle partial register
fills correctly.

With these changes, and on top of the two preceding patches, I've run a
full GDB test suite run (using gdbserver natively) on s390x-ibm-linux
using both 64-bit and 31-bit target executables.  Native s390-ibm-linux
runs also showed no regressions.

Bye,
Ulrich


ChangeLog:

	* linux-low.h (struct linux_target_ops): Change left_pad_xfer from
	boolean to function returning boolean.
	* linux-low.c (fetch_register): Pass register number into
	the_low_target.left_pad_xfer call.
	(usr_store_inferior_registers): Likewise.  Also, when changing
	partial register contents, do not pad with zero but leave old
	register contents unchanged.

	* linux-ppc64-low.c (ppc_left_pad_xfer): New function.
	(the_low_target): Use it as the_low_target.left_pad_xfer.

	* linux-s390-low.c (s390_left_pad_xfer): New function.
	(the_low_target): Use it as the_low_target.left_pad_xfer.
	(s390_fill_gregset): Handle partial register fills.


diff -urNp gdb-orig/gdb/gdbserver/linux-low.c gdb-head/gdb/gdbserver/linux-low.c
--- gdb-orig/gdb/gdbserver/linux-low.c	2008-01-18 01:00:50.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-low.c	2008-01-18 01:00:55.000000000 +0100
@@ -1384,7 +1384,7 @@ fetch_register (int regno)
 	  goto error_exit;
 	}
     }
-  if (the_low_target.left_pad_xfer
+  if (the_low_target.left_pad_xfer && the_low_target.left_pad_xfer (regno)
       && register_size (regno) < sizeof (PTRACE_XFER_TYPE))
     supply_register (regno, (buf + sizeof (PTRACE_XFER_TYPE)
 			     - register_size (regno)));
@@ -1431,7 +1431,10 @@ usr_store_inferior_registers (int regno)
 	     & - sizeof (PTRACE_XFER_TYPE);
       buf = alloca (size);
       memset (buf, 0, size);
-      if (the_low_target.left_pad_xfer
+      if (register_size (regno) < sizeof (PTRACE_XFER_TYPE))
+	*(PTRACE_XFER_TYPE *) buf = ptrace (PTRACE_PEEKUSER, inferior_pid,
+					    (PTRACE_ARG3_TYPE) regaddr, 0);
+      if (the_low_target.left_pad_xfer && the_low_target.left_pad_xfer (regno)
 	  && register_size (regno) < sizeof (PTRACE_XFER_TYPE))
 	collect_register (regno, (buf + sizeof (PTRACE_XFER_TYPE)
 				  - register_size (regno)));
diff -urNp gdb-orig/gdb/gdbserver/linux-low.h gdb-head/gdb/gdbserver/linux-low.h
--- gdb-orig/gdb/gdbserver/linux-low.h	2008-01-17 22:28:29.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-low.h	2008-01-18 01:00:55.000000000 +0100
@@ -71,7 +71,7 @@ struct linux_target_ops
 
   /* Whether to left-pad registers for PEEKUSR/POKEUSR if they are smaller
      than an xfer unit.  */
-  int left_pad_xfer;
+  int (*left_pad_xfer) (int regno);
 
   /* What string to report to GDB when it asks for the architecture,
      or NULL not to answer.  */
diff -urNp gdb-orig/gdb/gdbserver/linux-ppc64-low.c gdb-head/gdb/gdbserver/linux-ppc64-low.c
--- gdb-orig/gdb/gdbserver/linux-ppc64-low.c	2008-01-17 22:28:29.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-ppc64-low.c	2008-01-18 01:00:55.000000000 +0100
@@ -81,6 +81,12 @@ ppc_set_pc (CORE_ADDR pc)
   supply_register_by_name ("pc", &newpc);
 }
 
+static int
+ppc_left_pad_xfer (int regno)
+{
+  return 1;
+}
+
 /* Correct in either endianness.
    This instruction is "twge r2, r2", which GDB uses as a software
    breakpoint.  */
@@ -187,5 +193,5 @@ struct linux_target_ops the_low_target =
   NULL,
   NULL,
   NULL,
-  1
+  ppc_left_pad_xfer,
 };
diff -urNp gdb-orig/gdb/gdbserver/linux-s390-low.c gdb-head/gdb/gdbserver/linux-s390-low.c
--- gdb-orig/gdb/gdbserver/linux-s390-low.c	2008-01-18 01:00:50.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-s390-low.c	2008-01-18 01:00:55.000000000 +0100
@@ -79,6 +79,13 @@ s390_cannot_store_register (int regno)
   return 0;
 }
 
+static int
+s390_left_pad_xfer (int regno)
+{
+  return regno == find_regno ("pswa")
+         || (regno >= find_regno ("r0") && regno <= find_regno ("r15"));
+}
+
 /* Provide only a fill function for the general register set.  ps_lgetregs
    will use this for NPTL support.  */
 
@@ -87,7 +94,17 @@ static void s390_fill_gregset (void *buf
   int i;
 
   for (i = 0; i < 34; i++)
-    collect_register (i, (char *) buf + s390_regmap[i]);
+    {
+      int size = register_size (i);
+      int offset = s390_regmap[i];
+
+      memset ((char *) buf + offset, 0, sizeof (long));
+	
+      if (size < sizeof (long) && s390_left_pad_xfer (i))
+	offset += sizeof (long) - size;
+
+      collect_register (i, (char *) buf + offset);
+    }
 }
 
 struct regset_info target_regsets[] = {
@@ -179,5 +196,10 @@ struct linux_target_ops the_low_target =
   NULL,
   s390_breakpoint_len,
   s390_breakpoint_at,
+  NULL,
+  NULL,
+  NULL,
+  NULL,
+  s390_left_pad_xfer,
 };
 
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com