Fix Ericsson DICOS inferior function calls

Fix Ericsson DICOS inferior function calls

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 234 bytes --]

Hi,

This patch fixes inferior function calls on DICOS, by putting
the call dummy breakpoint location on the stack.  The comments
in the patch describe why.

Daniel approved this offline, so I checked it in.

Thanks,

-- 
Pedro Alves

[-- Attachment #2: dicos_call_dummy_on_stack.diff --]
[-- Type: text/x-diff, Size: 2548 bytes --]

2008-07-07  Pedro Alves  <pedro@codesourcery.com>

	* i386-dicos-tdep.c: Include "inferior.h".
	(i386_dicos_frame_align): New.
	(i386_dicos_init_abi): Register i386_dicos_frame_align.  Set call
	dummy location ON_STACK.
	* Makefile.in (i386-dicos-tdep.o): Depend on $(inferior_h).

---
 gdb/Makefile.in       |    2 +-
 gdb/i386-dicos-tdep.c |   18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-07-07 22:59:22.000000000 +0100
+++ src/gdb/Makefile.in	2008-07-07 23:00:53.000000000 +0100
@@ -2236,7 +2236,7 @@ i386-cygwin-tdep.o: i386-cygwin-tdep.c $
 	$(i386_tdep_h) $(regset_h) $(gdb_obstack_h) $(xml_support_h) \
 	$(gdbcore_h) $(solib_h) $(solib_target_h) $(i386_cygwin_tdep_h)
 i386-dicos-tdep.o: i386-dicos-tdep.c $(defs_h) $(osabi_h) $(gdb_string_h) \
-	$(solib_h) $(solib_target_h)
+	$(solib_h) $(solib_target_h) $(inferior_h)
 i386fbsd-nat.o: i386fbsd-nat.c $(defs_h) $(inferior_h) $(regcache_h) \
 	$(target_h) $(fbsd_nat_h) $(i386_tdep_h) $(i386bsd_nat_h) \
 	$(bsd_kvm_h)
Index: src/gdb/i386-dicos-tdep.c
===================================================================
--- src.orig/gdb/i386-dicos-tdep.c	2008-05-15 23:35:10.000000000 +0100
+++ src/gdb/i386-dicos-tdep.c	2008-07-07 23:00:53.000000000 +0100
@@ -22,6 +22,18 @@
 #include "gdb_string.h"
 #include "solib.h"
 #include "solib-target.h"
+#include "inferior.h"
+
+static CORE_ADDR
+i386_dicos_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
+{
+  /* Having a call dummy on the stack requires a gdbarch_frame_align
+     method to align the breakpoint instruction in the stack.
+     Strictly speaking, we could just return SP pristine on x86.  But,
+     as long as we're providing a frame align method, might as well
+     align for efficiency.  */
+  return sp & -(CORE_ADDR)16;
+}
 
 static void
 i386_dicos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
@@ -29,6 +41,12 @@ i386_dicos_init_abi (struct gdbarch_info
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   set_solib_ops (gdbarch, &solib_target_so_ops);
+
+  /* There's no (standard definition of) entry point or a guaranteed
+     text location we could find with a symbol where to place the call
+     dummy, so we put it on the stack.  */
+  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+  set_gdbarch_frame_align (gdbarch, i386_dicos_frame_align);
 }
 
 /* Look in the elf symbol table of ABFD for a symbol named WANTED.

Re: Fix Ericsson DICOS inferior function calls

[Mark Kettenis]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Mon, 7 Jul 2008 23:09:55 +0100
> 
> Hi,
> 
> This patch fixes inferior function calls on DICOS, by putting
> the call dummy breakpoint location on the stack.  The comments
> in the patch describe why.
> 
> Daniel approved this offline, so I checked it in.

Hmm, the reason for providing frame_align() is pretty bogus;
instruction alignment has little or nothing to do with stack
alignment.  It's just that most RISC ISAs require strict memory
alignment at something that happens to be a multiple of the
instruction size.

I'm also not sure the frame_align() you wrote does what you think it
does.  IIRC, on i386, you'll actually need to align on an odd 8-byte
border to get the performance benefit you mention;
i386_push_dummy_call() should take care of that (but may currently not
do that in all cases).

I think we should get rid of generic_push_dummy_code(), and instead
require that each target that puts the dummy breakpoint on the stack
provides its own push_dummy_code() function.  Could you provide one
for DICOS?

> 2008-07-07  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* i386-dicos-tdep.c: Include "inferior.h".
> 	(i386_dicos_frame_align): New.
> 	(i386_dicos_init_abi): Register i386_dicos_frame_align.  Set call
> 	dummy location ON_STACK.
> 	* Makefile.in (i386-dicos-tdep.o): Depend on $(inferior_h).

Re: Fix Ericsson DICOS inferior function calls

[Pedro Alves]

A Monday 07 July 2008 23:56:21, Mark Kettenis wrote:

> I think we should get rid of generic_push_dummy_code(), and instead
> require that each target that puts the dummy breakpoint on the stack
> provides its own push_dummy_code() function.  Could you provide one
> for DICOS?

Sure, I'll do that.  Thanks!

-- 
Pedro Alves

Re: Fix Ericsson DICOS inferior function calls

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

Hi Mark,

On Monday 07 July 2008 23:56:21, Mark Kettenis wrote:

> I think we should get rid of generic_push_dummy_code(), and instead
> require that each target that puts the dummy breakpoint on the stack
> provides its own push_dummy_code() function.  Could you provide one
> for DICOS?

Something like this?

It looks like the only other target that uses ON_STACK and doesn't
define a push_dummy_code is spu-tdep.c.

-- 
Pedro Alves

[-- Attachment #2: dicos_push_dummy_code.diff --]
[-- Type: text/x-diff, Size: 1930 bytes --]

2008-07-09  Pedro Alves  <pedro@codesourcery.com>

	* i386-dicos-tdep.c (i386_dicos_frame_align): Delete.
	(i386_dicos_push_dummy_code): New.
	(i386_dicos_init_abi): Don't register i386_dicos_frame_align.
	Register i386_dicos_push_dummy_code.

---
 gdb/i386-dicos-tdep.c |   23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Index: src/gdb/i386-dicos-tdep.c
===================================================================
--- src.orig/gdb/i386-dicos-tdep.c	2008-07-09 00:33:38.000000000 +0100
+++ src/gdb/i386-dicos-tdep.c	2008-07-09 00:41:36.000000000 +0100
@@ -25,14 +25,21 @@
 #include "inferior.h"
 
 static CORE_ADDR
-i386_dicos_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
+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)
 {
-  /* Having a call dummy on the stack requires a gdbarch_frame_align
-     method to align the breakpoint instruction in the stack.
-     Strictly speaking, we could just return SP pristine on x86.  But,
-     as long as we're providing a frame align method, might as well
-     align for efficiency.  */
-  return sp & -(CORE_ADDR)16;
+  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
@@ -46,7 +53,7 @@ i386_dicos_init_abi (struct gdbarch_info
      text location we could find with a symbol where to place the call
      dummy, so we put it on the stack.  */
   set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
-  set_gdbarch_frame_align (gdbarch, i386_dicos_frame_align);
+  set_gdbarch_push_dummy_code (gdbarch, i386_dicos_push_dummy_code);
 }
 
 /* Look in the elf symbol table of ABFD for a symbol named WANTED.

Re: Fix Ericsson DICOS inferior function calls

[Mark Kettenis]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 9 Jul 2008 16:48:10 +0100
> 
> Hi Mark,
> 
> On Monday 07 July 2008 23:56:21, Mark Kettenis wrote:
> 
> > I think we should get rid of generic_push_dummy_code(), and instead
> > require that each target that puts the dummy breakpoint on the stack
> > provides its own push_dummy_code() function.  Could you provide one
> > for DICOS?
> 
> Something like this?

Looks good to me.

> It looks like the only other target that uses ON_STACK and doesn't
> define a push_dummy_code is spu-tdep.c.

Let's see if I can convince the IBM guys to do something similar, and
then we can remove the somewhat bogus generic_push_dummy_code().

Thanks,

Mark

Re: Fix Ericsson DICOS inferior function calls

[Pedro Alves]

On Monday 14 July 2008 16:09:14, Mark Kettenis wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Something like this?
> Looks good to me.

Thanks Mark, I've checked it in.

-- 
Pedro Alves