Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

[Yao Qi <qiyaoltc at gmail dot com>]

On Tue, Feb 7, 2017 at 4:33 PM, Alan Hayward <Alan.Hayward@arm.com> wrote:

> 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);
>

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);

> -           frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch),
> -                                  &optimized, &unavailable, &lval, &addr,
> -                                  &realnum, value);
> +           gdb_assert (value != NULL);
>             /* 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");

-- 
Yao (齐尧)