Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@mips.com>
To: Daniel Jacobowitz <drow@false.org>
Cc: gdb-patches@sourceware.org, David Ung <davidu@mips.com>,
	     "Maciej W. Rozycki" <macro@linux-mips.org>
Subject: Re: mips-tdep.c: Sign-extend pointers for n32
Date: Thu, 20 Dec 2007 16:41:00 -0000	[thread overview]
Message-ID: <Pine.LNX.4.61.0712201614510.16321@perivale.mips.com> (raw)
In-Reply-To: <20071219161552.GA1280@caradoc.them.org>

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


  reply	other threads:[~2007-12-20 16:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-25 14:43 Maciej W. Rozycki
2007-12-16 18:49 ` Daniel Jacobowitz
2007-12-19 15:27   ` Maciej W. Rozycki
2007-12-19 15:28     ` Daniel Jacobowitz
2007-12-19 16:16       ` Maciej W. Rozycki
2007-12-19 17:08         ` Daniel Jacobowitz
2007-12-20 16:41           ` Maciej W. Rozycki [this message]
2007-12-20 17:07             ` Daniel Jacobowitz
2007-12-20 17:18               ` Maciej W. Rozycki
2007-12-20 19:37                 ` Daniel Jacobowitz
2007-12-21  4:36                 ` Joel Brobecker
2007-12-21 11:34                   ` Maciej W. Rozycki
2007-12-21 11:51                     ` Joel Brobecker
2007-12-21 12:02                       ` Maciej W. Rozycki
2007-12-22  5:30                         ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.61.0712201614510.16321@perivale.mips.com \
    --to=macro@mips.com \
    --cc=davidu@mips.com \
    --cc=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    --cc=macro@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox