public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-01 15:53 [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
  2018-10-01 15:53 ` [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call Alan Hayward
@ 2018-10-01 15:53 ` Alan Hayward
  2018-10-09 16:15   ` Pedro Alves
  2018-10-09  8:26 ` [PING][PATCH v2 0/2] " Alan Hayward
  2018-10-09 16:10 ` [PATCH " Pedro Alves
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Hayward @ 2018-10-01 15:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

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

Instead of incorrectly re-calculating lang_struct_return, reuse
the version passed in.

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

gdb/ChangeLog:

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

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

gdb/testsuite/ChangeLog:

2018-10-01  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                            | 27 +-----
 gdb/testsuite/gdb.base/condbreak-solib-lib.cc | 21 +++++
 .../gdb.base/condbreak-solib-main.cc          | 33 +++++++
 gdb/testsuite/gdb.base/condbreak-solib.exp    | 93 +++++++++++++++++++
 4 files changed, 149 insertions(+), 25 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 504b040c2e..f6eabc7029 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1513,14 +1513,11 @@ 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, int lang_struct_return_unused,
+			 int struct_return, int lang_struct_return,
 			 CORE_ADDR struct_addr)
 {
   int argnum;
   struct aarch64_call_info info;
-  struct type *func_type;
-  struct type *return_type;
-  int lang_struct_return;
 
   memset (&info, 0, sizeof (info));
 
@@ -1542,27 +1539,7 @@ 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.  */
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..02bf450c43
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
@@ -0,0 +1,21 @@
+/* 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..43d39d1da4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
@@ -0,0 +1,33 @@
+/* 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 bar ()
+{
+  return 1;
+}
+
+int main (void)
+{
+  cmp3 ((char*)"a");
+  bar ();
+  cmp3 ((char*)"a");
+  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..27639bd109
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-solib.exp
@@ -0,0 +1,93 @@
+# 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.
+
+if { [skip_cplus_tests] } { continue }
+
+if {![is_elf_target]} {
+    return 0
+}
+
+if [skip_shlib_tests] {
+    return 0
+}
+
+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}"
+set lib_so [standard_output_file ${lib_basename}.so]
+set lib_syms [shlib_symbol_file ${lib_so}]
+set lib_dlopen [shlib_target_file ${lib_basename}.so]
+
+
+if [get_compiler_info] {
+    return -1
+}
+
+# Compile a shared library containing cmp3
+
+if {[gdb_compile_shlib $libsrc $lib_so {debug c++}] != ""} {
+    untested "failed to compile shared library"
+    return
+}
+
+# Compile the main test linked against the shared library
+
+set exec_opts [list debug c++ "additional_flags= -I$srcdir/../../include/ -D$target_size\
+ -DSHLIB_NAME\\=\"$lib_dlopen\""]
+
+if {[prepare_for_testing "failed to prepare" $binfile "$srcfile $srcfile2" $exec_opts]} {
+    return
+}
+
+gdb_load_shlib ${lib_so}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return
+}
+
+# Create conditional breakpoint on cmp3(), which will fail the condition.
+
+gdb_test "b cmp3 if (int)strcmp(word,\"abc\") == 0" "Breakpoint .*"
+
+gdb_test "b bar" "Breakpoint .*"
+
+gdb_test "c" "Breakpoint .* bar .*"
+
+# Create conditional breakpoint on cmp3(), which will pass the condition.
+
+gdb_test "b cmp3 if (int)strcmp(word,\"stuff\") == 0" "Breakpoint .*"
+
+gdb_test "c" "Breakpoint .* cmp3 .*"
+
-- 
2.17.1 (Apple Git-112)

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

* [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call
  2018-10-01 15:53 [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
@ 2018-10-01 15:53 ` Alan Hayward
  2018-10-09 16:14   ` Pedro Alves
  2018-10-01 15:53 ` [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Hayward @ 2018-10-01 15:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

Make call_function_by_hand_dummy pass this down.

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

	* aarch64-tdep.c (aarch64_push_dummy_call): Add
	lang_struct_return_unused param.
	* alpha-tdep.c (alpha_push_dummy_call): Add lang_struct_return
	param.
	* amd64-tdep.c (amd64_push_dummy_call): Likewise.
	* amd64-windows-tdep.c: 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 (gdbarch_push_dummy_call): Regenerate.
	* gdbarch.h (CORE_ADDR): Likewise.
	(gdbarch_push_dummy_call): Likewise.
	* gdbarch.sh: Add lang_struct_return param.
	* 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): Pass lang_struct_return.
	* iq2000-tdep.c (iq2000_push_dummy_call): Add lang_struct_return
	param.
	* 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_push_dummy_call): Likewise.
	* sparc64-tdep.c (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        |  4 ++--
 gdb/alpha-tdep.c          |  3 ++-
 gdb/amd64-tdep.c          |  3 ++-
 gdb/amd64-windows-tdep.c  |  3 ++-
 gdb/arc-tdep.c            |  2 +-
 gdb/arm-tdep.c            |  2 +-
 gdb/avr-tdep.c            |  3 ++-
 gdb/bfin-tdep.c           |  1 +
 gdb/cris-tdep.c           |  3 ++-
 gdb/csky-tdep.c           |  3 ++-
 gdb/frv-tdep.c            |  3 ++-
 gdb/gdbarch.c             |  4 ++--
 gdb/gdbarch.h             |  4 ++--
 gdb/gdbarch.sh            |  2 +-
 gdb/h8300-tdep.c          |  3 ++-
 gdb/hppa-tdep.c           |  6 ++++--
 gdb/i386-darwin-tdep.c    |  3 ++-
 gdb/i386-tdep.c           |  2 +-
 gdb/ia64-tdep.c           |  3 ++-
 gdb/infcall.c             |  3 ++-
 gdb/iq2000-tdep.c         |  3 ++-
 gdb/lm32-tdep.c           |  3 ++-
 gdb/m32c-tdep.c           |  2 +-
 gdb/m32r-tdep.c           |  2 +-
 gdb/m68hc11-tdep.c        |  3 ++-
 gdb/m68k-tdep.c           |  2 +-
 gdb/mep-tdep.c            |  2 +-
 gdb/mips-tdep.c           | 15 +++++++++------
 gdb/mn10300-tdep.c        |  1 +
 gdb/msp430-tdep.c         |  3 ++-
 gdb/nds32-tdep.c          |  3 ++-
 gdb/nios2-tdep.c          |  3 ++-
 gdb/or1k-tdep.c           |  3 ++-
 gdb/ppc-sysv-tdep.c       |  6 ++++--
 gdb/ppc-tdep.h            |  2 ++
 gdb/riscv-tdep.c          |  1 +
 gdb/rl78-tdep.c           |  3 ++-
 gdb/rs6000-aix-tdep.c     |  3 ++-
 gdb/rs6000-lynx178-tdep.c |  3 ++-
 gdb/rx-tdep.c             |  2 +-
 gdb/s390-tdep.c           |  3 ++-
 gdb/score-tdep.c          |  3 ++-
 gdb/sh-tdep.c             |  2 ++
 gdb/sparc-tdep.c          |  3 ++-
 gdb/sparc64-tdep.c        |  3 ++-
 gdb/spu-tdep.c            |  3 ++-
 gdb/tic6x-tdep.c          |  3 ++-
 gdb/tilegx-tdep.c         |  1 +
 gdb/v850-tdep.c           |  1 +
 gdb/vax-tdep.c            |  2 +-
 gdb/xstormy16-tdep.c      |  1 +
 gdb/xtensa-tdep.c         |  1 +
 52 files changed, 98 insertions(+), 53 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 023e8eb453..504b040c2e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1512,8 +1512,8 @@ pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache *regcache,
 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,
+			 int nargs, struct value **args, CORE_ADDR sp,
+			 int struct_return, int lang_struct_return_unused,
 			 CORE_ADDR struct_addr)
 {
   int argnum;
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index e649bd2102..a154274091 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -295,7 +295,8 @@ 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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int i;
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 088542d72b..f805c8b5a5 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 904875bacc..a33908f1a2 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -245,7 +245,8 @@ 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)
+   CORE_ADDR sp, int struct_return, int lang_struct_return,
+   CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index fad9170978..f7f7ddfaee 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -593,7 +593,7 @@ 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,
-		     CORE_ADDR struct_addr)
+		     int lang_struct_return, CORE_ADDR struct_addr)
 {
   if (arc_debug)
     debug_printf ("arc: push_dummy_call (nargs = %d)\n", nargs);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index c3280ee211..ea69637986 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3680,7 +3680,7 @@ 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,
-		     CORE_ADDR struct_addr)
+		     int lang_struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 9e14007fc0..cf7daaf4ff 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)
+		     int struct_return, int lang_struct_return,
+		     CORE_ADDR struct_addr)
 {
   int i;
   gdb_byte buf[3];
diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index c84625c894..580eb0e013 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -499,6 +499,7 @@ bfin_push_dummy_call (struct gdbarch *gdbarch,
 		      struct value **args,
 		      CORE_ADDR sp,
 		      int struct_return,
+		      int lang_struct_return,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index e0371a2a28..df90e4ba70 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argreg;
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index 95bcead877..066b855414 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   int argnum;
   int argreg = CSKY_ABI_A0_REGNUM;
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index 1eed441f2b..8b67ac43e6 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)
+		     int struct_return, int lang_struct_return,
+		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argreg;
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index e2abf263b3..15de39d51c 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, int struct_return, int lang_struct_return, 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, struct_return, lang_struct_return, struct_addr);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index fc2f1a84a1..bcf7a954ab 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -394,8 +394,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, int struct_return, int lang_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, int lang_struct_return, 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 670ac30c03..8d4e1e9ced 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, int struct_return, int lang_struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, lang_struct_return, 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..caff83b3dc 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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int stack_alloc = 0, stack_offset = 0;
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 319096e056..6bea76d1fe 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)
+			int struct_return, int lang_struct_return,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
@@ -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)
+			int struct_return, int lang_struct_return,
+			CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/i386-darwin-tdep.c b/gdb/i386-darwin-tdep.c
index 5a1807ae4d..e1c0937a9c 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)
+			     int struct_return, int lang_struct_return,
+			     CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..9a8ce67d04 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2672,7 +2672,7 @@ 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,
-		      CORE_ADDR struct_addr)
+		      int lang_struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[4];
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 5e9ecb5151..8132c0d21b 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -3674,7 +3674,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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 96d43704fa..85e3d387b0 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1061,7 +1061,8 @@ call_function_by_hand_dummy (struct value *function,
      return address should be pointed.  */
   sp = gdbarch_push_dummy_call (gdbarch, function, get_current_regcache (),
 				bp_addr, nargs, args,
-				sp, struct_return, struct_addr);
+				sp, struct_return, hidden_first_param_p,
+				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
diff --git a/gdb/iq2000-tdep.c b/gdb/iq2000-tdep.c
index 9f7f35d287..6f82bf9fa0 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)
+			int struct_return, int lang_struct_return,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   const bfd_byte *val;
diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
index 942852140d..2cfe1a0b54 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int first_arg_reg = SIM_LM32_R1_REGNUM;
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 6fa24452da..7aed507512 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -2017,7 +2017,7 @@ 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,
-		      CORE_ADDR struct_addr)
+		      int lang_struct_return, CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index fd79f3f4cd..f807b2d5e3 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -660,7 +660,7 @@ 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,
-		      CORE_ADDR struct_addr)
+		      int lang_struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int stack_offset, stack_alloc;
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 1490ee2866..de1f7eeba3 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)
+			 int struct_return, int lang_struct_return,
+			 CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index a6e9b58a7d..f9357d6130 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -493,7 +493,7 @@ 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,
-		      CORE_ADDR struct_addr)
+		      int lang_struct_return, CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index 69e7fdda59..989d24e767 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -2259,7 +2259,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,
+		     int struct_return, int lang_struct_return,
                      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 5e0a60625b..934c8376ce 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)
+			   int struct_return, int lang_struct_return,
+			   CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -4889,7 +4890,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)
+			     int struct_return, int lang_struct_return,
+			     CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -5345,7 +5347,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)
+			  int struct_return, int lang_struct_return,
+			  CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
@@ -5867,9 +5870,9 @@ mips_o32_return_value (struct gdbarch *gdbarch, struct value *function,
 static CORE_ADDR
 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)
+			  int nargs, struct value **args, CORE_ADDR sp,
+			  int struct_return, int lang_struct_return,
+			  CORE_ADDR struct_addr)
 {
   int argreg;
   int float_argreg;
diff --git a/gdb/mn10300-tdep.c b/gdb/mn10300-tdep.c
index edc99a2fab..42d611f6fc 100644
--- a/gdb/mn10300-tdep.c
+++ b/gdb/mn10300-tdep.c
@@ -1192,6 +1192,7 @@ mn10300_push_dummy_call (struct gdbarch *gdbarch,
 			 int nargs, struct value **args,
 			 CORE_ADDR sp, 
 			 int struct_return,
+			 int lang_struct_return,
 			 CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index b6e062a380..a1b7add9ad 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)
+			int struct_return, int lang_struct_return,
+			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int write_pass;
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index b616cc9b2c..7282ed8cf8 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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   const int REND = 6;		/* End for register offset.  */
   int goff = 0;			/* Current gpr offset for argument.  */
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 008b1d4b22..fb113172ce 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -1814,7 +1814,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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   int argreg;
   int argnum;
diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
index c5104e3959..fca09347ad 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
 
   int argreg;
diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index 0bac225112..bebdc5cc7a 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)
+			      int struct_return, int lang_struct_return,
+			      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -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)
+				int struct_return, int lang_struct_return,
+				CORE_ADDR struct_addr)
 {
   CORE_ADDR func_addr = find_function_addr (function, NULL);
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index c3571cbd51..7517c3f06f 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -45,6 +45,7 @@ CORE_ADDR ppc_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 					CORE_ADDR bp_addr, int nargs,
 					struct value **args, CORE_ADDR sp,
 					int struct_return,
+					int lang_struct_return,
 					CORE_ADDR struct_addr);
 CORE_ADDR ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 					  struct value *function,
@@ -52,6 +53,7 @@ CORE_ADDR ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 					  CORE_ADDR bp_addr, int nargs,
 					  struct value **args, CORE_ADDR sp,
 					  int struct_return,
+					  int lang_struct_return,
 					  CORE_ADDR struct_addr);
 enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch,
 							  struct value *function,
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 319e01b4ab..ea8f8e829d 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -2234,6 +2234,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
 		       struct value **args,
 		       CORE_ADDR sp,
 		       int struct_return,
+		       int lang_struct_return,
 		       CORE_ADDR struct_addr)
 {
   int i;
diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index ace01b1171..92a780fc43 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[4];
diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 50a146a4f0..edb6d7e411 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)
+			int struct_return, int lang_struct_return,
+			CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/rs6000-lynx178-tdep.c b/gdb/rs6000-lynx178-tdep.c
index 44cd0d013d..c61e0942ad 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)
+				int struct_return, int lang_struct_return,
+				CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 94d57913a3..d2cdd79363 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -785,7 +785,7 @@ 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,
-		    CORE_ADDR struct_addr)
+		    int lang_struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int write_pass;
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index e962824ca0..3a30bf594f 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)
+		      int struct_return, int lang_struct_return,
+		      CORE_ADDR struct_addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
diff --git a/gdb/score-tdep.c b/gdb/score-tdep.c
index b2887c5eae..9b7fc17995 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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index fe64cf979a..05d71e036e 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -1063,6 +1063,7 @@ sh_push_dummy_call_fpu (struct gdbarch *gdbarch,
 			CORE_ADDR bp_addr, int nargs,
 			struct value **args,
 			CORE_ADDR sp, int struct_return,
+			int lang_struct_return,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1205,6 +1206,7 @@ sh_push_dummy_call_nofpu (struct gdbarch *gdbarch,
 			  CORE_ADDR bp_addr,
 			  int nargs, struct value **args,
 			  CORE_ADDR sp, int struct_return,
+			  int lang_struct_return,
 			  CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 7a50a8d4a9..7e8a099028 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -712,7 +712,8 @@ 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)
+			 int struct_return, int lang_struct_return,
+			 CORE_ADDR struct_addr)
 {
   CORE_ADDR call_pc = (struct_return ? (bp_addr - 12) : (bp_addr - 8));
 
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index b1ee6c1b57..92d751769d 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -1621,7 +1621,8 @@ 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)
+			 int struct_return, int lang_struct_return,
+			 CORE_ADDR struct_addr)
 {
   /* Set return address.  */
   regcache_cooked_write_unsigned (regcache, SPARC_O7_REGNUM, bp_addr - 8);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 6ae37f58c7..39191a7bf5 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)
+		     int struct_return, int lang_struct_return,
+		     CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR sp_delta;
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index b1711bad29..7cdc37de92 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -876,7 +876,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)
+		       int struct_return, int lang_struct_return,
+		       CORE_ADDR struct_addr)
 {
   int argreg = 0;
   int argnum;
diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c
index 9ed696630e..c6e88079a3 100644
--- a/gdb/tilegx-tdep.c
+++ b/gdb/tilegx-tdep.c
@@ -282,6 +282,7 @@ tilegx_push_dummy_call (struct gdbarch *gdbarch,
 			CORE_ADDR bp_addr, int nargs,
 			struct value **args,
 			CORE_ADDR sp, int struct_return,
+			int lang_struct_return,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index 2a3812d6c0..157c59e5fa 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -1014,6 +1014,7 @@ v850_push_dummy_call (struct gdbarch *gdbarch,
 		      struct value **args,
 		      CORE_ADDR sp,
 		      int struct_return,
+		      int lang_struct_return,
 		      CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/vax-tdep.c b/gdb/vax-tdep.c
index 21f2066da2..2855eca042 100644
--- a/gdb/vax-tdep.c
+++ b/gdb/vax-tdep.c
@@ -142,7 +142,7 @@ 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,
-		     CORE_ADDR struct_addr)
+		     int lang_struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR fp = sp;
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index b80d23e142..a6eccef30c 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -227,6 +227,7 @@ xstormy16_push_dummy_call (struct gdbarch *gdbarch,
 			   CORE_ADDR bp_addr, int nargs,
 			   struct value **args,
 			   CORE_ADDR sp, int struct_return,
+			   int lang_struct_return,
 			   CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 49a7f023e7..0928f47071 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -1696,6 +1696,7 @@ xtensa_push_dummy_call (struct gdbarch *gdbarch,
 			struct value **args,
 			CORE_ADDR sp,
 			int struct_return,
+			int lang_struct_return,
 			CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-- 
2.17.1 (Apple Git-112)

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

* [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls
@ 2018-10-01 15:53 Alan Hayward
  2018-10-01 15:53 ` [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call Alan Hayward
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Alan Hayward @ 2018-10-01 15:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

This is a reworking of a patch I posted in March.
V1 had a long discussion which was then paused to wait for
Pedro's IFUNC rewrite.


Prevent the int cast in 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 either 1) the call has a
cast or 2) an error has occured.
You can see this in infcall.c:call_function_by_hand_dummy():

  CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);

  if (values_type == NULL)
    values_type = default_return_type;
  if (values_type == NULL)
    {
      const char *name = get_function_name (funaddr,
					    name_buf, sizeof (name_buf));
      error (_("'%s' has unknown return type; "
	       "cast the call to its declared return type"),
	     name);
    }

In aarch64_push_dummy_call we do not have default_return_type, so cannot
determine between the two cases.

(In addition, aarch64_push_dummy_call incorrectly resolves the return
type for IFUNC).


However, aarch64_push_dummy_call only requires the return value in order
to calculate lang_struct_return ... which has previously been calculated
in the caller:

     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.

The fix is simple:
Patch 1: Update gdbarch interface to pass lang_struct_return.
Patch 2: Remove incorrect code and use the passed in lang_struct_return.

Built on x86 target all build and tested on aarch64.



Alan Hayward (2):
  Add lang_struct_return to _push_dummy_call
  Aarch64: Fix segfault when casting dummy calls

 gdb/aarch64-tdep.c                            | 29 +-----
 gdb/alpha-tdep.c                              |  3 +-
 gdb/amd64-tdep.c                              |  3 +-
 gdb/amd64-windows-tdep.c                      |  3 +-
 gdb/arc-tdep.c                                |  2 +-
 gdb/arm-tdep.c                                |  2 +-
 gdb/avr-tdep.c                                |  3 +-
 gdb/bfin-tdep.c                               |  1 +
 gdb/cris-tdep.c                               |  3 +-
 gdb/csky-tdep.c                               |  3 +-
 gdb/frv-tdep.c                                |  3 +-
 gdb/gdbarch.c                                 |  4 +-
 gdb/gdbarch.h                                 |  4 +-
 gdb/gdbarch.sh                                |  2 +-
 gdb/h8300-tdep.c                              |  3 +-
 gdb/hppa-tdep.c                               |  6 +-
 gdb/i386-darwin-tdep.c                        |  3 +-
 gdb/i386-tdep.c                               |  2 +-
 gdb/ia64-tdep.c                               |  3 +-
 gdb/infcall.c                                 |  3 +-
 gdb/iq2000-tdep.c                             |  3 +-
 gdb/lm32-tdep.c                               |  3 +-
 gdb/m32c-tdep.c                               |  2 +-
 gdb/m32r-tdep.c                               |  2 +-
 gdb/m68hc11-tdep.c                            |  3 +-
 gdb/m68k-tdep.c                               |  2 +-
 gdb/mep-tdep.c                                |  2 +-
 gdb/mips-tdep.c                               | 15 +--
 gdb/mn10300-tdep.c                            |  1 +
 gdb/msp430-tdep.c                             |  3 +-
 gdb/nds32-tdep.c                              |  3 +-
 gdb/nios2-tdep.c                              |  3 +-
 gdb/or1k-tdep.c                               |  3 +-
 gdb/ppc-sysv-tdep.c                           |  6 +-
 gdb/ppc-tdep.h                                |  2 +
 gdb/riscv-tdep.c                              |  1 +
 gdb/rl78-tdep.c                               |  3 +-
 gdb/rs6000-aix-tdep.c                         |  3 +-
 gdb/rs6000-lynx178-tdep.c                     |  3 +-
 gdb/rx-tdep.c                                 |  2 +-
 gdb/s390-tdep.c                               |  3 +-
 gdb/score-tdep.c                              |  3 +-
 gdb/sh-tdep.c                                 |  2 +
 gdb/sparc-tdep.c                              |  3 +-
 gdb/sparc64-tdep.c                            |  3 +-
 gdb/spu-tdep.c                                |  3 +-
 gdb/testsuite/gdb.base/condbreak-solib-lib.cc | 21 +++++
 .../gdb.base/condbreak-solib-main.cc          | 33 +++++++
 gdb/testsuite/gdb.base/condbreak-solib.exp    | 93 +++++++++++++++++++
 gdb/tic6x-tdep.c                              |  3 +-
 gdb/tilegx-tdep.c                             |  1 +
 gdb/v850-tdep.c                               |  1 +
 gdb/vax-tdep.c                                |  2 +-
 gdb/xstormy16-tdep.c                          |  1 +
 gdb/xtensa-tdep.c                             |  1 +
 55 files changed, 246 insertions(+), 77 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] 11+ messages in thread

* [PING][PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-01 15:53 [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
  2018-10-01 15:53 ` [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call Alan Hayward
  2018-10-01 15:53 ` [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
@ 2018-10-09  8:26 ` Alan Hayward
  2018-10-09 16:10 ` [PATCH " Pedro Alves
  3 siblings, 0 replies; 11+ messages in thread
From: Alan Hayward @ 2018-10-09  8:26 UTC (permalink / raw)
  To: GDB Patches; +Cc: nd

Ping.


> On 1 Oct 2018, at 16:52, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> This is a reworking of a patch I posted in March.
> V1 had a long discussion which was then paused to wait for
> Pedro's IFUNC rewrite.
> 
> 
> Prevent the int cast in 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 either 1) the call has a
> cast or 2) an error has occured.
> You can see this in infcall.c:call_function_by_hand_dummy():
> 
>  CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
> 
>  if (values_type == NULL)
>    values_type = default_return_type;
>  if (values_type == NULL)
>    {
>      const char *name = get_function_name (funaddr,
> 					    name_buf, sizeof (name_buf));
>      error (_("'%s' has unknown return type; "
> 	       "cast the call to its declared return type"),
> 	     name);
>    }
> 
> In aarch64_push_dummy_call we do not have default_return_type, so cannot
> determine between the two cases.
> 
> (In addition, aarch64_push_dummy_call incorrectly resolves the return
> type for IFUNC).
> 
> 
> However, aarch64_push_dummy_call only requires the return value in order
> to calculate lang_struct_return ... which has previously been calculated
> in the caller:
> 
>     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.
> 
> The fix is simple:
> Patch 1: Update gdbarch interface to pass lang_struct_return.
> Patch 2: Remove incorrect code and use the passed in lang_struct_return.
> 
> Built on x86 target all build and tested on aarch64.
> 
> 
> 
> Alan Hayward (2):
>  Add lang_struct_return to _push_dummy_call
>  Aarch64: Fix segfault when casting dummy calls
> 
> gdb/aarch64-tdep.c                            | 29 +-----
> gdb/alpha-tdep.c                              |  3 +-
> gdb/amd64-tdep.c                              |  3 +-
> gdb/amd64-windows-tdep.c                      |  3 +-
> gdb/arc-tdep.c                                |  2 +-
> gdb/arm-tdep.c                                |  2 +-
> gdb/avr-tdep.c                                |  3 +-
> gdb/bfin-tdep.c                               |  1 +
> gdb/cris-tdep.c                               |  3 +-
> gdb/csky-tdep.c                               |  3 +-
> gdb/frv-tdep.c                                |  3 +-
> gdb/gdbarch.c                                 |  4 +-
> gdb/gdbarch.h                                 |  4 +-
> gdb/gdbarch.sh                                |  2 +-
> gdb/h8300-tdep.c                              |  3 +-
> gdb/hppa-tdep.c                               |  6 +-
> gdb/i386-darwin-tdep.c                        |  3 +-
> gdb/i386-tdep.c                               |  2 +-
> gdb/ia64-tdep.c                               |  3 +-
> gdb/infcall.c                                 |  3 +-
> gdb/iq2000-tdep.c                             |  3 +-
> gdb/lm32-tdep.c                               |  3 +-
> gdb/m32c-tdep.c                               |  2 +-
> gdb/m32r-tdep.c                               |  2 +-
> gdb/m68hc11-tdep.c                            |  3 +-
> gdb/m68k-tdep.c                               |  2 +-
> gdb/mep-tdep.c                                |  2 +-
> gdb/mips-tdep.c                               | 15 +--
> gdb/mn10300-tdep.c                            |  1 +
> gdb/msp430-tdep.c                             |  3 +-
> gdb/nds32-tdep.c                              |  3 +-
> gdb/nios2-tdep.c                              |  3 +-
> gdb/or1k-tdep.c                               |  3 +-
> gdb/ppc-sysv-tdep.c                           |  6 +-
> gdb/ppc-tdep.h                                |  2 +
> gdb/riscv-tdep.c                              |  1 +
> gdb/rl78-tdep.c                               |  3 +-
> gdb/rs6000-aix-tdep.c                         |  3 +-
> gdb/rs6000-lynx178-tdep.c                     |  3 +-
> gdb/rx-tdep.c                                 |  2 +-
> gdb/s390-tdep.c                               |  3 +-
> gdb/score-tdep.c                              |  3 +-
> gdb/sh-tdep.c                                 |  2 +
> gdb/sparc-tdep.c                              |  3 +-
> gdb/sparc64-tdep.c                            |  3 +-
> gdb/spu-tdep.c                                |  3 +-
> gdb/testsuite/gdb.base/condbreak-solib-lib.cc | 21 +++++
> .../gdb.base/condbreak-solib-main.cc          | 33 +++++++
> gdb/testsuite/gdb.base/condbreak-solib.exp    | 93 +++++++++++++++++++
> gdb/tic6x-tdep.c                              |  3 +-
> gdb/tilegx-tdep.c                             |  1 +
> gdb/v850-tdep.c                               |  1 +
> gdb/vax-tdep.c                                |  2 +-
> gdb/xstormy16-tdep.c                          |  1 +
> gdb/xtensa-tdep.c                             |  1 +
> 55 files changed, 246 insertions(+), 77 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] 11+ messages in thread

* Re: [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-01 15:53 [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
                   ` (2 preceding siblings ...)
  2018-10-09  8:26 ` [PING][PATCH v2 0/2] " Alan Hayward
@ 2018-10-09 16:10 ` Pedro Alves
  2018-10-09 17:50   ` Alan Hayward
  3 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2018-10-09 16:10 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 10/01/2018 04:52 PM, Alan Hayward wrote:
> This is a reworking of a patch I posted in March.
> V1 had a long discussion which was then paused to wait for
> Pedro's IFUNC rewrite.
> 
> 
> Prevent the int cast in 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 either 1) the call has a
> cast or 2) an error has occured.

I'd think that "1) the call has a cast" is not accurate.
If the called function has debug info, then GDB will know
it's return type.  The issue is that the called function may
not have debug information, and then GDB does not know
its return type (so its NULL), and then the only way to
call the function is to add the cast.  Right?

It kind of sounds like IFUNCs were a red herring then.  :-/

> You can see this in infcall.c:call_function_by_hand_dummy():
> 
>   CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
> 
>   if (values_type == NULL)
>     values_type = default_return_type;
>   if (values_type == NULL)
>     {
>       const char *name = get_function_name (funaddr,
> 					    name_buf, sizeof (name_buf));
>       error (_("'%s' has unknown return type; "
> 	       "cast the call to its declared return type"),
> 	     name);
>     }
> 
> In aarch64_push_dummy_call we do not have default_return_type, so cannot
> determine between the two cases.
> 
> (In addition, aarch64_push_dummy_call incorrectly resolves the return
> type for IFUNC).

Can you expand a bit on this IFUNC remark?


> However, aarch64_push_dummy_call only requires the return value in order
> to calculate lang_struct_return ... which has previously been calculated
> in the caller:
> 
>      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.
> 

Ah, nice, the solution was right there.  :-)

> The fix is simple:
> Patch 1: Update gdbarch interface to pass lang_struct_return.
> Patch 2: Remove incorrect code and use the passed in lang_struct_return.
> 

Since cover letters don't end up in git, this info should be
somehow migrated into the commit logs of the two patches.

Thanks,
Pedro Alves

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

* Re: [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call
  2018-10-01 15:53 ` [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call Alan Hayward
@ 2018-10-09 16:14   ` Pedro Alves
  2018-10-10 11:54     ` Alan Hayward
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2018-10-09 16:14 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 10/01/2018 04:52 PM, Alan Hayward wrote:
> Make call_function_by_hand_dummy pass this down.
> 
> 2018-10-01  Alan Hayward  <alan.hayward@arm.com>
> 

> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 023e8eb453..504b040c2e 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1512,8 +1512,8 @@ pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache *regcache,
>  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,
> +			 int nargs, struct value **args, CORE_ADDR sp,
> +			 int struct_return, int lang_struct_return_unused,

Here this is called "lang_struct_return_unused", but all the other
cases were just "lang_struct_return".

I suppose it'd be clearer if "struct_return" were renamed to
"abi_struct_return", but I empathize with not having
changed it in this patch...

>  			 CORE_ADDR struct_addr)
>  {
>    int argnum;


> --- 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, int struct_return, int lang_struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, lang_struct_return, 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
>  

No documentation, no goddie.  What does the new parameter
mean?  

I'd think that a bool instead of an int would be better.
Or clearer still, an enum, like:

 enum class return_how { STRUCT, NORMAL };

If you want to do a preparatory patch adding the enum and
renaming "struct_return", it'd be awesome, I think.

But I suppose that given the existence of the "int struct_return"
parameter, I can't really complain.  We could always add such
an enum later on and change both parameters at the same time
throughout.  

So feel free to leave it be as is.

Thanks,
Pedro Alves

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

* Re: [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-01 15:53 ` [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
@ 2018-10-09 16:15   ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2018-10-09 16:15 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 10/01/2018 04:52 PM, Alan Hayward wrote:
> Prevent the following int cast causing a segfault on aarch64:
> (gdb) b foo if (int)strcmp(name,"abc") == 0
> (gdb) run
> 
> Instead of incorrectly re-calculating lang_struct_return, reuse
> the version passed in.
> 
> Add common test case using a shared library to ensure FUNC resolving.
> 

Isn't an important point here that the called function should
have no debug info, so that GDB does not know its return
type?  The testcase compiles the library with debug info,
which confuses me.

> gdb/ChangeLog:
> 
> 2018-10-01  Alan Hayward  <alan.hayward@arm.com>
> 
> 	PR gdb/22736:
> 	* aarch64-tdep.c (aarch64_push_dummy_call): Reuse
> 	lang_struct_return.
> 
> gdb/testsuite/ChangeLog:
> 
> 2018-10-01  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                            | 27 +-----
>  gdb/testsuite/gdb.base/condbreak-solib-lib.cc | 21 +++++
>  .../gdb.base/condbreak-solib-main.cc          | 33 +++++++
>  gdb/testsuite/gdb.base/condbreak-solib.exp    | 93 +++++++++++++++++++
>  4 files changed, 149 insertions(+), 25 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 504b040c2e..f6eabc7029 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1513,14 +1513,11 @@ 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, int lang_struct_return_unused,
> +			 int struct_return, int lang_struct_return,
>  			 CORE_ADDR struct_addr)
>  {
>    int argnum;
>    struct aarch64_call_info info;
> -  struct type *func_type;
> -  struct type *return_type;
> -  int lang_struct_return;
>  
>    memset (&info, 0, sizeof (info));
>  
> @@ -1542,27 +1539,7 @@ 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.  */
> 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..02bf450c43
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib-lib.cc
> @@ -0,0 +1,21 @@
> +/* 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)

Line break before cmp3.

> +{
> +  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..43d39d1da4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib-main.cc
> @@ -0,0 +1,33 @@
> +/* 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 bar ()

Write:

int
bar (void)
{


> +{
> +  return 1;
> +}
> +
> +int main (void)

Ditto.

> +{
> +  cmp3 ((char*)"a");

Write:

    cmp3 ((char*) "a");

I.e., space after case.  Or, make cmp3 take a "const char *"
and get rid of the casts?

> +  bar ();
> +  cmp3 ((char*)"a");

Ditto.

> +  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..27639bd109
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/condbreak-solib.exp
> @@ -0,0 +1,93 @@
> +# 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.

Add reference to gdb/22736 here.

> +
> +if { [skip_cplus_tests] } { continue }
> +
> +if {![is_elf_target]} {
> +    return 0
> +}

Why do you need this?

> +
> +if [skip_shlib_tests] {
> +    return 0
> +}
> +
> +set target_size TARGET_UNKNOWN
> +if {[is_lp64_target]} {
> +    set target_size TARGET_LP64
> +} elseif {[is_ilp32_target]} {
> +   set target_size TARGET_ILP32

Something odd with indentation here.

> +} 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}"
> +set lib_so [standard_output_file ${lib_basename}.so]
> +set lib_syms [shlib_symbol_file ${lib_so}]
> +set lib_dlopen [shlib_target_file ${lib_basename}.so]
> +
> +
> +if [get_compiler_info] {
> +    return -1
> +}
> +
> +# Compile a shared library containing cmp3
> +

Missing period.

> +if {[gdb_compile_shlib $libsrc $lib_so {debug c++}] != ""} {
> +    untested "failed to compile shared library"
> +    return
> +}
> +
> +# Compile the main test linked against the shared library

Missing period.

> +
> +set exec_opts [list debug c++ "additional_flags= -I$srcdir/../../include/ -D$target_size\
> + -DSHLIB_NAME\\=\"$lib_dlopen\""]

What's the -I..../include for?  And SHLIB_NAME/$lib_dlopen?  I didn't see
any dlopen in the testcase.

> +
> +if {[prepare_for_testing "failed to prepare" $binfile "$srcfile $srcfile2" $exec_opts]} {
> +    return
> +}
> +
> +gdb_load_shlib ${lib_so}
> +
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return
> +}
> +
> +# Create conditional breakpoint on cmp3(), which will fail the condition.
> +
> +gdb_test "b cmp3 if (int)strcmp(word,\"abc\") == 0" "Breakpoint .*"
> +
> +gdb_test "b bar" "Breakpoint .*"
> +
> +gdb_test "c" "Breakpoint .* bar .*"
> +
> +# Create conditional breakpoint on cmp3(), which will pass the condition.
> +
> +gdb_test "b cmp3 if (int)strcmp(word,\"stuff\") == 0" "Breakpoint .*"
> +
> +gdb_test "c" "Breakpoint .* cmp3 .*"
> +

Duplicate test names here.  See:

  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Thanks,
Pedro Alves

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

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



> On 9 Oct 2018, at 17:10, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/01/2018 04:52 PM, Alan Hayward wrote:
>> This is a reworking of a patch I posted in March.
>> V1 had a long discussion which was then paused to wait for
>> Pedro's IFUNC rewrite.
>> 
>> 
>> Prevent the int cast in 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 either 1) the call has a
>> cast or 2) an error has occured.
> 
> I'd think that "1) the call has a cast" is not accurate.
> If the called function has debug info, then GDB will know
> it's return type.  The issue is that the called function may
> not have debug information, and then GDB does not know
> its return type (so its NULL), and then the only way to
> call the function is to add the cast.  Right?
> 

That makes sense. I’d add that in the above example I’m able to do the
break without a cast and gdb does not segfault - the return type
of the function comes back as an int. 
 
> It kind of sounds like IFUNCs were a red herring then.  :-/

Yeah.

> 
>> You can see this in infcall.c:call_function_by_hand_dummy():
>> 
>>  CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
>> 
>>  if (values_type == NULL)
>>    values_type = default_return_type;
>>  if (values_type == NULL)
>>    {
>>      const char *name = get_function_name (funaddr,
>> 					    name_buf, sizeof (name_buf));
>>      error (_("'%s' has unknown return type; "
>> 	       "cast the call to its declared return type"),
>> 	     name);
>>    }
>> 
>> In aarch64_push_dummy_call we do not have default_return_type, so cannot
>> determine between the two cases.
>> 
>> (In addition, aarch64_push_dummy_call incorrectly resolves the return
>> type for IFUNC).
> 
> Can you expand a bit on this IFUNC remark?

Have a look at find_function_addr in infcall.c there is a section of code
beginning "if (TYPE_GNU_IFUNC (ftype))”. That is missing from the aarch64
code.

> 
> 
>> However, aarch64_push_dummy_call only requires the return value in order
>> to calculate lang_struct_return ... which has previously been calculated
>> in the caller:
>> 
>>     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.
>> 
> 
> Ah, nice, the solution was right there.  :-)
> 
>> The fix is simple:
>> Patch 1: Update gdbarch interface to pass lang_struct_return.
>> Patch 2: Remove incorrect code and use the passed in lang_struct_return.
>> 
> 
> Since cover letters don't end up in git, this info should be
> somehow migrated into the commit logs of the two patches.

Ok, I’ll merge it into the logs in the next version.

I’ll take a look at your other comments tomorrow.

Thanks for the review,
Alan.



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

* Re: [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-09 17:50   ` Alan Hayward
@ 2018-10-10  8:23     ` Pedro Alves
  2018-10-10 11:54       ` Alan Hayward
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2018-10-10  8:23 UTC (permalink / raw)
  To: Alan Hayward; +Cc: GDB Patches, nd

On 10/09/2018 06:50 PM, Alan Hayward wrote:
> 
> 
>> On 9 Oct 2018, at 17:10, Pedro Alves <palves@redhat.com> wrote:
>>
>> On 10/01/2018 04:52 PM, Alan Hayward wrote:
>>> This is a reworking of a patch I posted in March.
>>> V1 had a long discussion which was then paused to wait for
>>> Pedro's IFUNC rewrite.
>>>
>>>
>>> Prevent the int cast in 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 either 1) the call has a
>>> cast or 2) an error has occured.
>>
>> I'd think that "1) the call has a cast" is not accurate.
>> If the called function has debug info, then GDB will know
>> it's return type.  The issue is that the called function may
>> not have debug information, and then GDB does not know
>> its return type (so its NULL), and then the only way to
>> call the function is to add the cast.  Right?
>>
> 
> That makes sense. I’d add that in the above example I’m able to do the
> break without a cast and gdb does not segfault - the return type
> of the function comes back as an int. 

Please double check whether the proposed testcase crashes GDB without
the fix.  I suspect not, due to the library being compiled with
debug info.  If it does crash, then I think I'm missing something.

Thanks,
Pedro Alves

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

* Re: [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call
  2018-10-09 16:14   ` Pedro Alves
@ 2018-10-10 11:54     ` Alan Hayward
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Hayward @ 2018-10-10 11:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, nd



> On 9 Oct 2018, at 17:14, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/01/2018 04:52 PM, Alan Hayward wrote:
>> Make call_function_by_hand_dummy pass this down.
>> 
>> 2018-10-01  Alan Hayward  <alan.hayward@arm.com>
>> 
> 
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 023e8eb453..504b040c2e 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -1512,8 +1512,8 @@ pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache *regcache,
>> 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,
>> +			 int nargs, struct value **args, CORE_ADDR sp,
>> +			 int struct_return, int lang_struct_return_unused,
> 
> Here this is called "lang_struct_return_unused", but all the other
> cases were just "lang_struct_return".
> 
> I suppose it'd be clearer if "struct_return" were renamed to
> "abi_struct_return", but I empathize with not having
> changed it in this patch…
> 

This was because lang_struct_return is already a local parameter within aarch64_push_dummy_call.
The next patch renames the function arg back to lang_struct_return.

(However, if I do the enum below it should remove that issue).

>> 			 CORE_ADDR struct_addr)
>> {
>>   int argnum;
> 
> 
>> --- 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, int struct_return, int lang_struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, lang_struct_return, 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
>> 
> 
> No documentation, no goddie.  What does the new parameter
> mean?  
> 
> I'd think that a bool instead of an int would be better.
> Or clearer still, an enum, like:
> 
> enum class return_how { STRUCT, NORMAL };
> 
> If you want to do a preparatory patch adding the enum and
> renaming "struct_return", it'd be awesome, I think.

I considered doing this but didn’t want to add a global enum to gdbarch.h
for just a single use. Agreed it is cleaner. I’ll fix it up for v3.


> 
> But I suppose that given the existence of the "int struct_return"
> parameter, I can't really complain.  We could always add such
> an enum later on and change both parameters at the same time
> throughout.  
> 
> So feel free to leave it be as is.
> 
> Thanks,
> Pedro Alves


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

* Re: [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls
  2018-10-10  8:23     ` Pedro Alves
@ 2018-10-10 11:54       ` Alan Hayward
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Hayward @ 2018-10-10 11:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, nd



> On 10 Oct 2018, at 09:23, Pedro Alves <palves@redhat.com> wrote:
> 
> On 10/09/2018 06:50 PM, Alan Hayward wrote:
>> 
>> 
>>> On 9 Oct 2018, at 17:10, Pedro Alves <palves@redhat.com> wrote:
>>> 
>>> On 10/01/2018 04:52 PM, Alan Hayward wrote:
>>>> This is a reworking of a patch I posted in March.
>>>> V1 had a long discussion which was then paused to wait for
>>>> Pedro's IFUNC rewrite.
>>>> 
>>>> 
>>>> Prevent the int cast in 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 either 1) the call has a
>>>> cast or 2) an error has occured.
>>> 
>>> I'd think that "1) the call has a cast" is not accurate.
>>> If the called function has debug info, then GDB will know
>>> it's return type.  The issue is that the called function may
>>> not have debug information, and then GDB does not know
>>> its return type (so its NULL), and then the only way to
>>> call the function is to add the cast.  Right?
>>> 
>> 
>> That makes sense. I’d add that in the above example I’m able to do the
>> break without a cast and gdb does not segfault - the return type
>> of the function comes back as an int. 
> 
> Please double check whether the proposed testcase crashes GDB without
> the fix.  I suspect not, due to the library being compiled with
> debug info.  If it does crash, then I think I'm missing something.
> 

Checking this on latest clean head:

b cmp3 if (int)strcmp(word,"stuff") == 0
 - segfault

b cmp3 if strcmp(word,"stuff") == 0
 - works. Gdb complains that strcmp has unknown return type.

b bar if cmp3("stuff") == 0
 - works.

b bar if (char)cmp3("stuff") == 0
 - works.

I think I’ve gotten a little muddled when writing my test. I should instead
be compiling the library without debug info. And then do a conditional
break using cmp3 instead of strcmp. I’ll rework this in a v3.







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

end of thread, other threads:[~2018-10-10 11:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-01 15:53 [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
2018-10-01 15:53 ` [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call Alan Hayward
2018-10-09 16:14   ` Pedro Alves
2018-10-10 11:54     ` Alan Hayward
2018-10-01 15:53 ` [PATCH v2 2/2] Aarch64: Fix segfault when casting dummy calls Alan Hayward
2018-10-09 16:15   ` Pedro Alves
2018-10-09  8:26 ` [PING][PATCH v2 0/2] " Alan Hayward
2018-10-09 16:10 ` [PATCH " Pedro Alves
2018-10-09 17:50   ` Alan Hayward
2018-10-10  8:23     ` Pedro Alves
2018-10-10 11:54       ` 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).