[rfc] Displaced stepping with wrong entry point address

[rfc] Displaced stepping with wrong entry point address

[Ulrich Weigand]

Hello,

displaced stepping caused a regression for the Cell multi-arch debugger.
This is because the default displaced_step_at_entry_point routine uses
entry_point_address to determine where to put the displaced instruction.

This is a problem when debugging a stand-alone SPU executable using the
multi-arch debugger, because entry_point_address will point to the SPU
entry point, while we need a place in PowerPC address space to execute
displaced PowerPC code.

(SPU currently does not use displaced stepping, and this probably does
not really matter, as a single SPU context is always single-threaded.)


The following patch makes the multi-arch debugger work again, by using
the AT_ENTRY auxiliary vector to find the entry point address.  This
will always point to the PowerPC-side entry point.

I'm wondering whether this fix would be good for the general case too
-- there may be situations where entry_point_address does not work
(e.g. because the main executable file could not be loaded).  The
auxiliary vector, on targets where it is present, will probably be
more reliable ...

What do you think?

Tested on powerpc-linux and powerpc64-linux.

Bye,
Ulrich


ChangeLog:

	* arch-utils.c: Include "target.h", "auxv.h" and "elf/common.h".
	(displaced_step_at_entry_point): Use AT_ENTRY auxiliary vector
	to determine entry point address.

diff -urNp src-orig/gdb/arch-utils.c src/gdb/arch-utils.c
--- src-orig/gdb/arch-utils.c	2008-08-18 02:34:56.000000000 +0200
+++ src/gdb/arch-utils.c	2008-08-18 02:37:42.288892324 +0200
@@ -32,6 +32,9 @@
 #include "osabi.h"
 #include "target-descriptions.h"
 #include "objfiles.h"
+#include "target.h"
+#include "auxv.h"
+#include "elf/common.h"
 
 #include "version.h"
 
@@ -74,7 +77,10 @@ displaced_step_at_entry_point (struct gd
   CORE_ADDR addr;
   int bp_len;
 
-  addr = entry_point_address ();
+  /* Determine entry point from target auxiliary vector.  Fall back
+     to entry point from symbol file if not found.  */
+  if (target_auxv_search (&current_target, AT_ENTRY, &addr) <= 0)
+    addr = entry_point_address ();
 
   /* Make certain that the address points at real code, and not a
      function descriptor.  */
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[ping] Displaced stepping with wrong entry point address

[Ulrich Weigand]

Hello,

any comments on this?
http://sourceware.org/ml/gdb-patches/2008-08/msg00588.html

I'm wondering if it would be preferable to just use a ppc-linux
specific version of the displaced_step routine instead of 
modifying the default ...

Thanks,
Ulrich


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Displaced stepping with wrong entry point address

[Daniel Jacobowitz]

On Fri, Aug 22, 2008 at 07:48:46PM +0200, Ulrich Weigand wrote:
> I'm wondering whether this fix would be good for the general case too
> -- there may be situations where entry_point_address does not work
> (e.g. because the main executable file could not be loaded).  The
> auxiliary vector, on targets where it is present, will probably be
> more reliable ...

We really ought to cache this value; you'll go back and forth to the
target to read the auxv vector at every singlestep.

If SPU ever did support displaced stepping (not that this would be
terribly useful, but consider some other multi-architecture case),
would this be wrong for the SPU side code?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Displaced stepping with wrong entry point address

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Fri, Aug 22, 2008 at 07:48:46PM +0200, Ulrich Weigand wrote:
> > I'm wondering whether this fix would be good for the general case too
> > -- there may be situations where entry_point_address does not work
> > (e.g. because the main executable file could not be loaded).  The
> > auxiliary vector, on targets where it is present, will probably be
> > more reliable ...
> 
> We really ought to cache this value; you'll go back and forth to the
> target to read the auxv vector at every singlestep.

Good point.  I guess the cache would need to be reset from within a
inferior_created observer?  I'll give it a try ...
 
> If SPU ever did support displaced stepping (not that this would be
> terribly useful, but consider some other multi-architecture case),
> would this be wrong for the SPU side code?

Yes, of course -- we have different address spaces here, and we need
to find a location within the address space of the current thread where
to place the displaced instruction.  No single address would work for
both PowerPC and SPU code in a combined application.

But I guess SPU could always install its own callback to handle those
special cases ...  (just as we install the ON_STACK dummy call location
method because the AT_ENTRY method doesn't work for combined applications.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Displaced stepping with wrong entry point address

[Daniel Jacobowitz]

On Thu, Sep 04, 2008 at 09:28:06PM +0200, Ulrich Weigand wrote:
> > If SPU ever did support displaced stepping (not that this would be
> > terribly useful, but consider some other multi-architecture case),
> > would this be wrong for the SPU side code?
> 
> Yes, of course -- we have different address spaces here, and we need
> to find a location within the address space of the current thread where
> to place the displaced instruction.  No single address would work for
> both PowerPC and SPU code in a combined application.
> 
> But I guess SPU could always install its own callback to handle those
> special cases ...  (just as we install the ON_STACK dummy call location
> method because the AT_ENTRY method doesn't work for combined applications.)

In that case, maybe this is really specific to PowerPC; it sounds like
it won't work for any general multi-architecture target.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Displaced stepping with wrong entry point address

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Thu, Sep 04, 2008 at 09:28:06PM +0200, Ulrich Weigand wrote:
> > > If SPU ever did support displaced stepping (not that this would be
> > > terribly useful, but consider some other multi-architecture case),
> > > would this be wrong for the SPU side code?
> > 
> > Yes, of course -- we have different address spaces here, and we need
> > to find a location within the address space of the current thread where
> > to place the displaced instruction.  No single address would work for
> > both PowerPC and SPU code in a combined application.
> > 
> > But I guess SPU could always install its own callback to handle those
> > special cases ...  (just as we install the ON_STACK dummy call location
> > method because the AT_ENTRY method doesn't work for combined applications.)
> 
> In that case, maybe this is really specific to PowerPC; it sounds like
> it won't work for any general multi-architecture target.

OK, agreed.  I'll make this specific to PowerPC as part of the Cell
multi-arch support (b.t.w. I'll be posting an initial patch set for
that really soon now, I hope ...)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Displaced stepping with wrong entry point address

[Mark Kettenis]

> Date: Thu, 4 Sep 2008 21:28:06 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> 
> 
> But I guess SPU could always install its own callback to handle those
> special cases ...  (just as we install the ON_STACK dummy call location
> method because the AT_ENTRY method doesn't work for combined applications.)

Now you mention it, it would be nice if we could change the SPU code
such that it doesn't use generic_push_dummy_code().  This function
abuses frame_align(), and it would be nice if we could eliminate it.

Something like i386_dicos_push_dummy_code() is needed.  I can roll you
a diff if you want, but I won't be able to test it.

Thanks,

Mark

[commit, spu] Do not use generic_push_dummy_code

[Ulrich Weigand]

Mark Kettenis wrote:

> Now you mention it, it would be nice if we could change the SPU code
> such that it doesn't use generic_push_dummy_code().  This function
> abuses frame_align(), and it would be nice if we could eliminate it.
> 
> Something like i386_dicos_push_dummy_code() is needed.  I can roll you
> a diff if you want, but I won't be able to test it.

No problem.  The following patch adds a spu_push_dummy_code function.
If you want to eliminate generic_push_dummy_code now, that would
certainly be fine with me ...

Tested with no regressions on spu-elf.
Committed to mainline.

Bye,
Ulrich


ChangeLog:

	* spu-tdep.c (spu_push_dummy_code): New function.
	(spu_gdbarch_init): Install it.


Index: gdb/spu-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.c,v
retrieving revision 1.39
diff -c -p -r1.39 spu-tdep.c
*** gdb/spu-tdep.c	5 Sep 2008 11:37:17 -0000	1.39
--- gdb/spu-tdep.c	5 Sep 2008 15:06:47 -0000
*************** spu_frame_align (struct gdbarch *gdbarch
*** 1032,1037 ****
--- 1032,1053 ----
    return sp & ~15;
  }
  
+ static CORE_ADDR
+ spu_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)
+ {
+   /* Allocate space sufficient for a breakpoint, keeping the stack aligned.  */
+   sp = (sp - 4) & ~15;
+   /* Store the address of that breakpoint */
+   *bp_addr = sp;
+   /* The call starts at the callee's entry point.  */
+   *real_pc = funaddr;
+ 
+   return sp;
+ }
+ 
  static int
  spu_scalar_value_p (struct type *type)
  {
*************** spu_gdbarch_init (struct gdbarch_info in
*** 2108,2113 ****
--- 2124,2130 ----
    set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
    set_gdbarch_frame_align (gdbarch, spu_frame_align);
    set_gdbarch_frame_red_zone_size (gdbarch, 2000);
+   set_gdbarch_push_dummy_code (gdbarch, spu_push_dummy_code);
    set_gdbarch_push_dummy_call (gdbarch, spu_push_dummy_call);
    set_gdbarch_dummy_id (gdbarch, spu_dummy_id);
    set_gdbarch_return_value (gdbarch, spu_return_value);


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com