Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[Andreas Borchert]

Dear H.J. Lu,

gas out of binutils-2.9.1.0.23 (and older versions) has a severe bug
for the SPARC platform (sparc-sun-solaris2.6), i.e. it generates
wrong code for

        sethi   0x4000,%i0

(and probably in all other cases where sethi is used without the
%hi(...)-construct).

The sethi instruction has following suggested assembly syntax
(see SPARC Architecture Manual):

        sethi   const22,reg

const22 is a 22-bit constant that is placed by sethi in the most
significant 22 bits of reg (while it zeroes the 10 least significant
bits of reg). Therefore we would expect the value 0x100000 in %i0
after the execution of ``sethi 16384''. Instead of this, gas
modifies the given constant such that 0x4000 will be found in %i0
afterwards. This would have been equivalent to

        sethi %hi(0x4000),%i0

Anyway, I would like to take this opportunity to forward my severe
thanks to you. For my compilers gas has always been a life safer. The
assembler shipped with Solaris is not just significantly slower, it has
much more severe bugs that are not that easy to live with.

Kind regards, Andreas Borchert.

-- 
Andreas Borchert, Universitaet Ulm, SAI, Helmholtzstr. 18, 89069 Ulm,  Germany
E-Mail: borchert@mathematik.uni-ulm.de
WWW:	http://www.mathematik.uni-ulm.de/sai/borchert/
PGP key available via ``finger borchert@laborix.mathematik.uni-ulm.de''

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[H.J. Lu]

> gas out of binutils-2.9.1.0.23 (and older versions) has a severe bug
> for the SPARC platform (sparc-sun-solaris2.6), i.e. it generates
> wrong code for
> 
>         sethi   0x4000,%i0
> 
> (and probably in all other cases where sethi is used without the
> %hi(...)-construct).
> 
> The sethi instruction has following suggested assembly syntax
> (see SPARC Architecture Manual):
> 
>         sethi   const22,reg
> 
> const22 is a 22-bit constant that is placed by sethi in the most
> significant 22 bits of reg (while it zeroes the 10 least significant
> bits of reg). Therefore we would expect the value 0x100000 in %i0
> after the execution of ``sethi 16384''. Instead of this, gas
> modifies the given constant such that 0x4000 will be found in %i0
> afterwards. This would have been equivalent to
> 
>         sethi %hi(0x4000),%i0
> 

Please try this patch. I know next to nothing about Sparc asm. Please
make sure this patch won't break anything on Sparc and let me know
the result. I hope someone will make a real fix for this bug.

Thanks.


H.J.
----
Fri Apr  2 11:44:49 1999  H.J. Lu  (hjl@gnu.org)

        * config/tc-sparc.c (sparc_ip): Fix "sethi const22,reg".

Index: tc-sparc.c
===================================================================
RCS file: /local/work/cvs/gnu/binutils/gas/config/tc-sparc.c,v
retrieving revision 1.10
diff -u -p -r1.10 tc-sparc.c
--- tc-sparc.c	1999/03/31 17:24:48	1.10
+++ tc-sparc.c	1999/04/02 19:45:21
@@ -1126,6 +1126,7 @@ sparc_ip (str, pinsn)
   unsigned int mask = 0;
   int match = 0;
   int comma = 0;
+  int unary = 0;
   int v9_arg_p;
 
   for (s = str; islower ((unsigned char) *s) || (s > str && *s >= '0' && *s <= '8'); ++s)
@@ -1876,6 +1877,9 @@ sparc_ip (str, pinsn)
 		  };
 		  struct ops *o;
 
+		  /* We have seen "%xx". */
+		  unary = 1;
+
 		  for (o = ops; o->name; o++)
 		    if (strncmp (s + 1, o->name, o->len) == 0)
 		      break;
@@ -1940,6 +1944,21 @@ sparc_ip (str, pinsn)
 		      error_message = ": PC-relative operand can't be a constant";
 		      goto error;
 		    }
+		  else if (strcmp (insn->name, "sethi") == 0
+			   && unary == 0)
+		    /* The sethi instruction has following suggested
+		       assembly syntax (see SPARC Architecture Manual):
+
+			    sethi	const22,reg
+
+		       const22 is a 22-bit constant that is placed by
+		       sethi in the most significant 22 bits of reg
+		       (while it zeroes the 10 least significant bits
+		       of reg). It is different from
+
+			    sethi	%hi(value),reg
+		      */
+		    the_insn.reloc = BFD_RELOC_SPARC22;
 
 		  /* Constants that won't fit are checked in md_apply_fix3
 		     and bfd_install_relocation.

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[H.J. Lu]

> 
> gas out of binutils-2.9.1.0.23 (and older versions) has a severe bug
> for the SPARC platform (sparc-sun-solaris2.6), i.e. it generates
> wrong code for
> 
>         sethi   0x4000,%i0
> 
> (and probably in all other cases where sethi is used without the
> %hi(...)-construct).
> 
> The sethi instruction has following suggested assembly syntax
> (see SPARC Architecture Manual):
> 
>         sethi   const22,reg
> 
> const22 is a 22-bit constant that is placed by sethi in the most
> significant 22 bits of reg (while it zeroes the 10 least significant
> bits of reg). Therefore we would expect the value 0x100000 in %i0
> after the execution of ``sethi 16384''. Instead of this, gas
> modifies the given constant such that 0x4000 will be found in %i0
> afterwards. This would have been equivalent to
> 
>         sethi %hi(0x4000),%i0
> 

Hi,

Here is a new patch. Please discard my last patch to tc-sparc.c.
Let me know what you get.

Thanks.


H.J.
---
Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)

	* sparc-opc.c: Change "sethi" from "h,d" to "n,d".

Index: ChangeLog.linux
===================================================================
RCS file: /local/work/cvs/gnu/binutils/opcodes/ChangeLog.linux,v
retrieving revision 1.40
diff -u -p -r1.40 ChangeLog.linux
--- ChangeLog.linux	1999/03/31 17:24:49	1.40
+++ ChangeLog.linux	1999/04/03 00:05:33
@@ -1,3 +1,5 @@
+Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)
+
 Sun Mar 28 11:48:17 1999  H.J. Lu  (hjl@gnu.org)
 
 	* i386-dis.c (INSN_FWAIT): New, defined.
Index: sparc-opc.c
===================================================================
RCS file: /local/work/cvs/gnu/binutils/opcodes/sparc-opc.c,v
retrieving revision 1.7
diff -u -p -r1.7 sparc-opc.c
--- sparc-opc.c	1998/12/05 03:38:56	1.7
+++ sparc-opc.c	1999/04/03 00:00:22
@@ -1379,7 +1379,7 @@ CONDFC  ("fbule", "cb013", 0xe, 0),
 { "setsw",	F2(0x0, 0x4), F2(~0x0, ~0x4), "Sh,d", F_ALIAS, v9 },
 { "setx",	F2(0x0, 0x4), F2(~0x0, ~0x4), "S0,1,d", F_ALIAS, v9 },
 
-{ "sethi",	F2(0x0, 0x4), F2(~0x0, ~0x4), "h,d", 0, v6 },
+{ "sethi",	F2(0x0, 0x4), F2(~0x0, ~0x4), "n,d", 0, v6 },
 
 { "taddcc",	F3(2, 0x20, 0), F3(~2, ~0x20, ~0)|ASI(~0),	"1,2,d", 0, v6 },
 { "taddcc",	F3(2, 0x20, 1), F3(~2, ~0x20, ~1),		"1,i,d", 0, v6 },

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[Ian Lance Taylor]

   From: hjl@lucon.org (H.J. Lu)
   Date: Fri, 2 Apr 1999 16:10:48 -0800 (PST)

   > gas out of binutils-2.9.1.0.23 (and older versions) has a severe bug
   > for the SPARC platform (sparc-sun-solaris2.6), i.e. it generates
   > wrong code for
   > 
   >         sethi   0x4000,%i0
   > 
   > (and probably in all other cases where sethi is used without the
   > %hi(...)-construct).
   > 
   > The sethi instruction has following suggested assembly syntax
   > (see SPARC Architecture Manual):
   > 
   >         sethi   const22,reg
   > 
   > const22 is a 22-bit constant that is placed by sethi in the most
   > significant 22 bits of reg (while it zeroes the 10 least significant
   > bits of reg). Therefore we would expect the value 0x100000 in %i0
   > after the execution of ``sethi 16384''. Instead of this, gas
   > modifies the given constant such that 0x4000 will be found in %i0
   > afterwards. This would have been equivalent to
   > 
   >         sethi %hi(0x4000),%i0

   Here is a new patch. Please discard my last patch to tc-sparc.c.
   Let me know what you get.

   Thanks.


   H.J.
   ---
   Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)

	   * sparc-opc.c: Change "sethi" from "h,d" to "n,d".

I don't think that can be right.  I think it will do the wrong thing
for
    sethi foo,%i0

Ian

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[H.J. Lu]

> 
>    From: hjl@lucon.org (H.J. Lu)
>    Date: Fri, 2 Apr 1999 16:10:48 -0800 (PST)
> 
>    > gas out of binutils-2.9.1.0.23 (and older versions) has a severe bug
>    > for the SPARC platform (sparc-sun-solaris2.6), i.e. it generates
>    > wrong code for
>    > 
>    >         sethi   0x4000,%i0
>    > 
>    > (and probably in all other cases where sethi is used without the
>    > %hi(...)-construct).
>    > 
>    > The sethi instruction has following suggested assembly syntax
>    > (see SPARC Architecture Manual):
>    > 
>    >         sethi   const22,reg
>    > 
>    > const22 is a 22-bit constant that is placed by sethi in the most
>    > significant 22 bits of reg (while it zeroes the 10 least significant
>    > bits of reg). Therefore we would expect the value 0x100000 in %i0
>    > after the execution of ``sethi 16384''. Instead of this, gas
>    > modifies the given constant such that 0x4000 will be found in %i0
>    > afterwards. This would have been equivalent to
>    > 
>    >         sethi %hi(0x4000),%i0
> 
>    Here is a new patch. Please discard my last patch to tc-sparc.c.
>    Let me know what you get.
> 
>    Thanks.
> 
> 
>    H.J.
>    ---
>    Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)
> 
> 	   * sparc-opc.c: Change "sethi" from "h,d" to "n,d".
> 
> I don't think that can be right.  I think it will do the wrong thing
> for
>     sethi foo,%i0
> 

You are right. That shows I know nothing about Sparc. Do you have
a fix for the bug?

Thanks.


-- 
H.J. Lu (hjl@gnu.org)

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[Ian Lance Taylor]

   From: hjl@lucon.org (H.J. Lu)
   Date: Fri, 2 Apr 1999 16:41:52 -0800 (PST)

   You are right. That shows I know nothing about Sparc. Do you have
   a fix for the bug?

No, I haven't had a chance to look into it yet.

I know I am behind.  I'll try to catch up soon.

Ian

Re: Bug of gas (binutils-2.9.1.0.23): Wrong code for SETHI on SPARC

[H.J. Lu]

>    Let me know what you get.
> 
>    Thanks.
> 
> 
>    H.J.
>    ---
>    Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)
> 
> 	   * sparc-opc.c: Change "sethi" from "h,d" to "n,d".
> 
> I don't think that can be right.  I think it will do the wrong thing
> for
>     sethi foo,%i0
> 
 
First, the current gas handles

	sethi foo,%i0

differently than Solaris' as. Solaris' as generates R_SPARC_22. But gas
generates R_SPARC_HI22. The patch enclosed here makes gas matches
Solaris' as. But I don't know if it is correct :-).

Thanks.


-- 
H.J. Lu (hjl@gnu.org)
---
Index: gas/ChangeLog.linux
===================================================================
RCS file: /local/work/cvs/gnu/binutils/gas/ChangeLog.linux,v
retrieving revision 1.62
diff -u -p -r1.62 ChangeLog.linux
--- gas/ChangeLog.linux	1999/03/31 18:21:59	1.62
+++ gas/ChangeLog.linux	1999/04/03 01:22:45
@@ -1,3 +1,7 @@
+Fri Apr  2 17:22:33 1999  H.J. Lu  (hjl@gnu.org)
+
+	* config/tc-sparc.c (tc_gen_reloc): Handle BFD_RELOC_SPARC22.
+
 Sun Mar 28 15:32:42 1999  H.J. Lu  (hjl@gnu.org)
 
 	* Makefile.in: Regenerated with automake 1.4.
Index: gas/config/tc-sparc.c
===================================================================
RCS file: /local/work/cvs/gnu/binutils/gas/config/tc-sparc.c,v
retrieving revision 1.10
diff -u -p -r1.10 tc-sparc.c
--- gas/config/tc-sparc.c	1999/03/31 17:24:48	1.10
+++ gas/config/tc-sparc.c	1999/04/03 01:05:44
@@ -2721,6 +2721,7 @@ tc_gen_reloc (section, fixp)
     case BFD_RELOC_LO10:
     case BFD_RELOC_32_PCREL_S2:
     case BFD_RELOC_SPARC13:
+    case BFD_RELOC_SPARC22:
     case BFD_RELOC_SPARC_BASE13:
     case BFD_RELOC_SPARC_WDISP16:
     case BFD_RELOC_SPARC_WDISP19:
Index: opcodes/ChangeLog.linux
===================================================================
RCS file: /local/work/cvs/gnu/binutils/opcodes/ChangeLog.linux,v
retrieving revision 1.40
diff -u -p -r1.40 ChangeLog.linux
--- opcodes/ChangeLog.linux	1999/03/31 17:24:49	1.40
+++ opcodes/ChangeLog.linux	1999/04/03 01:07:01
@@ -1,3 +1,7 @@
+Fri Apr  2 16:05:25 1999  H.J. Lu  (hjl@gnu.org)
+
+	* sparc-opc.c: Change "sethi" from "h,d" to "n,d".
+
 Sun Mar 28 11:48:17 1999  H.J. Lu  (hjl@gnu.org)
 
 	* i386-dis.c (INSN_FWAIT): New, defined.
Index: opcodes/sparc-opc.c
===================================================================
RCS file: /local/work/cvs/gnu/binutils/opcodes/sparc-opc.c,v
retrieving revision 1.7
diff -u -p -r1.7 sparc-opc.c
--- opcodes/sparc-opc.c	1998/12/05 03:38:56	1.7
+++ opcodes/sparc-opc.c	1999/04/03 01:06:34
@@ -1379,7 +1379,7 @@ CONDFC  ("fbule", "cb013", 0xe, 0),
 { "setsw",	F2(0x0, 0x4), F2(~0x0, ~0x4), "Sh,d", F_ALIAS, v9 },
 { "setx",	F2(0x0, 0x4), F2(~0x0, ~0x4), "S0,1,d", F_ALIAS, v9 },
 
-{ "sethi",	F2(0x0, 0x4), F2(~0x0, ~0x4), "h,d", 0, v6 },
+{ "sethi",	F2(0x0, 0x4), F2(~0x0, ~0x4), "n,d", 0, v6 },
 
 { "taddcc",	F3(2, 0x20, 0), F3(~2, ~0x20, ~0)|ASI(~0),	"1,2,d", 0, v6 },
 { "taddcc",	F3(2, 0x20, 1), F3(~2, ~0x20, ~1),		"1,i,d", 0, v6 },