Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

Alan Hayward
2017-02-08T14:44:55Z

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

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,