[PATCH] Relax MIPS j/jal out-of-range check

[PATCH] Relax MIPS j/jal out-of-range check

[Thiemo Seufer]

Hello All,

I applied the appended patch. It relaxes the range checking for j and
jal addresses to a warning, since there are a few cases where the
effect of an out of range jump can be the intended result.


Thiemo


2006-05-09  David Ung  <davidu@mips.com>

	[ gas/ChangeLog ]
	* config/tc-mips.c (append_insn): Only warn about an out-of-range
	j or jal address.

	[ gas/testsuite/ChangeLog ]
	* gas/mips/jal-range.l: Only warn about an out-of-range j or jal
	address.


Index: gas/testsuite/gas/mips/jal-range.l
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/jal-range.l,v
retrieving revision 1.2
diff -u -p -r1.2 jal-range.l
--- gas/testsuite/gas/mips/jal-range.l	26 Sep 2002 09:00:08 -0000	1.2
+++ gas/testsuite/gas/mips/jal-range.l	2 May 2006 14:58:20 -0000
@@ -1,4 +1,4 @@
 .*: Assembler messages:
 .*:4: Error: jump to misaligned address \(0x1\)
 .*:6: Error: jump to misaligned address \(0xfffffff\)
-.*:7: Error: jump address range overflow \(0x10000000\)
+.*:7: Warning: jump address range overflow \(0x10000000\)
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.330
diff -u -p -r1.330 tc-mips.c
--- gas/config/tc-mips.c	30 Apr 2006 18:34:39 -0000	1.330
+++ gas/config/tc-mips.c	2 May 2006 14:58:19 -0000
@@ -2407,8 +2772,8 @@ append_insn (struct mips_cl_insn *ip, ex
 		as_bad (_("jump to misaligned address (0x%lx)"),
 			(unsigned long) address_expr->X_add_number);
 	      if (address_expr->X_add_number & ~0xfffffff)
-		as_bad (_("jump address range overflow (0x%lx)"),
-			(unsigned long) address_expr->X_add_number);
+		as_warn (_("jump address range overflow (0x%lx)"),
+			 (unsigned long) address_expr->X_add_number);
 	      ip->insn_opcode |= (address_expr->X_add_number >> 2) & 0x3ffffff;
 	      break;
 
@@ -2417,8 +2782,8 @@ append_insn (struct mips_cl_insn *ip, ex
 		as_bad (_("jump to misaligned address (0x%lx)"),
 			(unsigned long) address_expr->X_add_number);
 	      if (address_expr->X_add_number & ~0xfffffff)
-		as_bad (_("jump address range overflow (0x%lx)"),
-			(unsigned long) address_expr->X_add_number);
+		as_warn (_("jump address range overflow (0x%lx)"),
+			 (unsigned long) address_expr->X_add_number);
 	      ip->insn_opcode |=
 		(((address_expr->X_add_number & 0x7c0000) << 3)
 		 | ((address_expr->X_add_number & 0xf800000) >> 7)

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Daniel Jacobowitz]

On Tue, May 09, 2006 at 03:16:37PM +0100, Thiemo Seufer wrote:
> Hello All,
> 
> I applied the appended patch. It relaxes the range checking for j and
> jal addresses to a warning, since there are a few cases where the
> effect of an out of range jump can be the intended result.

Why?  Especially bearing in mind that we're in the assembler here. 
This doesn't make sense to me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Paul Koning]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

 Daniel> On Tue, May 09, 2006 at 03:16:37PM +0100, Thiemo Seufer
 Daniel> wrote:
 >> Hello All,
 >> 
 >> I applied the appended patch. It relaxes the range checking for j
 >> and jal addresses to a warning, since there are a few cases where
 >> the effect of an out of range jump can be the intended result.

 Daniel> Why?  Especially bearing in mind that we're in the assembler
 Daniel> here.  This doesn't make sense to me.

If you have a routine that's sometimes called from uncached code, and
the label is a KSEG0 address, you could call it from KSEG1 code simply
with
	jal	label
and the machine will do the right thing (you end up at the KSEG1
address corresponding to "label").

It's a bit of a stretch, I suppose, but I've run into this.

Without the patch you end up having to do this:
	jal	label+0x20000000
or perhaps even uglier things, if that address arithmetic is invalid
here. 

      paul

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Daniel Jacobowitz]

On Tue, May 09, 2006 at 10:43:51AM -0400, Paul Koning wrote:
> >>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
> 
>  Daniel> On Tue, May 09, 2006 at 03:16:37PM +0100, Thiemo Seufer
>  Daniel> wrote:
>  >> Hello All,
>  >> 
>  >> I applied the appended patch. It relaxes the range checking for j
>  >> and jal addresses to a warning, since there are a few cases where
>  >> the effect of an out of range jump can be the intended result.
> 
>  Daniel> Why?  Especially bearing in mind that we're in the assembler
>  Daniel> here.  This doesn't make sense to me.
> 
> If you have a routine that's sometimes called from uncached code, and
> the label is a KSEG0 address, you could call it from KSEG1 code simply
> with
> 	jal	label
> and the machine will do the right thing (you end up at the KSEG1
> address corresponding to "label").

How could you get the assembler to do this?  Normally you can't tell if
this is going to overflow until you've linked!  Linker errors are a
different kettle of fish.

I think the warning is just bogus, after looking at it.  We're warning
for "jal 0x10000000".  But we don't know the location of the jal yet,
so we haven't a clue if it's out of range, do we?


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Thiemo Seufer]

Paul Koning wrote:
> >>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
> 
>  Daniel> On Tue, May 09, 2006 at 03:16:37PM +0100, Thiemo Seufer
>  Daniel> wrote:
>  >> Hello All,
>  >> 
>  >> I applied the appended patch. It relaxes the range checking for j
>  >> and jal addresses to a warning, since there are a few cases where
>  >> the effect of an out of range jump can be the intended result.
> 
>  Daniel> Why?  Especially bearing in mind that we're in the assembler
>  Daniel> here.  This doesn't make sense to me.
> 
> If you have a routine that's sometimes called from uncached code, and
> the label is a KSEG0 address, you could call it from KSEG1 code simply
> with
> 	jal	label
> and the machine will do the right thing (you end up at the KSEG1
> address corresponding to "label").
> 
> It's a bit of a stretch, I suppose, but I've run into this.

Exactly that's the reason for the change. Since it is only needed in a
few cases like cache probing routines I kept the warning about it.


Thiemo

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Thiemo Seufer]

Daniel Jacobowitz wrote:
[snip]
> >  Daniel> Why?  Especially bearing in mind that we're in the assembler
> >  Daniel> here.  This doesn't make sense to me.
> > 
> > If you have a routine that's sometimes called from uncached code, and
> > the label is a KSEG0 address, you could call it from KSEG1 code simply
> > with
> > 	jal	label
> > and the machine will do the right thing (you end up at the KSEG1
> > address corresponding to "label").
> 
> How could you get the assembler to do this?  Normally you can't tell if
> this is going to overflow until you've linked!  Linker errors are a
> different kettle of fish.
> 
> I think the warning is just bogus, after looking at it.  We're warning
> for "jal 0x10000000".  But we don't know the location of the jal yet,
> so we haven't a clue if it's out of range, do we?

The patch is correct, but our explanation was misleading: The code
checks for absolute addresses, and warns now if it crosses a 256 MB
memory area. This allows e.g. to jump between KSEG0 and KSEG1 without
hassles.


Thiemo

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Daniel Jacobowitz]

On Wed, May 10, 2006 at 04:04:42PM +0100, Thiemo Seufer wrote:
> > How could you get the assembler to do this?  Normally you can't tell if
> > this is going to overflow until you've linked!  Linker errors are a
> > different kettle of fish.
> > 
> > I think the warning is just bogus, after looking at it.  We're warning
> > for "jal 0x10000000".  But we don't know the location of the jal yet,
> > so we haven't a clue if it's out of range, do we?
> 
> The patch is correct, but our explanation was misleading: The code
> checks for absolute addresses, and warns now if it crosses a 256 MB
> memory area. This allows e.g. to jump between KSEG0 and KSEG1 without
> hassles.

I don't think we're communicating.

drow@caradoc:~% cat a.s
        jal 0x10000000
drow@caradoc:~% /space/fsf/mips/install/bin/mips64-linux-as a.s
a.s: Assembler messages:
a.s:1: Error: jump address range overflow (0x10000000)

So... how can the assembler know that?  If you link the file into an
executable such that a.o(.text) is at 0x10000010, then the jal does not
overflow.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> On Wed, May 10, 2006 at 04:04:42PM +0100, Thiemo Seufer wrote:
> > > How could you get the assembler to do this?  Normally you can't tell if
> > > this is going to overflow until you've linked!  Linker errors are a
> > > different kettle of fish.
> > > 
> > > I think the warning is just bogus, after looking at it.  We're warning
> > > for "jal 0x10000000".  But we don't know the location of the jal yet,
> > > so we haven't a clue if it's out of range, do we?
> > 
> > The patch is correct, but our explanation was misleading: The code
> > checks for absolute addresses, and warns now if it crosses a 256 MB
> > memory area. This allows e.g. to jump between KSEG0 and KSEG1 without
> > hassles.
> 
> I don't think we're communicating.
> 
> drow@caradoc:~% cat a.s
>         jal 0x10000000
> drow@caradoc:~% /space/fsf/mips/install/bin/mips64-linux-as a.s
> a.s: Assembler messages:
> a.s:1: Error: jump address range overflow (0x10000000)
> 
> So... how can the assembler know that?  If you link the file into an
> executable such that a.o(.text) is at 0x10000010, then the jal does not
> overflow.

Hm, indeed. Do you like the appended patch better? Should something
similiar go in the branch?


Thiemo


2006-05-10  Thiemo Seufer  <ths@mips.com>

	[ gas/ChangeLog ]
	* config/tc-mips.c (append_insn): Don't check the range of j or
	jal addresses.

	[ gas/testsuite/ChangeLog ]
	* gas/mips/jal-range.l: Don't check the range of j or jal
	addresses.


Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.333
diff -u -p -r1.333 tc-mips.c
--- gas/config/tc-mips.c	9 May 2006 14:16:49 -0000	1.333
+++ gas/config/tc-mips.c	10 May 2006 16:50:30 -0000
@@ -2415,9 +2428,6 @@ append_insn (struct mips_cl_insn *ip, ex
 	      if ((address_expr->X_add_number & 3) != 0)
 		as_bad (_("jump to misaligned address (0x%lx)"),
 			(unsigned long) address_expr->X_add_number);
-	      if (address_expr->X_add_number & ~0xfffffff)
-		as_warn (_("jump address range overflow (0x%lx)"),
-			 (unsigned long) address_expr->X_add_number);
 	      ip->insn_opcode |= (address_expr->X_add_number >> 2) & 0x3ffffff;
 	      break;
 
@@ -2425,9 +2435,6 @@ append_insn (struct mips_cl_insn *ip, ex
 	      if ((address_expr->X_add_number & 3) != 0)
 		as_bad (_("jump to misaligned address (0x%lx)"),
 			(unsigned long) address_expr->X_add_number);
-	      if (address_expr->X_add_number & ~0xfffffff)
-		as_warn (_("jump address range overflow (0x%lx)"),
-			 (unsigned long) address_expr->X_add_number);
 	      ip->insn_opcode |=
 		(((address_expr->X_add_number & 0x7c0000) << 3)
 		 | ((address_expr->X_add_number & 0xf800000) >> 7)
Index: gas/testsuite/gas/mips/jal-range.l
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/jal-range.l,v
retrieving revision 1.3
diff -u -p -r1.3 jal-range.l
--- gas/testsuite/gas/mips/jal-range.l	9 May 2006 14:16:50 -0000	1.3
+++ gas/testsuite/gas/mips/jal-range.l	10 May 2006 16:50:31 -0000
@@ -1,4 +1,4 @@
 .*: Assembler messages:
 .*:4: Error: jump to misaligned address \(0x1\)
 .*:6: Error: jump to misaligned address \(0xfffffff\)
-.*:7: Warning: jump address range overflow \(0x10000000\)
+.*:8: Error: jump to misaligned address \(0x10000003\)
Index: gas/testsuite/gas/mips/jal-range.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/jal-range.s,v
retrieving revision 1.2
diff -u -p -r1.2 jal-range.s
--- gas/testsuite/gas/mips/jal-range.s	26 Sep 2002 09:00:08 -0000	1.2
+++ gas/testsuite/gas/mips/jal-range.s	10 May 2006 16:50:31 -0000
@@ -1,7 +1,8 @@
-# Source file use to test border cases of jumps
+# Source file used to test misaligned targets of absolute jumps
 
 	jal	0x0
 	jal	0x1
 	jal	0xffffffc
 	jal	0xfffffff
 	jal	0x10000000
+	jal	0x10000003

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Daniel Jacobowitz]

On Wed, May 10, 2006 at 06:00:23PM +0100, Thiemo Seufer wrote:
> Hm, indeed. Do you like the appended patch better? Should something
> similiar go in the branch?

I do - I'll leave it up to your judgement whether this is correct,
but feel free to apply it to the branch if it is.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Eric Christopher]

On May 10, 2006, at 10:02 AM, Daniel Jacobowitz wrote:

> On Wed, May 10, 2006 at 06:00:23PM +0100, Thiemo Seufer wrote:
>> Hm, indeed. Do you like the appended patch better? Should something
>> similiar go in the branch?
>
> I do - I'll leave it up to your judgement whether this is correct,
> but feel free to apply it to the branch if it is.

I'll agree as well, I think we should only be checking this via  
overflow in the linker.

-eric

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Richard Sandiford]

Eric Christopher <echristo@apple.com> writes:
> On May 10, 2006, at 10:02 AM, Daniel Jacobowitz wrote:
>
>> On Wed, May 10, 2006 at 06:00:23PM +0100, Thiemo Seufer wrote:
>>> Hm, indeed. Do you like the appended patch better? Should something
>>> similiar go in the branch?
>>
>> I do - I'll leave it up to your judgement whether this is correct,
>> but feel free to apply it to the branch if it is.
>
> I'll agree as well, I think we should only be checking this via  
> overflow in the linker.

But we won't be checking overflow in the linker in the case Dan gave:

        jal 0x10000000

Thiemo is patching code that handles fixups at assembly time, so after
the patch, gas will just silently assemble this as "jal 0".  I agree
that's the right thing to do though, for the reasons already discussed.
(Note that the linker doesn't warn about overflow for jals to local
symbols anyway, presumably for the same reasons that we don't want
an error here.)

Richard

Re: [PATCH] Relax MIPS j/jal out-of-range check

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> On Wed, May 10, 2006 at 06:00:23PM +0100, Thiemo Seufer wrote:
> > Hm, indeed. Do you like the appended patch better? Should something
> > similiar go in the branch?
> 
> I do - I'll leave it up to your judgement whether this is correct,
> but feel free to apply it to the branch if it is.

I checked in that patch, and a similiar one for the branch.


Thiemo