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

[Yao Qi <yao at codesourcery dot com>]

Hi, this patch series is about fixing return values in C++ in GDB inf-call.

In C++ ABI, section 3.1.4:

"...if the return value type has a non-trivial copy constructor or
destructor, the caller allocates space for a temporary, and passes a
pointer to the temporary as an implicit first parameter preceding both
the this parameter and user parameters. The callee constructs the return
value into this temporary."

Looks GDB is correct on handling C++ inf-call of return value (by checking
language_pass_by_reference).  Unfortunately, GCC is not correct here.
currently GCC processes the return value RTL expanding like this, in
gcc/function.c:assign_parms_augmented_arg_list():

  /* If struct value address is treated as the first argument, make it
so.  */
  if (aggregate_value_p (DECL_RESULT (fndecl), fndecl)
      && ! cfun->returns_pcc_struct
      && targetm.calls.struct_value_rtx (TREE_TYPE (fndecl), 1) == 0)
    {
      tree type = build_pointer_type (TREE_TYPE (fntype));
      tree decl;
      ...

The use of the TARGET_STRUCT_VALUE_RTX hook to determine whether to
honor this convention, effectively makes what should be a
language-dependent rule, into a target-dependent convention instead.

The ia64 hook takes care to detect the C++ types to return NULL (which
is referred to in the comments as an ABI bug resolved in G++ 3.4).
Looking across other targets, sh, c6x, m68k, are examples of targets
which always return non-NULL in their TARGET_STRUCT_VALUE_RTX hooks, and
are exactly those that I also predicted and reproduced the same
symptoms.

C++ ABI is intended to be cross-architecture, but C++ ABI ends up being
target-dependent at some points.  In practice, GDB has to know which
architecture is special.

As I said above, GCC changes a language-dependent rule to a
target-dependent rule 'by mistake', so GDB has to be aware of this.  That
is the motivation to create this new gdbarch hook.  If the hook is not
installed, the default version still complies to existing GDB's behavior,
nothing is changed.

gdb:

2012-04-16  Yao Qi  <yao@codesourcery.com>
	    Chung-Lin Tang <cltang@codesourcery.com>

	* arch-utils.c (default_return_with_first_hidden_param_p): New.
	* arch-utils.h: Declare.
	* gdbarch.sh: Add return_with_first_hidden_param_p.
	* gdbarch.c, gdbarch.h: Regenerated.
	* infcall.c (call_function_by_hand): Call gdbarch_return_with_first_hidden_param_p
	instead of language_pass_by_reference.
---
 gdb/arch-utils.c |    8 ++++++++
 gdb/arch-utils.h |    2 ++
 gdb/gdbarch.c    |   24 ++++++++++++++++++++++++
 gdb/gdbarch.h    |    9 +++++++++
 gdb/gdbarch.sh   |    6 ++++++
 gdb/infcall.c    |   12 ++++++------
 6 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index fabb515..f68d211 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"
 
@@ -805,6 +806,13 @@ default_target_signal_from_host (struct gdbarch *gdbarch, int signo)
   return target_signal_from_host (signo);
 }
 
+int
+default_return_with_first_hidden_param_p (struct gdbarch *gdbarch,
+					  struct type *type)
+{
+  return language_pass_by_reference (type);
+}
+
 /* */
 
 /* -Wmissing-prototypes */
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index c2c3398..15bd14d 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -177,4 +177,6 @@ extern enum target_signal default_target_signal_from_host (struct gdbarch *,
 extern int default_target_signal_to_host (struct gdbarch *,
 					  enum target_signal);
 
+extern int default_return_with_first_hidden_param_p (struct gdbarch *,
+						     struct type *);
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index c079932..63ccecc 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -199,6 +199,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_with_first_hidden_param_p_ftype *return_with_first_hidden_param_p;
   gdbarch_skip_prologue_ftype *skip_prologue;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue;
   gdbarch_inner_than_ftype *inner_than;
@@ -357,6 +358,7 @@ struct gdbarch startup_gdbarch =
   unsigned_address_to_pointer,  /* address_to_pointer */
   0,  /* integer_to_address */
   0,  /* return_value */
+  default_return_with_first_hidden_param_p,  /* return_with_first_hidden_param_p */
   0,  /* skip_prologue */
   0,  /* skip_main_prologue */
   0,  /* inner_than */
@@ -498,6 +500,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_with_first_hidden_param_p = default_return_with_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;
@@ -645,6 +648,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_with_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.  */
@@ -1210,6 +1214,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: return_value = <%s>\n",
                       host_address_to_string (gdbarch->return_value));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: return_with_first_hidden_param_p = <%s>\n",
+                      host_address_to_string (gdbarch->return_with_first_hidden_param_p));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: sdb_reg_to_regnum = <%s>\n",
                       host_address_to_string (gdbarch->sdb_reg_to_regnum));
   fprintf_unfiltered (file,
@@ -2486,6 +2493,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+int
+gdbarch_return_with_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->return_with_first_hidden_param_p != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_return_with_first_hidden_param_p called\n");
+  return gdbarch->return_with_first_hidden_param_p (gdbarch, type);
+}
+
+void
+set_gdbarch_return_with_first_hidden_param_p (struct gdbarch *gdbarch,
+                                              gdbarch_return_with_first_hidden_param_p_ftype return_with_first_hidden_param_p)
+{
+  gdbarch->return_with_first_hidden_param_p = return_with_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 84e6ff8..663251c 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -450,6 +450,15 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc
 extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct type *functype, 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_with_first_hidden_param_p_ftype) (struct gdbarch *gdbarch, struct type *type);
+extern int gdbarch_return_with_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type);
+extern void set_gdbarch_return_with_first_hidden_param_p (struct gdbarch *gdbarch, gdbarch_return_with_first_hidden_param_p_ftype *return_with_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 5831172..ade7840 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -515,6 +515,12 @@ 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 type *functype, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, 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_with_first_hidden_param_p:struct type *type:type::default_return_with_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 6c250e3..442965b 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_with_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.  */
@@ -708,7 +708,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);
 
@@ -734,7 +734,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;
 
@@ -1040,7 +1040,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));
-- 
1.7.0.4