[RFA] Patch for mips_o64_return_value to fix calling functions by hand

[RFA] Patch for mips_o64_return_value to fix calling functions by hand

[Fred Fish]

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

I'm doing some work with a mips64-elf toolchain and took a look at why
there are so many gdb testsuite failures.  One problem I found is that
calling functions by hand was broken, due to what looked to me like a
case of a development stub being left in.  I've attached a patch that
reduces the total number of testsuite failures by around 50%.

I'm testing with the following multilibs:

  mips64-sim/-EB
  mips64-sim/-EL
  mips64-sim/-EB/-msoft-float
  mips64-sim/-EL/-msoft-float

Before applying the attached patch, I get:

  # of expected passes            34613
  # of unexpected failures        1866
  # of unexpected successes       4
  # of expected failures          164
  # of known failures             196
  # of unresolved testcases       77
  # of untested testcases         36
  # of unsupported tests          152
  /links/build/latest/trunk/mips64-elf/gdb/gdb/testsuite/../../gdb/gdb version  6.4.50.20060422-cvs -nx
  
after applying the patch I get:

  # of expected passes            35540
  # of unexpected failures        938
  # of unexpected successes       4
  # of expected failures          164
  # of known failures             196
  # of unresolved testcases       76
  # of untested testcases         36
  # of unsupported tests          152
  /links/build/latest/trunk/mips64-elf/gdb/gdb/testsuite/../../gdb/gdb version  6.4.50.20060422-cvs -nx
  
There are still some issues calling functions by hand in code compiled
with -msoft-float, but these are not specific to 64-bit mips, or even
mips for that matter.  For example:

  Running target mips64-sim/-EB/-msoft-float
  Running /src/latest/trunk/src/gdb/gdb/testsuite/gdb.base/call-sc.exp ...
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tf
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-tf
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-td
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-td
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-tld
  
  Running target mips-sim/-EB/-msoft-float
  Running /src/latest/trunk/src/gdb/gdb/testsuite/gdb.base/call-sc.exp ...
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tf
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-tf
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-tf
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-td
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-td
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-td
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-tld
  FAIL: gdb.base/call-sc.exp: value foo returned; return call-sc-tld
  
  Running target powerpc-sim/-msoft-float/-mrelocatable-lib/-mno-eabi/-mstrict-align
  Running /src/latest/trunk/src/gdb/gdb/testsuite/gdb.base/call-sc.exp ...
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tf
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-tf
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-td
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-td
  FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
  FAIL: gdb.base/call-sc.exp: p/c L; call call-sc-tld

I'll investigate those failures and supply a separate patch.

-Fred


[-- Attachment #2: mips-tdep.patch --]
[-- Type: text/x-diff, Size: 2388 bytes --]

2006-04-27  Fred Fish  <fnf@specifix.com>

	* mips-tdep.c (mips_o64_return_value): Replace stub that always
	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.

Index: mips-tdep.c
===================================================================
RCS file: /cvsroots/latest/src/gdb/gdb/mips-tdep.c,v
retrieving revision 1.1.1.4
diff -c -p -r1.1.1.4 mips-tdep.c
*** mips-tdep.c	22 Apr 2006 17:31:07 -0000	1.1.1.4
--- mips-tdep.c	27 Apr 2006 14:14:51 -0000
*************** mips_o64_return_value (struct gdbarch *g
*** 3788,3794 ****
  		       struct type *type, struct regcache *regcache,
  		       gdb_byte *readbuf, const gdb_byte *writebuf)
  {
!   return RETURN_VALUE_STRUCT_CONVENTION;
  }
  
  /* Floating point register management.
--- 3788,3832 ----
  		       struct type *type, struct regcache *regcache,
  		       gdb_byte *readbuf, const gdb_byte *writebuf)
  {
!   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
! 
!   if (TYPE_CODE (type) == TYPE_CODE_STRUCT
!       || TYPE_CODE (type) == TYPE_CODE_UNION
!       || TYPE_CODE (type) == TYPE_CODE_ARRAY)
!     return RETURN_VALUE_STRUCT_CONVENTION;
!   else if (fp_register_arg_p (TYPE_CODE (type), type))
!     {
!       /* A floating-point value.  It fits in the least significant
!          part of FP0.  */
!       if (mips_debug)
! 	fprintf_unfiltered (gdb_stderr, "Return float in $fp0\n");
!       mips_xfer_register (regcache,
! 			  NUM_REGS + mips_regnum (current_gdbarch)->fp0,
! 			  TYPE_LENGTH (type),
! 			  TARGET_BYTE_ORDER, readbuf, writebuf, 0);
!       return RETURN_VALUE_REGISTER_CONVENTION;
!     }
!   else
!     {
!       /* A scalar extract each part but least-significant-byte
!          justified. */
!       int offset;
!       int regnum;
!       for (offset = 0, regnum = MIPS_V0_REGNUM;
! 	   offset < TYPE_LENGTH (type);
! 	   offset += mips_stack_argsize (gdbarch), regnum++)
! 	{
! 	  int xfer = mips_stack_argsize (gdbarch);
! 	  if (offset + xfer > TYPE_LENGTH (type))
! 	    xfer = TYPE_LENGTH (type) - offset;
! 	  if (mips_debug)
! 	    fprintf_unfiltered (gdb_stderr, "Return scalar+%d:%d in $%d\n",
! 				offset, xfer, regnum);
! 	  mips_xfer_register (regcache, NUM_REGS + regnum, xfer,
! 			      TARGET_BYTE_ORDER, readbuf, writebuf, offset);
! 	}
!       return RETURN_VALUE_REGISTER_CONVENTION;
!     }
  }
  
  /* Floating point register management.

Re: [RFA] Patch for mips_o64_return_value to fix calling functions by hand (additional patch)

[Fred Fish]

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

On Thursday 27 April 2006 10:04, Fred Fish wrote:
> I'm doing some work with a mips64-elf toolchain and took a look at why
> there are so many gdb testsuite failures.  One problem I found is that
> calling functions by hand was broken,

For mips_o64_push_dummy_call, mips_abi_regsize(gdbarch) always
evaluates to 8, and was suppressing a needed left shift of struct and
union arguments passed big endian.

Attached is an additional patch which fixes the following FAILs:

< FAIL: gdb.base/call-rt-st.exp: print print_bit_flags_char(*cflags)
< FAIL: gdb.base/call-rt-st.exp: print print_bit_flags_short(*sflags)
< FAIL: gdb.base/call-rt-st.exp: print print_bit_flags(*flags)
< FAIL: gdb.base/call-rt-st.exp: print print_three_chars(*three_char)
< FAIL: gdb.base/call-rt-st.exp: print print_five_chars(*five_char)
< FAIL: gdb.base/structs.exp: p/c L<n>; call 1 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 2 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 4 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 5 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 6 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 7 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 9 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 10 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 11 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 12 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 13 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 14 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 15 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 17 structs-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 1 structs-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 2 structs-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 5 structs-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 1 structs-ti
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-ti
< FAIL: gdb.base/structs.exp: p/c L<n>; call 1 structs-tl
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tl
< FAIL: gdb.base/structs.exp: p/c L<n>; call 1 structs-tf
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tf
< FAIL: gdb.base/structs.exp: p/c L<n>; call 2 structs-ts-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-ts-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 4 structs-ts-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 5 structs-ts-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 8 structs-ts-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 6 structs-ti-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 6 structs-tl-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 6 structs-tf-tc
< FAIL: gdb.base/structs.exp: p/c L<n>; call 2 structs-tc-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tc-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 5 structs-tc-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 6 structs-tc-ts
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tc-ti
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tc-tl
< FAIL: gdb.base/structs.exp: p/c L<n>; call 3 structs-tc-tf

-Fred


[-- Attachment #2: struct-union-fix.patch --]
[-- Type: text/x-diff, Size: 1518 bytes --]

2006-05-02  Fred Fish  <fnf@specifix.com>

	* mips-tdep.c (mips_o64_push_dummy_call): Left shift big endian
	structs or unions independent of ABI register size.

Index: mips-tdep.c
===================================================================
RCS file: /cvsroots/latest/src/gdb/gdb/mips-tdep.c,v
retrieving revision 1.1.1.4
diff -c -p -r1.1.1.4 mips-tdep.c
*** mips-tdep.c	22 Apr 2006 17:31:07 -0000	1.1.1.4
--- mips-tdep.c	2 May 2006 18:19:01 -0000
*************** mips_o64_push_dummy_call (struct gdbarch
*** 3721,3728 ****
  		     It does not seem to be necessary to do the
  		     same for integral types.
  
- 		     Also don't do this adjustment on O64 binaries.
- 
  		     cagney/2001-07-23: gdb/179: Also, GCC, when
  		     outputting LE O32 with sizeof (struct) <
  		     mips_abi_regsize(), generates a left shift as
--- 3721,3726 ----
*************** mips_o64_push_dummy_call (struct gdbarch
*** 3737,3744 ****
  		     identified as such and GDB gets tweaked
  		     accordingly.  */
  
! 		  if (mips_abi_regsize (gdbarch) < 8
! 		      && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
  		      && partial_len < mips_abi_regsize (gdbarch)
  		      && (typecode == TYPE_CODE_STRUCT ||
  			  typecode == TYPE_CODE_UNION))
--- 3735,3741 ----
  		     identified as such and GDB gets tweaked
  		     accordingly.  */
  
! 		  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
  		      && partial_len < mips_abi_regsize (gdbarch)
  		      && (typecode == TYPE_CODE_STRUCT ||
  			  typecode == TYPE_CODE_UNION))

Re: [RFA] Patch for mips_o64_return_value to fix calling functions by hand (additional patch)

[Daniel Jacobowitz]

On Tue, May 02, 2006 at 04:30:15PM -0400, Fred Fish wrote:
> On Thursday 27 April 2006 10:04, Fred Fish wrote:
> > I'm doing some work with a mips64-elf toolchain and took a look at why
> > there are so many gdb testsuite failures.  One problem I found is that
> > calling functions by hand was broken,
> 
> For mips_o64_push_dummy_call, mips_abi_regsize(gdbarch) always
> evaluates to 8, and was suppressing a needed left shift of struct and
> union arguments passed big endian.

So, why didn't it used to be done on o64 targets?  That's the
question.  This goes back to CVS revision 1.1.

GCC says: pad "upwards" on little endian (i.e. in the most significant
bits).  On big endian, "upwards" means least significant bits.  It pads
floats downward for o64, but other than that o32 and o64 are treated
the same.  So, I'm guessing GDB is just wrong, or else the ABI change
is farther back in history than I checked.

Aside: The comment below the one you're deleting was written well
before GCC 3.4, in particular http://gcc.gnu.org/gcc-3.4/mips-abi.html. 
So the FIXME is probably fixed for o32 now.  And this is now an o64
only function, so that comment could be deleted entirely.

> 2006-05-02  Fred Fish  <fnf@specifix.com>
> 
> 	* mips-tdep.c (mips_o64_push_dummy_call): Left shift big endian
> 	structs or unions independent of ABI register size.

OK.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch for mips_o64_return_value to fix calling functions by hand

[Daniel Jacobowitz]

On Thu, Apr 27, 2006 at 10:04:44AM -0400, Fred Fish wrote:
> 2006-04-27  Fred Fish  <fnf@specifix.com>
> 
> 	* mips-tdep.c (mips_o64_return_value): Replace stub that always
> 	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.

OK.

-- 
Daniel Jacobowitz
CodeSourcery

[commit] Patch for mips_o64_return_value to fix calling functions by hand

[Fred Fish]

On Friday 05 May 2006 13:13, Daniel Jacobowitz wrote:
> > 2006-04-27  Fred Fish  <fnf@specifix.com>
> > 
> > 	* mips-tdep.c (mips_o64_return_value): Replace stub that always
> > 	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.
> 
> OK.

Checked in now.

-Fred

[commit] Patch for mips_o64_return_value to fix calling functions by hand (additional patch)

[Fred Fish]

On Friday 05 May 2006 13:10, Daniel Jacobowitz wrote:
> Aside: The comment below the one you're deleting was written well
> before GCC 3.4, in particular http://gcc.gnu.org/gcc-3.4/mips-abi.html. 
> So the FIXME is probably fixed for o32 now.  And this is now an o64
> only function, so that comment could be deleted entirely.

OK, I added that to the change.

> > 2006-05-02  Fred Fish  <fnf@specifix.com>
> > 
> > 	* mips-tdep.c (mips_o64_push_dummy_call): Left shift big endian
> > 	structs or unions independent of ABI register size.
> 
> OK.

Checked in now.

-Fred

============================================================================

  2006-05-06  Fred Fish  <fnf@specifix.com>
 
	* mips-tdep.c (mips_o64_push_dummy_call): Left shift big endian
	structs or unions independent of ABI register size.

 
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.390
diff -u -p -r1.390 mips-tdep.c
--- mips-tdep.c	5 May 2006 17:50:33 -0000	1.390
+++ mips-tdep.c	5 May 2006 18:02:59 -0000
@@ -3719,26 +3719,9 @@ mips_o64_push_dummy_call (struct gdbarch
 		     big endian targets.
 
 		     It does not seem to be necessary to do the
-		     same for integral types.
+		     same for integral types. */
 
-		     Also don't do this adjustment on O64 binaries.
-
-		     cagney/2001-07-23: gdb/179: Also, GCC, when
-		     outputting LE O32 with sizeof (struct) <
-		     mips_abi_regsize(), generates a left shift as
-		     part of storing the argument in a register a
-		     register (the left shift isn't generated when
-		     sizeof (struct) >= mips_abi_regsize()).  Since
-		     it is quite possible that this is GCC
-		     contradicting the LE/O32 ABI, GDB has not been
-		     adjusted to accommodate this.  Either someone
-		     needs to demonstrate that the LE/O32 ABI
-		     specifies such a left shift OR this new ABI gets
-		     identified as such and GDB gets tweaked
-		     accordingly.  */
-
-		  if (mips_abi_regsize (gdbarch) < 8
-		      && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
+		  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
 		      && partial_len < mips_abi_regsize (gdbarch)
 		      && (typecode == TYPE_CODE_STRUCT ||
 			  typecode == TYPE_CODE_UNION))