This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 2/5] New gdb arch hook: return_in_first_hidden_param


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-05-22  Yao Qi  <yao@codesourcery.com>
	    Chung-Lin Tang <cltang@codesourcery.com>

	* c-lang.c (cplus_language_defn): Set field 'la_return_by_reference' to
	cp_return_by_reference.
	* cp-abi.c (cp_return_by_reference): New.
	* cp-abi.h: Declare cp_return_by_reference.
	* gdbarch.sh: Add return_in_first_hidden_param.
	* gdbarch.c, gdbarch.h: Regenerated.
---
 gdb/c-lang.c   |    2 +-
 gdb/cp-abi.c   |   13 +++++++++++++
 gdb/cp-abi.h   |    4 ++++
 gdb/gdbarch.c  |   33 +++++++++++++++++++++++++++++++++
 gdb/gdbarch.h  |   13 +++++++++++++
 gdb/gdbarch.sh |    8 ++++++++
 6 files changed, 72 insertions(+), 1 deletions(-)

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index da084be..cfa4b2d 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -988,7 +988,7 @@ const struct language_defn cplus_language_defn =
   cplus_language_arch_info,
   default_print_array_index,
   cp_pass_by_reference,
-  NULL,                          /* la_return_by_reference */
+  cp_return_by_reference,
   c_get_string,
   NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
diff --git a/gdb/cp-abi.c b/gdb/cp-abi.c
index 16b5356..cf33a71 100644
--- a/gdb/cp-abi.c
+++ b/gdb/cp-abi.c
@@ -187,6 +187,19 @@ cp_pass_by_reference (struct type *type)
   return (*current_cp_abi.pass_by_reference) (type);
 }
 
+int
+cp_return_by_reference (struct type *type)
+{
+  struct gdbarch *gdbarch = get_type_arch (type);
+
+  /* If the gdbarch hook method 'return_in_first_hidden_param' is installed,
+     we honor it.  */
+  if (gdbarch_return_in_first_hidden_param_p (gdbarch))
+    return gdbarch_return_in_first_hidden_param (gdbarch);
+
+  return cp_pass_by_reference (type);
+}
+
 /* Set the current C++ ABI to SHORT_NAME.  */
 
 static int
diff --git a/gdb/cp-abi.h b/gdb/cp-abi.h
index 8451450..c1513f9 100644
--- a/gdb/cp-abi.h
+++ b/gdb/cp-abi.h
@@ -189,6 +189,10 @@ CORE_ADDR cplus_skip_trampoline (struct frame_info *frame,
    reference instead of value.  */
 extern int cp_pass_by_reference (struct type *type);
 
+/*REturn non-zero if type TYPE should be returned by reference
+  instead of value.  */
+extern int cp_return_by_reference (struct type *type);
+
 struct cp_abi_ops
 {
   const char *shortname;
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index af2033b..2928836 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_in_first_hidden_param_ftype *return_in_first_hidden_param;
   gdbarch_skip_prologue_ftype *skip_prologue;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue;
   gdbarch_inner_than_ftype *inner_than;
@@ -367,6 +368,7 @@ struct gdbarch startup_gdbarch =
   unsigned_address_to_pointer,  /* address_to_pointer */
   0,  /* integer_to_address */
   0,  /* return_value */
+  0,  /* return_in_first_hidden_param */
   0,  /* skip_prologue */
   0,  /* skip_main_prologue */
   0,  /* inner_than */
@@ -665,6 +667,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, has predicate.  */
   if (gdbarch->skip_prologue == 0)
     fprintf_unfiltered (log, "\n\tskip_prologue");
   /* Skip verify of skip_main_prologue, has predicate.  */
@@ -1234,6 +1237,12 @@ 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: gdbarch_return_in_first_hidden_param_p() = %d\n",
+                      gdbarch_return_in_first_hidden_param_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: return_in_first_hidden_param = <%s>\n",
+                      host_address_to_string (gdbarch->return_in_first_hidden_param));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_return_value_p() = %d\n",
                       gdbarch_return_value_p (gdbarch));
   fprintf_unfiltered (file,
@@ -2552,6 +2561,30 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+int
+gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->return_in_first_hidden_param != NULL;
+}
+
+int
+gdbarch_return_in_first_hidden_param (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->return_in_first_hidden_param != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_return_in_first_hidden_param called\n");
+  return gdbarch->return_in_first_hidden_param ();
+}
+
+void
+set_gdbarch_return_in_first_hidden_param (struct gdbarch *gdbarch,
+                                          gdbarch_return_in_first_hidden_param_ftype return_in_first_hidden_param)
+{
+  gdbarch->return_in_first_hidden_param = return_in_first_hidden_param;
+}
+
 CORE_ADDR
 gdbarch_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR ip)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5bc4f4d..d30746c 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -451,6 +451,19 @@ 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.
+   F:int:return_in_first_hidden_param:struct type *type:type */
+
+extern int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_return_in_first_hidden_param_ftype) (void);
+extern int gdbarch_return_in_first_hidden_param (struct gdbarch *gdbarch);
+extern void set_gdbarch_return_in_first_hidden_param (struct gdbarch *gdbarch, gdbarch_return_in_first_hidden_param_ftype *return_in_first_hidden_param);
+
 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 3e3b126..b3aebec 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -515,6 +515,14 @@ 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.
+# F:int:return_in_first_hidden_param:struct type *type:type
+F:int:return_in_first_hidden_param:void
+
 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
-- 
1.7.0.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]