[Bug gas/1804] New: Wrong output for 64-bit difference of labels

The assembler output for the following label difference is incorrect:

        .dword  L$FE0061-L$FB0061

The order of the two 32-bit words in the 64-bit output are interchanged.
This doesn't happen in every label difference.  It has something to do
with label expressions since outputing the difference directly using a
hex constant works correctly.

As far as I can tell, this output is mainly done by generic code (cons).

Will attach the full test file shortly.

--
           Summary: Wrong output for 64-bit difference of labels
           Product: binutils
           Version: 2.17 (HEAD)
            Status: NEW
          Severity: critical
          Priority: P2
         Component: gas
        AssignedTo: unassigned at sources dot redhat dot com
        ReportedBy: danglin at gcc dot gnu dot org
                CC: bug-binutils at gnu dot org
 GCC build triplet: hppa64-hp-hpux11.11
  GCC host triplet: hppa64-hp-hpux11.11
GCC target triplet: hppa64-hp-hpux11.11


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From danglin at gcc dot gnu dot org  2005-11-04 16:14 -------
Created an attachment (id=743)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=743&action=view)
Assembler testcase.

The original assembler output generated by GCC has been hacked
a bit. The label xyzzy was added as a marker for the FDE that's
being incorrectly assembled.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From amodra at bigpond dot net dot au  2005-11-05 08:36 -------
This is caused by tc-hppa.c:md_apply_fix treating the fixup as only being 32 bit.
A simpler testcase is

 .data
 .dword  L$FE0061-L$FB0061

 .text
L$FB0061:
 nop
L$FE0061:


--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From dave at hiauly1 dot hia dot nrc dot ca  2005-11-06 00:27 -------
Subject: Re:  Wrong output for 64-bit difference of labels

> ------- Additional Comments From amodra at bigpond dot net dot au  2005-11-05 08:36 -------
> This is caused by tc-hppa.c:md_apply_fix treating the fixup as only being 32 bit.

Does this look like a reasonable fix?

Dave
--
J. David Anglin                                  dave.anglin@...
National Research Council of Canada              [phone] (FAX: [phone])

Index: config/tc-hppa.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-hppa.c,v
retrieving revision 1.126
diff -u -3 -p -r1.126 tc-hppa.c
--- config/tc-hppa.c 13 Oct 2005 01:06:01 -0000 1.126
+++ config/tc-hppa.c 6 Nov 2005 00:13:48 -0000
@@ -4436,7 +4436,7 @@ md_apply_fix (fixP, valP, seg)
      valueT *valP;
      segT seg ATTRIBUTE_UNUSED;
 {
-  unsigned char *buf;
+  char *fixpos;
   struct hppa_fix_struct *hppa_fixP;
   offsetT new_val;
   int insn, val, fmt;
@@ -4481,8 +4481,16 @@ md_apply_fix (fixP, valP, seg)
       return;
     }
 
-  buf = (unsigned char *) (fixP->fx_frag->fr_literal + fixP->fx_where);
-  insn = bfd_get_32 (stdoutput, buf);
+  fixpos = fixP->fx_frag->fr_literal + fixP->fx_where;
+
+  /* Handle fixups for complex expressions that evaluate to a constant.  */
+  if (fixP->fx_r_type == R_HPPA_COMPLEX && fixP->fx_done)
+    {
+      number_to_chars_bigendian (fixpos, *valP, fixP->fx_size);
+      return;
+    }
+
+  insn = bfd_get_32 (stdoutput, fixpos);
   fmt = bfd_hppa_insn2fmt (stdoutput, insn);
 
   /* If there is a symbol associated with this fixup, then it's something
@@ -4652,7 +4660,7 @@ md_apply_fix (fixP, valP, seg)
     }
 
   /* Insert the relocation.  */
-  bfd_put_32 (stdoutput, insn, buf);
+  bfd_put_32 (stdoutput, insn, fixpos);
 }
 
 /* Exactly what point is a PC-relative offset relative TO?


--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From amodra at bigpond dot net dot au  2005-11-06 03:12 -------
I'd be inclined to use

 if (fixP->fx_size > 4)
   {
     /* Handle constant output. */
     number_to_chars_bigendian (fixpos, *valP, fixP->fx_size);
     return;
   }

After all, if the fixup is for a dword, none of the insn handling code in
md_apply_fix is appropriate.  In fact, it would be even better to avoid anything
in md_apply_fix that assumes a fixup is for an insn, ie. bfd_hppa_insn2fmt
shouldn't be in md_apply_fix at all.  Instead, pass the insn fmt (if it is an
insn) in via fx_r_format.


--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From dave at hiauly1 dot hia dot nrc dot ca  2005-11-06 03:57 -------
Subject: Re:  Wrong output for 64-bit difference of labels

> ------- Additional Comments From amodra at bigpond dot net dot au  2005-11-06 03:12 -------
> I'd be inclined to use
>
>  if (fixP->fx_size > 4)

I was thinking that other sizes might in theory be used for expressions.
For example, GCC uses .word for label differences in switch tables.

>    {
>      /* Handle constant output. */
>      number_to_chars_bigendian (fixpos, *valP, fixP->fx_size);
>      return;
>    }
>
> After all, if the fixup is for a dword, none of the insn handling code in
> md_apply_fix is appropriate.  In fact, it would be even better to avoid anything
> in md_apply_fix that assumes a fixup is for an insn, ie. bfd_hppa_insn2fmt
> shouldn't be in md_apply_fix at all.  Instead, pass the insn fmt (if it is an
> insn) in via fx_r_format.

I agree.  It appears that there aren't any conflicts between data and
insn format values.  It also appears that insn formats are set by pa_ip,
so it shouldn't be necessary to call bfd_hppa_insn2fmt.

I'm amazed that data fixups work at all on hppa64.

Dave


--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From dave at hiauly1 dot hia dot nrc dot ca  2005-11-07 00:38 -------
Subject: Re:  Wrong output for 64-bit difference of labels

> > After all, if the fixup is for a dword, none of the insn handling code in
> > md_apply_fix is appropriate.  In fact, it would be even better to avoid anything
> > in md_apply_fix that assumes a fixup is for an insn, ie. bfd_hppa_insn2fmt
> > shouldn't be in md_apply_fix at all.  Instead, pass the insn fmt (if it is an
> > insn) in via fx_r_format.
>
> I agree.  It appears that there aren't any conflicts between data and
> insn format values.  It also appears that insn formats are set by pa_ip,
> so it shouldn't be necessary to call bfd_hppa_insn2fmt.

I took a quick look at this today.  It will take a bit of work to
sort this out as the format numbers generated by pa_ip are somewhat
inconsistent with those generated by bfd_hppa_insn2fmt.  The numbers
generated by pa_ip can't simply be changed to match bfd_hppa_insn2fmt
as they are used by elf_hppa_reloc_final_type.

Dave


--


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From danglin at gcc dot gnu dot org  2005-11-09 03:37 -------
Fixed.  http://sources.redhat.com/ml/binutils/2005-11/msg00111.html.


--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=1804

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils