Branch bug in gas on MIPS

Branch bug in gas on MIPS

[Ralf Baechle]

Below little test case demonstrates a gas bug that results in swapping
of the two branch instructions and use of bogus destination addresses
for the first of the two branches.

[ralf@lappi tmp]$ cat s.s
1:      beqzl   $2, 1b
        beq     $4, $5, 1b
[ralf@lappi tmp]$ mips-linux-as -mips2 -o s.o s.s
[ralf@lappi tmp]$ mips-linux-objdump -d s.o
 
s.o:     file format elf32-tradbigmips
 
Disassembly of section .text:
 
00000000 <.text>:
   0:   1085ffff        beq     a0,a1,0x0
   4:   00000000        nop
   8:   50400000        beqzl   v0,0xc
   c:   00000000        nop

Have a nice day ;-)

  Ralf

Re: Branch bug in gas on MIPS

[Ralf Baechle]

On Tue, Aug 17, 2004 at 06:01:10PM +0200, Ralf Baechle wrote:

I've uploaded fixed rpm packages of cross-binutils built on Fedora Core 2
for i386 to ftp.linux-mips.org which fix the swapped branch bug which I
posted yesterday.

03347fbaefda6451ad025e05dd43fc79  binutils-mips-linux-2.13.2.1-4.i386.rpm
6d3f22d1666497d6d02e2a2534426709  binutils-mips64-linux-2.13.2.1-4.i386.rpm
aca35612fa321ca01b02e84512cd2ae7  binutils-mips64el-linux-2.13.2.1-4.i386.rpm
1e87615b74173a2a2c5b94b60dc4bb2e  binutils-mipsel-linux-2.13.2.1-4.i386.rpm
a26ba1110361c2c167223ad01ed1acc2  cross-binutils-2.13.2.1-4.src.rpm

Please verify the checksums; for about 2h before this announcement I had
broken rpm packages online.

The unbundled fix (credits for which belong to Thiemo Seufer) is below.

  Ralf

diff -urN binutils-2.13.2.1.orig/gas/config/tc-mips.c binutils-2.13.2.1/gas/config/tc-mips.c
--- binutils-2.13.2.1.orig/gas/config/tc-mips.c	2002-11-05 23:03:40.000000000 +0100
+++ binutils-2.13.2.1/gas/config/tc-mips.c	2004-08-18 13:15:31.553748456 +0200
@@ -2466,6 +2466,7 @@
 	  prev_insn_reloc_type[1] = BFD_RELOC_UNUSED;
 	  prev_insn_reloc_type[2] = BFD_RELOC_UNUSED;
 	  prev_insn_extended = 0;
+	  prev_insn_is_delay_slot = 1;
 	}
       else
 	{

Re: Branch bug in gas on MIPS

[Thiemo Seufer]

Ralf Baechle wrote:
> Below little test case demonstrates a gas bug that results in swapping
> of the two branch instructions and use of bogus destination addresses
> for the first of the two branches.
> 
> [ralf@lappi tmp]$ cat s.s
> 1:      beqzl   $2, 1b
>         beq     $4, $5, 1b
> [ralf@lappi tmp]$ mips-linux-as -mips2 -o s.o s.s
> [ralf@lappi tmp]$ mips-linux-objdump -d s.o
>  
> s.o:     file format elf32-tradbigmips
>  
> Disassembly of section .text:
>  
> 00000000 <.text>:
>    0:   1085ffff        beq     a0,a1,0x0
>    4:   00000000        nop
>    8:   50400000        beqzl   v0,0xc
>    c:   00000000        nop

I applied the appended patch. Daniel, I think this should also go
in the branch.


Thiemo


/gas/ChangeLog
2004-08-18  Thiemo Seufer  <seufer@csv.ica.uni-stuttgart.de>
	* config/tc-mips.c (append_insn): Handle delay slots in branch likely
	correctly.

/gas/testsuite/ChangeLog
2004-08-18  Thiemo Seufer  <seufer@csv.ica.uni-stuttgart.de>
	* gas/mips/branch-swap.s: New testcase.
	* gas/mips/branch-swap.d: New testcase.
	* gas/mips/mips.exp: Run the testcase.


--- gas/config/tc-mips.c.old	2004-05-17 21:36:10.000000000 +0200
+++ gas/config/tc-mips.c	2004-08-17 20:00:43.000000000 +0200
@@ -2708,6 +2708,7 @@ append_insn (struct mips_cl_insn *ip, ex
 	  prev_insn_reloc_type[1] = BFD_RELOC_UNUSED;
 	  prev_insn_reloc_type[2] = BFD_RELOC_UNUSED;
 	  prev_insn_extended = 0;
+	  prev_insn_is_delay_slot = 1;
 	}
       else
 	{
--- gas/testsuite/gas/mips/mips.exp.old	2004-08-17 22:50:38.000000000 +0200
+++ gas/testsuite/gas/mips/mips.exp	2004-08-18 14:53:43.000000000 +0200
@@ -429,6 +429,7 @@ if { [istarget mips*-*-*] } then {
     run_dump_test_arches "branch-misc-1" [mips_arch_list_matching mips1]
     run_list_test_arches "branch-misc-2" "-32 -non_shared" [mips_arch_list_matching mips1]
     run_list_test_arches "branch-misc-2pic" "-32 -call_shared" [mips_arch_list_matching mips1]
+    run_dump_test "branch-swap"
 
     if $ilocks {
 	run_dump_test "div-ilocks"
--- gas/testsuite/gas/mips/branch-swap.s.old	1970-01-01 01:00:00.000000000 +0100
+++ gas/testsuite/gas/mips/branch-swap.s	2004-08-17 22:52:50.000000000 +0200
@@ -0,0 +1,9 @@
+	.set push
+	.set mips2
+1:	beqzl	$2, 1b
+	b	1b
+foo:	beqzl	$2, foo
+	b	foo
+
+	.set pop
+	.space 8
--- gas/testsuite/gas/mips/branch-swap.d.old	1970-01-01 01:00:00.000000000 +0100
+++ gas/testsuite/gas/mips/branch-swap.d	2004-08-17 22:57:36.000000000 +0200
@@ -0,0 +1,20 @@
+#as: -march=mips2
+#objdump: -dr
+#name: MIPS branch-swap
+
+.*:     file format .*mips.*
+
+Disassembly of section \.text:
+
+00000000 <foo-0x10>:
+   0:	5040ffff 	beqzl	v0,0 <foo-0x10>
+   4:	00000000 	nop
+   8:	1000fffd 	b	0 <foo-0x10>
+   c:	00000000 	nop
+
+00000010 <foo>:
+  10:	5040ffff 	beqzl	v0,10 <foo>
+  14:	00000000 	nop
+  18:	1000fffd 	b	10 <foo>
+  1c:	00000000 	nop
+	\.\.\.

Re: Branch bug in gas on MIPS

[Daniel Jacobowitz]

On Wed, Aug 18, 2004 at 05:58:19PM +0200, Thiemo Seufer wrote:
> Ralf Baechle wrote:
> > Below little test case demonstrates a gas bug that results in swapping
> > of the two branch instructions and use of bogus destination addresses
> > for the first of the two branches.
> > 
> > [ralf@lappi tmp]$ cat s.s
> > 1:      beqzl   $2, 1b
> >         beq     $4, $5, 1b
> > [ralf@lappi tmp]$ mips-linux-as -mips2 -o s.o s.s
> > [ralf@lappi tmp]$ mips-linux-objdump -d s.o
> >  
> > s.o:     file format elf32-tradbigmips
> >  
> > Disassembly of section .text:
> >  
> > 00000000 <.text>:
> >    0:   1085ffff        beq     a0,a1,0x0
> >    4:   00000000        nop
> >    8:   50400000        beqzl   v0,0xc
> >    c:   00000000        nop
> 
> I applied the appended patch. Daniel, I think this should also go
> in the branch.

OK.

-- 
Daniel Jacobowitz