public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 2/3] Pass return_method to _push_dummy_call
  2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls Alan Hayward
@ 2018-10-11 14:49 ` Alan Hayward
  2018-10-19 11:31   ` Pedro Alves
  2018-10-11 14:49 ` [PATCH v3 1/3] Use enum for return method for dummy calls Alan Hayward
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-11 14:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

gdb/ChangeLog:

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

	* aarch64-tdep.c (aarch64_push_dummy_call): Replace arg with
	return_method.
	* alpha-tdep.c (alpha_push_dummy_call): Likewise.
	* amd64-tdep.c (amd64_push_arguments): Likewise.
	(amd64_push_dummy_call): Likewise.
	* amd64-windows-tdep.c (amd64_windows_push_arguments): Likewise.
	* arc-tdep.c (arc_push_dummy_call): Likewise.
	* arm-tdep.c (arm_push_dummy_call): Likewise.
	* avr-tdep.c (avr_push_dummy_call): Likewise.
	* bfin-tdep.c (bfin_push_dummy_call): Likewise.
	* cris-tdep.c (cris_push_dummy_call): Likewise.
	* csky-tdep.c (csky_push_dummy_call): Likewise.
	* frv-tdep.c (frv_push_dummy_call): Likewise.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (gdbarch_push_dummy_call): Replace arg with
	return_method.
	* h8300-tdep.c (h8300_push_dummy_call): Likewise.
	* hppa-tdep.c (hppa32_push_dummy_call): Likewise.
	(hppa64_push_dummy_call): Likewise.
	* i386-darwin-tdep.c (i386_darwin_push_dummy_call): Likewise.
	* i386-tdep.c (i386_push_dummy_call): Likewise.
	* ia64-tdep.c (ia64_push_dummy_call): Likewise.
	* infcall.c (call_function_by_hand_dummy): Likewise.
	* iq2000-tdep.c (iq2000_push_dummy_call): Likewise.
	* lm32-tdep.c (lm32_push_dummy_call): Likewise.
	* m32c-tdep.c (m32c_push_dummy_call): Likewise.
	* m32r-tdep.c (m32r_push_dummy_call): Likewise.
	* m68hc11-tdep.c (m68hc11_push_dummy_call): Likewise.
	* m68k-tdep.c (m68k_push_dummy_call): Likewise.
	* mep-tdep.c (mep_push_dummy_call): Likewise.
	* mips-tdep.c (mips_eabi_push_dummy_call): Likewise.
	(mips_n32n64_push_dummy_call): Likewise.
	(mips_o32_push_dummy_call): Likewise.
	(mips_o64_push_dummy_call): Likewise.
	* mn10300-tdep.c (mn10300_push_dummy_call): Likewise.
	* msp430-tdep.c (msp430_push_dummy_call): Likewise.
	* nds32-tdep.c (nds32_push_dummy_call): Likewise.
	* nios2-tdep.c (nios2_push_dummy_call): Likewise.
	* or1k-tdep.c (or1k_push_dummy_call): Likewise.
	* ppc-sysv-tdep.c (ppc_sysv_abi_push_dummy_call): Likewise.
	(ppc64_sysv_abi_push_dummy_call): Likewise.
	* ppc-tdep.h (ppc_sysv_abi_push_dummy_call): Likewise.
	(ppc64_sysv_abi_push_dummy_call): Likewise.
	* riscv-tdep.c (riscv_push_dummy_call): Likewise.
	* rl78-tdep.c (rl78_push_dummy_call): Likewise.
	* rs6000-aix-tdep.c (rs6000_push_dummy_call): Likewise.
	* rs6000-lynx178-tdep.c (rs6000_lynx178_push_dummy_call): Likewise.
	* rx-tdep.c (rx_push_dummy_call): Likewise.
	* s390-tdep.c (s390_push_dummy_call): Likewise.
	* score-tdep.c (score_push_dummy_call): Likewise.
	* sh-tdep.c (sh_push_dummy_call_fpu): Likewise.
	(sh_push_dummy_call_nofpu): Likewise.
	* sparc-tdep.c (sparc32_store_arguments): Likewise.
	(sparc32_push_dummy_call): Likewise.
	* sparc64-tdep.c (sparc64_store_arguments): Likewise.
	(sparc64_push_dummy_call): Likewise.
	* spu-tdep.c (spu_push_dummy_call): Likewise.
	* tic6x-tdep.c (tic6x_push_dummy_call): Likewise.
	* tilegx-tdep.c (tilegx_push_dummy_call): Likewise.
	* v850-tdep.c (v850_push_dummy_call): Likewise.
	* vax-tdep.c (vax_push_dummy_call): Likewise.
	* xstormy16-tdep.c (xstormy16_push_dummy_call): Likewise.
	* xtensa-tdep.c (xtensa_push_dummy_call): Likewise.
---
 gdb/aarch64-tdep.c        |  5 +++--
 gdb/alpha-tdep.c          |  7 ++++---
 gdb/amd64-tdep.c          | 13 +++++++------
 gdb/amd64-windows-tdep.c  | 12 ++++++------
 gdb/arc-tdep.c            |  5 +++--
 gdb/arm-tdep.c            |  5 +++--
 gdb/avr-tdep.c            |  5 +++--
 gdb/bfin-tdep.c           |  4 ++--
 gdb/cris-tdep.c           |  9 ++++-----
 gdb/csky-tdep.c           |  5 +++--
 gdb/frv-tdep.c            |  5 +++--
 gdb/gdbarch.c             |  4 ++--
 gdb/gdbarch.h             |  4 ++--
 gdb/gdbarch.sh            |  2 +-
 gdb/h8300-tdep.c          |  5 +++--
 gdb/hppa-tdep.c           | 10 ++++++----
 gdb/i386-darwin-tdep.c    |  5 +++--
 gdb/i386-tdep.c           |  5 +++--
 gdb/ia64-tdep.c           | 11 +++++------
 gdb/infcall.c             |  3 +--
 gdb/iq2000-tdep.c         |  8 +++++---
 gdb/lm32-tdep.c           |  5 +++--
 gdb/m32c-tdep.c           |  5 +++--
 gdb/m32r-tdep.c           |  5 +++--
 gdb/m68hc11-tdep.c        |  9 ++++-----
 gdb/m68k-tdep.c           |  5 +++--
 gdb/mep-tdep.c            |  4 ++--
 gdb/mips-tdep.c           | 19 +++++++++++--------
 gdb/mn10300-tdep.c        |  6 +++---
 gdb/msp430-tdep.c         |  5 +++--
 gdb/nds32-tdep.c          |  5 +++--
 gdb/nios2-tdep.c          |  5 +++--
 gdb/or1k-tdep.c           |  5 +++--
 gdb/ppc-sysv-tdep.c       | 10 ++++++----
 gdb/ppc-tdep.h            | 25 +++++++++++--------------
 gdb/riscv-tdep.c          |  8 ++++----
 gdb/rl78-tdep.c           |  5 +++--
 gdb/rs6000-aix-tdep.c     |  5 +++--
 gdb/rs6000-lynx178-tdep.c |  5 +++--
 gdb/rx-tdep.c             |  8 +++++---
 gdb/s390-tdep.c           |  7 ++++---
 gdb/score-tdep.c          |  5 +++--
 gdb/sh-tdep.c             |  9 +++++----
 gdb/sparc-tdep.c          | 15 +++++++++------
 gdb/sparc64-tdep.c        | 14 ++++++++------
 gdb/spu-tdep.c            |  5 +++--
 gdb/tic6x-tdep.c          |  5 +++--
 gdb/tilegx-tdep.c         |  4 ++--
 gdb/v850-tdep.c           |  4 ++--
 gdb/vax-tdep.c            |  5 +++--
 gdb/xstormy16-tdep.c      |  5 +++--
 gdb/xtensa-tdep.c         | 10 +++++-----
 52 files changed, 201 insertions(+), 163 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 023e8eb453..63771dc21d 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1513,7 +1513,8 @@ 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,
+			 struct value **args, CORE_ADDR sp,
+			 function_call_return_method return_method,
 			 CORE_ADDR struct_addr)
 {
   int argnum;
@@ -1577,7 +1578,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     }
 
   /* The struct_return pointer occupies X8.  */
-  if (struct_return || lang_struct_return)
+  if (return_method != return_method_normal)
     {
       if (aarch64_debug)
 	{
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index e649bd2102..91bbe06ebf 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -295,11 +295,12 @@ static CORE_ADDR
 alpha_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int i;
-  int accumulate_size = struct_return ? 8 : 0;
+  int accumulate_size = (return_method == return_method_struct) ? 8 : 0;
   struct alpha_arg
     {
       const gdb_byte *contents;
@@ -446,7 +447,7 @@ alpha_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       /* Everything else goes to the stack.  */
       write_memory (sp + offset - sizeof(arg_reg_buffer), contents, len);
     }
-  if (struct_return)
+  if (return_method == return_method_struct)
     store_unsigned_integer (arg_reg_buffer, ALPHA_REGISTER_SIZE,
 			    byte_order, struct_addr);
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 088542d72b..cc46e1e156 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -856,8 +856,8 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 \f
 
 static CORE_ADDR
-amd64_push_arguments (struct regcache *regcache, int nargs,
-		      struct value **args, CORE_ADDR sp, int struct_return)
+amd64_push_arguments (struct regcache *regcache, int nargs, struct value **args,
+		      CORE_ADDR sp, function_call_return_method return_method)
 {
   static int integer_regnum[] =
   {
@@ -885,7 +885,7 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
   int i;
 
   /* Reserve a register for the "hidden" argument.  */
-  if (struct_return)
+if (return_method == return_method_struct)
     integer_reg++;
 
   for (i = 0; i < nargs; i++)
@@ -991,7 +991,8 @@ static CORE_ADDR
 amd64_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
@@ -1004,10 +1005,10 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   i387_reset_bnd_regs (gdbarch, regcache);
 
   /* Pass arguments.  */
-  sp = amd64_push_arguments (regcache, nargs, args, sp, struct_return);
+  sp = amd64_push_arguments (regcache, nargs, args, sp, return_method);
 
   /* Pass "hidden" argument".  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       store_unsigned_integer (buf, 8, byte_order, struct_addr);
       regcache->cooked_write (AMD64_RDI_REGNUM, buf);
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 904875bacc..9eedbde17d 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -157,7 +157,7 @@ amd64_windows_store_arg_in_reg (struct regcache *regcache,
 static CORE_ADDR
 amd64_windows_push_arguments (struct regcache *regcache, int nargs,
 			      struct value **args, CORE_ADDR sp,
-			      int struct_return)
+			      function_call_return_method return_method)
 {
   int reg_idx = 0;
   int i;
@@ -180,7 +180,7 @@ amd64_windows_push_arguments (struct regcache *regcache, int nargs,
   }
 
   /* Reserve a register for the "hidden" argument.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     reg_idx++;
 
   for (i = 0; i < nargs; i++)
@@ -244,18 +244,18 @@ static CORE_ADDR
 amd64_windows_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, CORE_ADDR struct_addr)
+   int nargs, struct value **args, CORE_ADDR sp,
+   function_call_return_method return_method, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
 
   /* Pass arguments.  */
   sp = amd64_windows_push_arguments (regcache, nargs, args, sp,
-				     struct_return);
+				     return_method);
 
   /* Pass "hidden" argument".  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       /* The "hidden" argument is passed throught the first argument
          register.  */
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index b9dcbbc1e5..c3c7839520 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -592,7 +592,8 @@ arc_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
 static CORE_ADDR
 arc_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,
+		     struct value **args, CORE_ADDR sp,
+		     function_call_return_method return_method,
 		     CORE_ADDR struct_addr)
 {
   if (arc_debug)
@@ -607,7 +608,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      value return?  If so, struct_addr is the address of the reserved space for
      the return structure to be written on the stack, and that address is
      passed to that function as a hidden first argument.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       /* Pass the return address in the first argument register.  */
       regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 53eee76926..c8d96498e7 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3679,7 +3679,8 @@ arm_vfp_abi_for_function (struct gdbarch *gdbarch, struct type *func_type)
 static CORE_ADDR
 arm_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,
+		     struct value **args, CORE_ADDR sp,
+		     function_call_return_method return_method,
 		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -3714,7 +3715,7 @@ arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* The struct_return pointer occupies the first parameter
      passing register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (arm_debug)
 	fprintf_unfiltered (gdb_stdlog, "struct return in %s = %s\n",
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 9e14007fc0..b70f06e27a 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -1263,7 +1263,8 @@ static CORE_ADDR
 avr_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, CORE_ADDR struct_addr)
+		     function_call_return_method return_method,
+		     CORE_ADDR struct_addr)
 {
   int i;
   gdb_byte buf[3];
@@ -1272,7 +1273,7 @@ avr_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int regnum = AVR_ARGN_REGNUM;
   struct stack_item *si = NULL;
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned
         (regcache, regnum--, (struct_addr >> 8) & 0xff);
diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index c84625c894..b064a6cef9 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -498,7 +498,7 @@ bfin_push_dummy_call (struct gdbarch *gdbarch,
 		      int nargs,
 		      struct value **args,
 		      CORE_ADDR sp,
-		      int struct_return,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -543,7 +543,7 @@ bfin_push_dummy_call (struct gdbarch *gdbarch,
 
   /* Store struct value address.  */
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, BFIN_P0_REGNUM, struct_addr);
 
   /* Set the dummy return value to bp_addr.
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index e0371a2a28..f9fdd86a3c 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -808,7 +808,8 @@ static CORE_ADDR
 cris_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argreg;
@@ -822,10 +823,8 @@ cris_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* Are we returning a value using a structure return or a normal value
      return?  struct_addr is the address of the reserved space for the return
      structure to be written on the stack.  */
-  if (struct_return)
-    {
-      regcache_cooked_write_unsigned (regcache, STR_REGNUM, struct_addr);
-    }
+  if (return_method == return_method_struct)
+    regcache_cooked_write_unsigned (regcache, STR_REGNUM, struct_addr);
 
   /* Now load as many as possible of the first arguments into registers,
      and push the rest onto the stack.  */
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index f843732310..ed56aed97a 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -336,7 +336,8 @@ static CORE_ADDR
 csky_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   int argnum;
   int argreg = CSKY_ABI_A0_REGNUM;
@@ -351,7 +352,7 @@ csky_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* The struct_return pointer occupies the first parameter
      passing register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (csky_debug)
 	{
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index dafab75654..f5c70007b2 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -1193,7 +1193,8 @@ static CORE_ADDR
 frv_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, CORE_ADDR struct_addr)
+		     function_call_return_method return_method,
+		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argreg;
@@ -1230,7 +1231,7 @@ frv_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   argreg = 8;
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, struct_return_regnum,
                                     struct_addr);
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index e2abf263b3..bc4f786501 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2356,13 +2356,13 @@ gdbarch_push_dummy_call_p (struct gdbarch *gdbarch)
 }
 
 CORE_ADDR
-gdbarch_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, CORE_ADDR struct_addr)
+gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->push_dummy_call != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_push_dummy_call called\n");
-  return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr);
+  return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, return_method, struct_addr);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5f9cf481fb..15e5e7d003 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -411,8 +411,8 @@ extern void set_gdbarch_deprecated_fp_regnum (struct gdbarch *gdbarch, int depre
 
 extern int gdbarch_push_dummy_call_p (struct gdbarch *gdbarch);
 
-typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, 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);
-extern CORE_ADDR gdbarch_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, CORE_ADDR struct_addr);
+typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr);
+extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr);
 extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call);
 
 extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 940f10e4d3..94cf187f3c 100755
--- 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, function_call_return_method return_method, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, return_method, 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
 
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 5a4802cfe5..a922981cad 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -634,7 +634,8 @@ static CORE_ADDR
 h8300_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int stack_alloc = 0, stack_offset = 0;
@@ -657,7 +658,7 @@ h8300_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      If we're returning a structure by value, then we must pass a
      pointer to the buffer for the return value as an invisible first
      argument.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, reg++, struct_addr);
 
   for (argument = 0; argument < nargs; argument++)
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 319096e056..d22b2cf5d2 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -714,7 +714,8 @@ static CORE_ADDR
 hppa32_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, CORE_ADDR struct_addr)
+			function_call_return_method return_method,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
@@ -849,7 +850,7 @@ hppa32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* If a structure has to be returned, set up register 28 to hold its
      address.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, 28, struct_addr);
 
   gp = tdep->find_global_pointer (gdbarch, function);
@@ -969,7 +970,8 @@ static CORE_ADDR
 hppa64_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, CORE_ADDR struct_addr)
+			function_call_return_method return_method,
+			CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1113,7 +1115,7 @@ hppa64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* If a structure has to be returned, set up GR 28 (%ret0) to hold
      its address.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, HPPA_RET0_REGNUM, struct_addr);
 
   /* Set up GR27 (%dp) to hold the global pointer (gp).  */
diff --git a/gdb/i386-darwin-tdep.c b/gdb/i386-darwin-tdep.c
index 5a1807ae4d..65aed998b6 100644
--- a/gdb/i386-darwin-tdep.c
+++ b/gdb/i386-darwin-tdep.c
@@ -153,7 +153,8 @@ static CORE_ADDR
 i386_darwin_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, CORE_ADDR struct_addr)
+			     function_call_return_method return_method,
+			     CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -169,7 +170,7 @@ i386_darwin_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       int args_space = 0;
       int num_m128 = 0;
 
-      if (struct_return)
+      if (return_method == return_method_struct)
 	{
 	  if (write_pass)
 	    {
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..4fd48a73ad 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2671,7 +2671,8 @@ i386_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
 static CORE_ADDR
 i386_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,
+		      struct value **args, CORE_ADDR sp,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -2695,7 +2696,7 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     {
       int args_space_used = 0;
 
-      if (struct_return)
+      if (return_method == return_method_struct)
 	{
 	  if (write_pass)
 	    {
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index d381ecc74f..6cacb2acac 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -3673,7 +3673,8 @@ static CORE_ADDR
 ia64_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -3828,11 +3829,9 @@ ia64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     }
 
   /* Store the struct return value in r8 if necessary.  */
-  if (struct_return)
-    {
-      regcache_cooked_write_unsigned (regcache, IA64_GR8_REGNUM,
-				      (ULONGEST) struct_addr);
-    }
+  if (return_method == return_method_struct)
+    regcache_cooked_write_unsigned (regcache, IA64_GR8_REGNUM,
+				    (ULONGEST) struct_addr);
 
   global_pointer = ia64_find_global_pointer (gdbarch, func_addr);
 
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 0c875ea4b9..1b1e7daf7a 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1052,8 +1052,7 @@ 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,
-				(return_method == return_method_struct),
+				bp_addr, nargs, args, sp, return_method,
 				struct_addr);
 
   /* Set up a frame ID for the dummy frame so we can pass it to
diff --git a/gdb/iq2000-tdep.c b/gdb/iq2000-tdep.c
index 9f7f35d287..767183594f 100644
--- a/gdb/iq2000-tdep.c
+++ b/gdb/iq2000-tdep.c
@@ -645,7 +645,8 @@ static CORE_ADDR
 iq2000_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, CORE_ADDR struct_addr)
+			function_call_return_method return_method,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   const bfd_byte *val;
@@ -657,7 +658,8 @@ iq2000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   CORE_ADDR struct_ptr;
 
   /* First determine how much stack space we will need.  */
-  for (i = 0, argreg = E_1ST_ARGREG + (struct_return != 0); i < nargs; i++)
+  for (i = 0, argreg = E_1ST_ARGREG + (return_method == return_method_struct);
+       i < nargs; i++)
     {
       type = value_type (args[i]);
       typelen = TYPE_LENGTH (type);
@@ -716,7 +718,7 @@ iq2000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   stackspace = 0;
 
   argreg = E_1ST_ARGREG;
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       /* A function that returns a struct will consume one argreg to do so.
        */
diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
index 694d30ee1c..609839d340 100644
--- a/gdb/lm32-tdep.c
+++ b/gdb/lm32-tdep.c
@@ -228,7 +228,8 @@ static CORE_ADDR
 lm32_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int first_arg_reg = SIM_LM32_R1_REGNUM;
@@ -240,7 +241,7 @@ lm32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* If we're returning a large struct, a pointer to the address to
      store it at is passed as a first hidden parameter.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, first_arg_reg, struct_addr);
       first_arg_reg++;
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 6fa24452da..841801dae5 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -2016,7 +2016,8 @@ m32c_reg_arg_type (struct type *type)
 static CORE_ADDR
 m32c_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,
+		      struct value **args, CORE_ADDR sp,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2054,7 +2055,7 @@ m32c_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* First, if the function returns an aggregate by value, push a
      pointer to a buffer for it.  This doesn't affect the way
      subsequent arguments are allocated to registers.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       int ptr_len = TYPE_LENGTH (tdep->ptr_voyd);
       sp -= ptr_len;
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index fd79f3f4cd..fda35bd4af 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -659,7 +659,8 @@ m32r_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
 static CORE_ADDR
 m32r_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,
+		      struct value **args, CORE_ADDR sp,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -683,7 +684,7 @@ m32r_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* If STRUCT_RETURN is true, then the struct return address (in
      STRUCT_ADDR) will consume the first argument-passing register.
      Both adjust the register count and store that value.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, argreg, struct_addr);
       argreg++;
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 1490ee2866..e90943ce09 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -1157,7 +1157,8 @@ static CORE_ADDR
 m68hc11_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, CORE_ADDR struct_addr)
+			 function_call_return_method return_method,
+			 CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
@@ -1167,10 +1168,8 @@ m68hc11_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   gdb_byte buf[2];
   
   first_stack_argnum = 0;
-  if (struct_return)
-    {
-      regcache_cooked_write_unsigned (regcache, HARD_D_REGNUM, struct_addr);
-    }
+  if (return_method == return_method_struct)
+    regcache_cooked_write_unsigned (regcache, HARD_D_REGNUM, struct_addr);
   else if (nargs > 0)
     {
       type = value_type (args[0]);
diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index a6e9b58a7d..4134c1a6a3 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -492,7 +492,8 @@ m68k_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
 static CORE_ADDR
 m68k_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,
+		      struct value **args, CORE_ADDR sp,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -522,7 +523,7 @@ m68k_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     }
 
   /* Store struct value address.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       store_unsigned_integer (buf, 4, byte_order, struct_addr);
       regcache->cooked_write (tdep->struct_value_regnum, buf);
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index ae9c4debca..b0c73f6c92 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -2258,7 +2258,7 @@ static CORE_ADDR
 mep_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
                      struct regcache *regcache, CORE_ADDR bp_addr,
                      int argc, struct value **argv, CORE_ADDR sp,
-                     int struct_return,
+		     function_call_return_method return_method,
                      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -2287,7 +2287,7 @@ mep_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* If we're returning a structure by value, push the pointer to the
      buffer as the first argument.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
       arg_reg++;
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index bf44c52f5d..93bd04b1be 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -4495,7 +4495,8 @@ static CORE_ADDR
 mips_eabi_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, CORE_ADDR struct_addr)
+			   function_call_return_method return_method,
+			   CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -4541,7 +4542,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   float_argreg = mips_fpa0_regnum (gdbarch);
 
   /* The struct_return pointer occupies the first parameter-passing reg.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (mips_debug)
 	fprintf_unfiltered (gdb_stdlog,
@@ -4890,7 +4891,8 @@ static CORE_ADDR
 mips_n32n64_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, CORE_ADDR struct_addr)
+			     function_call_return_method return_method,
+			     CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -4933,7 +4935,7 @@ mips_n32n64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   float_argreg = mips_fpa0_regnum (gdbarch);
 
   /* The struct_return pointer occupies the first parameter-passing reg.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (mips_debug)
 	fprintf_unfiltered (gdb_stdlog,
@@ -5347,7 +5349,8 @@ static CORE_ADDR
 mips_o32_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, CORE_ADDR struct_addr)
+			  function_call_return_method return_method,
+			  CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -5398,7 +5401,7 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   float_argreg = mips_fpa0_regnum (gdbarch);
 
   /* The struct_return pointer occupies the first parameter-passing reg.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (mips_debug)
 	fprintf_unfiltered (gdb_stdlog,
@@ -5872,7 +5875,7 @@ mips_o64_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, CORE_ADDR struct_addr)
+			  function_call_return_method return_method, CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -5920,7 +5923,7 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   float_argreg = mips_fpa0_regnum (gdbarch);
 
   /* The struct_return pointer occupies the first parameter-passing reg.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (mips_debug)
 	fprintf_unfiltered (gdb_stdlog,
diff --git a/gdb/mn10300-tdep.c b/gdb/mn10300-tdep.c
index edc99a2fab..25d73c0156 100644
--- a/gdb/mn10300-tdep.c
+++ b/gdb/mn10300-tdep.c
@@ -1191,7 +1191,7 @@ mn10300_push_dummy_call (struct gdbarch *gdbarch,
 			 CORE_ADDR bp_addr, 
 			 int nargs, struct value **args,
 			 CORE_ADDR sp, 
-			 int struct_return,
+			 function_call_return_method return_method,
 			 CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1211,7 +1211,7 @@ mn10300_push_dummy_call (struct gdbarch *gdbarch,
 
      XXX This doesn't appear to handle pass-by-invisible reference
      arguments.  */
-  regs_used = struct_return ? 1 : 0;
+  regs_used = (return_method == return_method_struct) ? 1 : 0;
   for (len = 0, argnum = 0; argnum < nargs; argnum++)
     {
       arg_len = (TYPE_LENGTH (value_type (args[argnum])) + 3) & ~3;
@@ -1226,7 +1226,7 @@ mn10300_push_dummy_call (struct gdbarch *gdbarch,
   /* Allocate stack space.  */
   sp -= len;
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regs_used = 1;
       regcache_cooked_write_unsigned (regcache, E_D0_REGNUM, struct_addr);
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 427f58c0ed..e034713025 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -669,7 +669,8 @@ static CORE_ADDR
 msp430_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, CORE_ADDR struct_addr)
+			function_call_return_method return_method,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int write_pass;
@@ -699,7 +700,7 @@ msp430_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	sp = align_down (sp - sp_off, 4);
       sp_off = 0;
 
-      if (struct_return)
+      if (return_method == return_method_struct)
 	{
 	  if (write_pass)
 	    regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index b616cc9b2c..974c0ef8a8 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -1490,7 +1490,8 @@ static CORE_ADDR
 nds32_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   const int REND = 6;		/* End for register offset.  */
   int goff = 0;			/* Current gpr offset for argument.  */
@@ -1511,7 +1512,7 @@ nds32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* If STRUCT_RETURN is true, then the struct return address (in
      STRUCT_ADDR) will consume the first argument-passing register.
      Both adjust the register count and store that value.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, NDS32_R0_REGNUM, struct_addr);
       goff++;
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index c972b4bae3..d6de7e8be5 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -1811,7 +1811,8 @@ static CORE_ADDR
 nios2_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   int argreg;
   int argnum;
@@ -1833,7 +1834,7 @@ nios2_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* The struct_return pointer occupies the first parameter-passing
      register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
 
   /* Now load as many as possible of the first arguments into
diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
index c5104e3959..a9ef44baac 100644
--- a/gdb/or1k-tdep.c
+++ b/gdb/or1k-tdep.c
@@ -595,7 +595,8 @@ static CORE_ADDR
 or1k_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
 
   int argreg;
@@ -617,7 +618,7 @@ or1k_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* Location for a returned structure.  This is passed as a silent first
      argument.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, OR1K_FIRST_ARG_REGNUM,
 				      struct_addr);
diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index ede666ab62..aeef8d8223 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -62,7 +62,8 @@ CORE_ADDR
 ppc_sysv_abi_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, CORE_ADDR struct_addr)
+			      function_call_return_method return_method,
+			      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -107,7 +108,7 @@ ppc_sysv_abi_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
          (which will be passed in r3) is used for struct return
          address.  In that case we should advance one word and start
          from r4 register to copy parameters.  */
-      if (struct_return)
+      if (return_method == return_method_struct)
 	{
 	  if (write_pass)
 	    regcache_cooked_write_signed (regcache,
@@ -1540,7 +1541,8 @@ ppc64_sysv_abi_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, CORE_ADDR struct_addr)
+				function_call_return_method return_method,
+				CORE_ADDR struct_addr)
 {
   CORE_ADDR func_addr = find_function_addr (function, NULL);
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -1624,7 +1626,7 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
          containing the address of that struct..  In that case we
          should advance one word and start from r4 register to copy
          parameters.  This also consumes one on-stack parameter slot.  */
-      if (struct_return)
+      if (return_method == return_method_struct)
 	ppc64_sysv_abi_push_integer (gdbarch, struct_addr, &argpos);
 
       for (argno = 0; argno < nargs; argno++)
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index c3571cbd51..63afd07225 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -39,20 +39,17 @@ enum return_value_convention ppc_sysv_abi_broken_return_value (struct gdbarch *g
 							       struct regcache *regcache,
 							       gdb_byte *readbuf,
 							       const gdb_byte *writebuf);
-CORE_ADDR ppc_sysv_abi_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,
-					CORE_ADDR struct_addr);
-CORE_ADDR ppc64_sysv_abi_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,
-					  CORE_ADDR struct_addr);
+
+CORE_ADDR ppc_sysv_abi_push_dummy_call
+  (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache,
+   CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp,
+   function_call_return_method return_method, CORE_ADDR struct_addr);
+
+CORE_ADDR ppc64_sysv_abi_push_dummy_call
+  (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache,
+   CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp,
+   function_call_return_method return_method, CORE_ADDR struct_addr);
+
 enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch,
 							  struct value *function,
 							  struct type *valtype,
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 3402241b97..76ed4c80e0 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -2233,7 +2233,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
 		       int nargs,
 		       struct value **args,
 		       CORE_ADDR sp,
-		       int struct_return,
+		       function_call_return_method return_method,
 		       CORE_ADDR struct_addr)
 {
   int i;
@@ -2248,7 +2248,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
   CORE_ADDR osp = sp;
 
   /* We'll use register $a0 if we're returning a struct.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     ++call_info.int_regs.next_regnum;
 
   for (i = 0; i < nargs; ++i)
@@ -2278,7 +2278,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
 	       (riscv_has_fp_abi (gdbarch) ? "is" : "is not"));
       fprintf_unfiltered (gdb_stdlog, ": xlen: %d\n: flen: %d\n",
 	       call_info.xlen, call_info.flen);
-      if (struct_return)
+      if (return_method == return_method_struct)
 	fprintf_unfiltered (gdb_stdlog,
 			    "[*] struct return pointer in register $A0\n");
       for (i = 0; i < nargs; ++i)
@@ -2305,7 +2305,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
 
   /* Now load the argument into registers, or onto the stack.  */
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       gdb_byte buf[sizeof (LONGEST)];
 
diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index ace01b1171..fe8c2de8e2 100644
--- a/gdb/rl78-tdep.c
+++ b/gdb/rl78-tdep.c
@@ -1336,7 +1336,8 @@ static CORE_ADDR
 rl78_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[4];
@@ -1355,7 +1356,7 @@ rl78_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     }
 
   /* Store struct value address.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       store_unsigned_integer (buf, 2, byte_order, struct_addr);
       sp -= 2;
diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 50a146a4f0..5cd5cdf9df 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -172,7 +172,8 @@ static CORE_ADDR
 rs6000_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, CORE_ADDR struct_addr)
+			function_call_return_method return_method,
+			CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -203,7 +204,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      (which will be passed in r3) is used for struct return address.
      In that case we should advance one word and start from r4
      register to copy parameters.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_raw_write_unsigned (regcache, tdep->ppc_gp0_regnum + 3,
 				   struct_addr);
diff --git a/gdb/rs6000-lynx178-tdep.c b/gdb/rs6000-lynx178-tdep.c
index 44cd0d013d..85d80b9292 100644
--- a/gdb/rs6000-lynx178-tdep.c
+++ b/gdb/rs6000-lynx178-tdep.c
@@ -33,7 +33,8 @@ rs6000_lynx178_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, CORE_ADDR struct_addr)
+				function_call_return_method return_method,
+				CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -63,7 +64,7 @@ rs6000_lynx178_push_dummy_call (struct gdbarch *gdbarch,
      (which will be passed in r3) is used for struct return address.
      In that case we should advance one word and start from r4
      register to copy parameters.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_raw_write_unsigned (regcache, tdep->ppc_gp0_regnum + 3,
 				   struct_addr);
diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 94d57913a3..2f797b168a 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -784,7 +784,8 @@ rx_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
 static CORE_ADDR
 rx_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,
+		    struct value **args, CORE_ADDR sp,
+		    function_call_return_method return_method,
 		    CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -830,7 +831,7 @@ rx_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	sp = align_down (sp - sp_off, 4);
       sp_off = 0;
 
-      if (struct_return)
+      if (return_method == return_method_struct)
 	{
 	  struct type *return_type = TYPE_TARGET_TYPE (func_type);
 
@@ -854,7 +855,8 @@ rx_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	  struct type *arg_type = check_typedef (value_type (arg));
 	  ULONGEST arg_size = TYPE_LENGTH (arg_type);
 
-	  if (i == 0 && struct_addr != 0 && !struct_return
+	  if (i == 0 && struct_addr != 0
+	      && return_method != return_method_struct
 	      && TYPE_CODE (arg_type) == TYPE_CODE_PTR
 	      && extract_unsigned_integer (arg_bits, 4,
 					   byte_order) == struct_addr)
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 81fa0329ea..871b777473 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -1864,7 +1864,8 @@ static CORE_ADDR
 s390_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, CORE_ADDR struct_addr)
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
@@ -1878,7 +1879,7 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
 
   arg_prep.copy = sp;
-  arg_prep.gr = struct_return ? 3 : 2;
+  arg_prep.gr = (return_method == return_method_struct) ? 3 : 2;
   arg_prep.fr = 0;
   arg_prep.vr = 0;
   arg_prep.argp = 0;
@@ -1907,7 +1908,7 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     error (_("Stack overflow"));
 
   /* Pass the structure return address in general register 2.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, S390_R2_REGNUM, struct_addr);
 
   /* Initialize arg_state for "write mode".  */
diff --git a/gdb/score-tdep.c b/gdb/score-tdep.c
index b2887c5eae..77299fc96d 100644
--- a/gdb/score-tdep.c
+++ b/gdb/score-tdep.c
@@ -511,7 +511,8 @@ static CORE_ADDR
 score_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
@@ -535,7 +536,7 @@ score_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* Step 3, Check if struct return then save the struct address to
      r4 and increase the stack_offset by 4.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
       stack_offset += SCORE_REGSIZE;
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index fe64cf979a..7a6a09e997 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -1062,7 +1062,7 @@ sh_push_dummy_call_fpu (struct gdbarch *gdbarch,
 			struct regcache *regcache,
 			CORE_ADDR bp_addr, int nargs,
 			struct value **args,
-			CORE_ADDR sp, int struct_return,
+			CORE_ADDR sp, function_call_return_method return_method,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1175,7 +1175,7 @@ sh_push_dummy_call_fpu (struct gdbarch *gdbarch,
 	}
     }
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (sh_is_renesas_calling_convention (func_type))
 	/* If the function uses the Renesas ABI, subtract another 4 bytes from
@@ -1204,7 +1204,8 @@ sh_push_dummy_call_nofpu (struct gdbarch *gdbarch,
 			  struct regcache *regcache,
 			  CORE_ADDR bp_addr,
 			  int nargs, struct value **args,
-			  CORE_ADDR sp, int struct_return,
+			  CORE_ADDR sp,
+			  function_call_return_method return_method,
 			  CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1278,7 +1279,7 @@ sh_push_dummy_call_nofpu (struct gdbarch *gdbarch,
 	}
     }
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       if (sh_is_renesas_calling_convention (func_type))
 	/* If the function uses the Renesas ABI, subtract another 4 bytes from
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 7a50a8d4a9..fae037cfe8 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -612,7 +612,8 @@ sparc32_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
 static CORE_ADDR
 sparc32_store_arguments (struct regcache *regcache, int nargs,
 			 struct value **args, CORE_ADDR sp,
-			 int struct_return, CORE_ADDR struct_addr)
+			 function_call_return_method return_method,
+			 CORE_ADDR struct_addr)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -697,7 +698,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
 
   gdb_assert (element == num_elements);
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       gdb_byte buf[4];
 
@@ -712,16 +713,18 @@ static CORE_ADDR
 sparc32_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, CORE_ADDR struct_addr)
+			 function_call_return_method return_method,
+			 CORE_ADDR struct_addr)
 {
-  CORE_ADDR call_pc = (struct_return ? (bp_addr - 12) : (bp_addr - 8));
+  CORE_ADDR call_pc = (return_method == return_method_struct
+		       ? (bp_addr - 12) : (bp_addr - 8));
 
   /* Set return address.  */
   regcache_cooked_write_unsigned (regcache, SPARC_O7_REGNUM, call_pc);
 
   /* Set up function arguments.  */
-  sp = sparc32_store_arguments (regcache, nargs, args, sp,
-				struct_return, struct_addr);
+  sp = sparc32_store_arguments (regcache, nargs, args, sp, return_method,
+				struct_addr);
 
   /* Allocate the 16-word window save area.  */
   sp -= 16 * 4;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index b1ee6c1b57..26e37394fc 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -1367,7 +1367,8 @@ sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
 static CORE_ADDR
 sparc64_store_arguments (struct regcache *regcache, int nargs,
 			 struct value **args, CORE_ADDR sp,
-			 int struct_return, CORE_ADDR struct_addr)
+			 function_call_return_method return_method,
+			 CORE_ADDR struct_addr)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   /* Number of extended words in the "parameter array".  */
@@ -1381,7 +1382,7 @@ sparc64_store_arguments (struct regcache *regcache, int nargs,
   /* First we calculate the number of extended words in the "parameter
      array".  While doing so we also convert some of the arguments.  */
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     num_elements++;
 
   for (i = 0; i < nargs; i++)
@@ -1477,7 +1478,7 @@ sparc64_store_arguments (struct regcache *regcache, int nargs,
      contents of any unused memory or registers in the "parameter
      array" are undefined.  */
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, SPARC_O0_REGNUM, struct_addr);
       element++;
@@ -1621,14 +1622,15 @@ static CORE_ADDR
 sparc64_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, CORE_ADDR struct_addr)
+			 function_call_return_method return_method,
+			 CORE_ADDR struct_addr)
 {
   /* Set return address.  */
   regcache_cooked_write_unsigned (regcache, SPARC_O7_REGNUM, bp_addr - 8);
 
   /* Set up function arguments.  */
-  sp = sparc64_store_arguments (regcache, nargs, args, sp,
-				struct_return, struct_addr);
+  sp = sparc64_store_arguments (regcache, nargs, args, sp, return_method,
+				struct_addr);
 
   /* Allocate the register save area.  */
   sp -= 16 * 8;
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 6ae37f58c7..1020167757 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1401,7 +1401,8 @@ static CORE_ADDR
 spu_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, CORE_ADDR struct_addr)
+		     function_call_return_method return_method,
+		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR sp_delta;
@@ -1418,7 +1419,7 @@ spu_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* If STRUCT_RETURN is true, then the struct return address (in
      STRUCT_ADDR) will consume the first argument-passing register.
      Both adjust the register count and store that value.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       memset (buf, 0, sizeof buf);
       store_unsigned_integer (buf, 4, byte_order, SPUADDR_ADDR (struct_addr));
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index 13ad67f974..b83074aa23 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -875,7 +875,8 @@ static CORE_ADDR
 tic6x_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, CORE_ADDR struct_addr)
+		       function_call_return_method return_method,
+		       CORE_ADDR struct_addr)
 {
   int argreg = 0;
   int argnum;
@@ -894,7 +895,7 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* The caller must pass an argument in A3 containing a destination address
      for the returned value.  The callee returns the object by copying it to
      the address in A3.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, 3, struct_addr);
 
   /* Determine the type of this function.  */
diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c
index 9ed696630e..e2e844bc02 100644
--- a/gdb/tilegx-tdep.c
+++ b/gdb/tilegx-tdep.c
@@ -281,7 +281,7 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch,
 			struct regcache *regcache,
 			CORE_ADDR bp_addr, int nargs,
 			struct value **args,
-			CORE_ADDR sp, int struct_return,
+			CORE_ADDR sp, function_call_return_method return_method,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -293,7 +293,7 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch,
 
   /* If struct_return is 1, then the struct return address will
      consume one argument-passing register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
 
   /* Arguments are passed in R0 - R9, and as soon as an argument
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index 119fb6d6bd..cc582ca54d 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -1013,7 +1013,7 @@ v850_push_dummy_call (struct gdbarch *gdbarch,
 		      int nargs,
 		      struct value **args,
 		      CORE_ADDR sp,
-		      int struct_return,
+		      function_call_return_method return_method,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1039,7 +1039,7 @@ v850_push_dummy_call (struct gdbarch *gdbarch,
 
   argreg = E_ARG0_REGNUM;
   /* The struct_return pointer occupies the first parameter register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
 
   /* Now load as many as possible of the first arguments into
diff --git a/gdb/vax-tdep.c b/gdb/vax-tdep.c
index 21f2066da2..797758b21a 100644
--- a/gdb/vax-tdep.c
+++ b/gdb/vax-tdep.c
@@ -141,7 +141,8 @@ vax_store_arguments (struct regcache *regcache, int nargs,
 static CORE_ADDR
 vax_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,
+		     struct value **args, CORE_ADDR sp,
+		     function_call_return_method return_method,
 		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -152,7 +153,7 @@ vax_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   sp = vax_store_arguments (regcache, nargs, args, sp);
 
   /* Store return value address.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     regcache_cooked_write_unsigned (regcache, VAX_R1_REGNUM, struct_addr);
 
   /* Store return address in the PC slot.  */
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index b3c2ba649c..5fae2773eb 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -226,7 +226,8 @@ xstormy16_push_dummy_call (struct gdbarch *gdbarch,
 			   struct regcache *regcache,
 			   CORE_ADDR bp_addr, int nargs,
 			   struct value **args,
-			   CORE_ADDR sp, int struct_return,
+			   CORE_ADDR sp,
+			   function_call_return_method return_method,
 			   CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -238,7 +239,7 @@ xstormy16_push_dummy_call (struct gdbarch *gdbarch,
 
   /* If struct_return is true, then the struct return address will
      consume one argument-passing register.  */
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       regcache_cooked_write_unsigned (regcache, E_PTR_RET_REGNUM, struct_addr);
       argreg++;
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 3b32bcfc82..5c095383c6 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -1695,7 +1695,7 @@ xtensa_push_dummy_call (struct gdbarch *gdbarch,
 			int nargs,
 			struct value **args,
 			CORE_ADDR sp,
-			int struct_return,
+			function_call_return_method return_method,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1725,9 +1725,9 @@ xtensa_push_dummy_call (struct gdbarch *gdbarch,
   if (xtensa_debug_level > 3)
     {
       DEBUGINFO ("[xtensa_push_dummy_call] nargs = %d\n", nargs);
-      DEBUGINFO ("[xtensa_push_dummy_call] sp=0x%x, struct_return=%d, "
+      DEBUGINFO ("[xtensa_push_dummy_call] sp=0x%x, return_method=%d, "
 		 "struct_addr=0x%x\n",
-		 (int) sp, (int) struct_return, (int) struct_addr);
+		 (int) sp, (int) return_method, (int) struct_addr);
 
       for (int i = 0; i < nargs; i++)
         {
@@ -1761,7 +1761,7 @@ xtensa_push_dummy_call (struct gdbarch *gdbarch,
   size = 0;
   onstack_size = 0;
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     size = REGISTER_SIZE;
 
   for (int i = 0; i < nargs; i++)
@@ -1838,7 +1838,7 @@ xtensa_push_dummy_call (struct gdbarch *gdbarch,
 
   /* Second Loop: Load arguments.  */
 
-  if (struct_return)
+  if (return_method == return_method_struct)
     {
       store_unsigned_integer (buf, REGISTER_SIZE, byte_order, struct_addr);
       regcache->cooked_write (ARG_1ST (gdbarch), buf);
-- 
2.17.1 (Apple Git-112)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/3] Use enum for return method for dummy calls
  2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls Alan Hayward
  2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
@ 2018-10-11 14:49 ` Alan Hayward
  2018-10-19 11:28   ` Pedro Alves
  2018-10-11 14:49 ` [PATCH v3 3/3] Aarch64: Fix segfault when casting " Alan Hayward
  2018-10-18  9:50 ` [PING][PATCH v3 0/3] " Alan Hayward
  3 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-11 14:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

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.
+   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);
-- 
2.17.1 (Apple Git-112)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls Alan Hayward
  2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
  2018-10-11 14:49 ` [PATCH v3 1/3] Use enum for return method for dummy calls Alan Hayward
@ 2018-10-11 14:49 ` Alan Hayward
  2018-10-19 11:36   ` Pedro Alves
  2018-10-18  9:50 ` [PING][PATCH v3 0/3] " Alan Hayward
  3 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-11 14:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

Prevent the following causing a segfault on aarch64:
(gdb) b foo if (int)strcmp(name,"abc") == 0
(gdb) run

This is because to aarch64_push_dummy_call determines the return type
of the function and then does not check for null pointer.

A null pointer for the return type means the call has no debug
information. For the code to get here, then either there has been an
error or the call has been cast - but we do not have this information
available.

In addition, aarch64_push_dummy_call does not check if the function is
an IFUNC.

However, aarch64_push_dummy_call only requires the return value in order
to calculate lang_struct_return. This information is available in the
return_method enum. The fix is to simply use this instead.

Add common test case using a shared library to ensure FUNC resolving.

gdb/ChangeLog:

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

	PR gdb/22736:
	* aarch64-tdep.c (aarch64_push_dummy_call): Remove
        lang_struct_return code.

gdb/testsuite/ChangeLog:

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

	PR gdb/22736:
	* gdb.base/condbreak-solib-lib.cc: New test.
	* gdb.base/condbreak-solib-main.cc: New test.
	* gdb.base/condbreak-solib.exp: New file.
---
 gdb/aarch64-tdep.c                            |  30 +---
 gdb/testsuite/gdb.base/condbreak-solib-lib.cc |  22 +++
 .../gdb.base/condbreak-solib-main.cc          |  39 +++++
 gdb/testsuite/gdb.base/condbreak-solib.exp    | 136 ++++++++++++++++++
 4 files changed, 200 insertions(+), 27 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-lib.cc
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-main.cc
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib.exp

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 63771dc21d..e541e45f61 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1519,9 +1519,6 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 {
   int argnum;
   struct aarch64_call_info info;
-  struct type *func_type;
-  struct type *return_type;
-  int lang_struct_return;
 
   memset (&info, 0, sizeof (info));
 
@@ -1543,35 +1540,14 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      If the language code decides to pass in memory we want to move
      the pointer inserted as the initial argument from the argument
      list and into X8, the conventional AArch64 struct return pointer
-     register.
-
-     This is slightly awkward, ideally the flag "lang_struct_return"
-     would be passed to the targets implementation of push_dummy_call.
-     Rather that change the target interface we call the language code
-     directly ourselves.  */
-
-  func_type = check_typedef (value_type (function));
-
-  /* Dereference function pointer types.  */
-  if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
-    func_type = TYPE_TARGET_TYPE (func_type);
-
-  gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC
-	      || TYPE_CODE (func_type) == TYPE_CODE_METHOD);
-
-  /* If language_pass_by_reference () returned true we will have been
-     given an additional initial argument, a hidden pointer to the
-     return slot in memory.  */
-  return_type = TYPE_TARGET_TYPE (func_type);
-  lang_struct_return = language_pass_by_reference (return_type);
+     register.  */
 
   /* Set the return address.  For the AArch64, the return breakpoint
      is always at BP_ADDR.  */
   regcache_cooked_write_unsigned (regcache, AARCH64_LR_REGNUM, bp_addr);
 
-  /* If we were given an initial argument for the return slot because
-     lang_struct_return was true, lose it.  */
-  if (lang_struct_return)
+  /* If we were given an initial argument for the return slot, lose it.  */
+  if (return_method == return_method_hidden_param)
     {
       args++;
       nargs--;
diff --git a/gdb/testsuite/gdb.base/condbreak-solib-lib.cc b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
new file mode 100644
index 0000000000..96dfe2182e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+cmp3 (char *name)
+{
+  return name[0] == '3';
+}
diff --git a/gdb/testsuite/gdb.base/condbreak-solib-main.cc b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
new file mode 100644
index 0000000000..10f2e4d474
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int cmp3 (char *name);
+
+const char *word = "stuff";
+
+int
+foo ()
+{
+  return 1;
+}
+
+int
+bar ()
+{
+  return 1;
+}
+
+int
+main (void)
+{
+  foo ();
+  bar ();
+}
diff --git a/gdb/testsuite/gdb.base/condbreak-solib.exp b/gdb/testsuite/gdb.base/condbreak-solib.exp
new file mode 100644
index 0000000000..38169ff81a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib.exp
@@ -0,0 +1,136 @@
+# This testcase is part of GDB, the GNU debugger.
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test conditional breakpoints on functions in shared libraries.
+# See gdb/22736 
+
+if { [skip_cplus_tests] || [skip_shlib_tests] } {
+    return 0
+}
+
+global target_size
+set target_size TARGET_UNKNOWN
+if {[is_lp64_target]} {
+    set target_size TARGET_LP64
+} elseif {[is_ilp32_target]} {
+    set target_size TARGET_ILP32
+} else {
+    return 0
+}
+
+set main_basename condbreak-solib-main
+set lib_basename condbreak-solib-lib
+standard_testfile $main_basename.cc $lib_basename.cc
+
+if [get_compiler_info "c++"] {
+    return -1
+}
+
+set libsrc "${srcdir}/${subdir}/${srcfile2}"
+
+
+# Run to main. Set breakpoint at bar. Set conditional breakpoint CBREAK.
+# Continue running. Test that program stops at function STOP, prefixed
+# with the text PREFIX.
+
+proc break_and_run { cbreak stop prefix} {
+    
+    if ![runto_main] then {
+        fail "can't run to main"
+        return
+    }
+
+    with_test_prefix $cbreak {
+        gdb_test "b bar" "Breakpoint .*"
+        gdb_test "b $cbreak" "Breakpoint .*"
+        gdb_test "c" ".*${prefix}.*Breakpoint .* $stop .*"
+    }
+}
+
+# Compile binary linked against shared library containing cmp3, using TYPE
+# to build the library as either DEBUG or NODEBUG. Then test conditional
+# breakpoints that make a call into the library.
+
+proc cond_break_test { type } {
+
+    global srcdir subdir srcfile srcfile2 binfile testfile
+    global target_size main_basename lib_basename libsrc
+
+    switch -regexp -- $type {
+        "^debug" {
+            set dir "debug"
+            set debug_flags "debug"
+        }
+        "^nodebug" {
+            set dir "nodebug"
+            set debug_flags ""
+        }
+    }
+
+    remote_exec build "rm -rf [standard_output_file ${dir}]"
+    remote_exec build "mkdir [standard_output_file ${dir}]"
+
+    set lib_so [standard_output_file ${dir}/${lib_basename}.so]
+    set lib_syms [shlib_symbol_file ${dir}/${lib_so}]
+    set binfile [standard_output_file ${dir}/${testfile}]
+
+    # Compile a shared library containing cmp3.
+
+    set lib_flags "c++ ldflags=-Wl,-Bsymbolic $debug_flags"
+
+    if {[gdb_compile_shlib $libsrc $lib_so ${lib_flags}] != ""} {
+        untested "failed to compile shared library"
+        return -1
+    }
+
+    # Compile the main test linked against the shared library.
+
+    set bin_flags "debug shlib=${lib_so}"
+
+    if { [gdb_compile $srcdir/$subdir/${srcfile} ${binfile} executable ${bin_flags}] != "" } {
+        untested "failed to compile"
+        return -1
+    }
+
+    clean_restart $binfile
+
+    with_test_prefix $type {
+
+        # Conditional break with casted function call that fails the condition.
+        break_and_run "foo if (int)cmp3(\"abc\") == 1" "bar" "Continuing.\r\n\r\n"
+
+        # Conditional break with casted function call that passes the condition.
+        break_and_run "foo if (int)cmp3(\"345\") == 1" "foo" "Continuing.\r\n\r\n"
+
+        # Conditional break with function call (no cast).
+        switch -regexp -- $type {
+            "^debug" {
+            	#Same results as conditional break with casted function.
+                break_and_run "foo if cmp3(\"abc\") == 1" "bar" "Continuing.\r\n\r\n"
+	        break_and_run "foo if cmp3(\"345\") == 1" "foo" "Continuing.\r\n\r\n"
+            }
+            "^nodebug" {
+                #Call will error due to lack of debug information, causing the condition to pass.
+                break_and_run "foo if cmp3(\"abc\") == 1" "foo" "Continuing.\r\nError in testing breakpoint condition"
+                break_and_run "foo if cmp3(\"345\") == 1" "foo" "Continuing.\r\nError in testing breakpoint condition"
+            }
+        }
+    }
+}
+
+cond_break_test debug
+cond_break_test nodebug
+
-- 
2.17.1 (Apple Git-112)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls
@ 2018-10-11 14:49 Alan Hayward
  2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Alan Hayward @ 2018-10-11 14:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

Aarch64: Fix segfault when casting dummy calls

Following Pedro's review I've squashed the new int param into a new enum
and rewritten the test.

Prevent the int cast in the following causing a segfault on aarch64:
(gdb) b foo if (int)strcmp(name,"abc") == 0
(gdb) run

The fix is to remove a bunch of code from aarch64_push_dummy_call,
instead passing down the information from the caller.

Patch 1 removes two ints from call_function_by_hand_dummy, replacing
them with an enum.
Patch 2 passes that enum down to _push_dummy_call.
Patch 3 makes use of the enum in aarch64_push_dummy_call and adds a
test case.

Tested with make check on aarch64 and build with all targets on x86.
Patch 2 needs a careful scan to make sure it doesn't break any other
targets.


Alan Hayward (3):
  Use enum for return method for dummy calls
  Pass return_method to _push_dummy_call
  Aarch64: Fix segfault when casting dummy calls

 gdb/aarch64-tdep.c                            |  35 +----
 gdb/alpha-tdep.c                              |   7 +-
 gdb/amd64-tdep.c                              |  13 +-
 gdb/amd64-windows-tdep.c                      |  12 +-
 gdb/arc-tdep.c                                |   5 +-
 gdb/arm-tdep.c                                |   5 +-
 gdb/avr-tdep.c                                |   5 +-
 gdb/bfin-tdep.c                               |   4 +-
 gdb/cris-tdep.c                               |   9 +-
 gdb/csky-tdep.c                               |   5 +-
 gdb/frv-tdep.c                                |   5 +-
 gdb/gdbarch.c                                 |   4 +-
 gdb/gdbarch.h                                 |  21 ++-
 gdb/gdbarch.sh                                |  19 ++-
 gdb/h8300-tdep.c                              |   5 +-
 gdb/hppa-tdep.c                               |  10 +-
 gdb/i386-darwin-tdep.c                        |   5 +-
 gdb/i386-tdep.c                               |   5 +-
 gdb/ia64-tdep.c                               |  11 +-
 gdb/infcall.c                                 |  28 ++--
 gdb/iq2000-tdep.c                             |   8 +-
 gdb/lm32-tdep.c                               |   5 +-
 gdb/m32c-tdep.c                               |   5 +-
 gdb/m32r-tdep.c                               |   5 +-
 gdb/m68hc11-tdep.c                            |   9 +-
 gdb/m68k-tdep.c                               |   5 +-
 gdb/mep-tdep.c                                |   4 +-
 gdb/mips-tdep.c                               |  19 +--
 gdb/mn10300-tdep.c                            |   6 +-
 gdb/msp430-tdep.c                             |   5 +-
 gdb/nds32-tdep.c                              |   5 +-
 gdb/nios2-tdep.c                              |   5 +-
 gdb/or1k-tdep.c                               |   5 +-
 gdb/ppc-sysv-tdep.c                           |  10 +-
 gdb/ppc-tdep.h                                |  25 ++--
 gdb/riscv-tdep.c                              |   8 +-
 gdb/rl78-tdep.c                               |   5 +-
 gdb/rs6000-aix-tdep.c                         |   5 +-
 gdb/rs6000-lynx178-tdep.c                     |   5 +-
 gdb/rx-tdep.c                                 |   8 +-
 gdb/s390-tdep.c                               |   7 +-
 gdb/score-tdep.c                              |   5 +-
 gdb/sh-tdep.c                                 |   9 +-
 gdb/sparc-tdep.c                              |  15 +-
 gdb/sparc64-tdep.c                            |  14 +-
 gdb/spu-tdep.c                                |   5 +-
 gdb/testsuite/gdb.base/condbreak-solib-lib.cc |  22 +++
 .../gdb.base/condbreak-solib-main.cc          |  39 +++++
 gdb/testsuite/gdb.base/condbreak-solib.exp    | 136 ++++++++++++++++++
 gdb/tic6x-tdep.c                              |   5 +-
 gdb/tilegx-tdep.c                             |   4 +-
 gdb/v850-tdep.c                               |   4 +-
 gdb/vax-tdep.c                                |   5 +-
 gdb/xstormy16-tdep.c                          |   5 +-
 gdb/xtensa-tdep.c                             |  10 +-
 55 files changed, 444 insertions(+), 206 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-lib.cc
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-main.cc
 create mode 100644 gdb/testsuite/gdb.base/condbreak-solib.exp

-- 
2.17.1 (Apple Git-112)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PING][PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls Alan Hayward
                   ` (2 preceding siblings ...)
  2018-10-11 14:49 ` [PATCH v3 3/3] Aarch64: Fix segfault when casting " Alan Hayward
@ 2018-10-18  9:50 ` Alan Hayward
  3 siblings, 0 replies; 18+ messages in thread
From: Alan Hayward @ 2018-10-18  9:50 UTC (permalink / raw)
  To: GDB Patches; +Cc: nd

Ping.

> On 11 Oct 2018, at 15:49, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> Aarch64: Fix segfault when casting dummy calls
> 
> Following Pedro's review I've squashed the new int param into a new enum
> and rewritten the test.
> 
> Prevent the int cast in the following causing a segfault on aarch64:
> (gdb) b foo if (int)strcmp(name,"abc") == 0
> (gdb) run
> 
> The fix is to remove a bunch of code from aarch64_push_dummy_call,
> instead passing down the information from the caller.
> 
> Patch 1 removes two ints from call_function_by_hand_dummy, replacing
> them with an enum.
> Patch 2 passes that enum down to _push_dummy_call.
> Patch 3 makes use of the enum in aarch64_push_dummy_call and adds a
> test case.
> 
> Tested with make check on aarch64 and build with all targets on x86.
> Patch 2 needs a careful scan to make sure it doesn't break any other
> targets.
> 
> 
> Alan Hayward (3):
>  Use enum for return method for dummy calls
>  Pass return_method to _push_dummy_call
>  Aarch64: Fix segfault when casting dummy calls
> 
> gdb/aarch64-tdep.c                            |  35 +----
> gdb/alpha-tdep.c                              |   7 +-
> gdb/amd64-tdep.c                              |  13 +-
> gdb/amd64-windows-tdep.c                      |  12 +-
> gdb/arc-tdep.c                                |   5 +-
> gdb/arm-tdep.c                                |   5 +-
> gdb/avr-tdep.c                                |   5 +-
> gdb/bfin-tdep.c                               |   4 +-
> gdb/cris-tdep.c                               |   9 +-
> gdb/csky-tdep.c                               |   5 +-
> gdb/frv-tdep.c                                |   5 +-
> gdb/gdbarch.c                                 |   4 +-
> gdb/gdbarch.h                                 |  21 ++-
> gdb/gdbarch.sh                                |  19 ++-
> gdb/h8300-tdep.c                              |   5 +-
> gdb/hppa-tdep.c                               |  10 +-
> gdb/i386-darwin-tdep.c                        |   5 +-
> gdb/i386-tdep.c                               |   5 +-
> gdb/ia64-tdep.c                               |  11 +-
> gdb/infcall.c                                 |  28 ++--
> gdb/iq2000-tdep.c                             |   8 +-
> gdb/lm32-tdep.c                               |   5 +-
> gdb/m32c-tdep.c                               |   5 +-
> gdb/m32r-tdep.c                               |   5 +-
> gdb/m68hc11-tdep.c                            |   9 +-
> gdb/m68k-tdep.c                               |   5 +-
> gdb/mep-tdep.c                                |   4 +-
> gdb/mips-tdep.c                               |  19 +--
> gdb/mn10300-tdep.c                            |   6 +-
> gdb/msp430-tdep.c                             |   5 +-
> gdb/nds32-tdep.c                              |   5 +-
> gdb/nios2-tdep.c                              |   5 +-
> gdb/or1k-tdep.c                               |   5 +-
> gdb/ppc-sysv-tdep.c                           |  10 +-
> gdb/ppc-tdep.h                                |  25 ++--
> gdb/riscv-tdep.c                              |   8 +-
> gdb/rl78-tdep.c                               |   5 +-
> gdb/rs6000-aix-tdep.c                         |   5 +-
> gdb/rs6000-lynx178-tdep.c                     |   5 +-
> gdb/rx-tdep.c                                 |   8 +-
> gdb/s390-tdep.c                               |   7 +-
> gdb/score-tdep.c                              |   5 +-
> gdb/sh-tdep.c                                 |   9 +-
> gdb/sparc-tdep.c                              |  15 +-
> gdb/sparc64-tdep.c                            |  14 +-
> gdb/spu-tdep.c                                |   5 +-
> gdb/testsuite/gdb.base/condbreak-solib-lib.cc |  22 +++
> .../gdb.base/condbreak-solib-main.cc          |  39 +++++
> gdb/testsuite/gdb.base/condbreak-solib.exp    | 136 ++++++++++++++++++
> gdb/tic6x-tdep.c                              |   5 +-
> gdb/tilegx-tdep.c                             |   4 +-
> gdb/v850-tdep.c                               |   4 +-
> gdb/vax-tdep.c                                |   5 +-
> gdb/xstormy16-tdep.c                          |   5 +-
> gdb/xtensa-tdep.c                             |  10 +-
> 55 files changed, 444 insertions(+), 206 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-lib.cc
> create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-main.cc
> create mode 100644 gdb/testsuite/gdb.base/condbreak-solib.exp
> 
> -- 
> 2.17.1 (Apple Git-112)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/3] Use enum for return method for dummy calls
  2018-10-11 14:49 ` [PATCH v3 1/3] Use enum for return method for dummy calls Alan Hayward
@ 2018-10-19 11:28   ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2018-10-19 11:28 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/3] Pass return_method to _push_dummy_call
  2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
@ 2018-10-19 11:31   ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2018-10-19 11:31 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 10/11/2018 03:49 PM, Alan Hayward wrote:
>    /* First determine how much stack space we will need.  */
> -  for (i = 0, argreg = E_1ST_ARGREG + (struct_return != 0); i < nargs; i++)
> +  for (i = 0, argreg = E_1ST_ARGREG + (return_method == return_method_struct);
> +       i < nargs; i++)

Please write:

  for (i = 0, argreg = E_1ST_ARGREG + (return_method == return_method_struct);
       i < nargs; 
       i++)

I.e., once you have to break one the statements, it reads better to break
them all.

> @@ -238,7 +239,7 @@ xstormy16_push_dummy_call (struct gdbarch *gdbarch,
>  
>    /* If struct_return is true, then the struct return address will
>       consume one argument-passing register.  */

This comment needs a slight update.

> -  if (struct_return)
> +  if (return_method == return_method_struct)


> @@ -1577,7 +1578,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>      }
>  
>    /* The struct_return pointer occupies X8.  */
> -  if (struct_return || lang_struct_return)
> +  if (return_method != return_method_normal)
>      {

I think in this patch, this should still read:

  if (return_method == return_method_struct || lang_struct_return)

So that the patch has no side effects other than passing the
return_method down.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-11 14:49 ` [PATCH v3 3/3] Aarch64: Fix segfault when casting " Alan Hayward
@ 2018-10-19 11:36   ` Pedro Alves
  2018-10-23 16:08     ` Alan Hayward
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2018-10-19 11:36 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 10/11/2018 03:49 PM, Alan Hayward wrote:
> Prevent the following causing a segfault on aarch64:
> (gdb) b foo if (int)strcmp(name,"abc") == 0
> (gdb) run
> 
> This is because to aarch64_push_dummy_call determines the return type
> of the function and then does not check for null pointer.
> 
> A null pointer for the return type means the call has no debug
> information. For the code to get here, then either there has been an
> error or the call has been cast - but we do not have this information
> available.

That "to get there, then either" is confusing, because assuming
"there" is the place that causes a crash, if there was an error,
we wouldn't ever get there.

I suggest this instead:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A null pointer for the return type means the call has no debug
information.  For the code to get here, then the call must have
been cast, otherwise we'd error out sooner.  In the case of a
no-debug-info call cast, the return type is the type the user
had cast the call to, but we do not have information
available here.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> 
> In addition, aarch64_push_dummy_call does not check if the function is
> an IFUNC.
> 
> However, aarch64_push_dummy_call only requires the return value in order

s/return value/return type/


> to calculate lang_struct_return. This information is available in the
> return_method enum. The fix is to simply use this instead.
> 
> Add common test case using a shared library to ensure FUNC resolving.

What does "common" mean here?

And what does "to ensure FUNC resolving" mean too, btw?
AFAICT, the only reason to use a shared library is to
compile it with or without debug information, right?
Come to think of it, you could instead eliminate the
shared library and compile a separate .o file instead, right?
That would simplify the testcase a bit and expose it to more
targets.


> 
> gdb/ChangeLog:
> 
> 2018-10-11  Alan Hayward  <alan.hayward@arm.com>
> 
> 	PR gdb/22736:
> 	* aarch64-tdep.c (aarch64_push_dummy_call): Remove
>         lang_struct_return code.

Indenting seems off here.  Make sure to indent the
like starting with lang_struct_return with a tab.

> 
> gdb/testsuite/ChangeLog:
> 
> 2018-10-11  Alan Hayward  <alan.hayward@arm.com>
> 
> 	PR gdb/22736:
> 	* gdb.base/condbreak-solib-lib.cc: New test.
> 	* gdb.base/condbreak-solib-main.cc: New test.
> 	* gdb.base/condbreak-solib.exp: New file.
> ---
>  gdb/aarch64-tdep.c                            |  30 +---
>  gdb/testsuite/gdb.base/condbreak-solib-lib.cc |  22 +++
>  .../gdb.base/condbreak-solib-main.cc          |  39 +++++
>  gdb/testsuite/gdb.base/condbreak-solib.exp    | 136 ++++++++++++++++++
>  4 files changed, 200 insertions(+), 27 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-lib.cc
>  create mode 100644 gdb/testsuite/gdb.base/condbreak-solib-main.cc
>  create mode 100644 gdb/testsuite/gdb.base/condbreak-solib.exp
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 63771dc21d..e541e45f61 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1519,9 +1519,6 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  {
>    int argnum;
>    struct aarch64_call_info info;
> -  struct type *func_type;
> -  struct type *return_type;
> -  int lang_struct_return;
>  
>    memset (&info, 0, sizeof (info));
>  
> @@ -1543,35 +1540,14 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>       If the language code decides to pass in memory we want to move
>       the pointer inserted as the initial argument from the argument
>       list and into X8, the conventional AArch64 struct return pointer
> -     register.
> -
> -     This is slightly awkward, ideally the flag "lang_struct_return"
> -     would be passed to the targets implementation of push_dummy_call.
> -     Rather that change the target interface we call the language code
> -     directly ourselves.  */
> -
> -  func_type = check_typedef (value_type (function));
> -
> -  /* Dereference function pointer types.  */
> -  if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
> -    func_type = TYPE_TARGET_TYPE (func_type);
> -
> -  gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC
> -	      || TYPE_CODE (func_type) == TYPE_CODE_METHOD);
> -
> -  /* If language_pass_by_reference () returned true we will have been
> -     given an additional initial argument, a hidden pointer to the
> -     return slot in memory.  */
> -  return_type = TYPE_TARGET_TYPE (func_type);
> -  lang_struct_return = language_pass_by_reference (return_type);
> +     register.  */
>  
>    /* Set the return address.  For the AArch64, the return breakpoint
>       is always at BP_ADDR.  */
>    regcache_cooked_write_unsigned (regcache, AARCH64_LR_REGNUM, bp_addr);
>  
> -  /* If we were given an initial argument for the return slot because
> -     lang_struct_return was true, lose it.  */
> -  if (lang_struct_return)
> +  /* If we were given an initial argument for the return slot, lose it.  */
> +  if (return_method == return_method_hidden_param)
>      {
>        args++;
>        nargs--;
> diff --git a/gdb/testsuite/gdb.base/condbreak-solib-lib.cc b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
> new file mode 100644
> index 0000000000..96dfe2182e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2018 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +cmp3 (char *name)

As mentioned in the previous review, use "const char *" here?

> +{
> +  return name[0] == '3';
> +}
> diff --git a/gdb/testsuite/gdb.base/condbreak-solib-main.cc b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
> new file mode 100644
> index 0000000000..10f2e4d474
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
> @@ -0,0 +1,39 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2018 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +extern int cmp3 (char *name);

And here.


> +
> +const char *word = "stuff";
> +
> +int
> +foo ()

And please do write the void, like:

  foo (void)

(void) and () are not the same in C.

> +{
> +  return 1;
> +}
> +
> +int
> +bar ()

Ditto.

> +{
> +  return 1;
> +}
> +
> +int
> +main (void)
> +{
> +  foo ();
> +  bar ();
> +}
> diff --git a/gdb/testsuite/gdb.base/condbreak-solib.exp b/gdb/testsuite/gdb.base/condbreak-solib.exp
> new file mode 100644
> index 0000000000..38169ff81a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib.exp
> @@ -0,0 +1,136 @@
> +# This testcase is part of GDB, the GNU debugger.
> +# Copyright 2018 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test conditional breakpoints on functions in shared libraries.

Please expand this to mention no debug info.

> +# See gdb/22736 
> +
> +if { [skip_cplus_tests] || [skip_shlib_tests] } {
> +    return 0
> +}
> +
> +global target_size
> +set target_size TARGET_UNKNOWN
> +if {[is_lp64_target]} {
> +    set target_size TARGET_LP64
> +} elseif {[is_ilp32_target]} {
> +    set target_size TARGET_ILP32
> +} else {
> +    return 0
> +}

Is this above actually used?  I mean, the target_size variable?

> +
> +set main_basename condbreak-solib-main
> +set lib_basename condbreak-solib-lib
> +standard_testfile $main_basename.cc $lib_basename.cc
> +
> +if [get_compiler_info "c++"] {
> +    return -1
> +}
> +
> +set libsrc "${srcdir}/${subdir}/${srcfile2}"
> +
> +
> +# Run to main. Set breakpoint at bar. Set conditional breakpoint CBREAK.
> +# Continue running. Test that program stops at function STOP, prefixed
> +# with the text PREFIX.

Double space after periods, please.  Same for other comments below.

Missing "the": "Test that the program".

> +
> +proc break_and_run { cbreak stop prefix} {

Space after "prefix" to match the space before "cbreak".
Or the other way around, as long as consistent.

> +    
> +    if ![runto_main] then {
> +        fail "can't run to main"
> +        return
> +    }

This should be within the with_test_prefix block too.

> +
> +    with_test_prefix $cbreak {
> +        gdb_test "b bar" "Breakpoint .*"
> +        gdb_test "b $cbreak" "Breakpoint .*"
> +        gdb_test "c" ".*${prefix}.*Breakpoint .* $stop .*"
> +    }
> +}
> +
> +# Compile binary linked against shared library containing cmp3, using TYPE
> +# to build the library as either DEBUG or NODEBUG. Then test conditional
> +# breakpoints that make a call into the library.
> +
> +proc cond_break_test { type } {
> +
> +    global srcdir subdir srcfile srcfile2 binfile testfile
> +    global target_size main_basename lib_basename libsrc
> +
> +    switch -regexp -- $type {
> +        "^debug" {
> +            set dir "debug"
> +            set debug_flags "debug"
> +        }
> +        "^nodebug" {
> +            set dir "nodebug"
> +            set debug_flags ""
> +        }
> +    }
> +
> +    remote_exec build "rm -rf [standard_output_file ${dir}]"
> +    remote_exec build "mkdir [standard_output_file ${dir}]"
> +
> +    set lib_so [standard_output_file ${dir}/${lib_basename}.so]
> +    set lib_syms [shlib_symbol_file ${dir}/${lib_so}]
> +    set binfile [standard_output_file ${dir}/${testfile}]
> +
> +    # Compile a shared library containing cmp3.
> +
> +    set lib_flags "c++ ldflags=-Wl,-Bsymbolic $debug_flags"
> +
> +    if {[gdb_compile_shlib $libsrc $lib_so ${lib_flags}] != ""} {
> +        untested "failed to compile shared library"
> +        return -1
> +    }
> +
> +    # Compile the main test linked against the shared library.
> +
> +    set bin_flags "debug shlib=${lib_so}"
> +
> +    if { [gdb_compile $srcdir/$subdir/${srcfile} ${binfile} executable ${bin_flags}] != "" } {
> +        untested "failed to compile"
> +        return -1
> +    }
> +
> +    clean_restart $binfile

The untested calls, and the clean_restart should be under the
with_test_prefix too.  But see further below.

> +
> +    with_test_prefix $type {
> +
> +        # Conditional break with casted function call that fails the condition.
> +        break_and_run "foo if (int)cmp3(\"abc\") == 1" "bar" "Continuing.\r\n\r\n"
> +
> +        # Conditional break with casted function call that passes the condition.
> +        break_and_run "foo if (int)cmp3(\"345\") == 1" "foo" "Continuing.\r\n\r\n"
> +
> +        # Conditional break with function call (no cast).
> +        switch -regexp -- $type {
> +            "^debug" {
> +            	#Same results as conditional break with casted function.
> +                break_and_run "foo if cmp3(\"abc\") == 1" "bar" "Continuing.\r\n\r\n"
> +	        break_and_run "foo if cmp3(\"345\") == 1" "foo" "Continuing.\r\n\r\n"

Odd indentation above.

> +            }
> +            "^nodebug" {
> +                #Call will error due to lack of debug information, causing the condition to pass.
> +                break_and_run "foo if cmp3(\"abc\") == 1" "foo" "Continuing.\r\nError in testing breakpoint condition"
> +                break_and_run "foo if cmp3(\"345\") == 1" "foo" "Continuing.\r\nError in testing breakpoint condition"
> +            }
> +        }
> +    }
> +}
> +
> +cond_break_test debug
> +cond_break_test nodebug

Instead of with_test_prefix, you can write instead here:

foreach_with_prefix type {debug nodebug} {
   cond_break_test $type
}

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-19 11:36   ` Pedro Alves
@ 2018-10-23 16:08     ` Alan Hayward
  2018-10-24 15:15       ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-23 16:08 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 19 Oct 2018, at 12:35, Pedro Alves <palves@redhat.com> wrote:
> 

<snip>

>> to calculate lang_struct_return. This information is available in the
>> return_method enum. The fix is to simply use this instead.
>> 
>> Add common test case using a shared library to ensure FUNC resolving.
> 
> What does "common" mean here?

By common I just meant not-aarch64-specific. I guess what I really meant
was any target that supported shared libraries.

> 
> And what does "to ensure FUNC resolving" mean too, btw?
> AFAICT, the only reason to use a shared library is to
> compile it with or without debug information, right?
> Come to think of it, you could instead eliminate the
> shared library and compile a separate .o file instead, right?
> That would simplify the testcase a bit and expose it to more
> targets.
> 

I could only get the bug to expose itself when using a .so

If I do the following using current HEAD then the bug is not present:

g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
g++ condbreak-solib-main.o condbreak-solib-lib.o

It causes the type of the return value to be detected as
TYPE_CODE_PTR->TYPE_CODE_INT.

Or was there another method you were thinking?



"Ok" for all the other comments (including 1/3 and 2/3).

As a side note, many of the GNU style issues I kept hitting in the tests
are due to me relying too much on check_GNU_style.sh (from gcc) which
ignores any files in the testsuite directory. Tools are great, until
they aren’t :)


Alan.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-23 16:08     ` Alan Hayward
@ 2018-10-24 15:15       ` Pedro Alves
  2018-10-29 11:58         ` Alan Hayward
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2018-10-24 15:15 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/23/2018 05:08 PM, Alan Hayward wrote:
> 
> 
>> On 19 Oct 2018, at 12:35, Pedro Alves <palves@redhat.com> wrote:
>>
> 
> <snip>
> 
>>> to calculate lang_struct_return. This information is available in the
>>> return_method enum. The fix is to simply use this instead.
>>>
>>> Add common test case using a shared library to ensure FUNC resolving.
>>
>> What does "common" mean here?
> 
> By common I just meant not-aarch64-specific. I guess what I really meant
> was any target that supported shared libraries.

I see.  (Suggest clarifying it in the commit log then.)

> 
>>
>> And what does "to ensure FUNC resolving" mean too, btw?
>> AFAICT, the only reason to use a shared library is to
>> compile it with or without debug information, right?
>> Come to think of it, you could instead eliminate the
>> shared library and compile a separate .o file instead, right?
>> That would simplify the testcase a bit and expose it to more
>> targets.
>>
> 
> I could only get the bug to expose itself when using a .so
> 
> If I do the following using current HEAD then the bug is not present:
> 
> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
> g++ condbreak-solib-main.o condbreak-solib-lib.o
> 
> It causes the type of the return value to be detected as
> TYPE_CODE_PTR->TYPE_CODE_INT.

Huh.  That's really strange.  Where is that pointer coming from?

What does "ptype cmp3" say for you?

(gdb) b foo if (int)cmp3("abc") == 1
Breakpoint 1 at 0x40048b
(gdb) ptype cmp3
type = <unknown return type> ()
(gdb) whatis cmp3
type = <text variable, no debug info>


I wonder if what you're looking at is actually the malloc call?
GDB will call malloc to allocate space for the "abc" string in
the inferior.  Look at the backtrace, see where the call is coming
from.

Actually, because of that, I think it would be better (more focused)
to pass in a variable instead of "abc".  You already have the
unused variable "word" there:

const char *word = "stuff";

So:

 (gdb) b foo if (int)cmp3(word) == 1

but please rename it to something else, because I tried that
locally and there's another symbol called "word"
in glibc's localeinfo.h, and gdb picks that one up.

Also, is there a reason for the test to be a C++ program?
If there's none, it'd be better to make it a C program, again
to expose it to more targets.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-24 15:15       ` Pedro Alves
@ 2018-10-29 11:58         ` Alan Hayward
  2018-10-29 12:38           ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-29 11:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 24 Oct 2018, at 16:14, Pedro Alves <palves@redhat.com> wrote:
> 
>> 
>>> 
>>> And what does "to ensure FUNC resolving" mean too, btw?
>>> AFAICT, the only reason to use a shared library is to
>>> compile it with or without debug information, right?
>>> Come to think of it, you could instead eliminate the
>>> shared library and compile a separate .o file instead, right?
>>> That would simplify the testcase a bit and expose it to more
>>> targets.
>>> 
>> 
>> I could only get the bug to expose itself when using a .so
>> 
>> If I do the following using current HEAD then the bug is not present:
>> 
>> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
>> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
>> g++ condbreak-solib-main.o condbreak-solib-lib.o
>> 
>> It causes the type of the return value to be detected as
>> TYPE_CODE_PTR->TYPE_CODE_INT.
> 
> Huh.  That's really strange.  Where is that pointer coming from?
> 
> What does "ptype cmp3" say for you?
> 
> (gdb) b foo if (int)cmp3("abc") == 1
> Breakpoint 1 at 0x40048b
> (gdb) ptype cmp3
> type = <unknown return type> ()
> (gdb) whatis cmp3
> type = <text variable, no debug info>
> 

Yes, I get the same.

Sounds to me like there might still be an issue in gdb? Or at least
a difference in the way the type is calculated.
This on it’s own is still a good fix, so I’ll send a new version.


> I wonder if what you're looking at is actually the malloc call?
> GDB will call malloc to allocate space for the "abc" string in
> the inferior.  Look at the backtrace, see where the call is coming
> from.


With the nodebug and debug shared library version:
(I need to run to main before I can set breakpoint on cmp3, otherwise
"Function "cmp3" not defined.”)

But with all versions, when stopped at cmp3, I get:
(gdb) bt
#0  0x00000000004005d4 in cmp3(char const*) ()
#1  <function called from gdb>
#2  0x00000000004005a4 in foo() ()
#3  0x00000000004005bc in main ()

> 
> Actually, because of that, I think it would be better (more focused)
> to pass in a variable instead of "abc".  You already have the
> unused variable "word" there:
> 
> const char *word = "stuff";
> 
> So:
> 
> (gdb) b foo if (int)cmp3(word) == 1
> 
> but please rename it to something else, because I tried that
> locally and there's another symbol called "word"
> in glibc's localeinfo.h, and gdb picks that one up.

Will do.
Using this still causes the bug in HEAD, so that’s ok.


> 
> Also, is there a reason for the test to be a C++ program?
> If there's none, it'd be better to make it a C program, again
> to expose it to more targets.


Switching to C causes the bug to vanish.
This is because C++ uses gnuv3_pass_by_reference(), and the
segfault happens inside there, whereas C uses
default_pass_by_reference(), which just returns 0.

I’ll expand the test to cover both C and C++.


Alan.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-29 11:58         ` Alan Hayward
@ 2018-10-29 12:38           ` Pedro Alves
  2018-10-29 14:56             ` Alan Hayward
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2018-10-29 12:38 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/29/2018 11:58 AM, Alan Hayward wrote:
> 
> 
>> On 24 Oct 2018, at 16:14, Pedro Alves <palves@redhat.com> wrote:
>>
>>>
>>>>
>>>> And what does "to ensure FUNC resolving" mean too, btw?
>>>> AFAICT, the only reason to use a shared library is to
>>>> compile it with or without debug information, right?
>>>> Come to think of it, you could instead eliminate the
>>>> shared library and compile a separate .o file instead, right?
>>>> That would simplify the testcase a bit and expose it to more
>>>> targets.
>>>>
>>>
>>> I could only get the bug to expose itself when using a .so
>>>
>>> If I do the following using current HEAD then the bug is not present:
>>>
>>> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
>>> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
>>> g++ condbreak-solib-main.o condbreak-solib-lib.o
>>>
>>> It causes the type of the return value to be detected as
>>> TYPE_CODE_PTR->TYPE_CODE_INT.
>>
>> Huh.  That's really strange.  Where is that pointer coming from?
>>
>> What does "ptype cmp3" say for you?
>>
>> (gdb) b foo if (int)cmp3("abc") == 1
>> Breakpoint 1 at 0x40048b
>> (gdb) ptype cmp3
>> type = <unknown return type> ()
>> (gdb) whatis cmp3
>> type = <text variable, no debug info>
>>
> 
> Yes, I get the same.
> 
> Sounds to me like there might still be an issue in gdb? Or at least
> a difference in the way the type is calculated.
> This on it’s own is still a good fix, so I’ll send a new version.

I think we should learn the answer to the "where is that pointer
coming from" question.  I'm really not understanding why the
shared library makes a difference.

> 
> 
>> I wonder if what you're looking at is actually the malloc call?
>> GDB will call malloc to allocate space for the "abc" string in
>> the inferior.  Look at the backtrace, see where the call is coming
>> from.
> 
> 
> With the nodebug and debug shared library version:
> (I need to run to main before I can set breakpoint on cmp3, otherwise
> "Function "cmp3" not defined.”)
> 
> But with all versions, when stopped at cmp3, I get:
> (gdb) bt
> #0  0x00000000004005d4 in cmp3(char const*) ()
> #1  <function called from gdb>
> #2  0x00000000004005a4 in foo() ()
> #3  0x00000000004005bc in main ()


That's a backtrace in the inferior.  I meant instead for you to set
a breakpoint on gdb's aarch64_push_dummy_call, figure out where the
TYPE_CODE_PTR->TYPE_CODE_INT pointer comes from.  
With "b foo if (int)cmp3("abc") == 1", gdb will do two
infcalls, one malloc call to allocate space for "abc"
string, and then the call to cmp3.

Thanks,
Pedro Alves

> 
>>
>> Actually, because of that, I think it would be better (more focused)
>> to pass in a variable instead of "abc".  You already have the
>> unused variable "word" there:
>>
>> const char *word = "stuff";
>>
>> So:
>>
>> (gdb) b foo if (int)cmp3(word) == 1
>>
>> but please rename it to something else, because I tried that
>> locally and there's another symbol called "word"
>> in glibc's localeinfo.h, and gdb picks that one up.
> 
> Will do.
> Using this still causes the bug in HEAD, so that’s ok.
> 
> 
>>
>> Also, is there a reason for the test to be a C++ program?
>> If there's none, it'd be better to make it a C program, again
>> to expose it to more targets.
> 
> 
> Switching to C causes the bug to vanish.
> This is because C++ uses gnuv3_pass_by_reference(), and the
> segfault happens inside there, whereas C uses
> default_pass_by_reference(), which just returns 0.
> 
> I’ll expand the test to cover both C and C++.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-29 12:38           ` Pedro Alves
@ 2018-10-29 14:56             ` Alan Hayward
  2018-10-29 18:13               ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-29 14:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 29 Oct 2018, at 12:38, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/29/2018 11:58 AM, Alan Hayward wrote:
>> 
>> 
>>> On 24 Oct 2018, at 16:14, Pedro Alves <palves@redhat.com> wrote:
>>> 
>>>> 
>>>>> 
>>>>> And what does "to ensure FUNC resolving" mean too, btw?
>>>>> AFAICT, the only reason to use a shared library is to
>>>>> compile it with or without debug information, right?
>>>>> Come to think of it, you could instead eliminate the
>>>>> shared library and compile a separate .o file instead, right?
>>>>> That would simplify the testcase a bit and expose it to more
>>>>> targets.
>>>>> 
>>>> 
>>>> I could only get the bug to expose itself when using a .so
>>>> 
>>>> If I do the following using current HEAD then the bug is not present:
>>>> 
>>>> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
>>>> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
>>>> g++ condbreak-solib-main.o condbreak-solib-lib.o
>>>> 
>>>> It causes the type of the return value to be detected as
>>>> TYPE_CODE_PTR->TYPE_CODE_INT.
>>> 
>>> Huh.  That's really strange.  Where is that pointer coming from?
>>> 
>>> What does "ptype cmp3" say for you?
>>> 
>>> (gdb) b foo if (int)cmp3("abc") == 1
>>> Breakpoint 1 at 0x40048b
>>> (gdb) ptype cmp3
>>> type = <unknown return type> ()
>>> (gdb) whatis cmp3
>>> type = <text variable, no debug info>
>>> 
>> 
>> Yes, I get the same.
>> 
>> Sounds to me like there might still be an issue in gdb? Or at least
>> a difference in the way the type is calculated.
>> This on it’s own is still a good fix, so I’ll send a new version.
> 
> I think we should learn the answer to the "where is that pointer
> coming from" question.  I'm really not understanding why the
> shared library makes a difference.
> 
>> 
>> 
>>> I wonder if what you're looking at is actually the malloc call?
>>> GDB will call malloc to allocate space for the "abc" string in
>>> the inferior.  Look at the backtrace, see where the call is coming
>>> from.
>> 
>> 
>> With the nodebug and debug shared library version:
>> (I need to run to main before I can set breakpoint on cmp3, otherwise
>> "Function "cmp3" not defined.”)
>> 
>> But with all versions, when stopped at cmp3, I get:
>> (gdb) bt
>> #0  0x00000000004005d4 in cmp3(char const*) ()
>> #1  <function called from gdb>
>> #2  0x00000000004005a4 in foo() ()
>> #3  0x00000000004005bc in main ()
> 
> 
> That's a backtrace in the inferior.  I meant instead for you to set
> a breakpoint on gdb's aarch64_push_dummy_call, figure out where the
> TYPE_CODE_PTR->TYPE_CODE_INT pointer comes from.  
> With "b foo if (int)cmp3("abc") == 1", gdb will do two
> infcalls, one malloc call to allocate space for "abc"
> string, and then the call to cmp3.
> 

A-ha! Now I understand why I get two calls into _push_dummy_call.

So to answer your question, the TYPE_CODE_PTR->TYPE_CODE_INT is the malloc call.

Then the next call to _push_dummy_call has a return type of 0, as expected.
This doesn’t segfault because it goes into language_pass_by_reference which
routes to default_pass_by_reference. The same as the C shared library version.


I’ve updated the test so it does {c,c++}*{debug nodebug}.
I can also update it to do both shared lib and non shared lib too. That should
cover everything.


Alan.











^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-29 14:56             ` Alan Hayward
@ 2018-10-29 18:13               ` Pedro Alves
  2018-10-30 11:13                 ` Alan Hayward
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2018-10-29 18:13 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/29/2018 02:56 PM, Alan Hayward wrote:

> A-ha! Now I understand why I get two calls into _push_dummy_call.
> 
> So to answer your question, the TYPE_CODE_PTR->TYPE_CODE_INT is the malloc call.
> 
> Then the next call to _push_dummy_call has a return type of 0, as expected.
> This doesn’t segfault because it goes into language_pass_by_reference which
> routes to default_pass_by_reference. The same as the C shared library version.
> 
> 
> I’ve updated the test so it does {c,c++}*{debug nodebug}.
> I can also update it to do both shared lib and non shared lib too. That should
> cover everything.
But still, why do you see a difference between shared library and non-shared
library?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-29 18:13               ` Pedro Alves
@ 2018-10-30 11:13                 ` Alan Hayward
  2018-10-30 16:31                   ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-30 11:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 29 Oct 2018, at 18:13, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/29/2018 02:56 PM, Alan Hayward wrote:
> 
>> A-ha! Now I understand why I get two calls into _push_dummy_call.
>> 
>> So to answer your question, the TYPE_CODE_PTR->TYPE_CODE_INT is the malloc call.
>> 
>> Then the next call to _push_dummy_call has a return type of 0, as expected.
>> This doesn’t segfault because it goes into language_pass_by_reference which
>> routes to default_pass_by_reference. The same as the C shared library version.
>> 
>> 
>> I’ve updated the test so it does {c,c++}*{debug nodebug}.
>> I can also update it to do both shared lib and non shared lib too. That should
>> cover everything.
> But still, why do you see a difference between shared library and non-shared
> library?

In all cases the function type is the same.

The difference is because with c++ && shared library, the code ends up in 
gnuv3_pass_by_reference(), which means it’s using the GNU G++ Version 3 ABI,
whereas with any other options (non shared or c) it ends up in
default_pass_by_reference().

Looking at the doc for GNU G++ Version 3 ABI:
https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
The library needs to be linked against libstdc++.so to use it.

A quick ldd shows only the c++ .so is linked against it.


Alan.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-30 11:13                 ` Alan Hayward
@ 2018-10-30 16:31                   ` Pedro Alves
  2018-10-30 17:09                     ` Alan Hayward
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2018-10-30 16:31 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/30/2018 11:13 AM, Alan Hayward wrote:

>> On 29 Oct 2018, at 18:13, Pedro Alves <palves@redhat.com> wrote:
>>
>> On 10/29/2018 02:56 PM, Alan Hayward wrote:
>>
>>> A-ha! Now I understand why I get two calls into _push_dummy_call.
>>>
>>> So to answer your question, the TYPE_CODE_PTR->TYPE_CODE_INT is the malloc call.
>>>
>>> Then the next call to _push_dummy_call has a return type of 0, as expected.
>>> This doesn’t segfault because it goes into language_pass_by_reference which
>>> routes to default_pass_by_reference. The same as the C shared library version.
>>>
>>>
>>> I’ve updated the test so it does {c,c++}*{debug nodebug}.
>>> I can also update it to do both shared lib and non shared lib too. That should
>>> cover everything.
>> But still, why do you see a difference between shared library and non-shared
>> library?
> 
> In all cases the function type is the same.
> 
> The difference is because with c++ && shared library, the code ends up in 
> gnuv3_pass_by_reference(), which means it’s using the GNU G++ Version 3 ABI,
> whereas with any other options (non shared or c) it ends up in
> default_pass_by_reference().

The function is the same, and should have been compiled using the calling
convention irrespective of whether it is linked into the main program,
or linked into the separate library.  Right?

So, either I'm missing something, or in one of the cases (shared
vs non-shared), we're calling the function incorrectly (along
with anything else that depends on call ABI), no?

What am I missing?

What does:

 (gdb) show cp-abi 
 The currently selected C++ ABI is "auto" (currently "gnu-v3").

show for you, in the shared and non-shared cases?

/me pokes a bit.

OK, I see what it is.

You've compiled the _main_ .cc without debug info as well:

 g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
 g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
 g++ condbreak-solib-main.o condbreak-solib-lib.o

And if you do that, the program ends up with no debug info at
all, and so GDB has no clue that this is a C++ program:

 (gdb) start
 Temporary breakpoint 1 at 0x4004c1
 Starting program: /tmp/a.out 
 
 Temporary breakpoint 1, 0x00000000004004c1 in main ()
 (gdb) show language 
 The current source language is "auto; currently c".
 (gdb)


If you compile (only) the main.cc with debug info, like this:

 - g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
 + g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline -g

then GDB will know that the program is a C++ program.  And you'd
still be calling a cmp3 function that has no debug info,
and should thus trigger the bug.



So when we call cmp3 with GDB's language set to C, we land
in default_pass_by_reference:

 (gdb) show language 
 The current source language is "auto; currently c".
 (gdb) p (int) cmp3(word)

 Thread 1 "gdb" hit Breakpoint 3, default_pass_by_reference (type=0x1b5a230) at src/gdb/language.c:669
 669       return 0;
 (top-gdb) c
 Continuing.

When the language is set to C++, we end up in gnuv3_pass_by_reference:

 (gdb) set language c++ 
 (gdb) p (int) cmp3(word)

 Thread 1 "gdb" hit Breakpoint 4, gnuv3_pass_by_reference (type=0x1b33290) at src/gdb/gnu-v3-abi.c:1255
 1255      type = check_typedef (type);
 (top-gdb) 

And this is because language_pass_by_reference uses the
current language, instead of the symbol's language (arguably a bug):

 (top-gdb) bt
 #0  0x000000000065be91 in gnuv3_pass_by_reference(type*) (type=0x1b33290)
     at src/gdb/gnu-v3-abi.c:1255
 #1  0x0000000000543e2a in cp_pass_by_reference(type*) (type=0x1b33290) at src/gdb/cp-abi.c:229
 #2  0x00000000006cc09b in language_pass_by_reference(type*) (type=0x1b33290)
     at src/gdb/language.c:660
 #3  0x000000000045a27a in default_return_in_first_hidden_param_p(gdbarch*, type*) (gdbarch=0x1b316b0, type=0x1b33290)
     at src/gdb/arch-utils.c:861
 #4  0x0000000000640a86 in gdbarch_return_in_first_hidden_param_p(gdbarch*, type*) (gdbarch=0x1b316b0, type=0x1b33290)
     at src/gdb/gdbarch.c:2739
 #5  0x00000000006a1011 in call_function_by_hand_dummy(value*, type*, int, value**, void (*)(void*, int), void*) (function=0x1b44730, default_return_type=0x1b33290, nargs=1, args=0x7fffffffc128, dummy_dtor=0x0, dummy_dtor_data=0x0)
     at src/gdb/infcall.c:881



So that's the real difference.  Shared vs non-shared is just
a kind of a red herring.  If you don't have debug info for
libstdc++, for example, then probably GDB won't know that the
no-debug-info program is a C++ program either.

So please adjust your test to eliminate use of the shared
library, and build just the cmp3 source file without
debug info.

> Looking at the doc for GNU G++ Version 3 ABI:
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
> The library needs to be linked against libstdc++.so to use it.
> 
> A quick ldd shows only the c++ .so is linked against it.

That wouldn't make much sense.  The whole program is using the
same compiler/call/mangling ABI, certainly, which is what
matters here.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-30 16:31                   ` Pedro Alves
@ 2018-10-30 17:09                     ` Alan Hayward
  2018-10-30 17:40                       ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Hayward @ 2018-10-30 17:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 30 Oct 2018, at 16:31, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/30/2018 11:13 AM, Alan Hayward wrote:
> 
>>> On 29 Oct 2018, at 18:13, Pedro Alves <palves@redhat.com> wrote:
>>> 
>>> On 10/29/2018 02:56 PM, Alan Hayward wrote:
>>> 
>>>> A-ha! Now I understand why I get two calls into _push_dummy_call.
>>>> 
>>>> So to answer your question, the TYPE_CODE_PTR->TYPE_CODE_INT is the malloc call.
>>>> 
>>>> Then the next call to _push_dummy_call has a return type of 0, as expected.
>>>> This doesn’t segfault because it goes into language_pass_by_reference which
>>>> routes to default_pass_by_reference. The same as the C shared library version.
>>>> 
>>>> 
>>>> I’ve updated the test so it does {c,c++}*{debug nodebug}.
>>>> I can also update it to do both shared lib and non shared lib too. That should
>>>> cover everything.
>>> But still, why do you see a difference between shared library and non-shared
>>> library?
>> 
>> In all cases the function type is the same.
>> 
>> The difference is because with c++ && shared library, the code ends up in 
>> gnuv3_pass_by_reference(), which means it’s using the GNU G++ Version 3 ABI,
>> whereas with any other options (non shared or c) it ends up in
>> default_pass_by_reference().
> 
> The function is the same, and should have been compiled using the calling
> convention irrespective of whether it is linked into the main program,
> or linked into the separate library.  Right?
> 
> So, either I'm missing something, or in one of the cases (shared
> vs non-shared), we're calling the function incorrectly (along
> with anything else that depends on call ABI), no?
> 
> What am I missing?
> 
> What does:
> 
> (gdb) show cp-abi 
> The currently selected C++ ABI is "auto" (currently "gnu-v3").
> 
> show for you, in the shared and non-shared cases?
> 
> /me pokes a bit.
> 
> OK, I see what it is.
> 
> You've compiled the _main_ .cc without debug info as well:
> 
> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
> g++ condbreak-solib-main.o condbreak-solib-lib.o
> 
> And if you do that, the program ends up with no debug info at
> all, and so GDB has no clue that this is a C++ program:
> 
> (gdb) start
> Temporary breakpoint 1 at 0x4004c1
> Starting program: /tmp/a.out 
> 
> Temporary breakpoint 1, 0x00000000004004c1 in main ()
> (gdb) show language 
> The current source language is "auto; currently c".
> (gdb)
> 
> 
> If you compile (only) the main.cc with debug info, like this:
> 
> - g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
> + g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline -g
> 
> then GDB will know that the program is a C++ program.  And you'd
> still be calling a cmp3 function that has no debug info,
> and should thus trigger the bug.
> 

Yes, I hit the bug this way.

Many thanks for looking through this.


> 
> 
> So when we call cmp3 with GDB's language set to C, we land
> in default_pass_by_reference:
> 
> (gdb) show language 
> The current source language is "auto; currently c".
> (gdb) p (int) cmp3(word)
> 
> Thread 1 "gdb" hit Breakpoint 3, default_pass_by_reference (type=0x1b5a230) at src/gdb/language.c:669
> 669       return 0;
> (top-gdb) c
> Continuing.
> 
> When the language is set to C++, we end up in gnuv3_pass_by_reference:
> 
> (gdb) set language c++ 
> (gdb) p (int) cmp3(word)
> 
> Thread 1 "gdb" hit Breakpoint 4, gnuv3_pass_by_reference (type=0x1b33290) at src/gdb/gnu-v3-abi.c:1255
> 1255      type = check_typedef (type);
> (top-gdb) 
> 
> And this is because language_pass_by_reference uses the
> current language, instead of the symbol's language (arguably a bug):
> 
> (top-gdb) bt
> #0  0x000000000065be91 in gnuv3_pass_by_reference(type*) (type=0x1b33290)
>     at src/gdb/gnu-v3-abi.c:1255
> #1  0x0000000000543e2a in cp_pass_by_reference(type*) (type=0x1b33290) at src/gdb/cp-abi.c:229
> #2  0x00000000006cc09b in language_pass_by_reference(type*) (type=0x1b33290)
>     at src/gdb/language.c:660
> #3  0x000000000045a27a in default_return_in_first_hidden_param_p(gdbarch*, type*) (gdbarch=0x1b316b0, type=0x1b33290)
>     at src/gdb/arch-utils.c:861
> #4  0x0000000000640a86 in gdbarch_return_in_first_hidden_param_p(gdbarch*, type*) (gdbarch=0x1b316b0, type=0x1b33290)
>     at src/gdb/gdbarch.c:2739
> #5  0x00000000006a1011 in call_function_by_hand_dummy(value*, type*, int, value**, void (*)(void*, int), void*) (function=0x1b44730, default_return_type=0x1b33290, nargs=1, args=0x7fffffffc128, dummy_dtor=0x0, dummy_dtor_data=0x0)
>     at src/gdb/infcall.c:881
> 
> 
> 
> So that's the real difference.  Shared vs non-shared is just
> a kind of a red herring.  If you don't have debug info for
> libstdc++, for example, then probably GDB won't know that the
> no-debug-info program is a C++ program either.
> 
> So please adjust your test to eliminate use of the shared
> library, and build just the cmp3 source file without
> debug info.

Will do.


> 
>> Looking at the doc for GNU G++ Version 3 ABI:
>> https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
>> The library needs to be linked against libstdc++.so to use it.
>> 
>> A quick ldd shows only the c++ .so is linked against it.
> 
> That wouldn't make much sense.  The whole program is using the
> same compiler/call/mangling ABI, certainly, which is what
> matters here.
> 
> Thanks,
> Pedro Alves


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls
  2018-10-30 17:09                     ` Alan Hayward
@ 2018-10-30 17:40                       ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2018-10-30 17:40 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/30/2018 05:09 PM, Alan Hayward wrote:
> 
> 
>> On 30 Oct 2018, at 16:31, Pedro Alves <palves@redhat.com> wrote:

>> So that's the real difference.  Shared vs non-shared is just
>> a kind of a red herring.  If you don't have debug info for
>> libstdc++, for example, then probably GDB won't know that the
>> no-debug-info program is a C++ program either.
>>
>> So please adjust your test to eliminate use of the shared
>> library, and build just the cmp3 source file without
>> debug info.
> 
> Will do.

Thinking a little bit more, the conditional breakpoint is also
kind of a red herring here.  What really matters is the infcall.

So the testcase could just do:

  (gdb) p (int) cmp3(word)

I'd simplify the testcase a little bit in that direction,
and call it gdb.cp/nodebug-infcall.exp, for example.

Note, the testcase should be gated with a 

 if [target_info exists gdb,cannot_call_functions] {

check.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-10-30 17:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting dummy calls Alan Hayward
2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
2018-10-19 11:31   ` Pedro Alves
2018-10-11 14:49 ` [PATCH v3 1/3] Use enum for return method for dummy calls Alan Hayward
2018-10-19 11:28   ` Pedro Alves
2018-10-11 14:49 ` [PATCH v3 3/3] Aarch64: Fix segfault when casting " Alan Hayward
2018-10-19 11:36   ` Pedro Alves
2018-10-23 16:08     ` Alan Hayward
2018-10-24 15:15       ` Pedro Alves
2018-10-29 11:58         ` Alan Hayward
2018-10-29 12:38           ` Pedro Alves
2018-10-29 14:56             ` Alan Hayward
2018-10-29 18:13               ` Pedro Alves
2018-10-30 11:13                 ` Alan Hayward
2018-10-30 16:31                   ` Pedro Alves
2018-10-30 17:09                     ` Alan Hayward
2018-10-30 17:40                       ` Pedro Alves
2018-10-18  9:50 ` [PING][PATCH v3 0/3] " Alan Hayward

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).