Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

Yao Qi
2017-02-08T12:24:45Z

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