mips-tdep.c: Sign-extend pointers for n32

mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

Hello,

 The change below fixes a few regressions in the GDB test suite for 
MIPS/n32 like the one below:

print (diamond.*left_base_vpmf) ()
UNPREDICTABLE: PC = 0x80020580
Quit
(gdb) FAIL: gdb.cp/member-ptr.exp: print (diamond.*left_base_vpmf) ()

 Tested for n32 using the mipsisa32-sde-elf target, with the 
mips-sim-sde64/-mips64/-EB and mips-sim-sde64/-mips64/-EL boards, fixing 6 
regressions (of 78) for the former and 3 regressions (of 74) for the 
latter.

2007-10-25  David Ung  <davidu@mips.com>

	* mips-tdep.c (mips_n32n64_push_dummy_call): Sign-extend the
	pointer value if the length of the variable is less than the
	width of the register size of the ABI.

 OK to apply?

  Maciej

12741-0.diff
Index: binutils-quilt/src/gdb/mips-tdep.c
===================================================================
--- binutils-quilt.orig/src/gdb/mips-tdep.c	2007-09-21 17:19:38.000000000 +0100
+++ binutils-quilt/src/gdb/mips-tdep.c	2007-09-21 17:19:38.000000000 +0100
@@ -3032,6 +3032,10 @@
 			  || typecode == TYPE_CODE_UNION))
 		    regval <<= ((MIPS64_REGSIZE - partial_len)
 				* TARGET_CHAR_BIT);
+		  /* Sign extend the value if it is an address.  */
+		  else if (partial_len < MIPS64_REGSIZE
+			   && typecode == TYPE_CODE_PTR)
+		    regval = extract_signed_integer (val, partial_len);
 
 		  if (mips_debug)
 		    fprintf_filtered (gdb_stdlog, " - reg=%d val=%s",

Re: mips-tdep.c: Sign-extend pointers for n32

[Daniel Jacobowitz]

On Thu, Oct 25, 2007 at 03:10:20PM +0100, Maciej W. Rozycki wrote:
> 2007-10-25  David Ung  <davidu@mips.com>
> 
> 	* mips-tdep.c (mips_n32n64_push_dummy_call): Sign-extend the
> 	pointer value if the length of the variable is less than the
> 	width of the register size of the ABI.
> 
>  OK to apply?

Why just pointers?  I'm not entirely sure what's going on above in the
float case, but in the integer case this seems clearly wrong.  Zero
extension is probably "right" for trailing bits of structures but what
about negative integers?

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Sun, 16 Dec 2007, Daniel Jacobowitz wrote:

> Why just pointers?  I'm not entirely sure what's going on above in the
> float case, but in the integer case this seems clearly wrong.  Zero
> extension is probably "right" for trailing bits of structures but what
> about negative integers?

 You are quite right here -- I have checked the N32 ABI document and it 
says:

"32-bit integer (int) parameters are always sign-extended when passed in 
registers, whether of signed or unsigned type. [This issue does not arise 
in the o32-bit ABI.]"

It does not actually say anything about pointers beyond:

"All pointers and addresses are 32-bit objects. [Under 64, pointers and 
addresses are 64bits.]"

but I gather this is because the document was meant to specify a user mode 
ABI for programs run under IRIX only, so addresses with the bit #31 set 
were not expected to be relevant.

 I have regression-tested the following rewrite of the original patch 
using the mipsisa32-sde-elf target, with the mips-sim-sde64/-mips64/-EB 
and mips-sim-sde64/-mips64/-EL boards, with the results the same as the 
original.

2007-12-19  David Ung  <davidu@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* mips-tdep.c (mips_n32n64_push_dummy_call): Sign-extend 32-bit
	pointers and integers as required by the ABI.

 OK to apply?

  Maciej

12741-0.diff
Index: binutils-quilt/src/gdb/mips-tdep.c
===================================================================
--- binutils-quilt.orig/src/gdb/mips-tdep.c	2007-12-19 15:09:31.000000000 +0000
+++ binutils-quilt/src/gdb/mips-tdep.c	2007-12-19 15:10:13.000000000 +0000
@@ -3184,8 +3184,17 @@
 	         purpose register.  */
 	      if (argreg <= MIPS_LAST_ARG_REGNUM)
 		{
-		  LONGEST regval =
-		    extract_unsigned_integer (val, partial_len);
+		  LONGEST regval;
+
+		  /* Sign extend pointers and 32-bit integers;
+		     everything else is taken as is.  */
+
+		  if (partial_len == 4 &&
+		      (typecode == TYPE_CODE_PTR
+		       || typecode == TYPE_CODE_INT))
+		    regval = extract_signed_integer (val, partial_len);
+		  else
+		    regval = extract_unsigned_integer (val, partial_len);
 
 		  /* A non-floating-point argument being passed in a
 		     general register.  If a struct or union, and if

Re: mips-tdep.c: Sign-extend pointers for n32

[Daniel Jacobowitz]

On Wed, Dec 19, 2007 at 03:20:20PM +0000, Maciej W. Rozycki wrote:
>  You are quite right here -- I have checked the N32 ABI document and it 
> says:
> 
> "32-bit integer (int) parameters are always sign-extended when passed in 
> registers, whether of signed or unsigned type. [This issue does not arise 
> in the o32-bit ABI.]"

What happens to an 8-bit unsigned char?  How about an 8-bit signed
char?

>  I have regression-tested the following rewrite of the original patch 
> using the mipsisa32-sde-elf target, with the mips-sim-sde64/-mips64/-EB 
> and mips-sim-sde64/-mips64/-EL boards, with the results the same as the 
> original.

What ABI is that?  I thought none of the ELF targets used n32 or n64,
but maybe SDE is an exception.

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Wed, 19 Dec 2007, Daniel Jacobowitz wrote:

> What happens to an 8-bit unsigned char?  How about an 8-bit signed
> char?

 Hmm, you have made me wonder...  Obviously 32-bit integer quantities are 
special -- they are always sign-extended so that 32-bit ALU operations may 
be used on them.  There is this statement in the ABI document:

"All integer parameters are promoted (that is, sign- or zero-extended to 
64-bit integers and passed in a single register). Typically, no code is 
required for the promotion."

This note about no code requirement may be misleading -- this may be true 
for the run time, because the use of "lb/lbu/lh/lhu" as appropriate would 
have had the effect of doing the correct extension when the value was 
fetched into a register originally, but here we are in a different 
position as it is GDB that is the caller.  Let me dig through the 
document...  Nothing relevant apparently found.

 But in practice it should not matter -- however you represent 8-bit and 
16-bit quantities you cannot overflow into the 64-bit data space as with a 
flip of the bit #31 the upper 32 bits follow and when a result is written 
back to memory or is otherwise finally processed (like output in a textual 
form) it has to be masked to its data width again (obviously "sb/sh" do 
this implicitly).  So I believe the change is correct as it is and the 
question is academic.  Let me know if you think otherwise.

> What ABI is that?  I thought none of the ELF targets used n32 or n64,
> but maybe SDE is an exception.

 N32, as documented in "MIPSpro N32 ABI Handbook" from SGI.  I can chase a 
link if you cannot locate the PDF.  The SDE target uses it as the default 
64-bit ABI implied by the "-mips64" switch.

  Maciej

Re: mips-tdep.c: Sign-extend pointers for n32

[Daniel Jacobowitz]

On Wed, Dec 19, 2007 at 04:07:13PM +0000, Maciej W. Rozycki wrote:
>  But in practice it should not matter -- however you represent 8-bit and 
> 16-bit quantities you cannot overflow into the 64-bit data space as with a 
> flip of the bit #31 the upper 32 bits follow and when a result is written 
> back to memory or is otherwise finally processed (like output in a textual 
> form) it has to be masked to its data width again (obviously "sb/sh" do 
> this implicitly).  So I believe the change is correct as it is and the 
> question is academic.  Let me know if you think otherwise.

I happen to know otherwise.  I once wasted several days tracking down
a bug in the Linux kernel's IP checksumming implementation which
resulted in incorrectly extended values in registers; there are MIPS
parts which really do behave unpredictably when you use 32-bit
arithmetic operations on them (specifically the Broadcom SB1).
And I believe a compiler would be justified in using such instructions
on a signed char argument.

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Wed, 19 Dec 2007, Daniel Jacobowitz wrote:

> I happen to know otherwise.  I once wasted several days tracking down
> a bug in the Linux kernel's IP checksumming implementation which
> resulted in incorrectly extended values in registers; there are MIPS
> parts which really do behave unpredictably when you use 32-bit
> arithmetic operations on them (specifically the Broadcom SB1).

 I recall the issue and the problem was broken inline assembly, which used 
64-bit operations to produce results associated with 32-bit integer types 
and register constraints.  This is a different issue and is doomed to fail 
indeed.

> And I believe a compiler would be justified in using such instructions
> on a signed char argument.

 Yes, of course -- it is actually meant to and thus will never produce 
results outside the sign-extended 32-bit range (remember that before 
extending, bits 31:16 or 31:8 as appropriate are zeroes, so after a 
zero-extension the values still fall within the signed 32-bit range).  
And most ALU operations do not care about the signedness, but there are 
two exceptions that came to my mind after I sent you the previous letter, 
specifically signed "div/mult", and GCC indeed uses signed/unsigned load 
operations for 16/8-bit types as appropriate, so even though chasing the 
relevant document that would state it explicitly seems a non-trivial task, 
I think sign-extension of such small signed integers is implied.

 Also n32 builds on compatibility with o32 and the most definite o32 
document which is: "SYSTEM V APPLICATION BINARY INTERFACE, MIPS RISC 
Processor Supplement, 3rd Edition" says:

"All integer-valued arguments are passed as 32-bit words, with signed or 
unsigned bytes and halfwords expanded (promoted) as necessary."

 Thus I have taken your suggestion into account and modified the change 
further as follows and regression tested it as previously, successfully.  
Well, I guess it actually means we do not really have a terribly good 
coverage here -- I suppose a set of test cases that would check that 
promotion is performed correctly with manual calls would be useful.

2007-12-19  David Ung  <davidu@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* mips-tdep.c (mips_n32n64_push_dummy_call): Sign-extend
	integers and 32-bit pointers as required by the ABI.

 OK to apply?

  Maciej

12741-0.diff
Index: binutils-quilt/src/gdb/mips-tdep.c
===================================================================
--- binutils-quilt.orig/src/gdb/mips-tdep.c	2007-12-19 15:15:45.000000000 +0000
+++ binutils-quilt/src/gdb/mips-tdep.c	2007-12-20 16:13:31.000000000 +0000
@@ -3184,8 +3184,21 @@
 	         purpose register.  */
 	      if (argreg <= MIPS_LAST_ARG_REGNUM)
 		{
-		  LONGEST regval =
-		    extract_unsigned_integer (val, partial_len);
+		  LONGEST regval;
+
+		  /* Sign extend pointers, 32-bit integers and signed
+		     16-bit and 8-bit integers; everything else is taken
+		     as is.  */
+
+		  if ((partial_len == 4
+		       && (typecode == TYPE_CODE_PTR
+			   || typecode == TYPE_CODE_INT))
+		      || (partial_len < 4
+			  && typecode == TYPE_CODE_INT
+			  && !TYPE_UNSIGNED (arg_type)))
+		    regval = extract_signed_integer (val, partial_len);
+		  else
+		    regval = extract_unsigned_integer (val, partial_len);
 
 		  /* A non-floating-point argument being passed in a
 		     general register.  If a struct or union, and if

Re: mips-tdep.c: Sign-extend pointers for n32

[Daniel Jacobowitz]

On Thu, Dec 20, 2007 at 04:37:31PM +0000, Maciej W. Rozycki wrote:
>  Thus I have taken your suggestion into account and modified the change 
> further as follows and regression tested it as previously, successfully.  
> Well, I guess it actually means we do not really have a terribly good 
> coverage here -- I suppose a set of test cases that would check that 
> promotion is performed correctly with manual calls would be useful.
> 
> 2007-12-19  David Ung  <davidu@mips.com>
>             Maciej W. Rozycki  <macro@mips.com>
> 
> 	* mips-tdep.c (mips_n32n64_push_dummy_call): Sign-extend
> 	integers and 32-bit pointers as required by the ABI.
> 
>  OK to apply?

OK.

I think there's still a problem here, though it is somewhat endemic to
the argument passing routines:

    TYPE_CODE_ENUM,             /* Enumeration type */
    TYPE_CODE_REF,              /* C++ Reference types */
    TYPE_CODE_CHAR,             /* *real* character type */
    TYPE_CODE_BOOL,

And possibly TYPE_CODE_MEMBERPTR too... that's a signed offset,
probably 32-bit in the n32 case.  Getting this right is a real
pain.

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Thu, 20 Dec 2007, Daniel Jacobowitz wrote:

> I think there's still a problem here, though it is somewhat endemic to
> the argument passing routines:
> 
>     TYPE_CODE_ENUM,             /* Enumeration type */
>     TYPE_CODE_REF,              /* C++ Reference types */
>     TYPE_CODE_CHAR,             /* *real* character type */
>     TYPE_CODE_BOOL,
> 
> And possibly TYPE_CODE_MEMBERPTR too... that's a signed offset,
> probably 32-bit in the n32 case.  Getting this right is a real
> pain.

 Hmm, these are obviously C-style types and I would expect other languages 
to have their own specific ones (Ada, anyone?).  As they map somehow to 
the integer types provided by the underlying architecture, wouldn't it be 
a good idea to actually record which of the plain CPU-specific types each 
of the language types corresponds to?

 This way the mapping would only be provided in a single place -- the 
language ABI for a given architecture -- and all the generic 
target-dependent and possibly target-independent code would not have to 
iterate over all the language types, which is obviously prone to errors 
and hard to fully cover in the test suite.

  Maciej

Re: mips-tdep.c: Sign-extend pointers for n32

[Daniel Jacobowitz]

On Thu, Dec 20, 2007 at 05:07:42PM +0000, Maciej W. Rozycki wrote:
>  Hmm, these are obviously C-style types and I would expect other languages 
> to have their own specific ones (Ada, anyone?).  As they map somehow to 
> the integer types provided by the underlying architecture, wouldn't it be 
> a good idea to actually record which of the plain CPU-specific types each 
> of the language types corresponds to?

Yes, that sounds like a good solution.  I don't think we need to fix
it today, though :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Sign-extend pointers for n32

[Joel Brobecker]

> > I think there's still a problem here, though it is somewhat endemic to
> > the argument passing routines:
> > 
> >     TYPE_CODE_ENUM,             /* Enumeration type */
> >     TYPE_CODE_REF,              /* C++ Reference types */
> >     TYPE_CODE_CHAR,             /* *real* character type */
> >     TYPE_CODE_BOOL,
> > 
> > And possibly TYPE_CODE_MEMBERPTR too... that's a signed offset,
> > probably 32-bit in the n32 case.  Getting this right is a real
> > pain.
> 
>  Hmm, these are obviously C-style types and I would expect other languages 
> to have their own specific ones (Ada, anyone?).

Are you asking which TYPE_CODE enumerates are specific to Ada?
I don't think there are any. I had a quick look at the current list,
and nothing stood out.

-- 
Joel

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Fri, 21 Dec 2007, Joel Brobecker wrote:

> Are you asking which TYPE_CODE enumerates are specific to Ada?

 Nope, not specifically -- just an example of a language that we support 
and which is considerably different from C (Java does not count -- it is 
close enough ;-) ).

  Maciej

Re: mips-tdep.c: Sign-extend pointers for n32

[Joel Brobecker]

> Nope, not specifically -- just an example of a language that we support 
> and which is considerably different from C (Java does not count -- it is 
> close enough ;-) ).

Ada is in many ways to Pascal, and I consider them to be close to C. 
My first time in Ada, I wrote a simple C program and then ran the GNAT
compiler and followed its instructions until I got a valid Ada program
;-).

For a language that's really different to C, how about Fortran?
There's scheme as well, but I don't know how well we support it.

-- 
Joel

Re: mips-tdep.c: Sign-extend pointers for n32

[Maciej W. Rozycki]

On Fri, 21 Dec 2007, Joel Brobecker wrote:

> Ada is in many ways to Pascal, and I consider them to be close to C. 

 Well, certainly both are procedural and thus closer to C than e.g. M4. ;)

> My first time in Ada, I wrote a simple C program and then ran the GNAT
> compiler and followed its instructions until I got a valid Ada program
> ;-).

 Good hint ;) -- I did some Ada hackery when trying to make the compiler 
work better for MIPS/Linux (any interest in that, BTW?) and it was 
somewhat tricky.

> For a language that's really different to C, how about Fortran?
> There's scheme as well, but I don't know how well we support it.

 I know neither of them well enough to make any comments.  Unfortunately I 
do not seem to see any interest in supporting languages like Lisp or 
Prolog with GCC yet. ;)

  Maciej

Re: mips-tdep.c: Sign-extend pointers for n32

[Joel Brobecker]

> > My first time in Ada, I wrote a simple C program and then ran the GNAT
> > compiler and followed its instructions until I got a valid Ada program
> > ;-).
> 
>  Good hint ;) -- I did some Ada hackery when trying to make the compiler 
> work better for MIPS/Linux (any interest in that, BTW?) and it was 
> somewhat tricky.

I'm already having enough trouble trying to keep up on mips-irix,
but thank you :). If it's Ada-specific questions that you have,
don't hesitate to ask me, though.

> > For a language that's really different to C, how about Fortran?
> > There's scheme as well, but I don't know how well we support it.
> 
>  I know neither of them well enough to make any comments.  Unfortunately I 
> do not seem to see any interest in supporting languages like Lisp or 
> Prolog with GCC yet. ;)

:-). I just had a look at Fortran, and although it's harder to read
when you're not used to their style, it looks pretty close to C or
Ada as well.

-- 
Joel