[patch/rfa] fix call-dummies for hppa

[patch/rfa] fix call-dummies for hppa

[Randolph Chung]

This fixes a few bugs in the 32-bit version of the hppa push_dummy_call
method.

Tested against hppa2.0w-hp-hpux11.11 and hppa-linux.

All of structs.exp pass, there are still some failures in call-sc.exp
and callfuncs.exp. In the latter case it appears most are due to
problems with calling malloc() in the inferior (at least on hppa-linux).

ok to apply?

thanks
randolph

2004-04-24  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa32_push_dummy_call): Fix small struct and floating 
	point argument passing, and make sure the stack frame is properly 
	aligned.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.151
diff -u -p -r1.151 hppa-tdep.c
--- hppa-tdep.c	24 Apr 2004 06:13:32 -0000	1.151
+++ hppa-tdep.c	24 Apr 2004 18:51:44 -0000
@@ -758,16 +759,13 @@ hppa64_register_name (int i)
 
    We simply allocate the appropriate amount of stack space and put
    arguments into their proper slots.  */
-   
+
 CORE_ADDR
 hppa32_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR func_addr,
 			struct regcache *regcache, CORE_ADDR bp_addr,
 			int nargs, struct value **args, CORE_ADDR sp,
 			int struct_return, CORE_ADDR struct_addr)
 {
-  /* NOTE: cagney/2004-02-27: This is a guess - its implemented by
-     reverse engineering testsuite failures.  */
-
   /* Stack base address at which any pass-by-reference parameters are
      stored.  */
   CORE_ADDR struct_end = 0;
@@ -781,12 +779,15 @@ hppa32_push_dummy_call (struct gdbarch *
   /* Two passes.  First pass computes the location of everything,
      second pass writes the bytes out.  */
   int write_pass;
+
   for (write_pass = 0; write_pass < 2; write_pass++)
     {
       CORE_ADDR struct_ptr = 0;
-      CORE_ADDR param_ptr = 0;
-      int reg = 27;	      /* NOTE: Registers go down.  */
+      /* The first parameter goes into sp-36, each stack slot is 4-bytes.  */
+      CORE_ADDR param_ptr = 32;
       int i;
+      int small_struct = 0;
+
       for (i = 0; i < nargs; i++)
 	{
 	  struct value *arg = args[i];
@@ -795,6 +796,7 @@ hppa32_push_dummy_call (struct gdbarch *
 	     stack, and [possibly] passed in a register.  */
 	  char param_val[8];
 	  int param_len;
+
 	  memset (param_val, 0, sizeof param_val);
 	  if (TYPE_LENGTH (type) > 8)
 	    {
@@ -813,27 +815,76 @@ hppa32_push_dummy_call (struct gdbarch *
 	      /* Integer value store, right aligned.  "unpack_long"
 		 takes care of any sign-extension problems.  */
 	      param_len = align_up (TYPE_LENGTH (type), 4);
+
 	      store_unsigned_integer (param_val, param_len,
 				      unpack_long (type,
 						   VALUE_CONTENTS (arg)));
 	    }
+	  else if (TYPE_CODE (type) == TYPE_CODE_FLT)
+            {
+	      param_len = align_up (TYPE_LENGTH (type), 4);
+	      memcpy (param_val, VALUE_CONTENTS (arg), param_len);
+            }
 	  else
 	    {
-	      /* Small struct value, store right aligned?  */
 	      param_len = align_up (TYPE_LENGTH (type), 4);
+
+	      /* Small struct value are stored right-aligned.  */
 	      memcpy (param_val + param_len - TYPE_LENGTH (type),
 		      VALUE_CONTENTS (arg), TYPE_LENGTH (type));
+
+	      /* Structures of size 5, 6 and 7 bytes are special in that
+	         the higher-ordered word is stored in the lower-ordered
+		 argument, and even though it is a 8-byte quantity the
+		 registers need not be 8-byte aligned.  */
+	      if (param_len > 4)
+		small_struct = 1;
 	    }
+
 	  param_ptr += param_len;
-	  reg -= param_len / 4;
+	  if (param_len == 8 && !small_struct)
+	    param_ptr += (param_ptr & 4);
+
+	  /* First 4 non-FP arguments are passed in gr26-gr23.
+	     First 4 32-bit FP arguments are passed in fr4L-fr7L.
+	     First 2 64-bit FP arguments are passed in fr5 and fr7.
+
+	     The rest go on the stack, starting at sp-36, towards lower
+	     addresses.  8-byte arguments must be aligned to a 8-byte
+	     stack boundary.  */
 	  if (write_pass)
 	    {
 	      write_memory (param_end - param_ptr, param_val, param_len);
-	      if (reg >= 23)
+
+	      /* There are some cases when we don't know the type
+		 expected by the callee (e.g. for variadic functions), so 
+		 pass the parameters in both general and fp regs.  */
+	      if (param_ptr <= 48)
 		{
-		  regcache_cooked_write (regcache, reg, param_val);
+		  int grreg = 26 - (param_ptr - 36) / 4;
+		  int fpLreg = 72 + (param_ptr - 36) / 4 * 2;
+		  int fpreg = 74 + (param_ptr - 40) / 8 * 4;
+
+		  regcache_cooked_write (regcache, grreg, param_val);
+		  regcache_cooked_write (regcache, fpLreg, param_val);
+
 		  if (param_len > 4)
-		    regcache_cooked_write (regcache, reg + 1, param_val + 4);
+		    {
+		      if (!small_struct)
+			{
+		          regcache_cooked_write (regcache, grreg - 1, 
+						 param_val + 4);
+
+			  regcache_cooked_write (regcache, fpreg, param_val);
+			  regcache_cooked_write (regcache, fpreg + 1, 
+						 param_val + 4);
+			}
+		      else
+			{
+		          regcache_cooked_write (regcache, grreg + 1, 
+						 param_val + 4);
+			}
+		    }
 		}
 	    }
 	}
@@ -841,13 +892,13 @@ hppa32_push_dummy_call (struct gdbarch *
       /* Update the various stack pointers.  */
       if (!write_pass)
 	{
-	  struct_end = sp + struct_ptr;
+	  struct_end = sp + align_up (struct_ptr, 64);
 	  /* PARAM_PTR already accounts for all the arguments passed
 	     by the user.  However, the ABI mandates minimum stack
 	     space allocations for outgoing arguments.  The ABI also
 	     mandates minimum stack alignments which we must
 	     preserve.  */
-	  param_end = struct_end + max (align_up (param_ptr, 8), 16);
+	  param_end = struct_end + align_up (param_ptr, 64);
 	}
     }
 
@@ -860,10 +911,9 @@ hppa32_push_dummy_call (struct gdbarch *
   regcache_cooked_write_unsigned (regcache, RP_REGNUM, bp_addr);
 
   /* Update the Stack Pointer.  */
-  regcache_cooked_write_unsigned (regcache, SP_REGNUM, param_end + 32);
+  regcache_cooked_write_unsigned (regcache, SP_REGNUM, param_end);
 
-  /* The stack will have 32 bytes of additional space for a frame marker.  */
-  return param_end + 32;
+  return param_end;
 }
 
 /* This function pushes a stack frame with arguments as part of the
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfa] fix call-dummies for hppa

[Andrew Cagney]

> This fixes a few bugs in the 32-bit version of the hppa push_dummy_call
> method.
> 
> Tested against hppa2.0w-hp-hpux11.11 and hppa-linux.

Which compilers?  I'm suspicious of GCC - it too often gets struct 
parameters and return values internally consistent but wrong :-(

> All of structs.exp pass, there are still some failures in call-sc.exp
> and callfuncs.exp. In the latter case it appears most are due to
> problems with calling malloc() in the inferior (at least on hppa-linux).

>     We simply allocate the appropriate amount of stack space and put
>     arguments into their proper slots.  */
> -   
> +

Be careful of white space change, this shouldn't be included.  If you 
want to fix some indention just do it separatly.

>  CORE_ADDR
>  hppa32_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR func_addr,
>  			struct regcache *regcache, CORE_ADDR bp_addr,
>  			int nargs, struct value **args, CORE_ADDR sp,
>  			int struct_return, CORE_ADDR struct_addr)
>  {
> -  /* NOTE: cagney/2004-02-27: This is a guess - its implemented by
> -     reverse engineering testsuite failures.  */
> -

(I've now got a copy of the 32-bit ABI but it doesn't help much)

>    /* Stack base address at which any pass-by-reference parameters are
>       stored.  */
>    CORE_ADDR struct_end = 0;
> @@ -781,12 +779,15 @@ hppa32_push_dummy_call (struct gdbarch *
>    /* Two passes.  First pass computes the location of everything,
>       second pass writes the bytes out.  */
>    int write_pass;
> +
>    for (write_pass = 0; write_pass < 2; write_pass++)
>      {
>        CORE_ADDR struct_ptr = 0;
> -      CORE_ADDR param_ptr = 0;
> -      int reg = 27;	      /* NOTE: Registers go down.  */

the comment doesn't match the assignment.

> +      /* The first parameter goes into sp-36, each stack slot is 4-bytes.  */
> +      CORE_ADDR param_ptr = 32;
>        int i;
> +      int small_struct = 0;
> +
>        for (i = 0; i < nargs; i++)
>  	{
>  	  struct value *arg = args[i];
> @@ -795,6 +796,7 @@ hppa32_push_dummy_call (struct gdbarch *
>  	     stack, and [possibly] passed in a register.  */
>  	  char param_val[8];
>  	  int param_len;
> +
>  	  memset (param_val, 0, sizeof param_val);
>  	  if (TYPE_LENGTH (type) > 8)
>  	    {
> @@ -813,27 +815,76 @@ hppa32_push_dummy_call (struct gdbarch *
>  	      /* Integer value store, right aligned.  "unpack_long"
>  		 takes care of any sign-extension problems.  */
>  	      param_len = align_up (TYPE_LENGTH (type), 4);
> +
white space
>  	      store_unsigned_integer (param_val, param_len,
>  				      unpack_long (type,
>  						   VALUE_CONTENTS (arg)));
>  	    }
> +	  else if (TYPE_CODE (type) == TYPE_CODE_FLT)
> +            {

more comments (the rest is well commented), ``&& TYPE_LENGTH () == 4'' 
test needed?

> +	      param_len = align_up (TYPE_LENGTH (type), 4);
> +	      memcpy (param_val, VALUE_CONTENTS (arg), param_len);
> +            }
>  	  else
>  	    {
> -	      /* Small struct value, store right aligned?  */
>  	      param_len = align_up (TYPE_LENGTH (type), 4);
> +
> +	      /* Small struct value are stored right-aligned.  */
>  	      memcpy (param_val + param_len - TYPE_LENGTH (type),
>  		      VALUE_CONTENTS (arg), TYPE_LENGTH (type));
> +
> +	      /* Structures of size 5, 6 and 7 bytes are special in that
> +	         the higher-ordered word is stored in the lower-ordered
> +		 argument, and even though it is a 8-byte quantity the
> +		 registers need not be 8-byte aligned.  */
> +	      if (param_len > 4)
> +		small_struct = 1;
>  	    }
> +
>  	  param_ptr += param_len;
> -	  reg -= param_len / 4;
> +	  if (param_len == 8 && !small_struct)
> +	    param_ptr += (param_ptr & 4);

align_up?

Otherwize, yep, thanks for fixing this.

Andrew

Re: [patch/rfa] fix call-dummies for hppa

[Randolph Chung]

> Which compilers?  I'm suspicious of GCC - it too often gets struct 
> parameters and return values internally consistent but wrong :-(

gcc only; I don't have access to the hp compilers. However, Dave 
(the hppa gcc maintainer) is quite careful about these things, so I 
think they are correct ;-)

> Be careful of white space change, this shouldn't be included.  If you 
> want to fix some indention just do it separatly.

ok, there were some stray tabs in the file so i was cleaning them up
along the way, but i'll remove that from this diff.

> (I've now got a copy of the 32-bit ABI but it doesn't help much)

this is the som runtime doc?  it's not particularly clear about small 
structs.....

> the comment doesn't match the assignment.
> 
> >+      /* The first parameter goes into sp-36, each stack slot is 4-bytes. 
> >*/
> >+      CORE_ADDR param_ptr = 32;

it does, actually, because the param_ptr is incremented by 4 for each
argument, so the first one goes to 36.

> >+	  else if (TYPE_CODE (type) == TYPE_CODE_FLT)
> >+            {
> 
> more comments (the rest is well commented), ``&& TYPE_LENGTH () == 4'' 
> test needed?
> 
> >+	      param_len = align_up (TYPE_LENGTH (type), 4);
> >+	      memcpy (param_val, VALUE_CONTENTS (arg), param_len);

yes, this bit is wrong. i found some more bugs in this function. will
send a new version with the whitespace changes removed and comments
added.

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfa] fix call-dummies for hppa

[Andrew Cagney]

>>> Which compilers?  I'm suspicious of GCC - it too often gets struct 
>>> parameters and return values internally consistent but wrong :-(
> 
> 
> gcc only; I don't have access to the hp compilers. However, Dave 
> (the hppa gcc maintainer) is quite careful about these things, so I 
> think they are correct ;-)
> 
> 
>>> Be careful of white space change, this shouldn't be included.  If you 
>>> want to fix some indention just do it separatly.
> 
> 
> ok, there were some stray tabs in the file so i was cleaning them up
> along the way, but i'll remove that from this diff.
> 
> 
>>> (I've now got a copy of the 32-bit ABI but it doesn't help much)
> 
> 
> this is the som runtime doc?  it's not particularly clear about small 
> structs.....

yes.  the 64-bit one is much better.

>>> the comment doesn't match the assignment.
>>> 
>>
>>>> >+      /* The first parameter goes into sp-36, each stack slot is 4-bytes. 
>>>> >*/
>>>> >+      CORE_ADDR param_ptr = 32;
> 
> 
> it does, actually, because the param_ptr is incremented by 4 for each
> argument, so the first one goes to 36.

ok, just expand the comment to clarify this.

>>>> >+	  else if (TYPE_CODE (type) == TYPE_CODE_FLT)
>>>> >+            {
>>
>>> 
>>> more comments (the rest is well commented), ``&& TYPE_LENGTH () == 4'' 
>>> test needed?
>>> 
>>
>>>> >+	      param_len = align_up (TYPE_LENGTH (type), 4);
>>>> >+	      memcpy (param_val, VALUE_CONTENTS (arg), param_len);
> 
> 
> yes, this bit is wrong. i found some more bugs in this function. will
> send a new version with the whitespace changes removed and comments
> added.

With a comment tweak, ok to commit.

Andrew

Re: [patch/rfa] fix call-dummies for hppa

[Randolph Chung]

In reference to a message from Andrew Cagney, dated Apr 28:
> >This fixes a few bugs in the 32-bit version of the hppa push_dummy_call
> >method.

Updated version, passes all of structs.exp and call-sc.exp on hppa-linux

2004-04-28  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa32_return_value): Handle both 4- and 8-byte fp
	return values.
	(hppa32_push_dummy_call): Fix small struct and floating point argument 
	passing, and make sure the stack frame is properly aligned.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.151
diff -u -p -r1.151 hppa-tdep.c
--- hppa-tdep.c	24 Apr 2004 06:13:32 -0000	1.151
+++ hppa-tdep.c	28 Apr 2004 17:14:54 -0000
@@ -121,22 +121,12 @@ hppa32_return_value (struct gdbarch *gdb
 		     struct type *type, struct regcache *regcache,
 		     void *readbuf, const void *writebuf)
 {
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    {
-      if (readbuf != NULL)
-	regcache_cooked_read_part (regcache, FP4_REGNUM, 0,
-				   TYPE_LENGTH (type), readbuf);
-      if (writebuf != NULL)
-	regcache_cooked_write_part (regcache, FP4_REGNUM, 0,
-				    TYPE_LENGTH (type), writebuf);
-      return RETURN_VALUE_REGISTER_CONVENTION;
-    }
   if (TYPE_LENGTH (type) <= 2 * 4)
     {
       /* The value always lives in the right hand end of the register
 	 (or register pair)?  */
       int b;
-      int reg = 28;
+      int reg = TYPE_CODE (type) == TYPE_CODE_FLT ? FP4_REGNUM : 28;
       int part = TYPE_LENGTH (type) % 4;
       /* The left hand register contains only part of the value,
 	 transfer that first so that the rest can be xfered as entire
@@ -765,9 +755,6 @@ hppa32_push_dummy_call (struct gdbarch *
 			int nargs, struct value **args, CORE_ADDR sp,
 			int struct_return, CORE_ADDR struct_addr)
 {
-  /* NOTE: cagney/2004-02-27: This is a guess - its implemented by
-     reverse engineering testsuite failures.  */
-
   /* Stack base address at which any pass-by-reference parameters are
      stored.  */
   CORE_ADDR struct_end = 0;
@@ -784,9 +771,11 @@ hppa32_push_dummy_call (struct gdbarch *
   for (write_pass = 0; write_pass < 2; write_pass++)
     {
       CORE_ADDR struct_ptr = 0;
-      CORE_ADDR param_ptr = 0;
-      int reg = 27;	      /* NOTE: Registers go down.  */
+      /* The first parameter goes into sp-36, each stack slot is 4-bytes.  */
+      CORE_ADDR param_ptr = 32;
       int i;
+      int small_struct = 0;
+
       for (i = 0; i < nargs; i++)
 	{
 	  struct value *arg = args[i];
@@ -817,23 +806,64 @@ hppa32_push_dummy_call (struct gdbarch *
 				      unpack_long (type,
 						   VALUE_CONTENTS (arg)));
 	    }
+	  else if (TYPE_CODE (type) == TYPE_CODE_FLT)
+            {
+	      /* Floating point value store, right aligned.  */
+	      param_len = align_up (TYPE_LENGTH (type), 4);
+	      memcpy (param_val, VALUE_CONTENTS (arg), param_len);
+            }
 	  else
 	    {
-	      /* Small struct value, store right aligned?  */
 	      param_len = align_up (TYPE_LENGTH (type), 4);
+
+	      /* Small struct value are stored right-aligned.  */
 	      memcpy (param_val + param_len - TYPE_LENGTH (type),
 		      VALUE_CONTENTS (arg), TYPE_LENGTH (type));
+
+	      /* Structures of size 5, 6 and 7 bytes are special in that
+	         the higher-ordered word is stored in the lower-ordered
+		 argument, and even though it is a 8-byte quantity the
+		 registers need not be 8-byte aligned.  */
+	      if (param_len > 4)
+		small_struct = 1;
 	    }
+
 	  param_ptr += param_len;
-	  reg -= param_len / 4;
+	  if (param_len == 8 && !small_struct)
+            param_ptr = align_up (param_ptr, 8);
+
+	  /* First 4 non-FP arguments are passed in gr26-gr23.
+	     First 4 32-bit FP arguments are passed in fr4L-fr7L.
+	     First 2 64-bit FP arguments are passed in fr5 and fr7.
+
+	     The rest go on the stack, starting at sp-36, towards lower
+	     addresses.  8-byte arguments must be aligned to a 8-byte
+	     stack boundary.  */
 	  if (write_pass)
 	    {
 	      write_memory (param_end - param_ptr, param_val, param_len);
-	      if (reg >= 23)
+
+	      /* There are some cases when we don't know the type
+		 expected by the callee (e.g. for variadic functions), so 
+		 pass the parameters in both general and fp regs.  */
+	      if (param_ptr <= 48)
 		{
-		  regcache_cooked_write (regcache, reg, param_val);
+		  int grreg = 26 - (param_ptr - 36) / 4;
+		  int fpLreg = 72 + (param_ptr - 36) / 4 * 2;
+		  int fpreg = 74 + (param_ptr - 32) / 8 * 4;
+
+		  regcache_cooked_write (regcache, grreg, param_val);
+		  regcache_cooked_write (regcache, fpLreg, param_val);
+
 		  if (param_len > 4)
-		    regcache_cooked_write (regcache, reg + 1, param_val + 4);
+		    {
+		      regcache_cooked_write (regcache, grreg + 1, 
+					     param_val + 4);
+
+		      regcache_cooked_write (regcache, fpreg, param_val);
+		      regcache_cooked_write (regcache, fpreg + 1, 
+					     param_val + 4);
+		    }
 		}
 	    }
 	}
@@ -841,13 +871,13 @@ hppa32_push_dummy_call (struct gdbarch *
       /* Update the various stack pointers.  */
       if (!write_pass)
 	{
-	  struct_end = sp + struct_ptr;
+	  struct_end = sp + align_up (struct_ptr, 64);
 	  /* PARAM_PTR already accounts for all the arguments passed
 	     by the user.  However, the ABI mandates minimum stack
 	     space allocations for outgoing arguments.  The ABI also
 	     mandates minimum stack alignments which we must
 	     preserve.  */
-	  param_end = struct_end + max (align_up (param_ptr, 8), 16);
+	  param_end = struct_end + align_up (param_ptr, 64);
 	}
     }
 
@@ -860,10 +890,9 @@ hppa32_push_dummy_call (struct gdbarch *
   regcache_cooked_write_unsigned (regcache, RP_REGNUM, bp_addr);
 
   /* Update the Stack Pointer.  */
-  regcache_cooked_write_unsigned (regcache, SP_REGNUM, param_end + 32);
+  regcache_cooked_write_unsigned (regcache, SP_REGNUM, param_end);
 
-  /* The stack will have 32 bytes of additional space for a frame marker.  */
-  return param_end + 32;
+  return param_end;
 }
 
 /* This function pushes a stack frame with arguments as part of the

Re: [patch/rfa] fix call-dummies for hppa

[Joel Brobecker]

> Updated version, passes all of structs.exp and call-sc.exp on hppa-linux
> 
> 2004-04-28  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-tdep.c (hppa32_return_value): Handle both 4- and 8-byte fp
> 	return values.
> 	(hppa32_push_dummy_call): Fix small struct and floating point argument 
> 	passing, and make sure the stack frame is properly aligned.

FYI: I just tested this patch on hppa-hpux11.00. It fixes 32 failures
without any regression.

-- 
Joel

Re: [patch/rfa] fix call-dummies for hppa

[Randolph Chung]

> FYI: I just tested this patch on hppa-hpux11.00. It fixes 32 failures
> without any regression.

Thanks Joel. Dave set me up with an hpux11.00 machine with some hpux
compilers. i'll try to setup a somewhat more extensive test env for
future changes.

is there a list of compilers that gdb is supposed to support?

right now i can test with 
gcc-3.3.3 hppa-linux and hppa2.0w-hp-hpux11.11
gcc-3.5 hppa-linux and hppa2.0w-hp-hpux11.11
aCC: HP ANSI C++ B3910B A.03.13 (once i figure out how to use it :)
and whatever the default cc is on hpux11.00

I haven't looked at hppa64-hp-hpux11.11 at all yet, but i will try once
i get the 32-bit ones in better shape.

I'm mostly interested in the hppa-linux port myself but will try not to
break hpux stuff too much :P

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/