Skip to contentSkip to conversation

/ 0 should send SIGFPE not SIGTRAP...

PATCH In review David Daney 6 participants 17 messages Jun 11, 2004 – Jun 28, 2004 2 messages not in the archive MIPS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 / 0 should send SIGFPE not SIGTRAP...

I am getting a SIGTRAP whenever an integer divide by 0 happens. It should be sending SIGFPE.

It looks like kernel/traps.c is a little messed up.

The attached patch fixes it for me.

The decoding of the break instruction was selecting the wrong bits. It looks like the trap instruction decoding was messed up also. The patch fixes trap also, but I could not figure out how to get gcc to generate the trap form of division, so that part is untested.

David Daney.

#1 lore raw

It might help if I attached the patch. Here it is...

Quoted 10 lines · David Daney

I am getting a SIGTRAP whenever an integer divide by 0 happens. It should be sending SIGFPE.

It looks like kernel/traps.c is a little messed up.

The attached patch fixes it for me.

The decoding of the break instruction was selecting the wrong bits. It looks like the trap instruction decoding was messed up also. The patch fixes trap also, but I could not figure out how to get gcc to generate the trap form of division, so that part is untested.

David Daney.

mips/kernel/traps.c renamed from linux-avtrex/linux/arch/mips/kernel/traps.c +2 -2
@@ -598,7 +598,7 @@
 	 * code starts left to bit 16 instead to bit 6 in the opcode.
 	 * Gas is bug-compatible ...
 	 */
-	bcode = ((opcode >> 16) & ((1 << 20) - 1));
+	bcode = ((opcode >> 6) & ((1 << 20) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all break
@@ -633,7 +633,7 @@
 
 	/* Immediate versions don't provide a code.  */
 	if (!(opcode & OPCODE))
-		tcode = ((opcode >> 6) & ((1 << 20) - 1));
+		tcode = ((opcode >> 6) & ((1 << 10) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all trap
#2 lore raw

I guess I didn't fully read the comment above this section of code.

/*
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
*/

I am using gcc-3.3.1/binutils-2.15. With this toolchain, I get break instructions that comply with the MIPS documentation for break instructions:

00000000 <do_div>:

  0:   3c1c0000        lui     gp,0x0
  4:   279c0000        addiu   gp,gp,0
  8:   0399e021        addu    gp,gp,t9
  c:   0085001a        div     zero,a0,a1
 10:   14a00002        bnez    a1,1c <do_div+0x1c>
 14:   00000000        nop
 18:   000001cd        break   0x7
 1c:   00001012        mflo    v0
 20:   03e00008        jr      ra
 24:   00000000        nop

What to do?

David Daney

Quoted 39 lines · David Daney

It might help if I attached the patch. Here it is...

Quoted 10 lines · David Daney

I am getting a SIGTRAP whenever an integer divide by 0 happens. It should be sending SIGFPE.

It looks like kernel/traps.c is a little messed up.

The attached patch fixes it for me.

The decoding of the break instruction was selecting the wrong bits. It looks like the trap instruction decoding was messed up also. The patch fixes trap also, but I could not figure out how to get gcc to generate the trap form of division, so that part is untested.

David Daney.

------------------------------------------------------------------------

mips/kernel/traps.c renamed from linux-avtrex/linux/arch/mips/kernel/traps.c +2 -2
@@ -598,7 +598,7 @@
 	 * code starts left to bit 16 instead to bit 6 in the opcode.
 	 * Gas is bug-compatible ...
 	 */
-	bcode = ((opcode >> 16) & ((1 << 20) - 1));
+	bcode = ((opcode >> 6) & ((1 << 20) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all break
@@ -633,7 +633,7 @@
 
 	/* Immediate versions don't provide a code.  */
 	if (!(opcode & OPCODE))
-		tcode = ((opcode >> 6) & ((1 << 20) - 1));
+		tcode = ((opcode >> 6) & ((1 << 10) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all trap
#3 lore raw
Quoted 10 lines · David Daney

I guess I didn't fully read the comment above this section of code.

/*
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
*/

I am using gcc-3.3.1/binutils-2.15. With this toolchain, I get break instructions that comply with the MIPS documentation for break instructions:

Well, I did some research and I'm afraid it dates back to a commit from
2000-12-01, when a different interpretation of "break" was introduced for the MIPS32/64 ISA. So the interpretation of the "break" instruction depends on the "-march" setting and moreover, only for instructions requested explicitly. For ones emitted implicitly as a result of division and multiplication macros, the interpretation is always the "traditional" one (the "c" vs the "B" code).

Quoted 13 lines

00000000 <do_div>:

   0:   3c1c0000        lui     gp,0x0
   4:   279c0000        addiu   gp,gp,0
   8:   0399e021        addu    gp,gp,t9
   c:   0085001a        div     zero,a0,a1
  10:   14a00002        bnez    a1,1c <do_div+0x1c>
  14:   00000000        nop
  18:   000001cd        break   0x7
  1c:   00001012        mflo    v0
  20:   03e00008        jr      ra
  24:   00000000        nop
Quoted 1 line

What to do?

1. I think Linux can intercept both the "traditional" and MIPS32/64 values -- break codes are not used that extensively this would risk running out of them. The set of known ones can be seen in <asm/break.h>. However, this won't aid userland trying to interpret code (there may be none such, though).

2. Gas should definitely use the codes consistently. And it's a pity the ABI got broken -- I think another mnemonic should have been chosen for the correct implementation of "break", available to any ISA.

3. GCC should probably use traps for anything above MIPS I, anyway. Perhaps with an option, like for gas, to select the alternative.

4. Perhaps something else.

Maciej

Signature · 3 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

A message that is not in the archive (sent off-list or not yet received): mailpost.1086981251.16853@news-sj1-1

#5 lore raw
Quoted 3 lines · Maciej W. Rozycki

2. Gas should definitely use the codes consistently. And it's a pity the ABI got broken -- I think another mnemonic should have been chosen for the correct implementation of "break", available to any ISA.

in retrospect, the 'B' variation probably wasn't the greatest idea.

If it were removed (leaving 'c' and 'c','q' variations), I don't know that any real harm would occur.

It may be very confusing to people who expect that the break code will translate into the instruction in an obvious way, and obviously it would mess up use of 20-bit codes, but i don't know how prevalent that is.

Unfortunately, at this point, Linux should probably accept the divide-by-zero code in both locations.

(Really, from day one, assemblers probably should have accepted a 20-bit code. I just checked my copy of the Kane r2000/r3000 book, and it was 20-bit all the way back then. If i had to guess, i'd guess that gas was copying a non-gnu assembler's behaviour. In any case, water under the bridge.)

cgd

#6 lore raw
Quoted 5 lines · On Fri, 11 Jun 2004 [email protected] wrote:
Quoted 3 lines

2. Gas should definitely use the codes consistently. And it's a pity the ABI got broken -- I think another mnemonic should have been chosen for the correct implementation of "break", available to any ISA.

in retrospect, the 'B' variation probably wasn't the greatest idea.

I guess it may be useful for something to have 20-bit codes available.
Though except these few special cases, breaks tend to be inserted at the run time, so it's the interested software that decides how to interpret them, not gas.

Quoted 4 lines

It may be very confusing to people who expect that the break code will translate into the instruction in an obvious way, and obviously it would mess up use of 20-bit codes, but i don't know how prevalent that is.

I was surprised at first, too.

Quoted 2 lines

Unfortunately, at this point, Linux should probably accept the divide-by-zero code in both locations.

I think that's not a big trouble for Linux -- the path is rare and not
critical for performance.

Quoted 5 lines

(Really, from day one, assemblers probably should have accepted a 20-bit code. I just checked my copy of the Kane r2000/r3000 book, and it was 20-bit all the way back then. If i had to guess, i'd guess that gas was copying a non-gnu assembler's behaviour. In any case, water under the bridge.)

Definitely they should have. It's bug-compatibility with the original
MIPS assembler, I'm told.

Signature · 3 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +
#7 lore raw
Quoted 13 lines · Maciej W. Rozycki
Quoted 4 lines · On Fri, 11 Jun 2004 [email protected] wrote:

Unfortunately, at this point, Linux should probably accept the divide-by-zero code in both locations.

I think that's not a big trouble for Linux -- the path is rare and not critical for performance.

How about the attached (lightly tested) patch?

David Daney.

*** ../linux-avtrex/linux/arch/mips/kernel/traps.c 2004-02-26 11:14:09.000000000 -0800
--- arch/mips/kernel/traps.c 2004-06-11 13:43:00.000000000 -0700
***************
*** 597,615 ****
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
! */
! bcode = ((opcode >> 16) & ((1 << 20) - 1));
!
! /*
* (A short test says that IRIX 5.3 sends SIGTRAP for all break
* insns, even for break codes that indicate arithmetic failures.
* Weird ...)
* But should we continue the brokenness??? --macro */
switch (bcode) {
! case 6:
! case 7:
! if (bcode == 7)
info.si_code = FPE_INTDIV;
else
info.si_code = FPE_INTOVF;
--- 597,621 ----
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
! *
* (A short test says that IRIX 5.3 sends SIGTRAP for all break
* insns, even for break codes that indicate arithmetic failures.
* Weird ...)
* But should we continue the brokenness??? --macro

+          *
+          * It seems some assemblers (binutils-2.15 for example) assemble
+          * break correctly.  So we check for the break code in either
+          * position.
+          *

*/
+
+ bcode = ((opcode >> 6) & ((1 << 20) - 1));
switch (bcode) {
! case 0x0006:
! case 0x0007:
! case 0x1800: /* 6 << 10 */
! case 0x1c00: /* 7 << 10 */
! if (bcode == 0x7 || bcode == 0x1c00)
info.si_code = FPE_INTDIV;
else
info.si_code = FPE_INTOVF;
***************
*** 633,639 ****

/* Immediate versions don't provide a code. */ if (!(opcode & OPCODE))
! tcode = ((opcode >> 6) & ((1 << 20) - 1));

/*
* (A short test says that IRIX 5.3 sends SIGTRAP for all trap
--- 639,645 ----

/* Immediate versions don't provide a code. */ if (!(opcode & OPCODE))
! tcode = ((opcode >> 6) & ((1 << 10) - 1));

/*
* (A short test says that IRIX 5.3 sends SIGTRAP for all trap

#8 lore raw

Re: [Patch] (revised patch) / 0 should send SIGFPE not SIGTRAP

Quoted 2 lines · David Daney

How about the attached (lightly tested) patch?

I will quit sending patches after this one. It is equivalent to the previous version, except it uses the symbolic names of the break codes instead of the numeric values.

David Daney.

*** ../linux-avtrex/linux/arch/mips/kernel/traps.c 2004-02-26 11:14:09.000000000 -0800
--- arch/mips/kernel/traps.c 2004-06-11 14:04:52.000000000 -0700
***************
*** 38,43 ****
--- 38,44 ----
#include <asm/traps.h>
#include <asm/uaccess.h>
#include <asm/mmu_context.h>
+ #include <asm/break.h>

extern asmlinkage void handle_mod(void);
extern asmlinkage void handle_tlbl(void);
***************
*** 597,615 ****
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
! */
! bcode = ((opcode >> 16) & ((1 << 20) - 1));
!
! /*
* (A short test says that IRIX 5.3 sends SIGTRAP for all break
* insns, even for break codes that indicate arithmetic failures.
* Weird ...)
* But should we continue the brokenness??? --macro */
switch (bcode) {
! case 6:
! case 7:
! if (bcode == 7)
info.si_code = FPE_INTDIV;
else
info.si_code = FPE_INTOVF;
--- 598,622 ----
* There is the ancient bug in the MIPS assemblers that the break
* code starts left to bit 16 instead to bit 6 in the opcode.
* Gas is bug-compatible ...
! *
* (A short test says that IRIX 5.3 sends SIGTRAP for all break
* insns, even for break codes that indicate arithmetic failures.
* Weird ...)
* But should we continue the brokenness??? --macro

+          *
+          * It seems some assemblers (binutils-2.15 for example) assemble
+          * break correctly.  So we check for the break code in either
+          * position.
+          *

*/
+
+ bcode = ((opcode >> 6) & ((1 << 20) - 1));
switch (bcode) {
! case BRK_OVERFLOW:
! case BRK_DIVZERO:
! case BRK_OVERFLOW << 10:
! case BRK_DIVZERO << 10:
! if (bcode == BRK_DIVZERO || bcode == (BRK_DIVZERO << 10))
info.si_code = FPE_INTDIV;
else
info.si_code = FPE_INTOVF;
***************
*** 633,639 ****

/* Immediate versions don't provide a code. */ if (!(opcode & OPCODE))
! tcode = ((opcode >> 6) & ((1 << 20) - 1));

/*
* (A short test says that IRIX 5.3 sends SIGTRAP for all trap
--- 640,646 ----

/* Immediate versions don't provide a code. */ if (!(opcode & OPCODE))
! tcode = ((opcode >> 6) & ((1 << 10) - 1));

/*
* (A short test says that IRIX 5.3 sends SIGTRAP for all trap

#9 lore raw

Re: [Patch] (revised patch) / 0 should send SIGFPE not SIGTRAP

Quoted 6 lines · David Daney
Quoted 2 lines · David Daney

How about the attached (lightly tested) patch?

I will quit sending patches after this one. It is equivalent to the previous version, except it uses the symbolic names of the break codes instead of the numeric values.

Please send one more, where you use `diff -up' :-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#10 lore raw

Re: [Patch] (revised patch) / 0 should send SIGFPE not SIGTRAP

Quoted 1 line · Geert Uytterhoeven

Please send one more, where you use `diff -up' :-)

No need to -- I've reimplemented it a bit differently meanwhile. Any
objections to the following changes?

Signature · 145 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

patch-mips-2.4.26-20040531-mips-bp-tr-0
diff -up --recursive --new-file linux-mips-2.4.26-20040531.macro/arch/mips/kernel/traps.c linux-mips-2.4.26-20040531/arch/mips/kernel/traps.c
--- linux-mips-2.4.26-20040531.macro/arch/mips/kernel/traps.c	2004-03-13 03:56:44.000000000 +0000
+++ linux-mips-2.4.26-20040531/arch/mips/kernel/traps.c	2004-06-13 20:24:32.000000000 +0000
@@ -9,7 +9,7 @@
  * Copyright (C) 1999 Silicon Graphics, Inc.
  * Kevin D. Kissell, [email protected] and Carsten Langgaard, [email protected]
  * Copyright (C) 2000, 01 MIPS Technologies, Inc.
- * Copyright (C) 2002, 2003  Maciej W. Rozycki
+ * Copyright (C) 2002, 2003, 2004  Maciej W. Rozycki
  */
 #include <linux/config.h>
 #include <linux/init.h>
@@ -22,6 +22,7 @@
 
 #include <asm/bootinfo.h>
 #include <asm/branch.h>
+#include <asm/break.h>
 #include <asm/cpu.h>
 #include <asm/fpu.h>
 #include <asm/cachectl.h>
@@ -596,9 +597,12 @@ asmlinkage void do_bp(struct pt_regs *re
 	/*
 	 * There is the ancient bug in the MIPS assemblers that the break
 	 * code starts left to bit 16 instead to bit 6 in the opcode.
-	 * Gas is bug-compatible ...
+	 * Gas is bug-compatible, but not always, grrr...
+	 * We handle both cases with a simple heuristics.  --macro
 	 */
-	bcode = ((opcode >> 16) & ((1 << 20) - 1));
+	bcode = ((opcode >> 6) & ((1 << 20) - 1));
+	if (bcode < (1 << 10))
+		bcode <<= 10;
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all break
@@ -607,9 +611,9 @@ asmlinkage void do_bp(struct pt_regs *re
 	 * But should we continue the brokenness???  --macro
 	 */
 	switch (bcode) {
-	case 6:
-	case 7:
-		if (bcode == 7)
+	case BRK_OVERFLOW << 10:
+	case BRK_DIVZERO << 10:
+		if (bcode == (BRK_DIVZERO << 10))
 			info.si_code = FPE_INTDIV;
 		else
 			info.si_code = FPE_INTOVF;
@@ -633,7 +637,7 @@ asmlinkage void do_tr(struct pt_regs *re
 
 	/* Immediate versions don't provide a code.  */
 	if (!(opcode & OPCODE))
-		tcode = ((opcode >> 6) & ((1 << 20) - 1));
+		tcode = ((opcode >> 6) & ((1 << 10) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all trap
@@ -642,9 +646,9 @@ asmlinkage void do_tr(struct pt_regs *re
 	 * But should we continue the brokenness???  --macro
 	 */
 	switch (tcode) {
-	case 6:
-	case 7:
-		if (tcode == 7)
+	case BRK_OVERFLOW:
+	case BRK_DIVZERO:
+		if (tcode == BRK_DIVZERO)
 			info.si_code = FPE_INTDIV;
 		else
 			info.si_code = FPE_INTOVF;
diff -up --recursive --new-file linux-mips-2.4.26-20040531.macro/arch/mips64/kernel/traps.c linux-mips-2.4.26-20040531/arch/mips64/kernel/traps.c
--- linux-mips-2.4.26-20040531.macro/arch/mips64/kernel/traps.c	2004-03-13 03:56:45.000000000 +0000
+++ linux-mips-2.4.26-20040531/arch/mips64/kernel/traps.c	2004-06-13 20:26:01.000000000 +0000
@@ -9,7 +9,7 @@
  * Copyright (C) 1999 Silicon Graphics, Inc.
  * Kevin D. Kissell, [email protected] and Carsten Langgaard, [email protected]
  * Copyright (C) 2000, 01 MIPS Technologies, Inc.
- * Copyright (C) 2002, 2003  Maciej W. Rozycki
+ * Copyright (C) 2002, 2003, 2004  Maciej W. Rozycki
  */
 #include <linux/config.h>
 #include <linux/init.h>
@@ -22,6 +22,7 @@
 
 #include <asm/bootinfo.h>
 #include <asm/branch.h>
+#include <asm/break.h>
 #include <asm/cpu.h>
 #include <asm/fpu.h>
 #include <asm/module.h>
@@ -606,9 +607,12 @@ asmlinkage void do_bp(struct pt_regs *re
 	/*
 	 * There is the ancient bug in the MIPS assemblers that the break
 	 * code starts left to bit 16 instead to bit 6 in the opcode.
-	 * Gas is bug-compatible ...
+	 * Gas is bug-compatible, but not always, grrr...
+	 * We handle both cases with a simple heuristics.  --macro
 	 */
-	bcode = ((opcode >> 16) & ((1 << 20) - 1));
+	bcode = ((opcode >> 6) & ((1 << 20) - 1));
+	if (bcode < (1 << 10))
+		bcode <<= 10;
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all break
@@ -617,9 +621,9 @@ asmlinkage void do_bp(struct pt_regs *re
 	 * But should we continue the brokenness???  --macro
 	 */
 	switch (bcode) {
-	case 6:
-	case 7:
-		if (bcode == 7)
+	case BRK_OVERFLOW << 10:
+	case BRK_DIVZERO << 10:
+		if (bcode == (BRK_DIVZERO << 10))
 			info.si_code = FPE_INTDIV;
 		else
 			info.si_code = FPE_INTOVF;
@@ -643,7 +647,7 @@ asmlinkage void do_tr(struct pt_regs *re
 
 	/* Immediate versions don't provide a code.  */
 	if (!(opcode & OPCODE))
-		tcode = ((opcode >> 6) & ((1 << 20) - 1));
+		tcode = ((opcode >> 6) & ((1 << 10) - 1));
 
 	/*
 	 * (A short test says that IRIX 5.3 sends SIGTRAP for all trap
@@ -652,9 +656,9 @@ asmlinkage void do_tr(struct pt_regs *re
 	 * But should we continue the brokenness???  --macro
 	 */
 	switch (tcode) {
-	case 6:
-	case 7:
-		if (tcode == 7)
+	case BRK_OVERFLOW:
+	case BRK_DIVZERO:
+		if (tcode == BRK_DIVZERO)
 			info.si_code = FPE_INTDIV;
 		else
 			info.si_code = FPE_INTOVF;
#11 lore raw
In reply to [email protected]
Quoted 14 lines · On Fri, 11 Jun 2004 [email protected] wrote:

in retrospect, the 'B' variation probably wasn't the greatest idea.

If it were removed (leaving 'c' and 'c','q' variations), I don't know that any real harm would occur.

It may be very confusing to people who expect that the break code will translate into the instruction in an obvious way, and obviously it would mess up use of 20-bit codes, but i don't know how prevalent that is.

Unfortunately, at this point, Linux should probably accept the divide-by-zero code in both locations.

Quoted 5 lines

(Really, from day one, assemblers probably should have accepted a 20-bit code. I just checked my copy of the Kane r2000/r3000 book, and it was 20-bit all the way back then. If i had to guess, i'd guess that gas was copying a non-gnu assembler's behaviour. In any case, water under the bridge.)

As it's at least annoying to have different break codes for divisions
expanded by gcc explicitly and ones created implicitly by gas, here's the most reasonable (IMO) approach to fix that. I think it should have been implemented this way originally (if at all).

gas/testsuite/:
2004-06-22 Maciej W. Rozycki <[email protected]>

* gas/mips/break20.s: Test the "break20" alias.
* gas/mips/break20.d: Results for the test.
* gas/mips/mips32.s: Replace "break" with "break20".
* gas/mips/set-arch.s: Likewise.
* gas/mips/mips32.d: Adjust for the new output.
* gas/mips/set-arch.d: Likewise.

opcodes/:
2004-06-22 Maciej W. Rozycki <[email protected]>

* mips-opc.c (mips_builtin_opcodes): Replace the MIPS32 ISA specific "break" encoding with a "break20" alias accepted for any ISA.

I decided to give a precedence to "break x,y" over "break20 z" to avoid
perhaps a bit surprising output in `objdump'.

The question is: does anyone know of possible troubles with this change?
Chances are someone depends on the current semantics for -march=mips32/mips64, but that's fragile anyway as building with a different setting results in different code.

Or should we get rid of the 20-bit "break" completely? The two-argument
version provides the same functionality, although the 10-bit codes to be used do not map to the 20-bit equivalent "optically" very well. Especially if decimal notation is used.

Note the same problem appears to be the case with "sdbbp". It could be
handled similarly, but given the limited use of "sdbbp" for non-MIPS32/MIPS64 ISAs it may not be worth the hassle. I see no problem writing a similar fix if desired, though.

Maciej

Signature · 104 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

binutils-2.15.91-20040615-mips-break20.patch
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/break20.d binutils-2.15.91-20040615/gas/testsuite/gas/mips/break20.d
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/break20.d	2003-05-08 03:25:31.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/break20.d	2004-06-17 21:03:30.000000000 +0000
@@ -10,9 +10,13 @@ Disassembly of section .text:
 0+0008 <[^>]*> break	0x14
 0+000c <[^>]*> break	0x14,0x28
 0+0010 <[^>]*> break	0x3ff,0x3ff
-0+0014 <[^>]*> sdbbp
-0+0018 <[^>]*> sdbbp
-0+001c <[^>]*> sdbbp	0x14
-0+0020 <[^>]*> sdbbp	0x14,0x28
-0+0024 <[^>]*> sdbbp	0x3ff,0x3ff
+0+0014 <[^>]*> break
+0+0018 <[^>]*> break	0x0,0x14
+0+001c <[^>]*> break	0x14,0x28
+0+0020 <[^>]*> break	0x3ff,0x3ff
+0+0024 <[^>]*> sdbbp
+0+0028 <[^>]*> sdbbp
+0+002c <[^>]*> sdbbp	0x14
+0+0030 <[^>]*> sdbbp	0x14,0x28
+0+0034 <[^>]*> sdbbp	0x3ff,0x3ff
 	...
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/break20.s binutils-2.15.91-20040615/gas/testsuite/gas/mips/break20.s
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/break20.s	1999-08-10 14:24:45.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/break20.s	2004-06-17 19:57:51.000000000 +0000
@@ -6,6 +6,11 @@ foo:	
 	break	20,40
 	break	1023,1023
 	
+	break20	0
+	break20	20
+	break20	20520
+	break20	1048575
+
 	sdbbp
 	sdbbp	0
 	sdbbp	20
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/mips32.d binutils-2.15.91-20040615/gas/testsuite/gas/mips/mips32.d
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/mips32.d	2003-05-08 03:25:35.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/mips32.d	2004-06-17 21:15:04.000000000 +0000
@@ -48,7 +48,7 @@ Disassembly of section .text:
 0+0098 <[^>]*> 4359e260 	wait	0x56789
 0+009c <[^>]*> 0000000d 	break
 0+00a0 <[^>]*> 0000000d 	break
-0+00a4 <[^>]*> 0048d14d 	break	0x12345
+0+00a4 <[^>]*> 0048d14d 	break	0x48,0x345
 0+00a8 <[^>]*> 7000003f 	sdbbp
 0+00ac <[^>]*> 7000003f 	sdbbp
 0+00b0 <[^>]*> 7159e27f 	sdbbp	0x56789
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/mips32.s binutils-2.15.91-20040615/gas/testsuite/gas/mips/mips32.s
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/mips32.s	2002-09-05 03:25:40.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/mips32.s	2004-06-17 21:13:22.000000000 +0000
@@ -62,7 +62,7 @@ text_label:
       # different.
       break
       break   0                       # disassembles without code
-      break   0x12345
+      break20 0x12345
       sdbbp
       sdbbp   0                       # disassembles without code
       sdbbp   0x56789
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/set-arch.d binutils-2.15.91-20040615/gas/testsuite/gas/mips/set-arch.d
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/set-arch.d	2003-10-01 03:25:36.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/set-arch.d	2004-06-17 21:17:46.000000000 +0000
@@ -160,7 +160,7 @@ Disassembly of section \.text:
 00000260 <[^>]*> 4359e260 	wait	0x56789
 00000264 <[^>]*> 0000000d 	break
 00000268 <[^>]*> 0000000d 	break
-0000026c <[^>]*> 0048d14d 	break	0x12345
+0000026c <[^>]*> 0048d14d 	break	0x48,0x345
 00000270 <[^>]*> 7000003f 	sdbbp
 00000274 <[^>]*> 7000003f 	sdbbp
 00000278 <[^>]*> 7159e27f 	sdbbp	0x56789
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/set-arch.s binutils-2.15.91-20040615/gas/testsuite/gas/mips/set-arch.s
--- binutils-2.15.91-20040615.macro/gas/testsuite/gas/mips/set-arch.s	2003-06-29 19:41:33.000000000 +0000
+++ binutils-2.15.91-20040615/gas/testsuite/gas/mips/set-arch.s	2004-06-17 21:18:01.000000000 +0000
@@ -204,7 +204,7 @@ text_label:	
 	# different.
 	break
 	break   0                       # disassembles without code
-	break   0x12345
+	break20 0x12345
 	sdbbp
 	sdbbp   0                       # disassembles without code
 	sdbbp   0x56789
diff -up --recursive --new-file binutils-2.15.91-20040615.macro/opcodes/mips-opc.c binutils-2.15.91-20040615/opcodes/mips-opc.c
--- binutils-2.15.91-20040615.macro/opcodes/mips-opc.c	2003-11-19 04:25:23.000000000 +0000
+++ binutils-2.15.91-20040615/opcodes/mips-opc.c	2004-06-17 19:46:23.000000000 +0000
@@ -274,9 +274,9 @@ const struct mips_opcode mips_builtin_op
 {"bnel",    "s,t,p",	0x54000000, 0xfc000000,	CBL|RD_s|RD_t, 		I2|T3	},
 {"bnel",    "s,I,p",	0,    (int) M_BNEL_I,	INSN_MACRO,		I2|T3	},
 {"break",   "",		0x0000000d, 0xffffffff,	TRAP,			I1	},
-{"break",   "B",        0x0000000d, 0xfc00003f, TRAP,           	I32     },
 {"break",   "c",	0x0000000d, 0xfc00ffff,	TRAP,			I1	},
 {"break",   "c,q",	0x0000000d, 0xfc00003f,	TRAP,			I1	},
+{"break20", "B",        0x0000000d, 0xfc00003f, TRAP,           	I1      },
 {"c.f.d",   "S,T",	0x46200030, 0xffe007ff,	RD_S|RD_T|WR_CC|FP_D,	I1	},
 {"c.f.d",   "M,S,T",    0x46200030, 0xffe000ff, RD_S|RD_T|WR_CC|FP_D,   I4|I32	},
 {"c.f.s",   "S,T",      0x46000030, 0xffe007ff, RD_S|RD_T|WR_CC|FP_S,   I1      },
#12 lore raw
Quoted 38 lines · Maciej W. Rozycki
Quoted 14 lines · On Fri, 11 Jun 2004 [email protected] wrote:

in retrospect, the 'B' variation probably wasn't the greatest idea.

If it were removed (leaving 'c' and 'c','q' variations), I don't know that any real harm would occur.

It may be very confusing to people who expect that the break code will translate into the instruction in an obvious way, and obviously it would mess up use of 20-bit codes, but i don't know how prevalent that is.

Unfortunately, at this point, Linux should probably accept the divide-by-zero code in both locations.

Quoted 7 lines

(Really, from day one, assemblers probably should have accepted a 20-bit code. I just checked my copy of the Kane r2000/r3000 book, and it was 20-bit all the way back then. If i had to guess, i'd guess that gas was copying a non-gnu assembler's behaviour. In any case, water under the bridge.)

As it's at least annoying to have different break codes for divisions expanded by gcc explicitly and ones created implicitly by gas, here's the most reasonable (IMO) approach to fix that. I think it should have been implemented this way originally (if at all).

gas/testsuite/:
2004-06-22 Maciej W. Rozycki <[email protected]>

* gas/mips/break20.s: Test the "break20" alias.
* gas/mips/break20.d: Results for the test.
* gas/mips/mips32.s: Replace "break" with "break20".
* gas/mips/set-arch.s: Likewise.
* gas/mips/mips32.d: Adjust for the new output.
* gas/mips/set-arch.d: Likewise.

Quoted 9 lines

opcodes/:
2004-06-22 Maciej W. Rozycki <[email protected]>

* mips-opc.c (mips_builtin_opcodes): Replace the MIPS32 ISA specific "break" encoding with a "break20" alias accepted for any ISA.

.
.
.
Just out of curiosity, do you propose this patch in lieu of the patch to Linux's traps.c?

Or would you do both?

It seems like both would be best, as there are already "broken" binutils floating around out there.

Also nobody has objected to the kernel patch...

David Daney.

#13 lore raw
Quoted 4 lines · David Daney

Just out of curiosity, do you propose this patch in lieu of the patch to Linux's traps.c?

Or would you do both?

Both.

Quoted 2 lines

It seems like both would be best, as there are already "broken" binutils floating around out there.

And broken binaries.

Signature · 3 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +
#14 lore raw
In reply to Maciej W. Rozycki
Quoted 4 lines · Maciej W. Rozycki

Or should we get rid of the 20-bit "break" completely? The two-argument
version provides the same functionality, although the 10-bit codes to be used do not map to the 20-bit equivalent "optically" very well. Especially if decimal notation is used.

I notice no-one's really responded to this question yet. FWIW, on gut instinct, I'd personally prefer to drop the 20-bit break than introduce a new, non-standard name for it.

Just an opinion though. I won't argue against anyone saying different. ;)

Richard

#15 lore raw
Quoted 8 lines · Richard Sandiford
Quoted 4 lines

Or should we get rid of the 20-bit "break" completely? The two-argument
version provides the same functionality, although the 10-bit codes to be used do not map to the 20-bit equivalent "optically" very well. Especially if decimal notation is used.

I notice no-one's really responded to this question yet. FWIW, on gut instinct, I'd personally prefer to drop the 20-bit break than introduce a new, non-standard name for it.

Well, this is essentially what the patch does. Or do you mean: "drop it
and if anyone screams, consider an alternative?" I'd find it acceptable, actually, but it's not my opinion that really matters here.

Signature · 3 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

A message that is not in the archive (sent off-list or not yet received): mailpost.1088102121.25381@news-sj1-1

#17 lore raw
Quoted 3 lines · Maciej W. Rozycki

Well, this is essentially what the patch does. Or do you mean: "drop it
and if anyone screams, consider an alternative?" I'd find it acceptable, actually, but it's not my opinion that really matters here.

(it's fine w/ me.)

#18 lore raw
Quoted 5 lines · On Thu, 24 Jun 2004 [email protected] wrote:
Quoted 3 lines

Well, this is essentially what the patch does. Or do you mean: "drop it
and if anyone screams, consider an alternative?" I'd find it acceptable, actually, but it's not my opinion that really matters here.

(it's fine w/ me.)

Here's an updated patch I use currently in case this is the solution to
be agreed upon.

opcodes/:
2004-06-28 Maciej W. Rozycki <[email protected]>

* mips-opc.c (mips_builtin_opcodes): Remove the MIPS32 ISA-specific "break" encoding.

gas/testsuite/:
2004-06-28 Maciej W. Rozycki <[email protected]>

* gas/mips/mips32.s: Adjust for the unified "break" syntax.
* gas/mips/set-arch.s: Likewise.
* gas/mips/mips32.d: Adjust for the new output.
* gas/mips/set-arch.d: Likewise.

Maciej

binutils-2.15.91-20040625-mips-break20.patch

gas/testsuite/gas/mips/mips32.d +1 -1
diff -up --recursive --new-file binutils-2.15.91-20040625.macro/gas/testsuite/gas/mips/mips32.d binutils-2.15.91-20040625/gas/testsuite/gas/mips/mips32.d
@@ -48,7 +48,7 @@ Disassembly of section .text:
 0+0098 <[^>]*> 4359e260 	wait	0x56789
 0+009c <[^>]*> 0000000d 	break
 0+00a0 <[^>]*> 0000000d 	break
-0+00a4 <[^>]*> 0048d14d 	break	0x12345
+0+00a4 <[^>]*> 0048d14d 	break	0x48,0x345
 0+00a8 <[^>]*> 7000003f 	sdbbp
 0+00ac <[^>]*> 7000003f 	sdbbp
 0+00b0 <[^>]*> 7159e27f 	sdbbp	0x56789
gas/testsuite/gas/mips/mips32.s +5 -3
diff -up --recursive --new-file binutils-2.15.91-20040625.macro/gas/testsuite/gas/mips/mips32.s binutils-2.15.91-20040625/gas/testsuite/gas/mips/mips32.s
@@ -58,11 +58,13 @@ text_label:
       wait    0                       # disassembles without code
       wait    0x56789
 
-      # Instructions in previous ISAs or CPUs which are now slightly
-      # different.
+      # Instructions that used to have compatibility problems.
       break
       break   0                       # disassembles without code
-      break   0x12345
+      break   0x48,0x345
+
+      # Instructions in previous ISAs or CPUs which are now slightly
+      # different.
       sdbbp
       sdbbp   0                       # disassembles without code
       sdbbp   0x56789
gas/testsuite/gas/mips/set-arch.d +1 -1
diff -up --recursive --new-file binutils-2.15.91-20040625.macro/gas/testsuite/gas/mips/set-arch.d binutils-2.15.91-20040625/gas/testsuite/gas/mips/set-arch.d
@@ -160,7 +160,7 @@ Disassembly of section \.text:
 00000260 <[^>]*> 4359e260 	wait	0x56789
 00000264 <[^>]*> 0000000d 	break
 00000268 <[^>]*> 0000000d 	break
-0000026c <[^>]*> 0048d14d 	break	0x12345
+0000026c <[^>]*> 0048d14d 	break	0x48,0x345
 00000270 <[^>]*> 7000003f 	sdbbp
 00000274 <[^>]*> 7000003f 	sdbbp
 00000278 <[^>]*> 7159e27f 	sdbbp	0x56789
gas/testsuite/gas/mips/set-arch.s +5 -3
diff -up --recursive --new-file binutils-2.15.91-20040625.macro/gas/testsuite/gas/mips/set-arch.s binutils-2.15.91-20040625/gas/testsuite/gas/mips/set-arch.s
@@ -200,11 +200,13 @@ text_label:	
 	wait    0                       # disassembles without code
 	wait    0x56789
 
-	# Instructions in previous ISAs or CPUs which are now slightly
-	# different.
+	# Instructions that used to have compatibility problems.
 	break
 	break   0                       # disassembles without code
-	break   0x12345
+	break   0x48,0x345
+
+	# Instructions in previous ISAs or CPUs which are now slightly
+	# different.
 	sdbbp
 	sdbbp   0                       # disassembles without code
 	sdbbp   0x56789
opcodes/mips-opc.c +0 -1
diff -up --recursive --new-file binutils-2.15.91-20040625.macro/opcodes/mips-opc.c binutils-2.15.91-20040625/opcodes/mips-opc.c
@@ -274,7 +274,6 @@ const struct mips_opcode mips_builtin_op
 {"bnel",    "s,t,p",	0x54000000, 0xfc000000,	CBL|RD_s|RD_t, 		I2|T3	},
 {"bnel",    "s,I,p",	0,    (int) M_BNEL_I,	INSN_MACRO,		I2|T3	},
 {"break",   "",		0x0000000d, 0xffffffff,	TRAP,			I1	},
-{"break",   "B",        0x0000000d, 0xfc00003f, TRAP,           	I32     },
 {"break",   "c",	0x0000000d, 0xfc00ffff,	TRAP,			I1	},
 {"break",   "c,q",	0x0000000d, 0xfc00003f,	TRAP,			I1	},
 {"c.f.d",   "S,T",	0x46200030, 0xffe007ff,	RD_S|RD_T|WR_CC|FP_D,	I1	},