Hey Simon!

Simon Marchi <simon.marchi@efficios.com> writes:

> Similar to the previous patches, change get_frame_register_bytes to take
> the "next frame" instead of "this frame".
>
> Change-Id: Ie8f35042bfa6e93565fcefaee71b6b3903f0fe9f
> Reviewed-By: John Baldwin <jhb@FreeBSD.org>
> ---
>  gdb/dwarf2/expr.c | 22 +++++++++++-----------
>  gdb/frame.c       | 16 ++++++----------
>  gdb/frame.h       | 10 +++++-----
>  gdb/i386-tdep.c   |  8 ++++----
>  gdb/i387-tdep.c   | 10 +++++-----
>  gdb/ia64-tdep.c   |  9 ++++-----
>  gdb/m68k-tdep.c   | 10 +++++-----
>  gdb/mips-tdep.c   |  7 ++++---
>  gdb/rs6000-tdep.c | 10 +++++-----
>  gdb/valops.c      |  3 +--
>  10 files changed, 50 insertions(+), 55 deletions(-)
>
[…]
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index c43039302bc3..643997e5bd7d 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -2713,11 +2713,11 @@ rs6000_register_to_value (frame_info_ptr frame,
>       fpr to vsr.  */
>    regnum = ieee_128_float_regnum_adjust (gdbarch, type, regnum);
>  
> -  if (!get_frame_register_bytes (frame, regnum, 0,
> -				 gdb::make_array_view (from,
> -						       register_size (gdbarch,
> -								      regnum)),
> -				 optimizedp, unavailablep))
> +  auto from_view
> +    = gdb::make_array_view (from, register_size (gdbarch, regnum));
> +  frame_info_ptr next_frame = get_next_frame_sentinel_okay (frame);
> +  if (!get_frame_register_bytes (frame, regnum, 0, from_view, optimizedp,
                                    ^^^^^

Should this have been changed to 'next_frame', as the commit message
suggests, and as was done in the other *-tdep.c?

Asking because I'm observing a regression on ppc-linux in our internal
testsuite.  I've reproduced that regression with the O2_float_param
testcase:

  cd gdb/testsuite/gdb.ada/O2_float_param
  powerpc-linux-gnu-gnatmake foo.adb \
      -cargs -g -O2 -bargs -static -largs -static
  scp foo $TARGET:
  […gdb…] foo                                   \
      -batch                                    \
      -ex 'tbreak callee.increment'             \
      -ex 'target remote | ssh gdbserver - foo' \
      -ex continue

Expected:

  > Temporary breakpoint 1, callee.increment (val=val@entry=99.0, msg=...) at callee.adb:19

Currently, since 2023-12-14 "gdb: make get_frame_register_bytes take the
next frame" (9fc79b42369):

  > Temporary breakpoint 1, callee.increment (val=<optimized out>, val@entry=0.0, msg=...) at callee.adb:19

Applying the attached patch gets us the expected output back, and does
not introduce further regressions that I could find.  I've tried it on
top of both master and your recently submitted "register value cleanups"
series, <20231221191716.257256-1-simon.marchi@efficios.com>.

Let me know if I've missed something!

> +				 unavailablep))
>      return 0;
>  
>    target_float_convert (from, builtin_type (gdbarch)->builtin_double,