Re: [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call

[Pedro Alves <palves@redhat.com>]

On 10/01/2018 04:52 PM, Alan Hayward wrote:
> Make call_function_by_hand_dummy pass this down.
> 
> 2018-10-01  Alan Hayward  <alan.hayward@arm.com>
> 

> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 023e8eb453..504b040c2e 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1512,8 +1512,8 @@ pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache *regcache,
>  static CORE_ADDR
>  aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  			 struct regcache *regcache, CORE_ADDR bp_addr,
> -			 int nargs,
> -			 struct value **args, CORE_ADDR sp, int struct_return,
> +			 int nargs, struct value **args, CORE_ADDR sp,
> +			 int struct_return, int lang_struct_return_unused,

Here this is called "lang_struct_return_unused", but all the other
cases were just "lang_struct_return".

I suppose it'd be clearer if "struct_return" were renamed to
"abi_struct_return", but I empathize with not having
changed it in this patch...

>  			 CORE_ADDR struct_addr)
>  {
>    int argnum;


> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -485,7 +485,7 @@ M;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame
>  # deprecated_fp_regnum.
>  v;int;deprecated_fp_regnum;;;-1;-1;;0
>  
> -M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr
> +M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, int lang_struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, lang_struct_return, struct_addr
>  v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0
>  M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
>  

No documentation, no goddie.  What does the new parameter
mean?  

I'd think that a bool instead of an int would be better.
Or clearer still, an enum, like:

 enum class return_how { STRUCT, NORMAL };

If you want to do a preparatory patch adding the enum and
renaming "struct_return", it'd be awesome, I think.

But I suppose that given the existence of the "int struct_return"
parameter, I can't really complain.  We could always add such
an enum later on and change both parameters at the same time
throughout.  

So feel free to leave it be as is.

Thanks,
Pedro Alves