[10/11] Add XTENSA_MAX_REGISTER_SIZE

Message ID D44F16DC-6E66-415D-A5E0-0C433CF70F73@arm.com
State New, archived
Headers

Commit Message

Alan Hayward April 6, 2017, 12:49 p.m. UTC
  > On 5 Apr 2017, at 11:23, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> @@ -559,7 +559,7 @@ 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);
>> +      gdb_byte *buf = (gdb_byte *) alloca (XTENSA_MAX_REGISTER_SIZE);
>>       enum register_status status;
>> 
>>       status = regcache_raw_read (regcache,
>> @@ -655,7 +655,7 @@ 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);
>> +      gdb_byte *buf = (gdb_byte *) alloca (XTENSA_MAX_REGISTER_SIZE);
>> 
>>       regcache_raw_read (regcache,
>> 			 gdbarch_tdep (gdbarch)->wb_regnum, buf);
> 
> The "buf" can be removed by using regcache_raw_read_unsigned.
> 
> -- 
> Yao (齐尧)

Ok, the patch below makes that change.

I want to make it clear that I do not have an xtensa machine.
With the previous patch I was confident that it would not cause any
regressions - as it only switched the define being used, and
shouldn't cause any functional change.

With the new patch there is more untested code and I am less confident
that I've not introduced a subtle error.

Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.

Ok to commit (or the previous version?

Alan.

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.
	(xtensa_pseudo_register_read): Use regcache_raw_read_unsigned.
	(xtensa_pseudo_register_write): Likewise.
  

Comments

Yao Qi April 25, 2017, 3:39 p.m. UTC | #1
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.
  

Patch

diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 13f1514e7651e47a268ef809b1324167f1f759bd..53c866075841fb00170cc34e446d6fc21c869032 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;
@@ -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);
     }

   /* We can always write 'core' registers.