Patchwork [v3,1/3] Use enum for return method for dummy calls

login
register
mail settings
Submitter Alan Hayward
Date Oct. 11, 2018, 2:49 p.m.
Message ID <20181011144905.66908-2-alan.hayward@arm.com>
Download mbox | patch
Permalink /patch/29706/
State New
Headers show

Comments

Alan Hayward - Oct. 11, 2018, 2:49 p.m.
In call_function_by_hand_dummy, struct_return and hidden_first_param_p
are used to represent a single concept. Replace with an enum.

gdb/ChangeLog:

2018-10-11  Alan Hayward  <alan.hayward@arm.com>

	* gdbarch.sh (enum function_call_return_method): Add enum.
	* gdbarch.h: Regenerate.
	* infcall.c (call_function_by_hand_dummy): Replace vars with enum.
---
 gdb/gdbarch.h  | 17 +++++++++++++++++
 gdb/gdbarch.sh | 17 +++++++++++++++++
 gdb/infcall.c  | 29 +++++++++++------------------
 3 files changed, 45 insertions(+), 18 deletions(-)
Pedro Alves - Oct. 19, 2018, 11:28 a.m.
On 10/11/2018 03:49 PM, Alan Hayward wrote:
> In call_function_by_hand_dummy, struct_return and hidden_first_param_p
> are used to represent a single concept. Replace with an enum.
> 
> gdb/ChangeLog:
> 
> 2018-10-11  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdbarch.sh (enum function_call_return_method): Add enum.
> 	* gdbarch.h: Regenerate.
> 	* infcall.c (call_function_by_hand_dummy): Replace vars with enum.
> ---
>  gdb/gdbarch.h  | 17 +++++++++++++++++
>  gdb/gdbarch.sh | 17 +++++++++++++++++
>  gdb/infcall.c  | 29 +++++++++++------------------
>  3 files changed, 45 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index fc2f1a84a1..5f9cf481fb 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -102,6 +102,23 @@ typedef void (iterate_over_regset_sections_cb)
>    (const char *sect_name, int supply_size, int collect_size,
>     const struct regset *regset, const char *human_name, void *cb_data);
>  
> +/* For a function call, are we returning a value using a normal value return
> +   or a structure return - passing a hidden argument pointing to storage.

Pedantically, this is not a property of the call action that
we're performing, but a property of the function's call ABI.
I'd suggest tweaking the sentence like this:

/* For a function call, does the function return a value using a
   normal value return or a structure return - passing a hidden
   argument pointing to storage.

> +   There are two cases: language-mandated structure return and target ABI
> +   structure return.  

That "There are two cases" is a bit confusing, because it makes you think
about normal vs not-normal.  So I'd suggest starting with "For the latter", like:

   For the latter, there are two cases: language-mandated structure
   return and target ABI [....]

The language version is handled by passing the return
> +   location as the first parameter to the function, even preceding "this".
> +   This is different from the target ABI version, which is target-specific; for
> +   instance, on ia64 the first argument is passed in out0 but the hidden
> +   structure return pointer would normally be passed in r8.  */
> +
> +enum function_call_return_method
> +{
> +  return_method_normal = 0,	/* Standard value return.  */
> +  return_method_struct,		/* target ABI structure return.  */

Uppercase "target" -> "Target".

> +  return_method_hidden_param	/* Return hidden in first param.  */

These last two are confusing if you consider the comment in the intro
that says:

 "or a structure return - passing a hidden argument pointing to storage."

I.e., as is, makes you wonder whether "return_method_struct" is
or is not a hidden param too?

> +};
> +

Keeping the enum/enumerators names, how about this:

/* For a function call, does the function return a value using a
   normal value return or a structure return - passing a hidden
   argument pointing to storage.  For the latter, there are two
   cases: language-mandated structure return and target ABI
   structure return.  */

enum function_call_return_method
{
  /* Standard value return.  */
  return_method_normal = 0,

  /* Language ABI structure return.  This is handled
     by passing the return location as the first parameter to
     the function, even preceding "this".
  return_method_hidden_param,

  /* Target ABI struct return.  This is target-specific; for instance,
     on ia64 the first argument is passed in out0 but the hidden
     structure return pointer would normally be passed in r8.  */
  return_method_struct,
};

Thanks,
Pedro Alves

Patch

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index fc2f1a84a1..5f9cf481fb 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -102,6 +102,23 @@  typedef void (iterate_over_regset_sections_cb)
   (const char *sect_name, int supply_size, int collect_size,
    const struct regset *regset, const char *human_name, void *cb_data);
 
+/* For a function call, are we returning a value using a normal value return
+   or a structure return - passing a hidden argument pointing to storage.
+   There are two cases: language-mandated structure return and target ABI
+   structure return.  The language version is handled by passing the return
+   location as the first parameter to the function, even preceding "this".
+   This is different from the target ABI version, which is target-specific; for
+   instance, on ia64 the first argument is passed in out0 but the hidden
+   structure return pointer would normally be passed in r8.  */
+
+enum function_call_return_method
+{
+  return_method_normal = 0,	/* Standard value return.  */
+  return_method_struct,		/* target ABI structure return.  */
+  return_method_hidden_param	/* Return hidden in first param.  */
+};
+
+
 
 /* The following are pre-initialized by GDBARCH.  */
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 670ac30c03..940f10e4d3 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1327,6 +1327,23 @@  typedef int (iterate_over_objfiles_in_search_order_cb_ftype)
 typedef void (iterate_over_regset_sections_cb)
   (const char *sect_name, int supply_size, int collect_size,
    const struct regset *regset, const char *human_name, void *cb_data);
+
+/* For a function call, are we returning a value using a normal value return
+   or a structure return - passing a hidden argument pointing to storage.
+   There are two cases: language-mandated structure return and target ABI
+   structure return.  The language version is handled by passing the return
+   location as the first parameter to the function, even preceding "this".
+   This is different from the target ABI version, which is target-specific; for
+   instance, on ia64 the first argument is passed in out0 but the hidden
+   structure return pointer would normally be passed in r8.  */
+
+enum function_call_return_method
+{
+  return_method_normal = 0,	/* Standard value return.  */
+  return_method_struct,		/* target ABI structure return.  */
+  return_method_hidden_param	/* Return hidden in first param.  */
+};
+
 EOF
 
 # function typedef's
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 96d43704fa..0c875ea4b9 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -719,7 +719,7 @@  call_function_by_hand_dummy (struct value *function,
 {
   CORE_ADDR sp;
   struct type *target_values_type;
-  unsigned char struct_return = 0, hidden_first_param_p = 0;
+  function_call_return_method return_method = return_method_normal;
   CORE_ADDR struct_addr = 0;
   CORE_ADDR real_pc;
   CORE_ADDR bp_addr;
@@ -876,20 +876,11 @@  call_function_by_hand_dummy (struct value *function,
 
   values_type = check_typedef (values_type);
 
-  /* Are we returning a value using a structure return (passing a
-     hidden argument pointing to storage) or a normal value return?
-     There are two cases: language-mandated structure return and
-     target ABI structure return.  The variable STRUCT_RETURN only
-     describes the latter.  The language version is handled by passing
-     the return location as the first parameter to the function,
-     even preceding "this".  This is different from the target
-     ABI version, which is target-specific; for instance, on ia64
-     the first argument is passed in out0 but the hidden structure
-     return pointer would normally be passed in r8.  */
+  /* Are we returning a value using a structure return?  */
 
   if (gdbarch_return_in_first_hidden_param_p (gdbarch, values_type))
     {
-      hidden_first_param_p = 1;
+      return_method = return_method_hidden_param;
 
       /* Tell the target specific argument pushing routine not to
 	 expect a value.  */
@@ -897,7 +888,8 @@  call_function_by_hand_dummy (struct value *function,
     }
   else
     {
-      struct_return = using_struct_return (gdbarch, function, values_type);
+      if (using_struct_return (gdbarch, function, values_type))
+	return_method = return_method_struct;
       target_values_type = values_type;
     }
 
@@ -1020,7 +1012,7 @@  call_function_by_hand_dummy (struct value *function,
      is being evaluated is OK because the thread is stopped until the
      expression is completely evaluated.  */
 
-  if (struct_return || hidden_first_param_p
+  if (return_method != return_method_normal
       || (stack_temporaries && class_or_union_p (values_type)))
     {
       if (gdbarch_inner_than (gdbarch, 1, 2))
@@ -1046,7 +1038,7 @@  call_function_by_hand_dummy (struct value *function,
     }
 
   std::vector<struct value *> new_args;
-  if (hidden_first_param_p)
+  if (return_method == return_method_hidden_param)
     {
       /* Add the new argument to the front of the argument list.  */
       new_args.push_back
@@ -1060,8 +1052,9 @@  call_function_by_hand_dummy (struct value *function,
      presumably, the ABI code knows where, in the call dummy, the
      return address should be pointed.  */
   sp = gdbarch_push_dummy_call (gdbarch, function, get_current_regcache (),
-				bp_addr, nargs, args,
-				sp, struct_return, struct_addr);
+				bp_addr, nargs, args, sp,
+				(return_method == return_method_struct),
+				struct_addr);
 
   /* Set up a frame ID for the dummy frame so we can pass it to
      set_momentary_breakpoint.  We need to give the breakpoint a frame
@@ -1157,7 +1150,7 @@  call_function_by_hand_dummy (struct value *function,
     sm = new_call_thread_fsm (current_ui, command_interp (),
 			      gdbarch, function,
 			      values_type,
-			      struct_return || hidden_first_param_p,
+			      return_method != return_method_normal,
 			      struct_addr);
 
     e = run_inferior_call (sm, call_thread.get (), real_pc);