[PATCH 2/2] S390: Fix gdbserver support for TDB

Andreas Arnez arnez@linux.vnet.ibm.com
Mon Dec 1 18:15:00 GMT 2014


On Wed, Nov 26 2014, Andreas Arnez wrote:

> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 01f11b7..5d1d9e1 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -4263,6 +4263,14 @@ regsets_fetch_inferior_registers (struct regsets_info *regsets_info,
>  	      free (buf);
>  	      continue;
>  	    }
> +	  else if (errno == ENODATA)
> +	    {
> +	      /* ENODATA may be returned if the regset is currently
> +		 not "active".  Provide a zeroed buffer and assume
> +		 that the store_function deals with this
> +		 appropriately.  */
> +	      memset (buf, 0, regset->size);
> +	    }

Well, providing a zeroed buffer may not actually the best thing to do
here.  It is just coincidence that the regset I targeted this for (the
TDB on S390) can be recognized as invalid if the buffer contains all
zeroes.  What really should be done is to enrich the store_function
interface such that "no data" can be distinguished from "zero data".
The reason I wrote it this way is to avoid impacting other platforms.
But after a quick grep through the Linux kernel it seems that there are
currently only two regsets for which ENODATA can be returned: the TDB on
S390 and the iWMMXt state on ARM.

Here's an updated version of the patch.  Instead of clearing the buffer,
this version passes a NULL pointer to the store function.  And the store
function for iWMMXt is adjusted accordingly.  Note that I've tested this
on S390, but not on ARM.


-- >8 --
Subject: [PATCH v2] S390: Fix gdbserver support for TDB

This makes gdbserver actually provide values for the TDB registers
when the inferior was stopped in a transaction.  The changes in
linux-low.c are needed for treating an unavailable TDB correctly and
without warnings.  In particular, ENODATA from ptrace is now handled
by passing a NULL pointer to the store function.  Since this may also
occur on ARM for the iWMMXt state, the patch ensures that the
respective store function can handle that.

The test case 's390-tdbregs.exp' passes with this patch and fails
without.

gdb/gdbserver/ChangeLog:

	* linux-low.c (regsets_fetch_inferior_registers): Upon ENODATA
	from ptrace, pass a NULL pointer to the store function.
	(regsets_store_inferior_registers): Skip regsets without a
	fill_function.
	* linux-arm-low.c (arm_store_wmmxregset): Accept a NULL pointer as
	buffer and then mark the registers as unavailable.
	* linux-s390-low.c (s390_fill_last_break): Remove.
	(s390_store_tdb): New.
	(s390_regsets): Set fill_function to NULL for NT_S390_LAST_BREAK.
	Add regset for NT_S390_TDB.
	(s390_arch_setup): Use regset's size instead of fill_function for
	loop end condition.
---
 gdb/gdbserver/linux-arm-low.c  |  7 +++++++
 gdb/gdbserver/linux-low.c      | 11 ++++++++++-
 gdb/gdbserver/linux-s390-low.c | 33 +++++++++++++++++++++++----------
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 8b72523..42dd521 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -199,6 +199,13 @@ arm_store_wmmxregset (struct regcache *regcache, const void *buf)
   if (!(arm_hwcap & HWCAP_IWMMXT))
     return;
 
+  if (buf == NULL)
+    {
+      for (i = 0; i < 22; i++)
+	supply_register (regcache, arm_num_regs + i, NULL);
+      return;
+    }
+
   for (i = 0; i < 16; i++)
     supply_register (regcache, arm_num_regs + i, (char *) buf + i * 8);
 
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 01f11b7..062140d 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4263,6 +4263,14 @@ regsets_fetch_inferior_registers (struct regsets_info *regsets_info,
 	      free (buf);
 	      continue;
 	    }
+	  else if (errno == ENODATA)
+	    {
+	      /* The regset is currently not "active".  For regsets
+		 where this can occur, store_function must be prepared
+		 to deal with a NULL pointer.  */
+	      free (buf);
+	      buf = NULL;
+	    }
 	  else
 	    {
 	      char s[256];
@@ -4300,7 +4308,8 @@ regsets_store_inferior_registers (struct regsets_info *regsets_info,
       void *buf, *data;
       int nt_type, res;
 
-      if (regset->size == 0 || regset_disabled (regsets_info, regset))
+      if (regset->size == 0 || regset_disabled (regsets_info, regset)
+	  || regset->fill_function == NULL)
 	{
 	  regset ++;
 	  continue;
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index 79fa6c0..2cb5a73 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -290,12 +290,6 @@ s390_fill_gregset (struct regcache *regcache, void *buf)
 /* Fill and store functions for extended register sets.  */
 
 static void
-s390_fill_last_break (struct regcache *regcache, void *buf)
-{
-  /* Last break address is read-only.  */
-}
-
-static void
 s390_store_last_break (struct regcache *regcache, const void *buf)
 {
   const char *p;
@@ -316,13 +310,32 @@ s390_store_system_call (struct regcache *regcache, const void *buf)
   supply_register_by_name (regcache, "system_call", buf);
 }
 
+static void
+s390_store_tdb (struct regcache *regcache, const void *buf)
+{
+  int tdb0 = find_regno (regcache->tdesc, "tdb0");
+  int tr0 = find_regno (regcache->tdesc, "tr0");
+  int i;
+
+  for (i = 0; i < 4; i++)
+    supply_register (regcache, tdb0 + i,
+		     buf ? (const char *) buf + 8 * i : NULL);
+
+  for (i = 0; i < 16; i++)
+    supply_register (regcache, tr0 + i,
+		     buf ? (const char *) buf + 8 * (16 + i) : NULL);
+}
+
 static struct regset_info s390_regsets[] = {
   { 0, 0, 0, 0, GENERAL_REGS, s390_fill_gregset, NULL },
-  /* Last break address is read-only; do not attempt PTRACE_SETREGSET.  */
-  { PTRACE_GETREGSET, PTRACE_GETREGSET, NT_S390_LAST_BREAK, 0,
-    EXTENDED_REGS, s390_fill_last_break, s390_store_last_break },
+  /* Last break address is read-only; no fill function.  */
+  { PTRACE_GETREGSET, -1, NT_S390_LAST_BREAK, 0, EXTENDED_REGS,
+    NULL, s390_store_last_break },
   { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_S390_SYSTEM_CALL, 0,
     EXTENDED_REGS, s390_fill_system_call, s390_store_system_call },
+  /* TDB is read-only.  */
+  { PTRACE_GETREGSET, -1, NT_S390_TDB, 0, EXTENDED_REGS,
+    NULL, s390_store_tdb },
   { 0, 0, 0, -1, -1, NULL, NULL }
 };
 
@@ -485,7 +498,7 @@ s390_arch_setup (void)
 #endif
 
   /* Update target_regsets according to available register sets.  */
-  for (regset = s390_regsets; regset->fill_function != NULL; regset++)
+  for (regset = s390_regsets; regset->size >= 0; regset++)
     if (regset->get_request == PTRACE_GETREGSET)
       switch (regset->nt_type)
 	{
-- 
1.8.4.2



More information about the Gdb-patches mailing list