Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

[Yao Qi <qiyaoltc@gmail.com>]

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 (齐尧)