Removal of uses of MAX_REGISTER_SIZE

Message ID 9DD2B6B8-9FC3-4339-996F-F58B387190EC@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Feb. 8, 2017, 2:44 p.m. UTC
  > On 8 Feb 2017, at 12:24, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> diff --git a/gdb/stack.c b/gdb/stack.c
>> index e00e2972cf20bc63917af19f86bf57f1c6b0b5b0..7ba7d68bde8d83ea1e700faa466c6951979e0f76 100644
>> --- a/gdb/stack.c
>> +++ b/gdb/stack.c
>> @@ -1650,33 +1650,35 @@ frame_info (char *addr_exp, int from_tty)
>>     int count;
>>     int i;
>>     int need_nl = 1;
>> +    int sp_regnum = gdbarch_sp_regnum (gdbarch);
>> 
>>     /* The sp is special; what's displayed isn't the save address, but
>>        the value of the previous frame's sp.  This is a legacy thing,
>>        at one stage the frame cached the previous frame's SP instead
>>        of its address, hence it was easiest to just display the cached
>>        value.  */
>> -    if (gdbarch_sp_regnum (gdbarch) >= 0)
>> +    if (sp_regnum >= 0)
>>       {
>> 	/* Find out the location of the saved stack pointer with out
>>            actually evaluating it.  */
>> -	frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch),
>> -			       &optimized, &unavailable, &lval, &addr,
>> -			       &realnum, NULL);
>> +	frame_register_unwind (fi, sp_regnum, &optimized, &unavailable, &lval,
>> +			       &addr, &realnum, NULL);
>> 	if (!optimized && !unavailable && lval == not_lval)
>> 	  {
>> 	    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -	    int sp_size = register_size (gdbarch, gdbarch_sp_regnum (gdbarch));
>> -	    gdb_byte value[MAX_REGISTER_SIZE];
>> +	    int sp_size = register_size (gdbarch, sp_regnum);
>> 	    CORE_ADDR sp;
>> +	    struct value *value = frame_unwind_register_value (fi, sp_regnum);
>> 
>> -	    frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch),
>> -				   &optimized, &unavailable, &lval, &addr,
>> -				   &realnum, value);
>> +	    gdb_assert (value != NULL);
> 
> Why don't you hoist frame_unwind_register_value above?, so the
> frame_register_unwind call is no longer needed,
> 
> 
>  struct value *value = frame_unwind_register_value (fi, sp_regnum);
> 
>  gdb_assert (value != NULL);
> 
>  if (!value_optimized_out (value) && value_entirely_available (value))
>     {
>       if (VALUE_LVAL (value) == not_lval)
>         {
>            sp = extract_unsigned_integer (value_contents_all (value),
>                                          sp_size, byte_order);
>         }
>       else if (VALUE_LVAL (value) == lval_memory)
>         {
>            // use value_address (value);
>         }
>       else if (VALUE_LVAL (value) == lval_register)
>         {
>            // use VALUE_REGNUM (value);
>         }
>     }
>   /* else keep quiet.  */
> 
>   release_value (value);
>   value_free (value);
> 
>> 	    /* NOTE: cagney/2003-05-22: This is assuming that the
>>                stack pointer was packed as an unsigned integer.  That
>>                may or may not be valid.  */
>> -	    sp = extract_unsigned_integer (value, sp_size, byte_order);
>> +	    sp = extract_unsigned_integer (value_contents_all (value), sp_size,
>> +					   byte_order);
>> +	    release_value (value);
>> +	    value_free (value);
>> +
>> 	    printf_filtered (" Previous frame's sp is ");
>> 	    fputs_filtered (paddress (gdbarch, sp), gdb_stdout);
>> 	    printf_filtered ("\n");
>> @@ -1702,7 +1704,7 @@ frame_info (char *addr_exp, int from_tty)
>>     numregs = gdbarch_num_regs (gdbarch)
>> 	      + gdbarch_num_pseudo_regs (gdbarch);
>>     for (i = 0; i < numregs; i++)
>> -      if (i != gdbarch_sp_regnum (gdbarch)
>> +      if (i != sp_regnum
>> 	  && gdbarch_register_reggroup_p (gdbarch, i, all_reggroup))
>> 	{
>> 	  /* Find out the location of the saved register without
> 
> -- 
> Yao (齐尧)


Ok, changed as requested.

Alan.

2017-02-08  Alan Hayward  <alan.hayward@arm.com>

	* stack.c (frame_info): Use frame_unwind_register_value to avoid buf.
  

Comments

Yao Qi Feb. 18, 2017, 11:18 p.m. UTC | #1
On 17-02-08 14:44:38, Alan Hayward wrote:
> 
> Ok, changed as requested.
> 
> Alan.
> 
> 2017-02-08  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* stack.c (frame_info): Use frame_unwind_register_value to avoid buf.

This line is too long.

Did you run regression test with board file unix and native-gdbserver?

Patch is good to me if it is regression-free.
  
Alan Hayward Feb. 20, 2017, 11:19 a.m. UTC | #2
> On 18 Feb 2017, at 23:18, Yao Qi <qiyaoltc@gmail.com> wrote:

> 

> On 17-02-08 14:44:38, Alan Hayward wrote:

>> 

>> Ok, changed as requested.

>> 

>> Alan.

>> 

>> 2017-02-08  Alan Hayward  <alan.hayward@arm.com>

>> 

>> 	* stack.c (frame_info): Use frame_unwind_register_value to avoid buf.

> 

> This line is too long.

> 

> Did you run regression test with board file unix and native-gdbserver?

> 

> Patch is good to me if it is regression-free.

> 

> -- 

> Yao 


Yes, I’ve run a unix and native-gdbserver run with the three patches:
*I386 + M68K changes
*stack.c changes
*mi-main changes


Alan.
  

Patch

diff --git a/gdb/stack.c b/gdb/stack.c
index e00e2972cf20bc63917af19f86bf57f1c6b0b5b0..97b600b6b7bb6b450e54c947dc6178be03f31e6b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1642,57 +1642,52 @@  frame_info (char *addr_exp, int from_tty)
   /* Print as much information as possible on the location of all the
      registers.  */
   {
-    enum lval_type lval;
-    int optimized;
-    int unavailable;
-    CORE_ADDR addr;
-    int realnum;
     int count;
     int i;
     int need_nl = 1;
+    int sp_regnum = gdbarch_sp_regnum (gdbarch);

     /* The sp is special; what's displayed isn't the save address, but
        the value of the previous frame's sp.  This is a legacy thing,
        at one stage the frame cached the previous frame's SP instead
        of its address, hence it was easiest to just display the cached
        value.  */
-    if (gdbarch_sp_regnum (gdbarch) >= 0)
+    if (sp_regnum >= 0)
       {
-	/* Find out the location of the saved stack pointer with out
-           actually evaluating it.  */
-	frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch),
-			       &optimized, &unavailable, &lval, &addr,
-			       &realnum, NULL);
-	if (!optimized && !unavailable && lval == not_lval)
-	  {
-	    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-	    int sp_size = register_size (gdbarch, gdbarch_sp_regnum (gdbarch));
-	    gdb_byte value[MAX_REGISTER_SIZE];
-	    CORE_ADDR sp;
-
-	    frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch),
-				   &optimized, &unavailable, &lval, &addr,
-				   &realnum, value);
-	    /* NOTE: cagney/2003-05-22: This is assuming that the
-               stack pointer was packed as an unsigned integer.  That
-               may or may not be valid.  */
-	    sp = extract_unsigned_integer (value, sp_size, byte_order);
-	    printf_filtered (" Previous frame's sp is ");
-	    fputs_filtered (paddress (gdbarch, sp), gdb_stdout);
-	    printf_filtered ("\n");
-	    need_nl = 0;
-	  }
-	else if (!optimized && !unavailable && lval == lval_memory)
-	  {
-	    printf_filtered (" Previous frame's sp at ");
-	    fputs_filtered (paddress (gdbarch, addr), gdb_stdout);
-	    printf_filtered ("\n");
-	    need_nl = 0;
-	  }
-	else if (!optimized && !unavailable && lval == lval_register)
+	struct value *value = frame_unwind_register_value (fi, sp_regnum);
+	gdb_assert (value != NULL);
+
+	if (!value_optimized_out (value) && value_entirely_available (value))
 	  {
-	    printf_filtered (" Previous frame's sp in %s\n",
-			     gdbarch_register_name (gdbarch, realnum));
+	    if (VALUE_LVAL (value) == not_lval)
+	      {
+		CORE_ADDR sp;
+		enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+		int sp_size = register_size (gdbarch, sp_regnum);
+
+		sp = extract_unsigned_integer (value_contents_all (value),
+					       sp_size, byte_order);
+
+		printf_filtered (" Previous frame's sp is ");
+		fputs_filtered (paddress (gdbarch, sp), gdb_stdout);
+		printf_filtered ("\n");
+	      }
+	    else if (VALUE_LVAL (value) == lval_memory)
+	      {
+		printf_filtered (" Previous frame's sp at ");
+		fputs_filtered (paddress (gdbarch, value_address (value)),
+				gdb_stdout);
+		printf_filtered ("\n");
+	      }
+	    else if (VALUE_LVAL (value) == lval_register)
+	      {
+		printf_filtered (" Previous frame's sp in %s\n",
+				 gdbarch_register_name (gdbarch,
+							VALUE_REGNUM (value)));
+	      }
+
+	    release_value (value);
+	    value_free (value);
 	    need_nl = 0;
 	  }
 	/* else keep quiet.  */
@@ -1702,9 +1697,15 @@  frame_info (char *addr_exp, int from_tty)
     numregs = gdbarch_num_regs (gdbarch)
 	      + gdbarch_num_pseudo_regs (gdbarch);
     for (i = 0; i < numregs; i++)
-      if (i != gdbarch_sp_regnum (gdbarch)
+      if (i != sp_regnum
 	  && gdbarch_register_reggroup_p (gdbarch, i, all_reggroup))
 	{
+	  enum lval_type lval;
+	  int optimized;
+	  int unavailable;
+	  CORE_ADDR addr;
+	  int realnum;
+
 	  /* Find out the location of the saved register without
              fetching the corresponding value.  */
 	  frame_register_unwind (fi, i, &optimized, &unavailable,