PowerPC64 skip_prologue patch

PowerPC64 skip_prologue patch

[David Edelsohn]

	What is the status of the PowerPC64 skip_prologue patch?  Without
this patch, GDB does not recognize standard PowerPC64 prologue
instructions which makes debugging very difficult.

Thanks, David


diff -Naur -X Diffs/diffX binGDB.orig/gdb/rs6000-tdep.c binGDB/gdb/rs6000-tdep.c
--- binGDB.orig/gdb/rs6000-tdep.c	Wed Feb 27 12:24:06 2002
+++ binGDB/gdb/rs6000-tdep.c	Wed Mar  6 12:48:10 2002
@@ -444,6 +438,7 @@
   int minimal_toc_loaded = 0;
   int prev_insn_was_prologue_insn = 1;
   int num_skip_non_prologue_insns = 0;
+  int wordsize = TDEP->wordsize;
 
   /* Attempt to find the end of the prologue when no limit is specified.
      Note that refine_prologue_limit() has been written so that it may
@@ -489,13 +484,13 @@
 
       if ((op & 0xfc1fffff) == 0x7c0802a6)
 	{			/* mflr Rx */
-	  lr_reg = (op & 0x03e00000) | 0x90010000;
+	  lr_reg = (op & 0x03e00000);
 	  continue;
 
 	}
       else if ((op & 0xfc1fffff) == 0x7c000026)
 	{			/* mfcr Rx */
-	  cr_reg = (op & 0x03e00000) | 0x90010000;
+	  cr_reg = (op & 0x03e00000);
 	  continue;
 
 	}
@@ -521,7 +516,7 @@
 	    {
 	      fdata->saved_gpr = reg;
 	      if ((op & 0xfc1f0003) == 0xf8010000)
-		op = (op >> 1) << 1;
+		op &= ~3UL;
 	      fdata->gpr_offset = SIGNED_SHORT (op) + offset;
 	    }
 	  continue;
@@ -553,19 +548,49 @@
 	  continue;
 
 	}
-      else if (lr_reg != -1 && (op & 0xffff0000) == lr_reg)
-	{			/* st Rx,NUM(r1) 
-				   where Rx == lr */
-	  fdata->lr_offset = SIGNED_SHORT (op) + offset;
+      else if (lr_reg != -1 &&
+	       /* std Rx || stdu Rx */
+	       (((op & 0xffff0000) == (lr_reg | 0xf8010000)) ||
+		/* stw Rx */
+		((op & 0xffff0000) == (lr_reg | 0x90010000)) ||
+		/* stwu Rx */
+		((op & 0xffff0000) == (lr_reg | 0x94010000))))
+	{	/* where Rx == lr */
+	  fdata->lr_offset = offset;
 	  fdata->nosavedpc = 0;
 	  lr_reg = 0;
+	  if ((op & 0xfc000003) == 0xf8000000 ||	/* std Rx */
+	      (op & 0xfc000000) == 0x90000000)		/* stw Rx */
+	    {
+	      /* does not update r1 add d to lr_offset */
+	      fdata->lr_offset = SIGNED_SHORT (op);
+	    }
 	  continue;
 
 	}
-      else if (cr_reg != -1 && (op & 0xffff0000) == cr_reg)
-	{			/* st Rx,NUM(r1) 
+      else if (cr_reg != -1 &&
+	       /* std Rx || stdu Rx */
+	       (((op & 0xffff0000) == (cr_reg | 0xf8010000)) ||
+		/* stw Rx */
+		((op & 0xffff0000) == (cr_reg | 0x90010000)) ||
+		/* stwu Rx */
+		((op & 0xffff0000) == (cr_reg | 0x94010000))))
+	{	/* where Rx == cr */
+	  fdata->cr_offset = offset;
+	  cr_reg = 0;
+	  if ((op & 0xfc000003) == 0xf8000000 ||
+	      (op & 0xfc000000) == 0x90000000)
+	    {
+	      /* does not update r1 add d to cr_offset */
+	      fdata->cr_offset += SIGNED_SHORT (op);
+	    }
+	  continue;
+
+	}
+      else if (cr_reg != -1 && (op & 0xffff0003) == cr_reg)
+	{			/* std Rx,NUM(r1) || stdu Rx,NUM(r1) 
 				   where Rx == cr */
-	  fdata->cr_offset = SIGNED_SHORT (op) + offset;
+	  fdata->cr_offset = SIGNED_SHORT (op & ~3UL) + offset;
 	  cr_reg = 0;
 	  continue;
 
@@ -619,30 +644,41 @@
 				   this branch */
 	  continue;
 
-	  /* update stack pointer */
 	}
-      else if ((op & 0xffff0000) == 0x94210000 ||	/* stu r1,NUM(r1) */
-	       (op & 0xffff0003) == 0xf8210001)		/* stdu r1,NUM(r1) */
-	{
+      /* update stack pointer */
+      else if ((op & 0xfc1f0000) == 0x94010000)
+	{		/* stu rX,NUM(r1) ||  stwu rX,NUM(r1) */
 	  fdata->frameless = 0;
-	  if ((op & 0xffff0003) == 0xf8210001)
-	    op = (op >> 1) << 1;
 	  fdata->offset = SIGNED_SHORT (op);
 	  offset = fdata->offset;
 	  continue;
-
 	}
-      else if (op == 0x7c21016e)
-	{			/* stwux 1,1,0 */
+      else if ((op & 0xfc1f016a) == 0x7c01016e)
+	{			/* stwux rX,r1,rY */
+	  /* no way to figure out what r1 is going to be */
 	  fdata->frameless = 0;
 	  offset = fdata->offset;
 	  continue;
-
-	  /* Load up minimal toc pointer */
 	}
-      else if ((op >> 22) == 0x20f
+      else if ((op & 0xfc1f0003) == 0xf8010001)
+	{			/* stdu rX,NUM(r1) */
+	  fdata->frameless = 0;
+	  fdata->offset = SIGNED_SHORT (op & ~3UL);
+	  offset = fdata->offset;
+	  continue;
+	}
+      else if ((op & 0xfc1f016a) == 0x7c01016a)
+	{			/* stdux rX,r1,rY */
+	  /* no way to figure out what r1 is going to be */
+	  fdata->frameless = 0;
+	  offset = fdata->offset;
+	  continue;
+	}
+      /* Load up minimal toc pointer */
+      else if (((op >> 22) == 0x20f	||	/* l r31,... or l r30,... */
+	       (op >> 22) == 0x3af)		/* ld r31,... or ld r30,... */
 	       && !minimal_toc_loaded)
-	{			/* l r31,... or l r30,... */
+	{
 	  minimal_toc_loaded = 1;
 	  continue;
 
@@ -842,9 +878,6 @@
 		       int nargs, struct value **args, struct type *type,
 		       int gcc_p)
 {
-#define	TOC_ADDR_OFFSET		20
-#define	TARGET_ADDR_OFFSET	28
-
   int ii;
   CORE_ADDR target_addr;
 
@@ -975,7 +1008,8 @@
 ran_out_of_registers_for_arguments:
 
   saved_sp = read_sp ();
-#ifndef ELF_OBJECT_FORMAT
+
+#if !defined (ELF_OBJECT_FORMAT) || defined (ELF64_OBJECT_FORMAT)
   /* location for 8 parameters are always reserved. */
   sp -= wordsize * 8;
 
@@ -984,7 +1018,7 @@
 
   /* stack pointer must be quadword aligned */
   sp &= -16;
-#endif
+#endif /* ! ELF_OBJECT_FORMAT || ELF64_OBJECT_FORMAT */
 
   /* if there are more arguments, allocate space for them in 
      the stack, then push them starting from the ninth one. */
@@ -1284,6 +1318,16 @@
       if (fi->next->signal_handler_caller)
 	return read_memory_addr (fi->next->frame + SIG_FRAME_LR_OFFSET,
 				 wordsize);
+      else if (wordsize == 8)
+	{
+	  /* FIXME: If this is ever called, cause skip prologue should
+	     get it right, then on a 64-bit target DEFAULT_LR_SAVE is
+	     different (should be 16 not 8), if target is 32 bits then
+	     we let the definition decide since it is ABI
+	     dependent. */
+	  return read_memory_addr (FRAME_CHAIN (fi) + 16,
+				     wordsize);
+	}
       else
 	return read_memory_addr (FRAME_CHAIN (fi) + DEFAULT_LR_SAVE,
 				 wordsize);

Re: PowerPC64 skip_prologue patch

[Kevin Buettner]

On Apr 23, 11:41pm, David Edelsohn wrote:

> 	What is the status of the PowerPC64 skip_prologue patch?  Without
> this patch, GDB does not recognize standard PowerPC64 prologue
> instructions which makes debugging very difficult.

I'm working on it.  Since we don't have a copyright assignment for
these changes, I've been told that I should pseudo-cleanroom it. 
Basically, what I'm going to do is look at the testsuite failures that
occur on PowerPC64 without the patch that you've posted, do my own
analysis, and create my own fixes.  I fully expect that the patches
that I come up with will look remarkably similar to the ones that
you've posted.  (Sigh.)

BTW, the patch you've just posted has more than just skip_prologue()
changes in it.  The following portions have already been dealt with:

> @@ -842,9 +878,6 @@
>  		       int nargs, struct value **args, struct type *type,
>  		       int gcc_p)
>  {
> -#define	TOC_ADDR_OFFSET		20
> -#define	TARGET_ADDR_OFFSET	28
> -
>    int ii;
>    CORE_ADDR target_addr;
>  

The above was taken care of by:

2002-04-08  Kevin Buettner  <kevinb@redhat.com>

	From Jimi X <jimix@watson.ibm.com>:
	* rs6000-tdep.c (rs6000_fix_call_dummy): Delete unused macro
	definitions for TOC_ADDR_OFFSET and TARGET_ADDR_OFFSET.


> @@ -975,7 +1008,8 @@
>  ran_out_of_registers_for_arguments:
>  
>    saved_sp = read_sp ();
> -#ifndef ELF_OBJECT_FORMAT
> +
> +#if !defined (ELF_OBJECT_FORMAT) || defined (ELF64_OBJECT_FORMAT)
>    /* location for 8 parameters are always reserved. */
>    sp -= wordsize * 8;
>  
> @@ -984,7 +1018,7 @@
>  
>    /* stack pointer must be quadword aligned */
>    sp &= -16;
> -#endif
> +#endif /* ! ELF_OBJECT_FORMAT || ELF64_OBJECT_FORMAT */
>  
>    /* if there are more arguments, allocate space for them in 
>       the stack, then push them starting from the ninth one. */

For the above patch, I decided that ELF_OBJECT_FORMAT was no
longer needed.  So I did:

2002-04-08  Kevin Buettner  <kevinb@redhat.com>

	* config/powerpc/tm-ppc-eabi.h (ELF_OBJECT_FORMAT): Delete.
	* rs6000-tdep.c (rs6000_push_arguments): Eliminate
	ELF_OBJECT_FORMAT ifdef.

> @@ -1284,6 +1318,16 @@
>        if (fi->next->signal_handler_caller)
>  	return read_memory_addr (fi->next->frame + SIG_FRAME_LR_OFFSET,
>  				 wordsize);
> +      else if (wordsize == 8)
> +	{
> +	  /* FIXME: If this is ever called, cause skip prologue should
> +	     get it right, then on a 64-bit target DEFAULT_LR_SAVE is
> +	     different (should be 16 not 8), if target is 32 bits then
> +	     we let the definition decide since it is ABI
> +	     dependent. */
> +	  return read_memory_addr (FRAME_CHAIN (fi) + 16,
> +				     wordsize);
> +	}
>        else
>  	return read_memory_addr (FRAME_CHAIN (fi) + DEFAULT_LR_SAVE,
>  				 wordsize);

I dealt with this differently too...

2002-04-12  Kevin Buettner  <kevinb@redhat.com>

	* ppc-tdep.h (struct gdbarch_tdep): Add new member ``lr_frame_offset''.
	* rs6000-tdep.c (rs6000_frame_saved_pc): Use ``lr_frame_offset''
	from tdep struct instead of DEFAULT_LR_SAVE.
	(rs6000_gdbarch_init): Initialize ``lr_frame_offset''.
	* config/powerpc/tm-ppc-eabi.h (DEFAULT_LR_SAVE): Delete.
	* config/rs6000/tm-rs6000.h (DEFAULT_LR_SAVE): Delete.

Re: PowerPC64 skip_prologue patch

[David Edelsohn]

>>>>> Kevin Buettner writes:

Kevin> I'm working on it.  Since we don't have a copyright assignment for
Kevin> these changes, I've been told that I should pseudo-cleanroom it. 
Kevin> Basically, what I'm going to do is look at the testsuite failures that
Kevin> occur on PowerPC64 without the patch that you've posted, do my own
Kevin> analysis, and create my own fixes.  I fully expect that the patches
Kevin> that I come up with will look remarkably similar to the ones that
Kevin> you've posted.  (Sigh.)

	The FSF does have copyright assignment from IBM.

David

Re: PowerPC64 skip_prologue patch

[Stan Shebs]

Kevin Buettner wrote:
> 
> On Apr 23, 11:41pm, David Edelsohn wrote:
> 
> >       What is the status of the PowerPC64 skip_prologue patch?  Without
> > this patch, GDB does not recognize standard PowerPC64 prologue
> > instructions which makes debugging very difficult.
> 
> I'm working on it.  Since we don't have a copyright assignment for
> these changes, [...]

Huh?  IBM has an overall corporate assignment, just like Cygnus
had and (I assume, not having seen it) Red Hat has now.

Stan

Re: PowerPC64 skip_prologue patch

[Christopher Faylor]

On Wed, Apr 24, 2002 at 08:34:07AM -0700, Stan Shebs wrote:
>Kevin Buettner wrote:
>>On Apr 23, 11:41pm, David Edelsohn wrote:
>>>What is the status of the PowerPC64 skip_prologue patch?  Without this
>>>patch, GDB does not recognize standard PowerPC64 prologue instructions
>>>which makes debugging very difficult.
>>
>>I'm working on it.  Since we don't have a copyright assignment for
>>these changes, [...]
>
>Huh?  IBM has an overall corporate assignment, just like Cygnus had and
>(I assume, not having seen it) Red Hat has now.

Can you point to where this is in the copyright assignment list?  For
Apple, I see this:

GDB BFD MMALLOC OPCODES Apple Computer, Inc.    1999-01-22
Assigns past and future changes.

I don't see anything remotely similar for IBM.

cgf

Re: PowerPC64 skip_prologue patch

[Andrew Cagney]

> Kevin Buettner wrote:
> 
>> 
>> On Apr 23, 11:41pm, David Edelsohn wrote:
>> 
> 
>> > What is the status of the PowerPC64 skip_prologue patch?  Without
>> > this patch, GDB does not recognize standard PowerPC64 prologue
>> > instructions which makes debugging very difficult.
> 
>> 
>> I'm working on it.  Since we don't have a copyright assignment for
>> these changes, [...]
> 
> 
> Huh?  IBM has an overall corporate assignment, just like Cygnus
> had and (I assume, not having seen it) Red Hat has now.

The last time IBM contributed stuff to GDB was the s390.  For that, 
individual electronic transfer letters were required.

Sounds like things have changed since then?

Andrew

Re: PowerPC64 skip_prologue patch

[David Edelsohn]

>>>>> Andrew Cagney writes:

Andrew> The last time IBM contributed stuff to GDB was the s390.  For that, 
Andrew> individual electronic transfer letters were required.

Andrew> Sounds like things have changed since then?

	The s390 and ppc64 toolchain contributions were approved by IBM a
long time ago.  It would help if you directly, privately contacted IBM
colleagues to inquire about the facts instead of sending obtuse comments
about "streams of patches" and making uninformed decisions.

David

Re: PowerPC64 skip_prologue patch

[Stan Shebs]

Christopher Faylor wrote:
> 
> On Wed, Apr 24, 2002 at 08:34:07AM -0700, Stan Shebs wrote:
> >Kevin Buettner wrote:
> >>On Apr 23, 11:41pm, David Edelsohn wrote:
> >>>What is the status of the PowerPC64 skip_prologue patch?  Without this
> >>>patch, GDB does not recognize standard PowerPC64 prologue instructions
> >>>which makes debugging very difficult.
> >>
> >>I'm working on it.  Since we don't have a copyright assignment for
> >>these changes, [...]
> >
> >Huh?  IBM has an overall corporate assignment, just like Cygnus had and
> >(I assume, not having seen it) Red Hat has now.
> 
> Can you point to where this is in the copyright assignment list?  For
> Apple, I see this:
> 
> GDB BFD MMALLOC OPCODES Apple Computer, Inc.    1999-01-22
> Assigns past and future changes.
> 
> I don't see anything remotely similar for IBM.

Is that all it shows for Apple?  We have a GCC assignment that's
more recent.  My list is out of date, I just remember discussing
IBM's assignment a while back.

Stan

Re: PowerPC64 skip_prologue patch

[Andrew Cagney]

> Andrew Cagney writes:
> 
> 
> Andrew> The last time IBM contributed stuff to GDB was the s390.  For that, 
> Andrew> individual electronic transfer letters were required.
> 
> Andrew> Sounds like things have changed since then?
> 
> 	The s390 and ppc64 toolchain contributions were approved by IBM a
> long time ago.  It would help if you directly, privately contacted IBM
> colleagues to inquire about the facts instead of sending obtuse comments
> about "streams of patches" and making uninformed decisions.

I'm already doing this (I'm obviously contacting the FSF and not IBM 
though).

For reference, the s390 contribution was via:

GDB     IBM Corporation ....
Assigns patch gdb-5.1pre-050901-s390.tar.gz available at the URL
http://....linux390/exp_src.html.
(Software Letter)

Andrew