[RFC,12/23] regcache: Add functions suitable for regset_supply/collect.

Message ID 87vbtt3hkr.fsf@br87z6lw.de.ibm.com
State Committed
Headers

Commit Message

Andreas Arnez April 28, 2014, 9:54 a.m. UTC
  These functions are intended to suit all targets that don't require too
special logic in their regset supply/collect methods.  Having such
generic functions helps reducing target-specific complexity.

gdb/
	* regcache.c: Include "regset.h".
	(regcache_supply_regset, regcache_collect_regset): New functions.
	* regcache.h (struct regcache_map_entry): New structure.
	(REGCACHE_MAP_SKIP_BYTES): New enum value.
	(regcache_supply_regset, regcache_collect_regset): New prototypes.
---
 gdb/regcache.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/regcache.h | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)
  

Comments

Mark Kettenis May 5, 2014, 10:02 a.m. UTC | #1
> From: Andreas Arnez <arnez@linux.vnet.ibm.com>
> Date: Mon, 28 Apr 2014 11:54:12 +0200
> 
> These functions are intended to suit all targets that don't require too
> special logic in their regset supply/collect methods.  Having such
> generic functions helps reducing target-specific complexity.
> 
> gdb/
> 	* regcache.c: Include "regset.h".
> 	(regcache_supply_regset, regcache_collect_regset): New functions.
> 	* regcache.h (struct regcache_map_entry): New structure.
> 	(REGCACHE_MAP_SKIP_BYTES): New enum value.
> 	(regcache_supply_regset, regcache_collect_regset): New prototypes.

Sorry, but this doesn't strike me as a particular good interface.
I'd say an approach that uses explicit offsets would be much better.
  
Andreas Arnez May 5, 2014, 3:34 p.m. UTC | #2
On Mon, May 05 2014, Mark Kettenis wrote:

>> From: Andreas Arnez <arnez@linux.vnet.ibm.com>
>> Date: Mon, 28 Apr 2014 11:54:12 +0200
>> 
>> These functions are intended to suit all targets that don't require too
>> special logic in their regset supply/collect methods.  Having such
>> generic functions helps reducing target-specific complexity.
>> 
>> gdb/
>> 	* regcache.c: Include "regset.h".
>> 	(regcache_supply_regset, regcache_collect_regset): New functions.
>> 	* regcache.h (struct regcache_map_entry): New structure.
>> 	(REGCACHE_MAP_SKIP_BYTES): New enum value.
>> 	(regcache_supply_regset, regcache_collect_regset): New prototypes.
>
> Sorry, but this doesn't strike me as a particular good interface.
> I'd say an approach that uses explicit offsets would be much better.

And this is because...?

Existing maps greatly differ.  There are lists of offset/regnum tuples,
or arrays of regnums where the indexes implicitly represent offsets, or
the other way around, or something completely special.  In addition to
maps, many targets have special logic in their supply/collect functions,
and in some cases there are not even any maps, but only code.  Thus,
finding a map format that suits most of the existing targets seems like
a fairly difficult task to me.  Nevertheless, I tried, and I considered
all of the above approaches (including "explicit-offset" map formats),
and more.

Here are some reasons why I ended up with this format:

* The "look and feel" is similar to a structure definition with array
  members.  This seems straightforward and makes a map easily comparable
  to a "struct pt_regs" or such.

* The same map is usable for different word sizes.

* Maps are usually shorter than with other existing map formats.

* With this format, there are cases where I could reduce or completely
  eliminate the special logic in the supply/collect functions.

If it helps to widen the use of the map format, I'd be happy to adjust
it, but I'm hesitant to give up these benefits.
  

Patch

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8b588c6..b38031e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -30,6 +30,7 @@ 
 #include "exceptions.h"
 #include "remote.h"
 #include "valprint.h"
+#include "regset.h"
 
 /*
  * DATA STRUCTURE
@@ -1031,6 +1032,71 @@  regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf)
   memcpy (buf, regbuf, size);
 }
 
+/* Supply register REGNUM from BUF to REGCACHE, using the register map
+   in REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+void
+regcache_supply_regset (const struct regset *regset,
+			struct regcache *regcache,
+			int regnum, const void *buf, size_t size)
+{
+  const struct regcache_map_entry *map = regset->descr;
+  int offs = 0;
+  int i, count;
+
+  for (i = 0; (count = map[i].count) != 0; i++)
+    {
+      int regno = map[i].regno;
+
+      if (regno == REGCACHE_MAP_SKIP_BYTES)
+	offs += count;
+      else
+	for (; count--; regno++)
+	  {
+	    int new_offs = offs + regcache->descr->sizeof_register[regno];
+
+	    if (new_offs <= size && (regnum == -1 || regnum == regno))
+	      regcache_raw_supply (regcache, regno,
+				   buf ? (const gdb_byte *) buf + offs
+				   : NULL);
+	    offs = new_offs;
+	  }
+    }
+}
+
+/* Collect register REGNUM from REGCACHE to BUF, using the register
+   map in REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+void
+regcache_collect_regset (const struct regset *regset,
+			 const struct regcache *regcache,
+			 int regnum, void *buf, size_t size)
+{
+  const struct regcache_map_entry *map = regset->descr;
+  int offs = 0;
+  int i, count;
+
+  for (i = 0; (count = map[i].count) != 0; i++)
+    {
+      int regno = map[i].regno;
+
+      if (regno == REGCACHE_MAP_SKIP_BYTES)
+	offs += count;
+      else
+	for (; count--; regno++)
+	  {
+	    int new_offs = offs + regcache->descr->sizeof_register[regno];
+
+	    if (new_offs <= size && (regnum == -1 || regnum == regno))
+	      regcache_raw_collect (regcache, regno,
+				    (gdb_byte *) buf + offs);
+	    offs = new_offs;
+	  }
+    }
+}
+
 
 /* Special handling for register PC.  */
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 8423f57..cc2bda8 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -147,6 +147,44 @@  extern void regcache_raw_supply (struct regcache *regcache,
 extern void regcache_raw_collect (const struct regcache *regcache,
 				  int regnum, void *buf);
 
+/* Mapping between register numbers and offsets in a buffer, for use
+   in the '*regset' functions below.  The first entry in a map refers
+   to offset 0, and each entry increases the offset by its size.
+   Specifying count=N and regno=R is equivalent to N consecutive
+   entries with count=1 and with regno ranging from R to R+N-1.  The
+   end of the map is marked with a dummy entry having count=0.  */
+
+struct regcache_map_entry
+{
+  int count;
+  int regno;
+};
+
+/* Special value for the 'regno' field in the struct above.  */
+
+enum
+  {
+    REGCACHE_MAP_SKIP_BYTES = -1,
+  };
+
+/* Transfer a set of registers (as described by REGSET) between
+   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
+   belonging to the regset, otherwise just the register numbered
+   REGNUM.  The REGSET's 'descr' field must point to an array of
+   'struct regcache_map_entry'.
+
+   These functions are suitable for the 'regset_supply' and
+   'regset_collect' fields in a regset structure.
+  */
+
+extern void regcache_supply_regset (const struct regset *regset,
+				    struct regcache *regcache,
+				    int regnum, const void *buf,
+				    size_t size);
+extern void regcache_collect_regset (const struct regset *regset,
+				     const struct regcache *regcache,
+				     int regnum, void *buf, size_t size);
+
 
 /* The type of a register.  This function is slightly more efficient
    then its gdbarch vector counterpart since it returns a precomputed