Remove MAX_REGISTER_SIZE from regcache.c

Message ID 1467DD03-A335-4CA3-9541-0A6DDB2FE7EA@arm.com
State New, archived
Headers

Commit Message

Alan Hayward May 3, 2017, 10:42 a.m. UTC
  > On 3 May 2017, at 09:21, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> That’s fine. My rebase should be easy enough.
>> If you remember, could you let me know when you have pushed your patch.
> 
> Hi Alan,
> My patch set is pushed in.  You can rebase your patch.
> 
> -- 
> Yao (齐尧)

Patch updated to head.

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

Ok to commit?

Alan.

2017-05-03  Alan Hayward  <alan.hayward@arm.com>

	* regcache.c (regcache_save): Avoid buffer use.
	(regcache_dump): Likewise.
  

Comments

Alan Hayward June 7, 2017, 8:49 a.m. UTC | #1
Ping.
Looks like this one got missed.
(Has been recently re-tested too).


Alan.

> On 3 May 2017, at 11:42, Alan Hayward <Alan.Hayward@arm.com> wrote:

> 

> 

>> On 3 May 2017, at 09:21, Yao Qi <qiyaoltc@gmail.com> wrote:

>> 

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

>> 

>>> That’s fine. My rebase should be easy enough.

>>> If you remember, could you let me know when you have pushed your patch.

>> 

>> Hi Alan,

>> My patch set is pushed in.  You can rebase your patch.

>> 

>> -- 

>> Yao (齐尧)

> 

> Patch updated to head.

> 

> Tested on a --enable-targets=all using make check with board files

> unix and native-gdbserver.

> 

> Ok to commit?

> 

> Alan.

> 

> 2017-05-03  Alan Hayward  <alan.hayward@arm.com>

> 

> 	* regcache.c (regcache_save): Avoid buffer use.

> 	(regcache_dump): Likewise.

> 

> 

> diff --git a/gdb/regcache.c b/gdb/regcache.c

> index 255246b2606036f178c7db3125ecd465dbc1af05..5d319153ad1184b754dbb0bc740c3bbb59dfa2e8 100644

> --- a/gdb/regcache.c

> +++ b/gdb/regcache.c

> @@ -327,7 +327,6 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,

> 		void *src)

> {

>   struct gdbarch *gdbarch = m_descr->gdbarch;

> -  gdb_byte buf[MAX_REGISTER_SIZE];

>   int regnum;

> 

>   /* The DST should be `read-only', if it wasn't then the save would

> @@ -345,18 +344,14 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,

>     {

>       if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))

> 	{

> -	  enum register_status status = cooked_read (src, regnum, buf);

> +	  gdb_byte *dst_buf = register_buffer (regnum);

> +	  enum register_status status = cooked_read (src, regnum, dst_buf);

> 

> -	  if (status == REG_VALID)

> -	    memcpy (register_buffer (regnum), buf,

> -		    register_size (gdbarch, regnum));

> -	  else

> -	    {

> -	      gdb_assert (status != REG_UNKNOWN);

> +	  gdb_assert (status != REG_UNKNOWN);

> +

> +	  if (status != REG_VALID)

> +	    memset (dst_buf, 0, register_size (gdbarch, regnum));

> 

> -	      memset (register_buffer (regnum), 0,

> -		      register_size (gdbarch, regnum));

> -	    }

> 	  m_register_status[regnum] = status;

> 	}

>     }

> @@ -1439,7 +1434,6 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)

>   int footnote_register_offset = 0;

>   int footnote_register_type_name_null = 0;

>   long register_offset = 0;

> -  gdb_byte buf[MAX_REGISTER_SIZE];

> 

> #if 0

>   fprintf_unfiltered (file, "nr_raw_registers %d\n",

> @@ -1565,8 +1559,8 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)

> 	    fprintf_unfiltered (file, "<unavailable>");

> 	  else

> 	    {

> -	      raw_read (regnum, buf);

> -	      print_hex_chars (file, buf,

> +	      raw_update (regnum);

> +	      print_hex_chars (file, register_buffer (regnum),

> 			       m_descr->sizeof_register[regnum],

> 			       gdbarch_byte_order (gdbarch));

> 	    }

> @@ -1579,9 +1573,30 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)

> 	    fprintf_unfiltered (file, "Cooked value");

> 	  else

> 	    {

> +	      const gdb_byte *buf = NULL;

> 	      enum register_status status;

> +	      struct value *value = NULL;

> +

> +	      if (regnum < m_descr->nr_raw_registers)

> +		{

> +		  raw_update (regnum);

> +		  status = get_register_status (regnum);

> +		  buf = register_buffer (regnum);

> +		}

> +	      else

> +		{

> +		  value = cooked_read_value (regnum);

> +

> +		  if (!value_optimized_out (value)

> +		      && value_entirely_available (value))

> +		    {

> +		      status = REG_VALID;

> +		      buf = value_contents_all (value);

> +		    }

> +		  else

> +		    status = REG_UNAVAILABLE;

> +		}

> 

> -	      status = cooked_read (regnum, buf);

> 	      if (status == REG_UNKNOWN)

> 		fprintf_unfiltered (file, "<invalid>");

> 	      else if (status == REG_UNAVAILABLE)

> @@ -1590,6 +1605,12 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)

> 		print_hex_chars (file, buf,

> 				 m_descr->sizeof_register[regnum],

> 				 gdbarch_byte_order (gdbarch));

> +

> +	      if (value != NULL)

> +		{

> +		  release_value (value);

> +		  value_free (value);

> +		}

> 	    }

> 	}

> 

>
  
Yao Qi June 7, 2017, 9:12 a.m. UTC | #2
Alan Hayward <Alan.Hayward@arm.com> writes:

> 2017-05-03  Alan Hayward  <alan.hayward@arm.com>
>
> 	* regcache.c (regcache_save): Avoid buffer use.

s/regcache_save/regcache::save/

> 	(regcache_dump): Likewise.

s/regcche_dump/regcache::dump/

Otherwise, patch is good to me.
  

Patch

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 255246b2606036f178c7db3125ecd465dbc1af05..5d319153ad1184b754dbb0bc740c3bbb59dfa2e8 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -327,7 +327,6 @@  regcache::save (regcache_cooked_read_ftype *cooked_read,
 		void *src)
 {
   struct gdbarch *gdbarch = m_descr->gdbarch;
-  gdb_byte buf[MAX_REGISTER_SIZE];
   int regnum;

   /* The DST should be `read-only', if it wasn't then the save would
@@ -345,18 +344,14 @@  regcache::save (regcache_cooked_read_ftype *cooked_read,
     {
       if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
 	{
-	  enum register_status status = cooked_read (src, regnum, buf);
+	  gdb_byte *dst_buf = register_buffer (regnum);
+	  enum register_status status = cooked_read (src, regnum, dst_buf);

-	  if (status == REG_VALID)
-	    memcpy (register_buffer (regnum), buf,
-		    register_size (gdbarch, regnum));
-	  else
-	    {
-	      gdb_assert (status != REG_UNKNOWN);
+	  gdb_assert (status != REG_UNKNOWN);
+
+	  if (status != REG_VALID)
+	    memset (dst_buf, 0, register_size (gdbarch, regnum));

-	      memset (register_buffer (regnum), 0,
-		      register_size (gdbarch, regnum));
-	    }
 	  m_register_status[regnum] = status;
 	}
     }
@@ -1439,7 +1434,6 @@  regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
   int footnote_register_offset = 0;
   int footnote_register_type_name_null = 0;
   long register_offset = 0;
-  gdb_byte buf[MAX_REGISTER_SIZE];

 #if 0
   fprintf_unfiltered (file, "nr_raw_registers %d\n",
@@ -1565,8 +1559,8 @@  regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	    fprintf_unfiltered (file, "<unavailable>");
 	  else
 	    {
-	      raw_read (regnum, buf);
-	      print_hex_chars (file, buf,
+	      raw_update (regnum);
+	      print_hex_chars (file, register_buffer (regnum),
 			       m_descr->sizeof_register[regnum],
 			       gdbarch_byte_order (gdbarch));
 	    }
@@ -1579,9 +1573,30 @@  regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	    fprintf_unfiltered (file, "Cooked value");
 	  else
 	    {
+	      const gdb_byte *buf = NULL;
 	      enum register_status status;
+	      struct value *value = NULL;
+
+	      if (regnum < m_descr->nr_raw_registers)
+		{
+		  raw_update (regnum);
+		  status = get_register_status (regnum);
+		  buf = register_buffer (regnum);
+		}
+	      else
+		{
+		  value = cooked_read_value (regnum);
+
+		  if (!value_optimized_out (value)
+		      && value_entirely_available (value))
+		    {
+		      status = REG_VALID;
+		      buf = value_contents_all (value);
+		    }
+		  else
+		    status = REG_UNAVAILABLE;
+		}

-	      status = cooked_read (regnum, buf);
 	      if (status == REG_UNKNOWN)
 		fprintf_unfiltered (file, "<invalid>");
 	      else if (status == REG_UNAVAILABLE)
@@ -1590,6 +1605,12 @@  regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 		print_hex_chars (file, buf,
 				 m_descr->sizeof_register[regnum],
 				 gdbarch_byte_order (gdbarch));
+
+	      if (value != NULL)
+		{
+		  release_value (value);
+		  value_free (value);
+		}
 	    }
 	}