[patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[Jan Kratochvil]

Hi,

this is Mark's variant of:
	[patch 2/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5
	http://sourceware.org/ml/gdb-patches/2012-03/msg00358.html

It fixes regression of gdb2495.exp on modern systems featuring .eh_frame for
.plt entries
	http://sourceware.org/bugzilla/show_bug.cgi?id=12570
and also for systems already featuring .eh_frame for _start; therefore having
no safe place with guaranteed no unwinding info.

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.


Thanks,
Jan


2012-06-13  Mark Kettenis  <kettenis@gnu.org>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Switch i386 and derived targets to ON_STACK.
	* amd64-dicos-tdep.c (amd64_dicos_push_dummy_code): Remove.
	(amd64_dicos_init_abi): Remove its installment.
	* dicos-tdep.c (dicos_init_abi): Remove the
	set_gdbarch_call_dummy_location call.  Update the comment here.
	* i386-dicos-tdep.c (i386_dicos_push_dummy_code): Remove.
	(i386_dicos_init_abi): Remove its installment.
	* i386-tdep.c (i386_push_dummy_code): New function.
	(i386_gdbarch_init): Call set_gdbarch_call_dummy_location, install
	i386_push_dummy_code.

--- a/gdb/amd64-dicos-tdep.c
+++ b/gdb/amd64-dicos-tdep.c
@@ -23,24 +23,6 @@
 #include "amd64-tdep.h"
 #include "dicos-tdep.h"
 
-static CORE_ADDR
-amd64_dicos_push_dummy_code (struct gdbarch *gdbarch,
-			     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)
-{
-  int bplen;
-  CORE_ADDR bppc = sp;
-
-  gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen);
-  *bp_addr = sp - bplen;
-  *real_pc = funaddr;
-
-  return *bp_addr;
-}
-
 static void
 amd64_dicos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -49,8 +31,6 @@ amd64_dicos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   amd64_init_abi (info, gdbarch);
 
   dicos_init_abi (gdbarch);
-
-  set_gdbarch_push_dummy_code (gdbarch, amd64_dicos_push_dummy_code);
 }
 
 static enum gdb_osabi
--- a/gdb/dicos-tdep.c
+++ b/gdb/dicos-tdep.c
@@ -43,8 +43,8 @@ dicos_init_abi (struct gdbarch *gdbarch)
 
   /* There's no (standard definition of) entry point or a guaranteed
      text location with a symbol where to place the call dummy, so we
-     put it on the stack.  */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+     need it on the stack.  Rely on i386_gdbarch_init used also for
+     amd64 to set up ON_STACK inferior calls.  */
 
   /* DICOS rewinds the PC itself.  */
   set_gdbarch_decr_pc_after_break (gdbarch, 0);
--- a/gdb/i386-dicos-tdep.c
+++ b/gdb/i386-dicos-tdep.c
@@ -22,32 +22,12 @@
 #include "gdb_string.h"
 #include "dicos-tdep.h"
 
-static CORE_ADDR
-i386_dicos_push_dummy_code (struct gdbarch *gdbarch,
-			    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)
-{
-  int bplen;
-  CORE_ADDR bppc = sp;
-
-  gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen);
-  *bp_addr = sp - bplen;
-  *real_pc = funaddr;
-
-  return *bp_addr;
-}
-
 static void
 i386_dicos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   dicos_init_abi (gdbarch);
-
-  set_gdbarch_push_dummy_code (gdbarch, i386_dicos_push_dummy_code);
 }
 
 static enum gdb_osabi
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2332,6 +2332,22 @@ i386_16_byte_align_p (struct type *type)
   return 0;
 }
 
+/* Implementation for set_gdbarch_push_dummy_code.  */
+
+static CORE_ADDR
+i386_push_dummy_code (struct gdbarch *gdbarch, 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)
+{
+  /* Use 0xcc breakpoint - 1 byte.  */
+  *bp_addr = sp - 1;
+  *real_pc = funaddr;
+
+  /* Keep the stack aligned.  */
+  return sp - 16;
+}
+
 static CORE_ADDR
 i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		      struct regcache *regcache, CORE_ADDR bp_addr, int nargs,
@@ -7707,6 +7723,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_get_longjmp_target (gdbarch, i386_get_longjmp_target);
 
   /* Call dummy code.  */
+  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+  set_gdbarch_push_dummy_code (gdbarch, i386_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, i386_frame_align);
 

Possible future NX ON_STACk regression Re: [patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[Jan Kratochvil]

On Wed, 13 Jun 2012 17:51:03 +0200, Jan Kratochvil wrote:
> +  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);

In https://bugzilla.redhat.com/show_bug.cgi?id=832534 Nathan Sidwell reports
that inferior calls segfault with inferior non-executable stack and Fedora GDB
already containing this patch.

I do not have it reproducible, I guess it may be CPU firmware dependent.


Regards,
Jan

Re: Possible future NX ON_STACk regression Re: [patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[Jan Kratochvil]

On Mon, 18 Jun 2012 10:09:07 +0200, Jan Kratochvil wrote:
> In https://bugzilla.redhat.com/show_bug.cgi?id=832534 Nathan Sidwell reports
> that inferior calls segfault with inferior non-executable stack and Fedora GDB
> already containing this patch.

I have heuristically found the problem affects only one Fedora kernel
version/arch:
	https://bugzilla.redhat.com/show_bug.cgi?id=832534#c9

So the GDB patch should be compatible with Linux kernels in general I hope.
Feel free to run the testcase on your kernel/CPU:
	http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ret-to-nxpage.c?cvsroot=systemtap
	gcc -o ret-to-nxpage ret-to-nxpage.c -Wall -g; ./ret-to-nxpage; echo $?
	0 == PASS


Regards,
Jan

Re: Possible future NX ON_STACk regression Re: [patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[Mark Kettenis]

> Date: Mon, 18 Jun 2012 16:20:55 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Mon, 18 Jun 2012 10:09:07 +0200, Jan Kratochvil wrote:
> > In https://bugzilla.redhat.com/show_bug.cgi?id=832534 Nathan Sidwell reports
> > that inferior calls segfault with inferior non-executable stack and Fedora GDB
> > already containing this patch.
> 
> I have heuristically found the problem affects only one Fedora kernel
> version/arch:
> 	https://bugzilla.redhat.com/show_bug.cgi?id=832534#c9
> 
> So the GDB patch should be compatible with Linux kernels in general I hope.
> Feel free to run the testcase on your kernel/CPU:
> 	http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ret-to-nxpage.c?cvsroot=systemtap
> 	gcc -o ret-to-nxpage ret-to-nxpage.c -Wall -g; ./ret-to-nxpage; echo $?
> 	0 == PASS

OK, go for it then.

[commit] Re: Possible future NX ON_STACk regression Re: [patch 3/3] Use ON_STACK for i386/amd64 (gdb2495.exp regression)

[Jan Kratochvil]

On Mon, 18 Jun 2012 16:51:30 +0200, Mark Kettenis wrote:
> > So the GDB patch should be compatible with Linux kernels in general I hope.
[...]
> OK, go for it then.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2012-06/msg00140.html

gdb/
	Switch i386 and derived targets to ON_STACK.
	* amd64-dicos-tdep.c (amd64_dicos_push_dummy_code): Remove.
	(amd64_dicos_init_abi): Remove its installment.
	* dicos-tdep.c (dicos_init_abi): Remove the
	set_gdbarch_call_dummy_location call.  Update the comment here.
	* i386-dicos-tdep.c (i386_dicos_push_dummy_code): Remove.
	(i386_dicos_init_abi): Remove its installment.
	* i386-tdep.c (i386_push_dummy_code): New function.
	(i386_gdbarch_init): Call set_gdbarch_call_dummy_location, install
	i386_push_dummy_code.


Thanks,
Jan