Re: [PATCH 1/4] New gdb arch hook: return_with_first_hidden_param_p

[Yao Qi <yao at codesourcery dot com>]

On 05/17/2012 07:03 AM, Mark Kettenis wrote:
>> > Ok with those fixes.
> If this hasn't been committed yet, please wait.

Mark, 
Haven't heard you on the V2 of this patch series,

  [V2 PATCH 0/5] Handle first hidden parameter
  http://sourceware.org/ml/gdb-patches/2012-05/msg00885.html

Committed this version with the fixes to address Tom's comments.
Regression tested on x86_64-linux.

http://sourceware.org/ml/gdb-cvs/2012-06/msg00063.html

-- 
Yao (éå) 

gdb:

2012-06-08  Yao Qi  <yao@codesourcery.com>
	    Chung-Lin Tang <cltang@codesourcery.com>

	* arch-utils.c (default_return_in_first_hidden_param_p): New.
	* arch-utils.h: Declare.
	* gdbarch.sh: Add return_in_first_hidden_param_p.
	* gdbarch.c, gdbarch.h: Regenerated.
	* infcall.c (call_function_by_hand): Call
	gdbarch_return_in_first_hidden_param_p instead of
	language_pass_by_reference.

	* m68k-tdep.c (m68k_return_in_first_hidden_param_p): New.
	(m68k_gdbarch_init): Install m68k_return_in_first_hidden_param_p.
	* sh-tdep.c (sh_return_in_first_hidden_param_p): New.
	(sh_gdbarch_init): Install sh_return_in_first_hidden_param_p.
	* tic6x-tdep.c (tic6x_push_dummy_call): Remove local variable
	`cplus_return_struct_by_reference'.
	(tic6x_return_value): Handle language cplusplus.
	(tic6x_return_in_first_hidden_param_p): New.
	(tic6x_gdbarch_init): Install tic6x_return_in_first_hidden_param_p.
---
 gdb/arch-utils.c |   11 +++++++++
 gdb/arch-utils.h |    2 +
 gdb/gdbarch.c    |   24 ++++++++++++++++++++
 gdb/gdbarch.h    |   10 ++++++++
 gdb/gdbarch.sh   |    7 +++++
 gdb/infcall.c    |   12 +++++-----
 gdb/m68k-tdep.c  |   12 ++++++++++
 gdb/sh-tdep.c    |   13 ++++++++++
 gdb/tic6x-tdep.c |   65 ++++++++++++++++++++++++-----------------------------
 9 files changed, 114 insertions(+), 42 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e683a2d..f4414b9 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -31,6 +31,7 @@
 #include "osabi.h"
 #include "target-descriptions.h"
 #include "objfiles.h"
+#include "language.h"
 
 #include "version.h"
 
@@ -793,6 +794,16 @@ default_gen_return_address (struct gdbarch *gdbarch,
   error (_("This architecture has no method to collect a return address."));
 }
 
+int
+default_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
+					struct type *type)
+{
+  /* Usually, the return value's address is stored the in the "first hidden"
+     parameter if the return value should be passed by reference, as
+     specified in ABI.  */
+  return language_pass_by_reference (type);
+}
+
 /* */
 
 /* -Wmissing-prototypes */
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 7c398b3..ef4cb26 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -172,4 +172,6 @@ extern void default_gen_return_address (struct gdbarch *gdbarch,
 extern const char *default_auto_charset (void);
 extern const char *default_auto_wide_charset (void);
 
+extern int default_return_in_first_hidden_param_p (struct gdbarch *,
+						   struct type *);
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4d46a5d..f3d81a1 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -200,6 +200,7 @@ struct gdbarch
   gdbarch_address_to_pointer_ftype *address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address;
   gdbarch_return_value_ftype *return_value;
+  gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p;
   gdbarch_skip_prologue_ftype *skip_prologue;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue;
   gdbarch_inner_than_ftype *inner_than;
@@ -368,6 +369,7 @@ struct gdbarch startup_gdbarch =
   unsigned_address_to_pointer,  /* address_to_pointer */
   0,  /* integer_to_address */
   0,  /* return_value */
+  default_return_in_first_hidden_param_p,  /* return_in_first_hidden_param_p */
   0,  /* skip_prologue */
   0,  /* skip_main_prologue */
   0,  /* inner_than */
@@ -519,6 +521,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->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch->remote_breakpoint_from_pc = default_remote_breakpoint_from_pc;
   gdbarch->memory_insert_breakpoint = default_memory_insert_breakpoint;
   gdbarch->memory_remove_breakpoint = default_memory_remove_breakpoint;
@@ -665,6 +668,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 return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
     fprintf_unfiltered (log, "\n\tskip_prologue");
   /* Skip verify of skip_main_prologue, has predicate.  */
@@ -1243,6 +1247,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: remote_register_number = <%s>\n",
                       host_address_to_string (gdbarch->remote_register_number));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n",
+                      host_address_to_string (gdbarch->return_in_first_hidden_param_p));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_return_value_p() = %d\n",
                       gdbarch_return_value_p (gdbarch));
   fprintf_unfiltered (file,
@@ -2555,6 +2562,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+int
+gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->return_in_first_hidden_param_p != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_return_in_first_hidden_param_p called\n");
+  return gdbarch->return_in_first_hidden_param_p (gdbarch, type);
+}
+
+void
+set_gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
+                                            gdbarch_return_in_first_hidden_param_p_ftype return_in_first_hidden_param_p)
+{
+  gdbarch->return_in_first_hidden_param_p = return_in_first_hidden_param_p;
+}
+
 CORE_ADDR
 gdbarch_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR ip)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index a82e8bb..a239aa4 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -457,6 +457,16 @@ 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 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
+   implement it to a target-dependent feature.  So that we need such hook here
+   to be aware of this in GDB. */
+
+typedef int (gdbarch_return_in_first_hidden_param_p_ftype) (struct gdbarch *gdbarch, struct type *type);
+extern int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type);
+extern void set_gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p);
+
 typedef CORE_ADDR (gdbarch_skip_prologue_ftype) (struct gdbarch *gdbarch, CORE_ADDR ip);
 extern CORE_ADDR gdbarch_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR ip);
 extern void set_gdbarch_skip_prologue (struct gdbarch *gdbarch, gdbarch_skip_prologue_ftype *skip_prologue);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 2324138..06294ab 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -515,6 +515,13 @@ M:CORE_ADDR:integer_to_address:struct type *type, const gdb_byte *buf:type, buf
 # for instance).
 M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:function, valtype, regcache, readbuf, writebuf
 
+# 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
+# implement it to a target-dependent feature.  So that we need such hook here
+# to be aware of this in GDB.
+m:int:return_in_first_hidden_param_p:struct type *type:type::default_return_in_first_hidden_param_p::0
+
 m:CORE_ADDR:skip_prologue:CORE_ADDR ip:ip:0:0
 M:CORE_ADDR:skip_main_prologue:CORE_ADDR ip:ip
 f:int:inner_than:CORE_ADDR lhs, CORE_ADDR rhs:lhs, rhs:0:0
diff --git a/gdb/infcall.c b/gdb/infcall.c
index a3496d6..20a2c2b 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -464,7 +464,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
 {
   CORE_ADDR sp;
   struct type *values_type, *target_values_type;
-  unsigned char struct_return = 0, lang_struct_return = 0;
+  unsigned char struct_return = 0, hidden_first_param_p = 0;
   CORE_ADDR struct_addr = 0;
   struct infcall_control_state *inf_status;
   struct cleanup *inf_status_cleanup;
@@ -598,9 +598,9 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
      the first argument is passed in out0 but the hidden structure
      return pointer would normally be passed in r8.  */
 
-  if (language_pass_by_reference (values_type))
+  if (gdbarch_return_in_first_hidden_param_p (gdbarch, values_type))
     {
-      lang_struct_return = 1;
+      hidden_first_param_p = 1;
 
       /* Tell the target specific argument pushing routine not to
 	 expect a value.  */
@@ -680,7 +680,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
      stack, if necessary.  Make certain that the value is correctly
      aligned.  */
 
-  if (struct_return || lang_struct_return)
+  if (struct_return || hidden_first_param_p)
     {
       int len = TYPE_LENGTH (values_type);
 
@@ -706,7 +706,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
 	}
     }
 
-  if (lang_struct_return)
+  if (hidden_first_param_p)
     {
       struct value **new_args;
 
@@ -1012,7 +1012,7 @@ When the function is done executing, GDB will silently stop."),
     /* Figure out the value returned by the function.  */
     retval = allocate_value (values_type);
 
-    if (lang_struct_return)
+    if (hidden_first_param_p)
       read_value_memory (retval, 0, 1, struct_addr,
 			 value_contents_raw (retval),
 			 TYPE_LENGTH (values_type));
diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index cc1df34..b1e2cd4 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -1050,6 +1050,16 @@ m68k_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
 }
 
 
+/* This is the implementation of gdbarch method
+   return_in_first_hidden_param_p.  */
+
+static int
+m68k_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
+				     struct type *type)
+{
+  return 0;
+}
+
 /* System V Release 4 (SVR4).  */
 
 void
@@ -1235,6 +1245,8 @@ m68k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Function call & return.  */
   set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
   set_gdbarch_return_value (gdbarch, m68k_return_value);
+  set_gdbarch_return_in_first_hidden_param_p (gdbarch,
+					      m68k_return_in_first_hidden_param_p);
 
 
   /* Disassembler.  */
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index c2af736..a8e31b1 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -2209,6 +2209,17 @@ sh_regset_from_core_section (struct gdbarch *gdbarch, const char *sect_name,
 
   return NULL;
 }
+
+/* This is the implementation of gdbarch method
+   return_in_first_hidden_param_p.  */
+
+static int
+sh_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
+				     struct type *type)
+{
+  return 0;
+}
+
 
 
 static struct gdbarch *
@@ -2260,6 +2271,8 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
 
   set_gdbarch_push_dummy_call (gdbarch, sh_push_dummy_call_nofpu);
+  set_gdbarch_return_in_first_hidden_param_p (gdbarch,
+					      sh_return_in_first_hidden_param_p);
 
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
 
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index 8573222..1fefcf3 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -822,6 +822,19 @@ tic6x_return_value (struct gdbarch *gdbarch, struct value *function,
 		    struct type *type, struct regcache *regcache,
 		    gdb_byte *readbuf, const gdb_byte *writebuf)
 {
+  /* In C++, when function returns an object, even its size is small
+     enough, it stii has to be passed via reference, pointed by register
+     A3.  */
+  if (current_language->la_language == language_cplus)
+    {
+      if (type != NULL)
+	{
+	  CHECK_TYPEDEF (type);
+	  if (language_pass_by_reference (type))
+	    return RETURN_VALUE_STRUCT_CONVENTION;
+	}
+    }
+
   if (TYPE_LENGTH (type) > 8)
     return RETURN_VALUE_STRUCT_CONVENTION;
 
@@ -912,32 +925,7 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* The first arg passed on stack.  Mostly the first 10 args are passed by
      registers.  */
   int first_arg_on_stack = 10;
-  /* If this inf-call is a cpp method call, and return value is passed by
-     reference, this flag is set to 1, otherwise set to 0.  We need this flag
-     because computation of the return location in
-     infcall.c:call_function_by_hand is wrong for C6000 ELF ABI.  In
-     call_function_by_hand, the language is considered first, and then
-     target ABI is considered.  If language_pass_by_reference returns true,
-     the return location is passed as the first parameter to the function,
-     which is conflict with C6000 ELF ABI.  If this flag is true, we should
-     adjust args and return locations accordingly to comply with C6000 ELF
-     ABI.  */
-  int cplus_return_struct_by_reference = 0;
-
-  if (current_language->la_language == language_cplus)
-    {
-      struct type *values_type;
-
-      find_function_addr (function, &values_type);
-
-      if (values_type)
-	{
-	  CHECK_TYPEDEF (values_type);
-	  if (language_pass_by_reference (values_type))
-	    cplus_return_struct_by_reference = 1;
-	}
 
-    }
   /* Set the return address register to point to the entry point of
      the program, where a breakpoint lies in wait.  */
   regcache_cooked_write_unsigned (regcache, TIC6X_RA_REGNUM, bp_addr);
@@ -947,12 +935,6 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      the address in A3.  */
   if (struct_return)
     regcache_cooked_write_unsigned (regcache, 3, struct_addr);
-  else if (cplus_return_struct_by_reference)
-    /* When cplus_return_struct_by_reference is 1, means local variable
-       lang_struct_return in call_function_by_hand is 1, so struct is
-       returned by reference, even STRUCT_RETURN is 0.  Note that STRUCT_ADDR
-       is still valid in this case.  */
-    regcache_cooked_write_unsigned (regcache, 3, struct_addr);
 
   /* Determine the type of this function.  */
   func_type = check_typedef (func_type);
@@ -967,10 +949,8 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   if (TYPE_VARARGS (func_type))
     first_arg_on_stack = TYPE_NFIELDS (func_type) - 1;
 
-  /* Now make space on the stack for the args.  If
-     cplus_return_struct_by_reference is 1, means GDB pass an extra parameter
-     in ARGS, which is useless here, skip it.  */
-  for (argnum = cplus_return_struct_by_reference; argnum < nargs; argnum++)
+  /* Now make space on the stack for the args.  */
+  for (argnum = 0; argnum < nargs; argnum++)
     {
       int len = align_up (TYPE_LENGTH (value_type (args[argnum])), 4);
       if (argnum >= 10 - argreg)
@@ -986,7 +966,7 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* Now load as many as possible of the first arguments into
      registers, and push the rest onto the stack.  Loop through args
      from first to last.  */
-  for (argnum = cplus_return_struct_by_reference; argnum < nargs; argnum++)
+  for (argnum = 0; argnum < nargs; argnum++)
     {
       const gdb_byte *val;
       struct value *arg = args[argnum];
@@ -1207,6 +1187,16 @@ tic6x_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
   return 1;
 }
 
+/* This is the implementation of gdbarch method
+   return_in_first_hidden_param_p.  */
+
+static int
+tic6x_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
+				      struct type *type)
+{
+  return 0;
+}
+
 static struct gdbarch *
 tic6x_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -1365,6 +1355,9 @@ tic6x_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_in_function_epilogue_p (gdbarch, tic6x_in_function_epilogue_p);
 
+  set_gdbarch_return_in_first_hidden_param_p (gdbarch,
+					      tic6x_return_in_first_hidden_param_p);
+
   /* Hook in ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
-- 
1.7.0.4