diff mbox

Removal of uses of MAX_REGISTER_SIZE

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

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.
diff mbox

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,