diff mbox

[10/11] Add XTENSA_MAX_REGISTER_SIZE

Message ID 49176F63-F5C5-43C2-A21D-A7642F4ACB80@arm.com
State New
Headers show

Commit Message

Alan Hayward April 26, 2017, 9:59 a.m. UTC
> On 25 Apr 2017, at 16:39, Yao Qi <qiyaoltc@gmail.com> wrote:

> 

> Alan Hayward <Alan.Hayward@arm.com> writes:

> 

>> @@ -559,16 +562,15 @@ xtensa_pseudo_register_read (struct gdbarch *gdbarch,

>>       && (regnum >= gdbarch_tdep (gdbarch)->a0_base)

>>       && (regnum <= gdbarch_tdep (gdbarch)->a0_base + 15))

>>     {

>> -      gdb_byte *buf = (gdb_byte *) alloca (MAX_REGISTER_SIZE);

>> +      ULONGEST value;

>>       enum register_status status;

>> 

>> -      status = regcache_raw_read (regcache,

>> -				  gdbarch_tdep (gdbarch)->wb_regnum,

>> -				  buf);

>> +      status = regcache_raw_read_unsigned (regcache,

>> +					   gdbarch_tdep (gdbarch)->wb_regnum,

>> +					   &value);

>>       if (status != REG_VALID)

>> 	return status;

>> -      regnum = arreg_number (gdbarch, regnum,

>> -			     extract_unsigned_integer (buf, 4, byte_order));

>> +      regnum = arreg_number (gdbarch, regnum, value);

>>     }

>> 

>>   /* We can always read non-pseudo registers.  */

>> @@ -656,12 +658,10 @@ xtensa_pseudo_register_write (struct gdbarch *gdbarch,

>>       && (regnum >= gdbarch_tdep (gdbarch)->a0_base)

>>       && (regnum <= gdbarch_tdep (gdbarch)->a0_base + 15))

>>     {

>> -      gdb_byte *buf = (gdb_byte *) alloca (MAX_REGISTER_SIZE);

>> -

>> -      regcache_raw_read (regcache,

>> -			 gdbarch_tdep (gdbarch)->wb_regnum, buf);

>> -      regnum = arreg_number (gdbarch, regnum,

>> -			     extract_unsigned_integer (buf, 4, byte_order));

>> +      ULONGEST value;

>> +      regcache_raw_read_unsigned (regcache,

>> +				  gdbarch_tdep (gdbarch)->wb_regnum, &value);

>> +      regnum = arreg_number (gdbarch, regnum, value);

> 

> This part of patch is OK, but the part using XTENSA_MAX_REGISTER_SIZE

> still needs some review.

> 

> -- 

> Yao (齐尧)



Ok, Above committed with:

2017-04-26  Alan Hayward  <alan.hayward@arm.com>

	* xtensa-tdep.c (xtensa_pseudo_register_read): Use
	regcache_raw_read_unsigned.
	(xtensa_pseudo_register_write): Likewise.


This leaves the following left to review (below).

Looking at the code, I'm still a little unsure exactly how "value" is
being used. "value" is declared as max register size, rounded up to the
next int. Looking at the use of value, I think the code requires max
size of a register multiplied by the value in reg->mask + 1 - and checking
xtensa-config.c, this never goes above one. The comment also states
the size of a masked register is never greater than 32bits.
Maybe the value should be given a size of 32bits * (mask->count + 1) ?


2017-04-06  Alan Hayward  <alan.hayward@arm.com>

	* xtensa-tdep.c (XTENSA_MAX_REGISTER_SIZE): Add.
        (xtensa_register_write_masked): Use XTENSA_MAX_REGISTER_SIZE.
	(xtensa_register_read_masked): Likewise.
diff mbox

Patch

diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 0a4ed37cf9582a7aa43ec601fc013f1f93624a39..e79c925b345a8bd03e3a674c731c00e86f1ea39a 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -120,6 +120,9 @@  static unsigned int xtensa_debug_level = 0;
 #define PS_WOE			(1<<18)
 #define PS_EXC			(1<<4)

+/* Big enough to hold the size of the largest register in bytes.  */
+#define XTENSA_MAX_REGISTER_SIZE	16
+
 static int
 windowing_enabled (struct gdbarch *gdbarch, unsigned int ps)
 {
@@ -370,7 +373,7 @@  static void
 xtensa_register_write_masked (struct regcache *regcache,
 			      xtensa_register_t *reg, const gdb_byte *buffer)
 {
-  unsigned int value[(MAX_REGISTER_SIZE + 3) / 4];
+  unsigned int value[(XTENSA_MAX_REGISTER_SIZE + 3) / 4];
   const xtensa_mask_t *mask = reg->mask;

   int shift = 0;		/* Shift for next mask (mod 32).  */
@@ -454,7 +457,7 @@  static enum register_status
 xtensa_register_read_masked (struct regcache *regcache,
 			     xtensa_register_t *reg, gdb_byte *buffer)
 {
-  unsigned int value[(MAX_REGISTER_SIZE + 3) / 4];
+  unsigned int value[(XTENSA_MAX_REGISTER_SIZE + 3) / 4];
   const xtensa_mask_t *mask = reg->mask;

   int shift = 0;