[2/2,ver,2] PowerPC, fix support for printing the function return value for non-trivial values.

  GDB maintainers:

Version 2, updated comments per Kevin's comments.  Changed "frame_info
*" to frame_info_ptr per Bruno's comments.  Discussed supressing the
new warning after printing it with Kevin.  In the end we decided to not
suppress the warnings.  Suppression can be added later if deemed
necessary.  The white space introduced by the gdb/gdbarch-components.py 
needs to be addressed by a future patch to fix the script and the
generated files.

On PowerPC, a non-trivial return value for a function cannot be
reliably determined with the current PowerPC ABI.  The issue is the
address of the return buffer for a non-trivial value is passed to the
function in register r3.  However, at the end of the function the ABI
does not guarantee r3 will not be changed.  Hence it cannot be reliably
used to get the address of the return buffer.

This patch adds a new GDB ABI to allow GDB to obtain the input value of
r3 when the function returns.  This is done by using the DW_OP_entry
value for the DWARF entries to obtain the value of r3 on entry to the
function.  The ABI allows GDB to get the correct address for the return
buffer on exit from the function.

This patch fixes the five test failures in gdb.cp/non-trivial-
retval.exp.

The patch has been re-tested on PowerPC and X86-64 with no regression
failures.

Please let me know if this version of the patch is acceptable for GDB
mainline.  Thanks.

                         Carl Love


-----------------------------------
PowerPC, fix support for printing the function return value for non-trivial values.

Currently, a non-trivial return value from a function cannot currently be
reliably determined on PowerPC.  This is due to the fact that the PowerPC
ABI uses register r3 to store the address of the buffer containing the
non-trivial return value when the function is called.  The PowerPC ABI
does not guarantee the value in register r3 is not modified in the
function.  Thus the value in r3 cannot be reliably used to obtain the
return addreses on exit from the function.

This patch adds a new gdbarch method to allow PowerPC to access the value
of r3 on entry to a function. On PowerPC, the new gdbarch method attempts
to use the DW_OP_entry_value for the DWARF entries, when exiting the
function, to determine the value of r3 on entry to the function.  This
requires the use of the -fvar-tracking compiler option to compile the
user application thus generating the DW_OP_entry_value in the binary.  The
DW_OP_entry_value entries in the binary file allows GDB to resolve the
DW_TAG_call_site entries.  This new gdbarch method is used to get the
return buffer address, in the case of a function returning a nontrivial
data type, on exit from the function.  The GDB function should_stop checks
to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set to
zero by the new gdbarch method call for all architectures except PowerPC.
The get_return_value function will be used to obtain the return value on
all other architectures as is currently being done if RETURN_BUF is zero.
On PowerPC, the new gdbarch method will return a nonzero address in
RETURN_BUF if the value can be determined.  The value_at function uses the
return buffer address to get the return value.

This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp.
The correct function return values are now reported.

Note this patch is dependent on patch: "PowerPC, function
ppc64_sysv_abi_return_value add missing return value convention".

This patch has been tested on Power 10 and x86-64 with no regressions.

Signed-off-by: Carl Love <cel@us.ibm.com>
---
 gdb/arch-utils.c                            |  6 +++
 gdb/arch-utils.h                            |  4 ++
 gdb/dwarf2/loc.c                            | 10 +----
 gdb/dwarf2/loc.h                            | 11 ++++++
 gdb/gdbarch-components.py                   | 15 ++++++++
 gdb/gdbarch-gen.h                           | 11 ++++++
 gdb/gdbarch.c                               | 23 ++++++++++++
 gdb/infcmd.c                                | 41 +++++++++++++++++++--
 gdb/ppc-sysv-tdep.c                         | 41 +++++++++++++++++++++
 gdb/ppc-tdep.h                              |  2 +
 gdb/rs6000-tdep.c                           |  6 ++-
 gdb/testsuite/gdb.cp/non-trivial-retval.exp |  9 ++++-
 gdb/testsuite/lib/gdb.exp                   |  8 ++++
 13 files changed, 173 insertions(+), 14 deletions(-)
  

Kevin, Bruno:

I haven't seen any comments on the updated patch.  Just wanted to check
and see if the updated patches look OK to you. Thanks.

                   Carl 

On Tue, 2022-10-18 at 11:55 -0700, Carl Love wrote:
> GDB maintainers:
> 
> Version 2, updated comments per Kevin's comments.  Changed
> "frame_info
> *" to frame_info_ptr per Bruno's comments.  Discussed supressing the
> new warning after printing it with Kevin.  In the end we decided to
> not
> suppress the warnings.  Suppression can be added later if deemed
> necessary.  The white space introduced by the gdb/gdbarch-
> components.py 
> needs to be addressed by a future patch to fix the script and the
> generated files.
> 
> On PowerPC, a non-trivial return value for a function cannot be
> reliably determined with the current PowerPC ABI.  The issue is the
> address of the return buffer for a non-trivial value is passed to the
> function in register r3.  However, at the end of the function the ABI
> does not guarantee r3 will not be changed.  Hence it cannot be
> reliably
> used to get the address of the return buffer.
> 
> This patch adds a new GDB ABI to allow GDB to obtain the input value
> of
> r3 when the function returns.  This is done by using the DW_OP_entry
> value for the DWARF entries to obtain the value of r3 on entry to the
> function.  The ABI allows GDB to get the correct address for the
> return
> buffer on exit from the function.
> 
> This patch fixes the five test failures in gdb.cp/non-trivial-
> retval.exp.
> 
> The patch has been re-tested on PowerPC and X86-64 with no regression
> failures.
> 
> Please let me know if this version of the patch is acceptable for GDB
> mainline.  Thanks.
> 
>                          Carl Love
> 
> 
> -----------------------------------
> PowerPC, fix support for printing the function return value for non-
> trivial values.
> 
> Currently, a non-trivial return value from a function cannot
> currently be
> reliably determined on PowerPC.  This is due to the fact that the
> PowerPC
> ABI uses register r3 to store the address of the buffer containing
> the
> non-trivial return value when the function is called.  The PowerPC
> ABI
> does not guarantee the value in register r3 is not modified in the
> function.  Thus the value in r3 cannot be reliably used to obtain the
> return addreses on exit from the function.
> 
> This patch adds a new gdbarch method to allow PowerPC to access the
> value
> of r3 on entry to a function. On PowerPC, the new gdbarch method
> attempts
> to use the DW_OP_entry_value for the DWARF entries, when exiting the
> function, to determine the value of r3 on entry to the
> function.  This
> requires the use of the -fvar-tracking compiler option to compile the
> user application thus generating the DW_OP_entry_value in the
> binary.  The
> DW_OP_entry_value entries in the binary file allows GDB to resolve
> the
> DW_TAG_call_site entries.  This new gdbarch method is used to get the
> return buffer address, in the case of a function returning a
> nontrivial
> data type, on exit from the function.  The GDB function should_stop
> checks
> to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set
> to
> zero by the new gdbarch method call for all architectures except
> PowerPC.
> The get_return_value function will be used to obtain the return value
> on
> all other architectures as is currently being done if RETURN_BUF is
> zero.
> On PowerPC, the new gdbarch method will return a nonzero address in
> RETURN_BUF if the value can be determined.  The value_at function
> uses the
> return buffer address to get the return value.
> 
> This patch fixes five testcase failures in gdb.cp/non-trivial-
> retval.exp.
> The correct function return values are now reported.
> 
> Note this patch is dependent on patch: "PowerPC, function
> ppc64_sysv_abi_return_value add missing return value convention".
> 
> This patch has been tested on Power 10 and x86-64 with no
> regressions.
> 
> Signed-off-by: Carl Love <cel@us.ibm.com>
> ---
>  gdb/arch-utils.c                            |  6 +++
>  gdb/arch-utils.h                            |  4 ++
>  gdb/dwarf2/loc.c                            | 10 +----
>  gdb/dwarf2/loc.h                            | 11 ++++++
>  gdb/gdbarch-components.py                   | 15 ++++++++
>  gdb/gdbarch-gen.h                           | 11 ++++++
>  gdb/gdbarch.c                               | 23 ++++++++++++
>  gdb/infcmd.c                                | 41
> +++++++++++++++++++--
>  gdb/ppc-sysv-tdep.c                         | 41
> +++++++++++++++++++++
>  gdb/ppc-tdep.h                              |  2 +
>  gdb/rs6000-tdep.c                           |  6 ++-
>  gdb/testsuite/gdb.cp/non-trivial-retval.exp |  9 ++++-
>  gdb/testsuite/lib/gdb.exp                   |  8 ++++
>  13 files changed, 173 insertions(+), 14 deletions(-)
> 
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 6a72b3f5efd..1358987c02f 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1092,6 +1092,12 @@ default_read_core_file_mappings
>  {
>  }
> 
> +CORE_ADDR
> +default_get_return_buf_addr (struct type *val_type, frame_info_ptr
> cur_frame)
> +{
> +  return 0;
> +}
> +
>  /* Non-zero if we want to trace architecture code.  */
> 
>  #ifndef GDBARCH_DEBUG
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index f6229f434fd..46018a6fbbb 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings
>     struct bfd *cbfd,
>     read_core_file_mappings_pre_loop_ftype pre_loop_cb,
>     read_core_file_mappings_loop_ftype loop_cb);
> +
> +/* Default implementation of gdbarch default_get_return_buf_addr
> method.  */
> +extern CORE_ADDR default_get_return_buf_addr (struct type
> *val_typegdbarch,
> +					      frame_info_ptr
> cur_frame);
>  #endif /* ARCH_UTILS_H */
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 791648d6e7e..b88226db610 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -1325,14 +1325,8 @@ static const struct lval_funcs
> entry_data_value_funcs =
>    entry_data_value_free_closure
>  };
> 
> -/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND
> and KIND_U
> -   are used to match DW_AT_location at the caller's
> -   DW_TAG_call_site_parameter.
> -
> -   Function always returns non-NULL value.  It throws
> NO_ENTRY_VALUE_ERROR if it
> -   cannot resolve the parameter for any reason.  */
> -
> -static struct value *
> +/* See dwarf2/loc.h.  */
> +struct value *
>  value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame,
>  			  enum call_site_parameter_kind kind,
>  			  union call_site_parameter_u kind_u)
> diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
> index d6709f2e342..9156e1ee533 100644
> --- a/gdb/dwarf2/loc.h
> +++ b/gdb/dwarf2/loc.h
> @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer
>     dwarf2_per_objfile *per_objfile, frame_info_ptr frame,
>     struct type *type, bool resolve_abstract_p = false);
> 
> +/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND
> and KIND_U
> +   are used to match DW_AT_location at the caller's
> +   DW_TAG_call_site_parameter.
> +
> +   Function always returns non-NULL value.  It throws
> NO_ENTRY_VALUE_ERROR if
> +   it cannot resolve the parameter for any reason.  */
> +
> +extern struct value *value_of_dwarf_reg_entry (struct type *type,
> +					       struct frame_info_ptr
> frame,
> +					       enum
> call_site_parameter_kind kind,
> +					       union
> call_site_parameter_u kind_u);
>  #endif /* DWARF2LOC_H */
> diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
> index 46e7565f293..325801256a2 100644
> --- a/gdb/gdbarch-components.py
> +++ b/gdb/gdbarch-components.py
> @@ -868,6 +868,21 @@ for instance).
>      invalid=True,
>  )
> 
> +Function(
> +    comment="""
> +Return the address at which the value being returned from
> +the current function will be stored.  This routine is only
> +called if the current function uses the the "struct return
> +convention".
> +
> +May return 0 when unable to determine that address.""",
> +    type="CORE_ADDR",
> +    name="get_return_buf_addr",
> +    params=[("struct type *", "val_type"), ("frame_info_ptr",
> "cur_frame")],
> +    predefault="default_get_return_buf_addr",
> +    invalid=False,
> +)
> +
>  Method(
>      comment="""
>  Return true if the return value of function is stored in the first
> hidden
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 840de585869..66e2e761385 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -441,6 +441,17 @@ typedef enum return_value_convention
> (gdbarch_return_value_ftype) (struct gdbarc
>  extern enum return_value_convention gdbarch_return_value (struct
> gdbarch *gdbarch, struct value *function, struct type *valtype,
> struct regcache *regcache, gdb_byte *readbuf, const gdb_byte
> *writebuf);
>  extern void set_gdbarch_return_value (struct gdbarch *gdbarch,
> gdbarch_return_value_ftype *return_value);
> 
> +/* Return the address at which the value being returned from
> +   the current function will be stored.  This routine is only
> +   called if the current function uses the the "struct return
> +   convention".
> +
> +   May return 0 when unable to determine that address. */
> +
> +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type
> *val_type, frame_info_ptr cur_frame);
> +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch
> *gdbarch, struct type *val_type, frame_info_ptr cur_frame);
> +extern void set_gdbarch_get_return_buf_addr (struct gdbarch
> *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr);
> +
>  /* Return true if the return value of function is stored in the
> first hidden
>     parameter.  In theory, this feature should be language-dependent, 
> specified
>     by language and its ABI, such as C++.  Unfortunately, compiler
> may
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 559e92dee58..3bbb70e7be9 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -116,6 +116,7 @@ struct gdbarch
>    gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr;
>    gdbarch_integer_to_address_ftype *integer_to_address = nullptr;
>    gdbarch_return_value_ftype *return_value = nullptr;
> +  gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr;
>    gdbarch_return_in_first_hidden_param_p_ftype
> *return_in_first_hidden_param_p = nullptr;
>    gdbarch_skip_prologue_ftype *skip_prologue = nullptr;
>    gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
> @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
>    gdbarch->value_from_register = default_value_from_register;
>    gdbarch->pointer_to_address = unsigned_pointer_to_address;
>    gdbarch->address_to_pointer = unsigned_address_to_pointer;
> +  gdbarch->get_return_buf_addr = default_get_return_buf_addr;
>    gdbarch->return_in_first_hidden_param_p =
> default_return_in_first_hidden_param_p;
>    gdbarch->breakpoint_from_pc = default_breakpoint_from_pc;
>    gdbarch->sw_breakpoint_from_kind = NULL;
> @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>    /* Skip verify of address_to_pointer, invalid_p == 0 */
>    /* Skip verify of integer_to_address, has predicate.  */
>    /* Skip verify of return_value, has predicate.  */
> +  /* Skip verify of get_return_buf_addr, invalid_p == 0 */
>    /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0
> */
>    if (gdbarch->skip_prologue == 0)
>      log.puts ("\n\tskip_prologue");
> @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct
> ui_file *file)
>    gdb_printf (file,
>                        "gdbarch_dump: return_value = <%s>\n",
>                        host_address_to_string (gdbarch-
> >return_value));
> +  gdb_printf (file,
> +                      "gdbarch_dump: get_return_buf_addr = <%s>\n",
> +                      host_address_to_string (gdbarch-
> >get_return_buf_addr));
>    gdb_printf (file,
>                        "gdbarch_dump: return_in_first_hidden_param_p
> = <%s>\n",
>                        host_address_to_string (gdbarch-
> >return_in_first_hidden_param_p));
> @@ -2684,6 +2690,23 @@ set_gdbarch_return_value (struct gdbarch
> *gdbarch,
>    gdbarch->return_value = return_value;
>  }
> 
> +CORE_ADDR
> +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type
> *val_type, frame_info_ptr cur_frame)
> +{
> +  gdb_assert (gdbarch != NULL);
> +  gdb_assert (gdbarch->get_return_buf_addr != NULL);
> +  if (gdbarch_debug >= 2)
> +    gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n");
> +  return gdbarch->get_return_buf_addr (val_type, cur_frame);
> +}
> +
> +void
> +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch,
> +                                 gdbarch_get_return_buf_addr_ftype
> get_return_buf_addr)
> +{
> +  gdbarch->get_return_buf_addr = get_return_buf_addr;
> +}
> +
>  int
>  gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
> struct type *type)
>  {
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index d729732c81c..2e7cca1a1aa 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -55,6 +55,7 @@
>  #include "gdbsupport/gdb_optional.h"
>  #include "source.h"
>  #include "cli/cli-style.h"
> +#include "dwarf2/loc.h"
> 
>  /* Local functions: */
> 
> @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm
>       return value.  */
>    struct return_value_info return_value_info {};
> 
> +  /* If the current function uses the "struct return convention",
> +     this holds the address at which the value being returned will
> +     be stored, or zero if that address could not be determined or
> +     the "struct return convention" is not being used.  */
> +  CORE_ADDR return_buf;
> +
>    explicit finish_command_fsm (struct interp *cmd_interp)
>      : thread_fsm (cmd_interp)
>    {
> @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct
> thread_info *tp)
>  	  struct value *func;
> 
>  	  func = read_var_value (function, NULL, get_current_frame ());
> -	  rv->value = get_return_value (function, func);
> +
> +	  if (return_buf != 0)
> +	    /* Retrieve return value from the buffer where it was
> saved.  */
> +	      rv->value = value_at (rv->type, return_buf);
> +	  else
> +	      rv->value = get_return_value (function, func);
> +
>  	  if (rv->value != NULL)
>  	    rv->value_history_index = record_latest_value (rv->value);
>  	}
> @@ -1852,8 +1865,28 @@ finish_command (const char *arg, int from_tty)
>      }
> 
>    /* Find the function we will return from.  */
> -
> -  sm->function = find_pc_function (get_frame_pc (get_selected_frame
> (NULL)));
> +  frame_info_ptr callee_frame = get_selected_frame (NULL);
> +  sm->function = find_pc_function (get_frame_pc (callee_frame));
> +
> +  /* Determine the return convention.  If it is
> RETURN_VALUE_STRUCT_CONVENTION,
> +     attempt to determine the address of the return buffer.  */
> +  enum return_value_convention return_value;
> +  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
> +
> +  struct type * val_type
> +    = check_typedef (sm->function->type()->target_type ());
> +
> +  return_value = gdbarch_return_value (gdbarch,
> +				       read_var_value (sm->function,
> NULL,
> +						       callee_frame),
> +				       val_type, NULL, NULL, NULL);
> +
> +  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
> +      && val_type->code() != TYPE_CODE_VOID)
> +    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
> +						  callee_frame);
> +  else
> +    sm->return_buf = 0;
> 
>    /* Print info on the selected frame, including level number but
> not
>       source.  */
> @@ -1871,7 +1904,7 @@ finish_command (const char *arg, int from_tty)
>  	  gdb_printf (_("Run till exit from "));
>  	}
> 
> -      print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0);
> +      print_stack_frame (callee_frame, 1, LOCATION, 0);
>      }
>    frame.reinflate ();
> 
> diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
> index 14effb93210..f720e87e63b 100644
> --- a/gdb/ppc-sysv-tdep.c
> +++ b/gdb/ppc-sysv-tdep.c
> @@ -28,6 +28,7 @@
>  #include "objfiles.h"
>  #include "infcall.h"
>  #include "dwarf2.h"
> +#include "dwarf2/loc.h"
>  #include "target-float.h"
>  #include <algorithm>
> 
> @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch
> *gdbarch, struct value *function,
>    return RETURN_VALUE_STRUCT_CONVENTION;
>  }
> 
> +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type,
> +					  frame_info_ptr cur_frame)
> +{
> +  /* The PowerPC ABI specifies aggregates that are not returned by
> value
> +     are returned in a storage buffer provided by the caller.  The
> +     address of the storage buffer is provided as a hidden first
> input
> +     arguement in register r3.  The PowerPC ABI does not guarantee
> that
> +     register r3 will not be changed while executing the
> function.  Hence, it
> +     cannot be assumed that r3 will still contain the address of the
> storage
> +     buffer when execution reaches the end of the function.
> +
> +     This function attempts to determine the value of r3 on entry to
> the
> +     function using the DW_OP_entry_value DWARF entries.  This
> requires
> +     compiling the user program with -fvar-tracking to resolve the
> +     DW_TAG_call_sites in the binary file.  */
> +
> +  union call_site_parameter_u kind_u;
> +  enum call_site_parameter_kind kind;
> +  CORE_ADDR return_val = 0;
> +
> +  kind_u.dwarf_reg = 3;  /* First passed arg/return value is in
> r3.  */
> +  kind = CALL_SITE_PARAMETER_DWARF_REG;
> +
> +  /* val_type is the type of the return value.  Need the pointer
> type
> +     to the return value.  */
> +  val_type = lookup_pointer_type (val_type);
> +
> +  try
> +    {
> +      return_val = value_as_address (value_of_dwarf_reg_entry
> (val_type,
> +							       cur_fram
> e,
> +							       kind,
> kind_u));
> +    }
> +  catch (const gdb_exception_error &e)
> +    {
> +      warning ("Cannot determine the function return value.\n"
> +	       "Try compiling with -fvar-tracking.");
> +    }
> +  return return_val;
> +}
> diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
> index 34b250849b9..cbccd87820d 100644
> --- a/gdb/ppc-tdep.h
> +++ b/gdb/ppc-tdep.h
> @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct
> regset *regset,
>  				  const struct regcache *regcache,
>  				  int regnum, void *vsxregs, size_t
> len);
> 
> +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*,
> frame_info_ptr);
> +
>  /* Private data that this module attaches to struct gdbarch.  */
> 
>  /* ELF ABI version used by the inferior.  */
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index 8b6d666bbe7..637cbb41651 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -8234,7 +8234,11 @@ rs6000_gdbarch_init (struct gdbarch_info info,
> struct gdbarch_list *arches)
>    set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum);
> 
>    if (wordsize == 8)
> -    set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
> +    {
> +      set_gdbarch_return_value (gdbarch,
> ppc64_sysv_abi_return_value);
> +      set_gdbarch_get_return_buf_addr (gdbarch,
> +				       ppc64_sysv_get_return_buf_addr);
> +    }
>    else
>      set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value);
> 
> diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> index 21c390bc937..93a3a6832f7 100644
> --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> @@ -15,11 +15,18 @@
> 
>  # This file is part of the gdb testsuite
> 
> +set additional_flags ""
> +
>  if {[skip_cplus_tests]} { continue }
> 
>  standard_testfile .cc
> 
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile
> {debug c++}]} {
> +if {[have_fvar_tracking]} {
> +    set additional_flags "additional_flags= -fvar-tracking"
> +}
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile
> [list debug c++ $additional_flags]]} {
> +
>      return -1
>  }
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index bfa9fec628e..d3b77b4869b 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -8582,6 +8582,14 @@ gdb_caching_proc have_fuse_ld_gold {
>      return [gdb_simple_compile $me $src executable $flags]
>  }
> 
> +# Return 1 if compiler supports fvar-tracking, otherwise return 0.
> +gdb_caching_proc have_fvar_tracking {
> +    set me "have_fvar_tracking"
> +    set flags "additional_flags=-fvar-tracking"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags]
> +}
> +
>  # Return 1 if linker supports -Ttext-segment, otherwise return 0.
>  gdb_caching_proc linker_supports_Ttext_segment_flag {
>      set me "linker_supports_Ttext_segment_flag"
  

Cheers,
Bruno

On 18/10/2022 20:55, Carl Love wrote:
> GDB maintainers:
>
> Version 2, updated comments per Kevin's comments.  Changed "frame_info
> *" to frame_info_ptr per Bruno's comments.  Discussed supressing the
> new warning after printing it with Kevin.  In the end we decided to not
> suppress the warnings.  Suppression can be added later if deemed
> necessary.  The white space introduced by the gdb/gdbarch-components.py
> needs to be addressed by a future patch to fix the script and the
> generated files.
>
> On PowerPC, a non-trivial return value for a function cannot be
> reliably determined with the current PowerPC ABI.  The issue is the
> address of the return buffer for a non-trivial value is passed to the
> function in register r3.  However, at the end of the function the ABI
> does not guarantee r3 will not be changed.  Hence it cannot be reliably
> used to get the address of the return buffer.
>
> This patch adds a new GDB ABI to allow GDB to obtain the input value of
> r3 when the function returns.  This is done by using the DW_OP_entry
> value for the DWARF entries to obtain the value of r3 on entry to the
> function.  The ABI allows GDB to get the correct address for the return
> buffer on exit from the function.
>
> This patch fixes the five test failures in gdb.cp/non-trivial-
> retval.exp.
>
> The patch has been re-tested on PowerPC and X86-64 with no regression
> failures.
>
> Please let me know if this version of the patch is acceptable for GDB
> mainline.  Thanks.
>
>                           Carl Love

This whole area of code is pure sorcery for me, but I do see that the 
patch fails to apply because of conflicts on gdbarch.c, due to Tom 
Tromey's "Inline initialization of gdbarch members" patch, and some 
style nits inlined.

Sorry for taking so long to reply to this that it is not applying again, 
but in the future I wouldn't wait for my review if someone approves the 
patch.

>
> -----------------------------------
> PowerPC, fix support for printing the function return value for non-trivial values.
>
> Currently, a non-trivial return value from a function cannot currently be
> reliably determined on PowerPC.  This is due to the fact that the PowerPC
> ABI uses register r3 to store the address of the buffer containing the
> non-trivial return value when the function is called.  The PowerPC ABI
> does not guarantee the value in register r3 is not modified in the
> function.  Thus the value in r3 cannot be reliably used to obtain the
> return addreses on exit from the function.
>
> This patch adds a new gdbarch method to allow PowerPC to access the value
> of r3 on entry to a function. On PowerPC, the new gdbarch method attempts
> to use the DW_OP_entry_value for the DWARF entries, when exiting the
> function, to determine the value of r3 on entry to the function.  This
> requires the use of the -fvar-tracking compiler option to compile the
> user application thus generating the DW_OP_entry_value in the binary.  The
> DW_OP_entry_value entries in the binary file allows GDB to resolve the
> DW_TAG_call_site entries.  This new gdbarch method is used to get the
> return buffer address, in the case of a function returning a nontrivial
> data type, on exit from the function.  The GDB function should_stop checks
> to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set to
> zero by the new gdbarch method call for all architectures except PowerPC.
> The get_return_value function will be used to obtain the return value on
> all other architectures as is currently being done if RETURN_BUF is zero.
> On PowerPC, the new gdbarch method will return a nonzero address in
> RETURN_BUF if the value can be determined.  The value_at function uses the
> return buffer address to get the return value.
>
> This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp.
> The correct function return values are now reported.
>
> Note this patch is dependent on patch: "PowerPC, function
> ppc64_sysv_abi_return_value add missing return value convention".
>
> This patch has been tested on Power 10 and x86-64 with no regressions.
>
> Signed-off-by: Carl Love <cel@us.ibm.com>
> ---
>   gdb/arch-utils.c                            |  6 +++
>   gdb/arch-utils.h                            |  4 ++
>   gdb/dwarf2/loc.c                            | 10 +----
>   gdb/dwarf2/loc.h                            | 11 ++++++
>   gdb/gdbarch-components.py                   | 15 ++++++++
>   gdb/gdbarch-gen.h                           | 11 ++++++
>   gdb/gdbarch.c                               | 23 ++++++++++++
>   gdb/infcmd.c                                | 41 +++++++++++++++++++--
>   gdb/ppc-sysv-tdep.c                         | 41 +++++++++++++++++++++
>   gdb/ppc-tdep.h                              |  2 +
>   gdb/rs6000-tdep.c                           |  6 ++-
>   gdb/testsuite/gdb.cp/non-trivial-retval.exp |  9 ++++-
>   gdb/testsuite/lib/gdb.exp                   |  8 ++++
>   13 files changed, 173 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 6a72b3f5efd..1358987c02f 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1092,6 +1092,12 @@ default_read_core_file_mappings
>   {
>   }
>   
> +CORE_ADDR
> +default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame)
> +{
> +  return 0;
> +}
> +
>   /* Non-zero if we want to trace architecture code.  */
>   
>   #ifndef GDBARCH_DEBUG
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index f6229f434fd..46018a6fbbb 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings
>      struct bfd *cbfd,
>      read_core_file_mappings_pre_loop_ftype pre_loop_cb,
>      read_core_file_mappings_loop_ftype loop_cb);
> +
> +/* Default implementation of gdbarch default_get_return_buf_addr method.  */
> +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch,
> +					      frame_info_ptr cur_frame);
>   #endif /* ARCH_UTILS_H */
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 791648d6e7e..b88226db610 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -1325,14 +1325,8 @@ static const struct lval_funcs entry_data_value_funcs =
>     entry_data_value_free_closure
>   };
>   
> -/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND and KIND_U
> -   are used to match DW_AT_location at the caller's
> -   DW_TAG_call_site_parameter.
> -
> -   Function always returns non-NULL value.  It throws NO_ENTRY_VALUE_ERROR if it
> -   cannot resolve the parameter for any reason.  */
> -
> -static struct value *
> +/* See dwarf2/loc.h.  */
> +struct value *
>   value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame,
>   			  enum call_site_parameter_kind kind,
>   			  union call_site_parameter_u kind_u)
> diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
> index d6709f2e342..9156e1ee533 100644
> --- a/gdb/dwarf2/loc.h
> +++ b/gdb/dwarf2/loc.h
> @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer
>      dwarf2_per_objfile *per_objfile, frame_info_ptr frame,
>      struct type *type, bool resolve_abstract_p = false);
>   
> +/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND and KIND_U
> +   are used to match DW_AT_location at the caller's
> +   DW_TAG_call_site_parameter.
> +
> +   Function always returns non-NULL value.  It throws NO_ENTRY_VALUE_ERROR if
> +   it cannot resolve the parameter for any reason.  */
> +
> +extern struct value *value_of_dwarf_reg_entry (struct type *type,
> +					       struct frame_info_ptr frame,
> +					       enum call_site_parameter_kind kind,
> +					       union call_site_parameter_u kind_u);
>   #endif /* DWARF2LOC_H */
> diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
> index 46e7565f293..325801256a2 100644
> --- a/gdb/gdbarch-components.py
> +++ b/gdb/gdbarch-components.py
> @@ -868,6 +868,21 @@ for instance).
>       invalid=True,
>   )
>   
> +Function(
> +    comment="""
> +Return the address at which the value being returned from
> +the current function will be stored.  This routine is only
> +called if the current function uses the the "struct return
> +convention".
> +
> +May return 0 when unable to determine that address.""",
> +    type="CORE_ADDR",
> +    name="get_return_buf_addr",
> +    params=[("struct type *", "val_type"), ("frame_info_ptr", "cur_frame")],
> +    predefault="default_get_return_buf_addr",
> +    invalid=False,
> +)
> +
>   Method(
>       comment="""
>   Return true if the return value of function is stored in the first hidden
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 840de585869..66e2e761385 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -441,6 +441,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc
>   extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
>   extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value);
>   
> +/* Return the address at which the value being returned from
> +   the current function will be stored.  This routine is only
> +   called if the current function uses the the "struct return
> +   convention".
> +
> +   May return 0 when unable to determine that address. */
> +
> +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info_ptr cur_frame);
> +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame);
> +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr);
> +
>   /* Return true if the return value of function is stored in the first hidden
>      parameter.  In theory, this feature should be language-dependent, specified
>      by language and its ABI, such as C++.  Unfortunately, compiler may
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 559e92dee58..3bbb70e7be9 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -116,6 +116,7 @@ struct gdbarch
>     gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr;
>     gdbarch_integer_to_address_ftype *integer_to_address = nullptr;
>     gdbarch_return_value_ftype *return_value = nullptr;
> +  gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr;
>     gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr;
>     gdbarch_skip_prologue_ftype *skip_prologue = nullptr;
>     gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
> @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
>     gdbarch->value_from_register = default_value_from_register;
>     gdbarch->pointer_to_address = unsigned_pointer_to_address;
>     gdbarch->address_to_pointer = unsigned_address_to_pointer;
> +  gdbarch->get_return_buf_addr = default_get_return_buf_addr;
>     gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
>     gdbarch->breakpoint_from_pc = default_breakpoint_from_pc;
>     gdbarch->sw_breakpoint_from_kind = NULL;
> @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>     /* Skip verify of address_to_pointer, invalid_p == 0 */
>     /* Skip verify of integer_to_address, has predicate.  */
>     /* Skip verify of return_value, has predicate.  */
> +  /* Skip verify of get_return_buf_addr, invalid_p == 0 */
>     /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
>     if (gdbarch->skip_prologue == 0)
>       log.puts ("\n\tskip_prologue");
> @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>     gdb_printf (file,
>                         "gdbarch_dump: return_value = <%s>\n",
>                         host_address_to_string (gdbarch->return_value));
> +  gdb_printf (file,
> +                      "gdbarch_dump: get_return_buf_addr = <%s>\n",
> +                      host_address_to_string (gdbarch->get_return_buf_addr));
>     gdb_printf (file,
>                         "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n",
>                         host_address_to_string (gdbarch->return_in_first_hidden_param_p));
> @@ -2684,6 +2690,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
>     gdbarch->return_value = return_value;
>   }
>   
> +CORE_ADDR
> +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame)
> +{
> +  gdb_assert (gdbarch != NULL);
> +  gdb_assert (gdbarch->get_return_buf_addr != NULL);
> +  if (gdbarch_debug >= 2)
> +    gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n");
> +  return gdbarch->get_return_buf_addr (val_type, cur_frame);
> +}
> +
> +void
> +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch,
> +                                 gdbarch_get_return_buf_addr_ftype get_return_buf_addr)
> +{
> +  gdbarch->get_return_buf_addr = get_return_buf_addr;
> +}
> +
>   int
>   gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
>   {
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index d729732c81c..2e7cca1a1aa 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -55,6 +55,7 @@
>   #include "gdbsupport/gdb_optional.h"
>   #include "source.h"
>   #include "cli/cli-style.h"
> +#include "dwarf2/loc.h"
>   
>   /* Local functions: */
>   
> @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm
>        return value.  */
>     struct return_value_info return_value_info {};
>   
> +  /* If the current function uses the "struct return convention",
> +     this holds the address at which the value being returned will
> +     be stored, or zero if that address could not be determined or
> +     the "struct return convention" is not being used.  */
> +  CORE_ADDR return_buf;
> +
>     explicit finish_command_fsm (struct interp *cmd_interp)
>       : thread_fsm (cmd_interp)
>     {
> @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct thread_info *tp)
>   	  struct value *func;
>   
>   	  func = read_var_value (function, NULL, get_current_frame ());
> -	  rv->value = get_return_value (function, func);
> +
> +	  if (return_buf != 0)
> +	    /* Retrieve return value from the buffer where it was saved.  */
> +	      rv->value = value_at (rv->type, return_buf);
> +	  else
> +	      rv->value = get_return_value (function, func);
> +
>   	  if (rv->value != NULL)
>   	    rv->value_history_index = record_latest_value (rv->value);
>   	}
> @@ -1852,8 +1865,28 @@ finish_command (const char *arg, int from_tty)
>       }
>   
>     /* Find the function we will return from.  */
> -
> -  sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
> +  frame_info_ptr callee_frame = get_selected_frame (NULL);
> +  sm->function = find_pc_function (get_frame_pc (callee_frame));
> +
> +  /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
> +     attempt to determine the address of the return buffer.  */
> +  enum return_value_convention return_value;
> +  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
> +
> +  struct type * val_type
> +    = check_typedef (sm->function->type()->target_type ());
missing space before (.
> +
> +  return_value = gdbarch_return_value (gdbarch,
> +				       read_var_value (sm->function, NULL,
> +						       callee_frame),
> +				       val_type, NULL, NULL, NULL);
> +
> +  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
> +      && val_type->code() != TYPE_CODE_VOID)
same here.
> +    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
> +						  callee_frame);
> +  else
> +    sm->return_buf = 0;
>   
>     /* Print info on the selected frame, including level number but not
>        source.  */
> @@ -1871,7 +1904,7 @@ finish_command (const char *arg, int from_tty)
>   	  gdb_printf (_("Run till exit from "));
>   	}
>   
> -      print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0);
> +      print_stack_frame (callee_frame, 1, LOCATION, 0);
>       }
>     frame.reinflate ();
>   
> diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
> index 14effb93210..f720e87e63b 100644
> --- a/gdb/ppc-sysv-tdep.c
> +++ b/gdb/ppc-sysv-tdep.c
> @@ -28,6 +28,7 @@
>   #include "objfiles.h"
>   #include "infcall.h"
>   #include "dwarf2.h"
> +#include "dwarf2/loc.h"
>   #include "target-float.h"
>   #include <algorithm>
>   
> @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
>     return RETURN_VALUE_STRUCT_CONVENTION;
>   }
>   
> +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type,
> +					  frame_info_ptr cur_frame)
> +{
> +  /* The PowerPC ABI specifies aggregates that are not returned by value
> +     are returned in a storage buffer provided by the caller.  The
> +     address of the storage buffer is provided as a hidden first input
> +     arguement in register r3.  The PowerPC ABI does not guarantee that
> +     register r3 will not be changed while executing the function.  Hence, it
> +     cannot be assumed that r3 will still contain the address of the storage
> +     buffer when execution reaches the end of the function.
> +
> +     This function attempts to determine the value of r3 on entry to the
> +     function using the DW_OP_entry_value DWARF entries.  This requires
> +     compiling the user program with -fvar-tracking to resolve the
> +     DW_TAG_call_sites in the binary file.  */
> +
> +  union call_site_parameter_u kind_u;
> +  enum call_site_parameter_kind kind;
> +  CORE_ADDR return_val = 0;
> +
> +  kind_u.dwarf_reg = 3;  /* First passed arg/return value is in r3.  */
> +  kind = CALL_SITE_PARAMETER_DWARF_REG;
> +
> +  /* val_type is the type of the return value.  Need the pointer type
> +     to the return value.  */
> +  val_type = lookup_pointer_type (val_type);
> +
> +  try
> +    {
> +      return_val = value_as_address (value_of_dwarf_reg_entry (val_type,
> +							       cur_frame,
> +							       kind, kind_u));
> +    }
> +  catch (const gdb_exception_error &e)
> +    {
> +      warning ("Cannot determine the function return value.\n"
> +	       "Try compiling with -fvar-tracking.");
> +    }
> +  return return_val;
> +}
> diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
> index 34b250849b9..cbccd87820d 100644
> --- a/gdb/ppc-tdep.h
> +++ b/gdb/ppc-tdep.h
> @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
>   				  const struct regcache *regcache,
>   				  int regnum, void *vsxregs, size_t len);
>   
> +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
> +
>   /* Private data that this module attaches to struct gdbarch.  */
>   
>   /* ELF ABI version used by the inferior.  */
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index 8b6d666bbe7..637cbb41651 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -8234,7 +8234,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum);
>   
>     if (wordsize == 8)
> -    set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
> +    {
> +      set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
> +      set_gdbarch_get_return_buf_addr (gdbarch,
> +				       ppc64_sysv_get_return_buf_addr);
> +    }
>     else
>       set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value);
>   
> diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> index 21c390bc937..93a3a6832f7 100644
> --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
> @@ -15,11 +15,18 @@
>   
>   # This file is part of the gdb testsuite
>   
> +set additional_flags ""
> +
>   if {[skip_cplus_tests]} { continue }
>   
>   standard_testfile .cc
>   
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> +if {[have_fvar_tracking]} {
> +    set additional_flags "additional_flags= -fvar-tracking"
> +}
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} {
> +
>       return -1
>   }
>   
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index bfa9fec628e..d3b77b4869b 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -8582,6 +8582,14 @@ gdb_caching_proc have_fuse_ld_gold {
>       return [gdb_simple_compile $me $src executable $flags]
>   }
>   
> +# Return 1 if compiler supports fvar-tracking, otherwise return 0.
> +gdb_caching_proc have_fvar_tracking {
> +    set me "have_fvar_tracking"
> +    set flags "additional_flags=-fvar-tracking"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags]
> +}
> +
>   # Return 1 if linker supports -Ttext-segment, otherwise return 0.
>   gdb_caching_proc linker_supports_Ttext_segment_flag {
>       set me "linker_supports_Ttext_segment_flag"
  

Bruno:

On Mon, 2022-11-07 at 15:56 +0100, Bruno Larsen wrote:
> This whole area of code is pure sorcery for me, but I do see that
> the 
> patch fails to apply because of conflicts on gdbarch.c, due to Tom 
> Tromey's "Inline initialization of gdbarch members" patch, and some 
> style nits inlined.
> 
> Sorry for taking so long to reply to this that it is not applying
> again, 
> but in the future I wouldn't wait for my review if someone approves
> the 
> patch.

Thanks for the reply.  I was hoping that Kevin would also let me know
if all of his concerns had been addressed.  I did fix up the two
formatting issues you mentioned, rebased and retested the patch on the
latest gdb source code tree.

I will post version 3 of the patch and ping version 2 of patch 1/2 and
see if we can get a maintainer to review the patches.  Thanks for all
the help and input.

                       Carl
  

GDB maintainers:

Version 3, fixed two formatting issues noted by Bruno.  Patch was
rebased on the latest gdb source tree and retested with no regression
failures.

Version 2, updated comments per Kevin's comments.  Changed "frame_info
*" to frame_info_ptr per Bruno's comments.  Discussed supressing the
new warning after printing it with Kevin.  In the end we decided to not
suppress the warnings.  Suppression can be added later if deemed
necessary.  The white space introduced by the gdb/gdbarch-
components.py 
needs to be addressed by a future patch to fix the script and the
generated files.

On PowerPC, a non-trivial return value for a function cannot be
reliably determined with the current PowerPC ABI.  The issue is the
address of the return buffer for a non-trivial value is passed to the
function in register r3.  However, at the end of the function the ABI
does not guarantee r3 will not be changed.  Hence it cannot be reliably
used to get the address of the return buffer.

This patch adds a new GDB ABI to allow GDB to obtain the input value of
r3 when the function returns.  This is done by using the DW_OP_entry
value for the DWARF entries to obtain the value of r3 on entry to the
function.  The ABI allows GDB to get the correct address for the return
buffer on exit from the function.

This patch fixes the five test failures in gdb.cp/non-trivial-
retval.exp.

The patch has been re-tested on PowerPC and X86-64 with no regression
failures.

Please let me know if this version of the patch is acceptable for GDB
mainline.  Thanks.

                         Carl Love


-----------------------------------

PowerPC, fix support for printing the function return value for non-trivial values.

Currently, a non-trivial return value from a function cannot currently be
reliably determined on PowerPC.  This is due to the fact that the PowerPC
ABI uses register r3 to store the address of the buffer containing the
non-trivial return value when the function is called.  The PowerPC ABI
does not guarantee the value in register r3 is not modified in the
function.  Thus the value in r3 cannot be reliably used to obtain the
return addreses on exit from the function.

This patch adds a new gdbarch method to allow PowerPC to access the value
of r3 on entry to a function. On PowerPC, the new gdbarch method attempts
to use the DW_OP_entry_value for the DWARF entries, when exiting the
function, to determine the value of r3 on entry to the function.  This
requires the use of the -fvar-tracking compiler option to compile the
user application thus generating the DW_OP_entry_value in the binary.  The
DW_OP_entry_value entries in the binary file allows GDB to resolve the
DW_TAG_call_site entries.  This new gdbarch method is used to get the
return buffer address, in the case of a function returning a nontrivial
data type, on exit from the function.  The GDB function should_stop checks
to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set to
zero by the new gdbarch method call for all architectures except PowerPC.
The get_return_value function will be used to obtain the return value on
all other architectures as is currently being done if RETURN_BUF is zero.
On PowerPC, the new gdbarch method will return a nonzero address in
RETURN_BUF if the value can be determined.  The value_at function uses the
return buffer address to get the return value.

This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp.
The correct function return values are now reported.

Note this patch is dependent on patch: "PowerPC, function
ppc64_sysv_abi_return_value add missing return value convention".

This patch has been tested on Power 10 and x86-64 with no regressions.
---
 gdb/arch-utils.c                            |  6 +++
 gdb/arch-utils.h                            |  4 ++
 gdb/dwarf2/loc.c                            | 10 +----
 gdb/dwarf2/loc.h                            | 11 ++++++
 gdb/gdbarch-components.py                   | 15 ++++++++
 gdb/gdbarch-gen.h                           | 11 ++++++
 gdb/gdbarch.c                               | 22 +++++++++++
 gdb/infcmd.c                                | 41 +++++++++++++++++++--
 gdb/ppc-sysv-tdep.c                         | 41 +++++++++++++++++++++
 gdb/ppc-tdep.h                              |  2 +
 gdb/rs6000-tdep.c                           |  6 ++-
 gdb/testsuite/gdb.cp/non-trivial-retval.exp |  9 ++++-
 gdb/testsuite/lib/gdb.exp                   |  8 ++++
 13 files changed, 172 insertions(+), 14 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 5218bfc05e1..7b84daf046e 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1084,6 +1084,12 @@ default_read_core_file_mappings
 {
 }
 
+CORE_ADDR
+default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame)
+{
+  return 0;
+}
+
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index f6229f434fd..46018a6fbbb 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -300,4 +300,8 @@ extern void default_read_core_file_mappings
    struct bfd *cbfd,
    read_core_file_mappings_pre_loop_ftype pre_loop_cb,
    read_core_file_mappings_loop_ftype loop_cb);
+
+/* Default implementation of gdbarch default_get_return_buf_addr method.  */
+extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch,
+					      frame_info_ptr cur_frame);
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index c42359ab96e..8355aa44333 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1325,14 +1325,8 @@ static const struct lval_funcs entry_data_value_funcs =
   entry_data_value_free_closure
 };
 
-/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND and KIND_U
-   are used to match DW_AT_location at the caller's
-   DW_TAG_call_site_parameter.
-
-   Function always returns non-NULL value.  It throws NO_ENTRY_VALUE_ERROR if it
-   cannot resolve the parameter for any reason.  */
-
-static struct value *
+/* See dwarf2/loc.h.  */
+struct value *
 value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame,
 			  enum call_site_parameter_kind kind,
 			  union call_site_parameter_u kind_u)
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index d6709f2e342..9156e1ee533 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer
    dwarf2_per_objfile *per_objfile, frame_info_ptr frame,
    struct type *type, bool resolve_abstract_p = false);
 
+/* Read parameter of TYPE at (callee) FRAME's function entry.  KIND and KIND_U
+   are used to match DW_AT_location at the caller's
+   DW_TAG_call_site_parameter.
+
+   Function always returns non-NULL value.  It throws NO_ENTRY_VALUE_ERROR if
+   it cannot resolve the parameter for any reason.  */
+
+extern struct value *value_of_dwarf_reg_entry (struct type *type,
+					       struct frame_info_ptr frame,
+					       enum call_site_parameter_kind kind,
+					       union call_site_parameter_u kind_u);
 #endif /* DWARF2LOC_H */
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index b2c7b784761..9b688998a7b 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -868,6 +868,21 @@ for instance).
     invalid=True,
 )
 
+Function(
+    comment="""
+Return the address at which the value being returned from
+the current function will be stored.  This routine is only
+called if the current function uses the the "struct return
+convention".
+
+May return 0 when unable to determine that address.""",
+    type="CORE_ADDR",
+    name="get_return_buf_addr",
+    params=[("struct type *", "val_type"), ("frame_info_ptr", "cur_frame")],
+    predefault="default_get_return_buf_addr",
+    invalid=False,
+)
+
 Method(
     comment="""
 Return true if the return value of function is stored in the first hidden
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index e0d7a08ff6a..a663316df16 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -441,6 +441,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc
 extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value);
 
+/* Return the address at which the value being returned from
+   the current function will be stored.  This routine is only
+   called if the current function uses the the "struct return
+   convention".
+
+   May return 0 when unable to determine that address. */
+
+typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info_ptr cur_frame);
+extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame);
+extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr);
+
 /* Return true if the return value of function is stored in the first hidden
    parameter.  In theory, this feature should be language-dependent, specified
    by language and its ABI, such as C++.  Unfortunately, compiler may
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 9d929da6da9..3227e945880 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -116,6 +116,7 @@ struct gdbarch
   gdbarch_address_to_pointer_ftype *address_to_pointer = unsigned_address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address = nullptr;
   gdbarch_return_value_ftype *return_value = nullptr;
+  gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = default_get_return_buf_addr;
   gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch_skip_prologue_ftype *skip_prologue = 0;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
@@ -369,6 +370,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate.  */
   /* Skip verify of return_value, has predicate.  */
+  /* Skip verify of get_return_buf_addr, invalid_p == 0 */
   /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
     log.puts ("\n\tskip_prologue");
@@ -780,6 +782,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
                       "gdbarch_dump: return_value = <%s>\n",
                       host_address_to_string (gdbarch->return_value));
+  gdb_printf (file,
+                      "gdbarch_dump: get_return_buf_addr = <%s>\n",
+                      host_address_to_string (gdbarch->get_return_buf_addr));
   gdb_printf (file,
                       "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n",
                       host_address_to_string (gdbarch->return_in_first_hidden_param_p));
@@ -2587,6 +2592,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+CORE_ADDR
+gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->get_return_buf_addr != NULL);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n");
+  return gdbarch->get_return_buf_addr (val_type, cur_frame);
+}
+
+void
+set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch,
+                                 gdbarch_get_return_buf_addr_ftype get_return_buf_addr)
+{
+  gdbarch->get_return_buf_addr = get_return_buf_addr;
+}
+
 int
 gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
 {
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c03ca103c91..ef00504969f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -55,6 +55,7 @@
 #include "gdbsupport/gdb_optional.h"
 #include "source.h"
 #include "cli/cli-style.h"
+#include "dwarf2/loc.h"
 
 /* Local functions: */
 
@@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm
      return value.  */
   struct return_value_info return_value_info {};
 
+  /* If the current function uses the "struct return convention",
+     this holds the address at which the value being returned will
+     be stored, or zero if that address could not be determined or
+     the "struct return convention" is not being used.  */
+  CORE_ADDR return_buf;
+
   explicit finish_command_fsm (struct interp *cmd_interp)
     : thread_fsm (cmd_interp)
   {
@@ -1635,7 +1642,13 @@ finish_command_fsm::should_stop (struct thread_info *tp)
 	  struct value *func;
 
 	  func = read_var_value (function, NULL, get_current_frame ());
-	  rv->value = get_return_value (function, func);
+
+	  if (return_buf != 0)
+	    /* Retrieve return value from the buffer where it was saved.  */
+	      rv->value = value_at (rv->type, return_buf);
+	  else
+	      rv->value = get_return_value (function, func);
+
 	  if (rv->value != NULL)
 	    rv->value_history_index = record_latest_value (rv->value);
 	}
@@ -1851,8 +1864,28 @@ finish_command (const char *arg, int from_tty)
     }
 
   /* Find the function we will return from.  */
-
-  sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
+  frame_info_ptr callee_frame = get_selected_frame (NULL);
+  sm->function = find_pc_function (get_frame_pc (callee_frame));
+
+  /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
+     attempt to determine the address of the return buffer.  */
+  enum return_value_convention return_value;
+  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
+
+  struct type * val_type
+    = check_typedef (sm->function->type ()->target_type ());
+
+  return_value = gdbarch_return_value (gdbarch,
+				       read_var_value (sm->function, NULL,
+						       callee_frame),
+				       val_type, NULL, NULL, NULL);
+
+  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
+      && val_type->code () != TYPE_CODE_VOID)
+    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
+						  callee_frame);
+  else
+    sm->return_buf = 0;
 
   /* Print info on the selected frame, including level number but not
      source.  */
@@ -1870,7 +1903,7 @@ finish_command (const char *arg, int from_tty)
 	  gdb_printf (_("Run till exit from "));
 	}
 
-      print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0);
+      print_stack_frame (callee_frame, 1, LOCATION, 0);
     }
   frame.reinflate ();
 
diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index 450162dd46e..1cbaaf2a4e6 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -28,6 +28,7 @@
 #include "objfiles.h"
 #include "infcall.h"
 #include "dwarf2.h"
+#include "dwarf2/loc.h"
 #include "target-float.h"
 #include <algorithm>
 
@@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
   return RETURN_VALUE_STRUCT_CONVENTION;
 }
 
+CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type,
+					  frame_info_ptr cur_frame)
+{
+  /* The PowerPC ABI specifies aggregates that are not returned by value
+     are returned in a storage buffer provided by the caller.  The
+     address of the storage buffer is provided as a hidden first input
+     arguement in register r3.  The PowerPC ABI does not guarantee that
+     register r3 will not be changed while executing the function.  Hence, it
+     cannot be assumed that r3 will still contain the address of the storage
+     buffer when execution reaches the end of the function.
+
+     This function attempts to determine the value of r3 on entry to the
+     function using the DW_OP_entry_value DWARF entries.  This requires
+     compiling the user program with -fvar-tracking to resolve the
+     DW_TAG_call_sites in the binary file.  */
+
+  union call_site_parameter_u kind_u;
+  enum call_site_parameter_kind kind;
+  CORE_ADDR return_val = 0;
+
+  kind_u.dwarf_reg = 3;  /* First passed arg/return value is in r3.  */
+  kind = CALL_SITE_PARAMETER_DWARF_REG;
+
+  /* val_type is the type of the return value.  Need the pointer type
+     to the return value.  */
+  val_type = lookup_pointer_type (val_type);
+
+  try
+    {
+      return_val = value_as_address (value_of_dwarf_reg_entry (val_type,
+							       cur_frame,
+							       kind, kind_u));
+    }
+  catch (const gdb_exception_error &e)
+    {
+      warning ("Cannot determine the function return value.\n"
+	       "Try compiling with -fvar-tracking.");
+    }
+  return return_val;
+}
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index 34b250849b9..cbccd87820d 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
 				  const struct regcache *regcache,
 				  int regnum, void *vsxregs, size_t len);
 
+extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
+
 /* Private data that this module attaches to struct gdbarch.  */
 
 /* ELF ABI version used by the inferior.  */
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 866d43ded7a..cbd84514795 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -8243,7 +8243,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum);
 
   if (wordsize == 8)
-    set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
+    {
+      set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
+      set_gdbarch_get_return_buf_addr (gdbarch,
+				       ppc64_sysv_get_return_buf_addr);
+    }
   else
     set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value);
 
diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
index 21c390bc937..93a3a6832f7 100644
--- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp
+++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
@@ -15,11 +15,18 @@
 
 # This file is part of the gdb testsuite
 
+set additional_flags ""
+
 if {[skip_cplus_tests]} { continue }
 
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[have_fvar_tracking]} {
+    set additional_flags "additional_flags= -fvar-tracking"
+}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} {
+
     return -1
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e2cda30b95a..ed70c08d1f2 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8796,6 +8796,14 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Return 1 if compiler supports fvar-tracking, otherwise return 0.
+gdb_caching_proc have_fvar_tracking {
+    set me "have_fvar_tracking"
+    set flags "additional_flags=-fvar-tracking"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
 # Return 1 if linker supports -Ttext-segment, otherwise return 0.
 gdb_caching_proc linker_supports_Ttext_segment_flag {
     set me "linker_supports_Ttext_segment_flag"
  

Carl Love <cel@us.ibm.com> wrote:

>PowerPC, fix support for printing the function return value for non-trivial values.
>
>Currently, a non-trivial return value from a function cannot currently be
>reliably determined on PowerPC.  This is due to the fact that the PowerPC
>ABI uses register r3 to store the address of the buffer containing the
>non-trivial return value when the function is called.  The PowerPC ABI
>does not guarantee the value in register r3 is not modified in the
>function.  Thus the value in r3 cannot be reliably used to obtain the
>return addreses on exit from the function.
>
>This patch adds a new gdbarch method to allow PowerPC to access the value
>of r3 on entry to a function. On PowerPC, the new gdbarch method attempts
>to use the DW_OP_entry_value for the DWARF entries, when exiting the
>function, to determine the value of r3 on entry to the function.  This
>requires the use of the -fvar-tracking compiler option to compile the
>user application thus generating the DW_OP_entry_value in the binary.  The
>DW_OP_entry_value entries in the binary file allows GDB to resolve the
>DW_TAG_call_site entries.  This new gdbarch method is used to get the
>return buffer address, in the case of a function returning a nontrivial
>data type, on exit from the function.  The GDB function should_stop checks
>to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set to
>zero by the new gdbarch method call for all architectures except PowerPC.
>The get_return_value function will be used to obtain the return value on
>all other architectures as is currently being done if RETURN_BUF is zero.
>On PowerPC, the new gdbarch method will return a nonzero address in
>RETURN_BUF if the value can be determined.  The value_at function uses the
>return buffer address to get the return value.
>
>This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp.
>The correct function return values are now reported.
>
>Note this patch is dependent on patch: "PowerPC, function
>ppc64_sysv_abi_return_value add missing return value convention".
>
>This patch has been tested on Power 10 and x86-64 with no regressions.

I believe all of Kevin's and Bruno's comments have been addressed
in this version.  I've reviewed it myself as well, and it looks
good to me.

This is OK.

Thanks,
Ulrich
  

On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote:
> Carl Love <cel@us.ibm.com> wrote:
> 
>> PowerPC, fix support for printing the function return value for non-trivial values.
>>
>> Currently, a non-trivial return value from a function cannot currently be
>> reliably determined on PowerPC.  This is due to the fact that the PowerPC
>> ABI uses register r3 to store the address of the buffer containing the
>> non-trivial return value when the function is called.  The PowerPC ABI
>> does not guarantee the value in register r3 is not modified in the
>> function.  Thus the value in r3 cannot be reliably used to obtain the
>> return addreses on exit from the function.
>>
>> This patch adds a new gdbarch method to allow PowerPC to access the value
>> of r3 on entry to a function. On PowerPC, the new gdbarch method attempts
>> to use the DW_OP_entry_value for the DWARF entries, when exiting the
>> function, to determine the value of r3 on entry to the function.  This
>> requires the use of the -fvar-tracking compiler option to compile the
>> user application thus generating the DW_OP_entry_value in the binary.  The
>> DW_OP_entry_value entries in the binary file allows GDB to resolve the
>> DW_TAG_call_site entries.  This new gdbarch method is used to get the
>> return buffer address, in the case of a function returning a nontrivial
>> data type, on exit from the function.  The GDB function should_stop checks
>> to see if RETURN_BUF is non-zero.  By default, RETURN_BUF will be set to
>> zero by the new gdbarch method call for all architectures except PowerPC.
>> The get_return_value function will be used to obtain the return value on
>> all other architectures as is currently being done if RETURN_BUF is zero.
>> On PowerPC, the new gdbarch method will return a nonzero address in
>> RETURN_BUF if the value can be determined.  The value_at function uses the
>> return buffer address to get the return value.
>>
>> This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp.
>> The correct function return values are now reported.
>>
>> Note this patch is dependent on patch: "PowerPC, function
>> ppc64_sysv_abi_return_value add missing return value convention".
>>
>> This patch has been tested on Power 10 and x86-64 with no regressions.
> 
> I believe all of Kevin's and Bruno's comments have been addressed
> in this version.  I've reviewed it myself as well, and it looks
> good to me.
> 
> This is OK.
> 

On x86_64-linux, I run into a segfault:
...
PASS: gdb.asm/asm-source.exp: info source asmsrc1.s
ERROR: GDB process no longer exists
UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3
...

Reproduced on command line:
...
$ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1
...

The problem seems to be that:
...
Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
0x000000000043de7a in symbol::type (this=0x0) at 
/home/vries/gdb_versions/devel/src/gdb/symtab.h:1287
1287        return m_type;
...
because:
...
(gdb) up
#1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
     at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887
1887        = check_typedef (sm->function->type ()->target_type ());
(gdb) p sm->function
$1 = (symbol *) 0x0
...

Thanks,
- Tom
  

Tom de Vries <tdevries@suse.de> wrote:
>On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote:
> >I believe all of Kevin's and Bruno's comments have been addressed
> >in this version.  I've reviewed it myself as well, and it looks
> >good to me.
> 
> >This is OK.

>On x86_64-linux, I run into a segfault:
>[...]
>(gdb) up
>#1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
>     at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887
>1887        = check_typedef (sm->function->type ()->target_type ());
>(gdb) p sm->function
>$1 = (symbol *) 0x0

Ah, I missed that possibility, sorry.

Carl, if sm->function is NULL, then the whole check needs
to be skipped, and sm->return_buf should be set to 0.

Can you come up with a fix along those lines (or else,
just revert the patch for now)?   Thanks!

Bye,
Ulrich
  

On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote:
> Tom de Vries <tdevries@suse.de> wrote:
> > On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote:
> > > I believe all of Kevin's and Bruno's comments have been addressed
> > > in this version.  I've reviewed it myself as well, and it looks
> > > good to me.
> > > This is OK.
> > On x86_64-linux, I run into a segfault:
> > [...]
> > (gdb) up
> > #1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
> >     at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887
> > 1887        = check_typedef (sm->function->type ()->target_type
> > ());
> > (gdb) p sm->function
> > $1 = (symbol *) 0x0
> 
> Ah, I missed that possibility, sorry.
> 
> Carl, if sm->function is NULL, then the whole check needs
> to be skipped, and sm->return_buf should be set to 0.
> 
> Can you come up with a fix along those lines (or else,
> just revert the patch for now)?   Thanks!
> 
Yes, I will take a look and see about fixing or reverting it ASAP.

                 Carl
  

On 11/15/22 11:04, Carl Love via Gdb-patches wrote:
> On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote:
>> Tom de Vries <tdevries@suse.de> wrote:
>>> On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote:
>>>> I believe all of Kevin's and Bruno's comments have been addressed
>>>> in this version.  I've reviewed it myself as well, and it looks
>>>> good to me.
>>>> This is OK.
>>> On x86_64-linux, I run into a segfault:
>>> [...]
>>> (gdb) up
>>> #1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
>>>     at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887
>>> 1887        = check_typedef (sm->function->type ()->target_type
>>> ());
>>> (gdb) p sm->function
>>> $1 = (symbol *) 0x0
>>
>> Ah, I missed that possibility, sorry.
>>
>> Carl, if sm->function is NULL, then the whole check needs
>> to be skipped, and sm->return_buf should be set to 0.
>>
>> Can you come up with a fix along those lines (or else,
>> just revert the patch for now)?   Thanks!
>>
> Yes, I will take a look and see about fixing or reverting it ASAP.
> 
>                  Carl 
> 

Just so you can test properly, I also see these failures that are likely
due to the same bug:

UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork parent follow, finish after tcatch vfork: finish
UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork child follow, finish after tcatch vfork: finish
UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork parent follow, finish after tcatch vfork: finish
UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork child follow, finish after tcatch vfork: finish
UNRESOLVED: gdb.base/sigaltstack.exp: finish to throw INNER
UNRESOLVED: gdb.base/sigstep.exp: finish from handleri: leave signal trampoline

Simon
  

Tom, Ulrich:

On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote:
> Tom de Vries <tdevries@suse.de> wrote:
> > On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote:
> > > I believe all of Kevin's and Bruno's comments have been addressed
> > > in this version.  I've reviewed it myself as well, and it looks
> > > good to me.
> > > This is OK.
> > On x86_64-linux, I run into a segfault:
> > [...]
> > (gdb) up
> > #1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
> >     at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887
> > 1887        = check_typedef (sm->function->type ()->target_type
> > ());
> > (gdb) p sm->function
> > $1 = (symbol *) 0x0
> 
> Ah, I missed that possibility, sorry.
> 
> Carl, if sm->function is NULL, then the whole check needs
> to be skipped, and sm->return_buf should be set to 0.
> 
> Can you come up with a fix along those lines (or else,
> just revert the patch for now)?   Thanks!
> 

I tested the last mainline code with the command on an X86-64 box.

   make check RUNTESTFLAGS='GDB=/home/carll/bin/gdb  gdb.asm/asm-source.exp  '

Which generated an error.  I then ran the command, from Tom's email:

 gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1

from directory gdb/testsuite which generated a segmentation fault as
expected.

Once I applied the following patch, recompiled and retested, the make
check seemed to run fine and the gdb -q batch test ran without
generating a segmentation fault.

The patch appears to fix the issue.  Not sure why this didn't show up
in my original X86 testing?  I will go back and look some more to see
if I can track down why I didn't see the issue.

Sorry for the bug.  

Tom, can you take a look at the patch and try it out to make sure it
fixes the issues on your system.  Thanks.

                  Carl Love

----------------------------------------------------------------
Bug fix in commit for printing the function return value for non-trivial values

The recent commit:

  commit a0eda3df5b750ae32576a9be092b361281a41787
  Author: Carl Love <cel@us.ibm.com>
  Date:   Mon Nov 14 16:22:37 2022 -0500

    PowerPC, fix support for printing the function return value for non-trivial values.

Is generating a segmentation fault on x86_64-linux.

  segfault:
  ...
  PASS: gdb.asm/asm-source.exp: info source asmsrc1.s
  ERROR: GDB process no longer exists
  UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3
  ...

  Reproduced on command line:
  ...
  $ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1
  ...

  The problem seems to be that:
  ...
  Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
  0x000000000043de7a in symbol::type (this=0x0) at
  .../gdb_versions/devel/src/gdb/symtab.h:1287
  1287        return m_type;
  ...
  because:
  ...
  (gdb) up
  #1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
     at .../gdb_versions/devel/src/gdb/infcmd.c:1887
  1887        = check_typedef (sm->function->type ()->target_type ());
  (gdb) p sm->function
  $1 = (symbol *) 0x0

The code is not checking if sm->function is NULL.  If sm->function is NULL
the check for the return buffer should be skipped.
---
 gdb/infcmd.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index b71dc10370b..2876f6d9c9d 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1880,23 +1880,28 @@ finish_command (const char *arg, int from_tty)
 
   /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
      attempt to determine the address of the return buffer.  */
-  enum return_value_convention return_value;
-  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
+  if (sm->function != NULL)
+    {
+      enum return_value_convention return_value;
+      struct gdbarch *gdbarch = get_frame_arch (callee_frame);
 
-  struct type * val_type
-    = check_typedef (sm->function->type ()->target_type ());
+      struct type * val_type
+	= check_typedef (sm->function->type ()->target_type ());
 
-  return_value = gdbarch_return_value (gdbarch,
-				       read_var_value (sm->function, NULL,
-						       callee_frame),
-				       val_type, NULL, NULL, NULL);
+      return_value = gdbarch_return_value (gdbarch,
+					   read_var_value (sm->function, NULL,
+							   callee_frame),
+					   val_type, NULL, NULL, NULL);
 
-  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
-      && val_type->code () != TYPE_CODE_VOID)
-    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
-						  callee_frame);
+      if (return_value == RETURN_VALUE_STRUCT_CONVENTION
+	  && val_type->code () != TYPE_CODE_VOID)
+	sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
+						      callee_frame);
+      else
+	sm->return_buf = 0;
+    }
   else
-    sm->return_buf = 0;
+    sm->return_buf = 0;  /* Return buffer address is not available.  */
 
   /* Print info on the selected frame, including level number but not
      source.  */
  

Carl Love <cel@us.ibm.com> wrote:

>+      else
>+	sm->return_buf = 0;
>+    }
>   else
>-    sm->return_buf = 0;
>+    sm->return_buf = 0;  /* Return buffer address is not available.  */

Just as a minor nit, it might be cleaner to initialize
sm->return_buf to 0 just once up-front.

Patch is OK otherwise.


Thanks,
Ulrich
  

Ulrich, Simon, Tom, GDB maintainers:

> 
> Just so you can test properly, I also see these failures that are
> likely
> due to the same bug:
> 
> UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork parent follow,
> finish after tcatch vfork: finish
> UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork child follow, finish
> after tcatch vfork: finish
> UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork parent follow,
> finish after tcatch vfork: finish
> UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork child follow, finish
> after tcatch vfork: finish
> UNRESOLVED: gdb.base/sigaltstack.exp: finish to throw INNER
> UNRESOLVED: gdb.base/sigstep.exp: finish from handleri: leave signal
> trampoline

I have dug into why I didn't catch the issue earlier.  Looking
carefully at the regression test output, the foll-vfork.exp,
sigaltstack.exp, sigstep.exp and asm-source.exp tests are all failing
similarly.  In the GDB regression test output I see:

Running /home/carll/GDB/build-non-trivial/gdb/testsuite/../../../binutils-gdb-n\
on-trivial/gdb/testsuite/gdb.base/foll-vfork.exp ...
ERROR: GDB process no longer exists
ERROR: GDB process no longer exists
ERROR: GDB process no longer exists
ERROR: GDB process no longer exists

Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.asm/asm-source.exp ...
ERROR: GDB process no longer exists

Running ../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/sigaltstack.exp ...
ERROR: GDB process no longer exists
ERROR: Couldn't send finish to GDB.
ERROR: can not find channel named "exp17"

Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/sigstep.exp ...
ERROR: GDB process no longer exists

The failures then end up being recorded in the summary as "# of
unresolved testcases" not as "# of unexpected failures".  I can see
with the current mainline code base there are 8 unresolved  testcases. 
I updated the fix that I sent to Tom earlier per the comments from
Ulrich.  With that fix applied, the number of unresolved test cases
drops to 1.  If I run the regression tests with the tree backed up the
the commit just before my commits from Monday, I see one unresolved
test case.  So, clearly my commit added 7 unresolved test cases, the
six that Simon mentioned above and the one from Tom.  The original
unresolved test case is:

Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/gdb-sigterm.exp ...
FAIL: gdb.base/gdb-sigterm.exp: pass=0: expect eof (GDB internal error)
ERROR: Could not resync from internal error (recursive internal problem)

I always check that "# of unexpected failures" did not change with my
patch applied.  I didn't notice that the number of unresolved testcases
changed and hence missed the breakage my patch caused.  My bad.  I will
be sure to watch the other stats in the gdb summary a lot closer in the
future.

Thanks for pointing out the failures and the for help resolving the
issue. 

I will post my updated fix for Tom to test/review.  I think we have
this all figured out and fixed.  Sorry for breaking things.

                      Carl Love
  

Ulrich, Tom, Simon:

On Tue, 2022-11-15 at 18:05 +0000, Ulrich Weigand wrote:
> Carl Love <cel@us.ibm.com> wrote:
> 
> > +      else
> > +	sm->return_buf = 0;
> > +    }
> >   else
> > -    sm->return_buf = 0;
> > +    sm->return_buf = 0;  /* Return buffer address is not
> > available.  */
> 
> Just as a minor nit, it might be cleaner to initialize
> sm->return_buf to 0 just once up-front.
> 
> Patch is OK otherwise.
> 
> 
> Thanks,
> Ulrich

Yes, I agree, it would be best to just set sm->return_buf = 0
initially.  Then if the return buffer is available update it as needed.
That makes things a lot cleaner.

The updated patch is below.  Note, it has been tested on PowerPC and
X86_64.  It does not introduce any additional regression errors.  It
does fix the seven unresolved testcases that the initial patch
introduced and were missed by my testing of the initial patch. 

Tom, if you can verify this fix works on your system it would be
greatly appreciated.  Thanks.

                        Carl Love
---------------------------------------------------
Bug fix in commit for printing the function return value for non-trivial values

The recent commit:

  commit a0eda3df5b750ae32576a9be092b361281a41787
  Author: Carl Love <cel@us.ibm.com>
  Date:   Mon Nov 14 16:22:37 2022 -0500

    PowerPC, fix support for printing the function return value for non-trivial values.

Is generating a segmentation fault on x86_64-linux.

  segfault:
  ...
  PASS: gdb.asm/asm-source.exp: info source asmsrc1.s
  ERROR: GDB process no longer exists
  UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3
  ...

  Reproduced on command line:
  ...
  $ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1
  ...

  The problem seems to be that:
  ...
  Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
  0x000000000043de7a in symbol::type (this=0x0) at
  .../gdb_versions/devel/src/gdb/symtab.h:1287
  1287        return m_type;
  ...
  because:
  ...
  (gdb) up
  #1  0x0000000000852d94 in finish_command (arg=0x0, from_tty=0)
     at .../gdb_versions/devel/src/gdb/infcmd.c:1887
  1887        = check_typedef (sm->function->type ()->target_type ());
  (gdb) p sm->function
  $1 = (symbol *) 0x0

The code is not checking if sm->function is NULL.  If sm->function is NULL
the check for the return buffer should be skipped.
---
 gdb/infcmd.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index b71dc10370b..a72df2d6a01 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1877,26 +1877,28 @@ finish_command (const char *arg, int from_tty)
   /* Find the function we will return from.  */
   frame_info_ptr callee_frame = get_selected_frame (NULL);
   sm->function = find_pc_function (get_frame_pc (callee_frame));
+  sm->return_buf = 0;    /* Initialize buffer address is not available.  */
 
   /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
      attempt to determine the address of the return buffer.  */
-  enum return_value_convention return_value;
-  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
+  if (sm->function != NULL)
+    {
+      enum return_value_convention return_value;
+      struct gdbarch *gdbarch = get_frame_arch (callee_frame);
 
-  struct type * val_type
-    = check_typedef (sm->function->type ()->target_type ());
+      struct type * val_type
+	= check_typedef (sm->function->type ()->target_type ());
 
-  return_value = gdbarch_return_value (gdbarch,
-				       read_var_value (sm->function, NULL,
-						       callee_frame),
-				       val_type, NULL, NULL, NULL);
+      return_value = gdbarch_return_value (gdbarch,
+					   read_var_value (sm->function, NULL,
+							   callee_frame),
+					   val_type, NULL, NULL, NULL);
 
-  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
-      && val_type->code () != TYPE_CODE_VOID)
-    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
-						  callee_frame);
-  else
-    sm->return_buf = 0;
+      if (return_value == RETURN_VALUE_STRUCT_CONVENTION
+	  && val_type->code () != TYPE_CODE_VOID)
+	sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
+						      callee_frame);
+    }
 
   /* Print info on the selected frame, including level number but not
      source.  */
  

Carl Love <cel@us.ibm.com> wrote:

>Bug fix in commit for printing the function return value for non-trivial values

This is OK.

Thanks,
Ulrich
  

On 11/16/22 02:01, Carl Love wrote:
> Tom, if you can verify this fix works on your system it would be
> greatly appreciated.  Thanks.
> 

Hi Carl,

it works, all fails are gone, I got my usual result again on openSUSE 
Leap 15.4 x86_64:
...
                 === gdb Summary ===

# of unexpected core files      1
# of expected passes            117502
# of expected failures          150
# of known failures             89
# of untested testcases         26
# of unsupported tests          107
...

Thanks,
- Tom
  

Hi Carl,

Thanks for the fix.

> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index b71dc10370b..a72df2d6a01 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1877,26 +1877,28 @@ finish_command (const char *arg, int from_tty)
>    /* Find the function we will return from.  */
>    frame_info_ptr callee_frame = get_selected_frame (NULL);
>    sm->function = find_pc_function (get_frame_pc (callee_frame));
> +  sm->return_buf = 0;    /* Initialize buffer address is not available.  */
>  
>    /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
>       attempt to determine the address of the return buffer.  */
> -  enum return_value_convention return_value;
> -  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
> +  if (sm->function != NULL)

I know that Ulrich has already approved the patch, but to follow the
coding standard the NULL should be replaced by nullptr.  If this is not
too much trouble and is OK with maintainers, I think it would be nice to
do this minor tweak.

> +    {
> +      enum return_value_convention return_value;
> +      struct gdbarch *gdbarch = get_frame_arch (callee_frame);
>  
> -  struct type * val_type
> -    = check_typedef (sm->function->type ()->target_type ());
> +      struct type * val_type
> +	= check_typedef (sm->function->type ()->target_type ());
>  
> -  return_value = gdbarch_return_value (gdbarch,
> -				       read_var_value (sm->function, NULL,
> -						       callee_frame),
> -				       val_type, NULL, NULL, NULL);
> +      return_value = gdbarch_return_value (gdbarch,
> +					   read_var_value (sm->function, NULL,
> +							   callee_frame),
> +					   val_type, NULL, NULL, NULL);

While at it, those NULL could probably be updated I guess.

Best,
Lancelot.

>  
> -  if (return_value == RETURN_VALUE_STRUCT_CONVENTION
> -      && val_type->code () != TYPE_CODE_VOID)
> -    sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
> -						  callee_frame);
> -  else
> -    sm->return_buf = 0;
> +      if (return_value == RETURN_VALUE_STRUCT_CONVENTION
> +	  && val_type->code () != TYPE_CODE_VOID)
> +	sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
> +						      callee_frame);
> +    }
>  
>    /* Print info on the selected frame, including level number but not
>       source.  */
> -- 
> 2.31.1
> 
>
  

On Wed, 2022-11-16 at 10:20 +0000, Lancelot SIX wrote:
> Hi Carl,
> 
<snip>
> > s RETURN_VALUE_STRUCT_CONVENTION,
> >       attempt to determine the address of the return buffer.  */
> > -  enum return_value_convention return_value;
> > -  struct gdbarch *gdbarch = get_frame_arch (callee_frame);
> > +  if (sm->function != NULL)
> 
> I know that Ulrich has already approved the patch, but to follow the
> coding standard the NULL should be replaced by nullptr.  If this is
> not
> too much trouble and is OK with maintainers, I think it would be nice
> to
> do this minor tweak.
> 
> > +    {
> > +      enum return_value_convention return_value;
> > +      struct gdbarch *gdbarch = get_frame_arch (callee_frame);
> >  
> > -  struct type * val_type
> > -    = check_typedef (sm->function->type ()->target_type ());
> > +      struct type * val_type
> > +	= check_typedef (sm->function->type ()->target_type ());
> >  
> > -  return_value = gdbarch_return_value (gdbarch,
> > -				       read_var_value (sm->function,
> > NULL,
> > -						       callee_frame),
> > -				       val_type, NULL, NULL, NULL);
> > +      return_value = gdbarch_return_value (gdbarch,
> > +					   read_var_value (sm-
> > >function, NULL,
> > +							   callee_frame
> > ),
> > +					   val_type, NULL, NULL, NULL);
> 
> While at it, those NULL could probably be updated I guess.

I think at this point it is probably best to do a new clean up patch
particularly since the other NULLs are not part of the fix patch.

I will go ahead and commit the fix and then put together a clean up
patch to address all the NULLs.   Thanks for letting me know about it.

                   Carl Love